Add function to CancelableTaskTracker and convert BootTimeLoader

BUG=155883


Review URL: https://chromiumcodereview.appspot.com/11410073

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@168134 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 177 insertions, 50 deletions

diff --git a/chrome/browser/chromeos/boot_times_loader.cc b/chrome/browser/chromeos/boot_times_loader.cc
index 2137cd8..4aac9e2 100644
--- a/chrome/browser/chromeos/boot_times_loader.cc
+++ b/chrome/browser/chromeos/boot_times_loader.cc
@@ -11,7 +11,9 @@
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/lazy_instance.h"
+#include "base/location.h"
 #include "base/message_loop.h"
+#include "base/message_loop_proxy.h"
 #include "base/metrics/histogram.h"
 #include "base/process_util.h"
 #include "base/string_number_conversions.h"
@@ -67,8 +69,19 @@ const std::string GetTabUrl(RenderWidgetHost* rwh) {
   }
   return std::string();
 }
+
+void PostCallbackIfNotCanceled(
+    const CancelableTaskTracker::IsCanceledCallback& is_canceled_cb,
+    base::TaskRunner* task_runner,
+    const chromeos::BootTimesLoader::GetBootTimesCallback& callback,
+    const chromeos::BootTimesLoader::BootTimes& boot_times) {
+  if (is_canceled_cb.Run())
+    return;
+  task_runner->PostTask(FROM_HERE, base::Bind(callback, boot_times));
 }
 
+}  // namespace
+
 namespace chromeos {
 
 #define FPL(value) FILE_PATH_LITERAL(value)
@@ -122,31 +135,34 @@ BootTimesLoader* BootTimesLoader::Get() {
   return g_boot_times_loader.Pointer();
 }
 
-BootTimesLoader::Handle BootTimesLoader::GetBootTimes(
-    CancelableRequestConsumerBase* consumer,
-    const GetBootTimesCallback& callback) {
+CancelableTaskTracker::TaskId BootTimesLoader::GetBootTimes(
+      const GetBootTimesCallback& callback,
+      CancelableTaskTracker* tracker) {
   if (!BrowserThread::IsMessageLoopValid(BrowserThread::FILE)) {
     // This should only happen if Chrome is shutting down, so we don't do
     // anything.
-    return 0;
+    return CancelableTaskTracker::kBadTaskId;
   }
 
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
   if (command_line.HasSwitch(switches::kTestType)) {
     // TODO(davemoore) This avoids boottimes for tests. This needs to be
     // replaced with a mock of BootTimesLoader.
-    return 0;
+    return CancelableTaskTracker::kBadTaskId;
   }
 
-  scoped_refptr<CancelableRequest<GetBootTimesCallback> > request(
-      new CancelableRequest<GetBootTimesCallback>(callback));
-  AddRequest(request, consumer);
+  CancelableTaskTracker::IsCanceledCallback is_canceled;
+  CancelableTaskTracker::TaskId id = tracker->NewTrackedTaskId(&is_canceled);
 
+  GetBootTimesCallback callback_runner =
+      base::Bind(&PostCallbackIfNotCanceled,
+                 is_canceled, base::MessageLoopProxy::current(), callback);
   BrowserThread::PostTask(
       BrowserThread::FILE,
       FROM_HERE,
-      base::Bind(&Backend::GetBootTimes, backend_, request));
-  return request->handle();
+      base::Bind(&Backend::GetBootTimesAndRunCallback,
+                 backend_, is_canceled, callback_runner));
+  return id;
 }
 
 // Extracts the uptime value from files located in /tmp, returning the
@@ -214,8 +230,12 @@ static void SendBootTimesToUMA(const BootTimesLoader::BootTimes& boot_times) {
   DCHECK(file_util::PathExists(sent));
 }
 
-void BootTimesLoader::Backend::GetBootTimes(
-    const scoped_refptr<GetBootTimesRequest>& request) {
+void BootTimesLoader::Backend::GetBootTimesAndRunCallback(
+    const CancelableTaskTracker::IsCanceledCallback& is_canceled_cb,
+    const GetBootTimesCallback& callback) {
+  if (is_canceled_cb.Run())
+    return;
+
   const FilePath::CharType kFirmwareBootTime[] = FPL("firmware-boot-time");
   const FilePath::CharType kPreStartup[] = FPL("pre-startup");
   const FilePath::CharType kChromeExec[] = FPL("chrome-exec");
@@ -224,9 +244,6 @@ void BootTimesLoader::Backend::GetBootTimes(
   const FilePath::CharType kLoginPromptReady[] = FPL("login-prompt-ready");
   const FilePath::StringType uptime_prefix = kUptimePrefix;
 
-  if (request->canceled())
-    return;
-
   // Wait until firmware-boot-time file exists by reposting.
   FilePath log_dir(kLogPath);
   FilePath log_file = log_dir.Append(kFirmwareBootTime);
@@ -234,7 +251,8 @@ void BootTimesLoader::Backend::GetBootTimes(
     BrowserThread::PostDelayedTask(
         BrowserThread::FILE,
         FROM_HERE,
-        base::Bind(&Backend::GetBootTimes, this, request),
+        base::Bind(&Backend::GetBootTimesAndRunCallback,
+                   this, is_canceled_cb, callback),
         base::TimeDelta::FromMilliseconds(kReadAttemptDelayMs));
     return;
   }
@@ -258,7 +276,7 @@ void BootTimesLoader::Backend::GetBootTimes(
 
   SendBootTimesToUMA(boot_times);
 
-  request->ForwardResult(request->handle(), boot_times);
+  callback.Run(boot_times);
 }
 
 // Appends the given buffer into the file. Returns the number of bytes
diff --git a/chrome/browser/chromeos/boot_times_loader.h b/chrome/browser/chromeos/boot_times_loader.h
index 691a44ff..dc420e8 100644
--- a/chrome/browser/chromeos/boot_times_loader.h
+++ b/chrome/browser/chromeos/boot_times_loader.h
@@ -12,7 +12,7 @@
 #include "base/callback_forward.h"
 #include "base/compiler_specific.h"
 #include "base/time.h"
-#include "chrome/browser/common/cancelable_request.h"
+#include "chrome/common/cancelable_task_tracker.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/render_widget_host.h"
@@ -22,17 +22,14 @@ namespace chromeos {
 // BootTimesLoader loads the bootimes of Chrome OS from the file system.
 // Loading is done asynchronously on the file thread. Once loaded,
 // BootTimesLoader calls back to a method of your choice with the boot times.
-// To use BootTimesLoader do the following:
+// To use BootTimesLoader, do the following:
 //
 // . In your class define a member field of type chromeos::BootTimesLoader and
-//   CancelableRequestConsumerBase.
+//   CancelableTaskTracker.
 // . Define the callback method, something like:
-//   void OnBootTimesLoader(chromeos::BootTimesLoader::Handle,
-//                             BootTimesLoader::BootTimes boot_times);
-// . When you want the version invoke: loader.GetBootTimes(&consumer, callback);
-class BootTimesLoader
-    : public CancelableRequestProvider,
-      public content::NotificationObserver {
+//   void OnBootTimesLoaded(const BootTimesLoader::BootTimes& boot_times);
+// . When you want the version invoke: loader.GetBootTimes(callback, &tracker_);
+class BootTimesLoader : public content::NotificationObserver {
  public:
   BootTimesLoader();
   virtual ~BootTimesLoader();
@@ -60,17 +57,14 @@ class BootTimesLoader
                   total(0) {}
   } BootTimes;
 
-  // Signature
-  typedef base::Callback<void(Handle, BootTimes)> GetBootTimesCallback;
-
-  typedef CancelableRequest<GetBootTimesCallback> GetBootTimesRequest;
-
   static BootTimesLoader* Get();
 
+  typedef base::Callback<void(const BootTimes&)> GetBootTimesCallback;
+
   // Asynchronously requests the info.
-  Handle GetBootTimes(
-      CancelableRequestConsumerBase* consumer,
-      const GetBootTimesCallback& callback);
+  CancelableTaskTracker::TaskId GetBootTimes(
+      const GetBootTimesCallback& callback,
+      CancelableTaskTracker* tracker);
 
   // Add a time marker for login. A timeline will be dumped to
   // /tmp/login-times-sent after login is done. If |send_to_uma| is true
@@ -119,7 +113,9 @@ class BootTimesLoader
    public:
     Backend() {}
 
-    void GetBootTimes(const scoped_refptr<GetBootTimesRequest>& request);
+    void GetBootTimesAndRunCallback(
+        const CancelableTaskTracker::IsCanceledCallback& is_canceled_cb,
+        const GetBootTimesCallback& callback);
 
    private:
     friend class base::RefCountedThreadSafe<Backend>;
diff --git a/chrome/browser/chromeos/login/version_info_updater.cc b/chrome/browser/chromeos/login/version_info_updater.cc
index 2c9ccc5..42aa2e4 100644
--- a/chrome/browser/chromeos/login/version_info_updater.cc
+++ b/chrome/browser/chromeos/login/version_info_updater.cc
@@ -59,10 +59,10 @@ void VersionInfoUpdater::StartUpdate(bool is_official_build) {
         base::Bind(&VersionInfoUpdater::OnVersion, base::Unretained(this)),
         &tracker_);
     boot_times_loader_.GetBootTimes(
-        &boot_times_consumer_,
-        base::Bind(is_official_build ? &VersionInfoUpdater::OnBootTimesNoop :
-                                       &VersionInfoUpdater::OnBootTimes,
-                   base::Unretained(this)));
+        base::Bind(is_official_build ? &VersionInfoUpdater::OnBootTimesNoop
+                                     : &VersionInfoUpdater::OnBootTimes,
+                   base::Unretained(this)),
+        &tracker_);
   } else {
     UpdateVersionLabel();
   }
@@ -195,11 +195,10 @@ void VersionInfoUpdater::OnVersion(const std::string& version) {
 }
 
 void VersionInfoUpdater::OnBootTimesNoop(
-    BootTimesLoader::Handle handle, BootTimesLoader::BootTimes boot_times) {
-}
+    const BootTimesLoader::BootTimes& boot_times) {}
 
 void VersionInfoUpdater::OnBootTimes(
-    BootTimesLoader::Handle handle, BootTimesLoader::BootTimes boot_times) {
+    const BootTimesLoader::BootTimes& boot_times) {
   const char* kBootTimesNoChromeExec =
       "Non-firmware boot took %.2f seconds (kernel %.2fs, system %.2fs)";
   const char* kBootTimesChromeExec =
diff --git a/chrome/browser/chromeos/login/version_info_updater.h b/chrome/browser/chromeos/login/version_info_updater.h
index c02ff71..fc9868d 100644
--- a/chrome/browser/chromeos/login/version_info_updater.h
+++ b/chrome/browser/chromeos/login/version_info_updater.h
@@ -75,21 +75,19 @@ class VersionInfoUpdater : public policy::CloudPolicySubsystem::Observer,
   // Callback from chromeos::VersionLoader giving the version.
   void OnVersion(const std::string& version);
   // Callback from chromeos::InfoLoader giving the boot times.
-  void OnBootTimes(
-      BootTimesLoader::Handle handle, BootTimesLoader::BootTimes boot_times);
+  void OnBootTimes(const BootTimesLoader::BootTimes& boot_times);
   // Null callback from chromeos::InfoLoader.
-  void OnBootTimesNoop(
-      BootTimesLoader::Handle handle, BootTimesLoader::BootTimes boot_times);
+  void OnBootTimesNoop(const BootTimesLoader::BootTimes& boot_times);
 
   // Handles asynchronously loading the version.
   VersionLoader version_loader_;
   // Used to request the version.
-  CancelableTaskTracker tracker_;
+  CancelableRequestConsumer version_consumer_;
 
   // Handles asynchronously loading the boot times.
   BootTimesLoader boot_times_loader_;
-  // Used to request the boot times.
-  CancelableRequestConsumer boot_times_consumer_;
+  // Used to request boot times.
+  CancelableTaskTracker tracker_;
 
   // Information pieces for version label.
   std::string version_text_;
diff --git a/chrome/common/cancelable_task_tracker.cc b/chrome/common/cancelable_task_tracker.cc
index 3570e05..28ee436 100644
--- a/chrome/common/cancelable_task_tracker.cc
+++ b/chrome/common/cancelable_task_tracker.cc
@@ -8,6 +8,7 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/location.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop_proxy.h"
 #include "base/synchronization/cancellation_flag.h"
@@ -33,6 +34,18 @@ void RunIfNotCanceledThenUntrack(const CancellationFlag* flag,
   untrack.Run();
 }
 
+bool IsCanceled(const CancellationFlag* flag,
+                base::ScopedClosureRunner* untrack_runner) {
+  return flag->IsSet();
+}
+
+void RunOrPostToTaskRunner(TaskRunner* task_runner, const Closure& closure) {
+  if (task_runner->RunsTasksOnCurrentThread())
+    closure.Run();
+  else
+    task_runner->PostTask(FROM_HERE, closure);
+}
+
 }  // namespace
 
 // static
@@ -84,6 +97,29 @@ CancelableTaskTracker::TaskId CancelableTaskTracker::PostTaskAndReply(
   return id;
 }
 
+CancelableTaskTracker::TaskId CancelableTaskTracker::NewTrackedTaskId(
+    IsCanceledCallback* is_canceled_cb) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(base::MessageLoopProxy::current());
+
+  TaskId id = next_id_;
+  next_id_++;  // int64 is big enough that we ignore the potential overflow.
+
+  // Both owned by |is_canceled_cb|.
+  CancellationFlag* flag = new CancellationFlag();
+  base::ScopedClosureRunner* untrack_runner = new base::ScopedClosureRunner(
+      Bind(&RunOrPostToTaskRunner,
+           base::MessageLoopProxy::current(),
+           Bind(&CancelableTaskTracker::Untrack,
+                weak_factory_.GetWeakPtr(), id)));
+
+  *is_canceled_cb = Bind(&IsCanceled,
+                         base::Owned(flag),
+                         base::Owned(untrack_runner));
+  Track(id, flag);
+  return id;
+}
+
 void CancelableTaskTracker::TryCancel(TaskId id) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
diff --git a/chrome/common/cancelable_task_tracker.h b/chrome/common/cancelable_task_tracker.h
index 0a7289b..86450c3 100644
--- a/chrome/common/cancelable_task_tracker.h
+++ b/chrome/common/cancelable_task_tracker.h
@@ -28,6 +28,8 @@
 // commonly used to cancel all outstanding tasks.
 //
 // 2. Both task and reply are deleted on the originating thread.
+//
+// 3. IsCanceledCallback is thread safe and can be run or deleted on any thread.
 
 #ifndef CHROME_COMMON_CANCELABLE_TASK_TRACKER_H_
 #define CHROME_COMMON_CANCELABLE_TASK_TRACKER_H_
@@ -53,6 +55,8 @@ class CancelableTaskTracker {
   typedef int64 TaskId;
   static const TaskId kBadTaskId;
 
+  typedef base::Callback<bool()> IsCanceledCallback;
+
   CancelableTaskTracker();
 
   // Cancels all tracked tasks.
@@ -67,6 +71,14 @@ class CancelableTaskTracker {
                           const base::Closure& task,
                           const base::Closure& reply);
 
+  // Creates a tracked TaskId and an associated IsCanceledCallback. Client can
+  // later call TryCancel() with the returned TaskId, and run |is_canceled_cb|
+  // to check whether the TaskId is canceled.
+  //
+  // Note. This function is used to address some special cancelation requirement
+  // in existing code. You SHOULD NOT need this function in new code.
+  TaskId NewTrackedTaskId(IsCanceledCallback* is_canceled_cb);
+
   // After calling this function, |task| and |reply| will not run. If the
   // cancelation happens when |task| is running or has finished running, |reply|
   // will not run. If |reply| is running or has finished running, cancellation
diff --git a/chrome/common/cancelable_task_tracker_unittest.cc b/chrome/common/cancelable_task_tracker_unittest.cc
index 60315bc..c6a7a85 100644
--- a/chrome/common/cancelable_task_tracker_unittest.cc
+++ b/chrome/common/cancelable_task_tracker_unittest.cc
@@ -67,12 +67,14 @@ class CancelableTaskTrackerTest : public testing::Test {
   virtual void TearDown() {
     UnblockTaskThread();
 
-    // Create tracker on client thread.
+    // Destroy tracker on client thread.
     WaitableEvent tracker_destroyed(true, false);
     client_thread_runner_->PostTask(
         FROM_HERE,
         Bind(&CancelableTaskTrackerTest::DestroyTrackerOnClientThread,
              Unretained(this), &tracker_destroyed));
+
+    // This will also wait for any pending tasks on client thread.
     tracker_destroyed.Wait();
 
     client_thread_->Stop();
@@ -112,6 +114,13 @@ class CancelableTaskTrackerTest : public testing::Test {
                 &test_data_, event);
   }
 
+  Closure IncreaseTestDataIfNotCanceledAndSignalClosure(
+      const CancelableTaskTracker::IsCanceledCallback& is_canceled_cb,
+      WaitableEvent* event) {
+    return Bind(&CancelableTaskTrackerTest::IncreaseDataIfNotCanceledAndSignal,
+                &test_data_, is_canceled_cb, event);
+  }
+
   Closure DecreaseTestDataClosure(WaitableEvent* event) {
     return Bind(&CancelableTaskTrackerTest::DecreaseData,
                 Owned(new WaitableEventScoper(event)), &test_data_);
@@ -134,6 +143,16 @@ class CancelableTaskTrackerTest : public testing::Test {
       event->Signal();
   }
 
+  static void IncreaseDataIfNotCanceledAndSignal(
+      int* data,
+      const CancelableTaskTracker::IsCanceledCallback& is_canceled_cb,
+      WaitableEvent* event) {
+    if (!is_canceled_cb.Run())
+      (*data)++;
+    if (event)
+      event->Signal();
+  }
+
   static void DecreaseData(WaitableEventScoper* event_scoper, int* data) {
     (*data) -= 2;
   }
@@ -145,6 +164,7 @@ class CancelableTaskTrackerTest : public testing::Test {
 };
 
 #if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON)) && GTEST_HAS_DEATH_TEST
+
 typedef CancelableTaskTrackerTest CancelableTaskTrackerDeathTest;
 
 TEST_F(CancelableTaskTrackerDeathTest, PostFromDifferentThread) {
@@ -325,4 +345,52 @@ TEST_F(CancelableTaskTrackerTest, TrackerDestructedAfterTask) {
   EXPECT_EQ(1, test_data_);
 }
 
+void CheckTrackedTaskIdOnSameThread_Test(CancelableTaskTrackerTest* test,
+                                         WaitableEvent* event) {
+  CancelableTaskTracker::IsCanceledCallback is_canceled_cb;
+  test->task_id_ = test->tracker_->NewTrackedTaskId(&is_canceled_cb);
+  ASSERT_NE(CancelableTaskTracker::kBadTaskId, test->task_id_);
+
+  EXPECT_FALSE(is_canceled_cb.Run());
+
+  test->tracker_->TryCancel(test->task_id_);
+  EXPECT_TRUE(is_canceled_cb.Run());
+
+  test->task_id_ = test->tracker_->NewTrackedTaskId(&is_canceled_cb);
+  EXPECT_FALSE(is_canceled_cb.Run());
+
+  // Destroy tracker will cancel all tasks.
+  test->tracker_.reset();
+  EXPECT_TRUE(is_canceled_cb.Run());
+
+  event->Signal();
+}
+
+TEST_F(CancelableTaskTrackerTest, CheckTrackedTaskIdOnSameThread) {
+  RunOnClientAndWait(&CheckTrackedTaskIdOnSameThread_Test);
+}
+
+void CheckTrackedTaskIdOnDifferentThread_Test(CancelableTaskTrackerTest* test,
+                                              WaitableEvent* event) {
+  CancelableTaskTracker::IsCanceledCallback is_canceled_cb;
+  test->task_id_ = test->tracker_->NewTrackedTaskId(&is_canceled_cb);
+  ASSERT_NE(CancelableTaskTracker::kBadTaskId, test->task_id_);
+
+  // Post task to task thread.
+  test->task_thread_runner_->PostTask(
+      FROM_HERE,
+      test->IncreaseTestDataIfNotCanceledAndSignalClosure(is_canceled_cb,
+                                                          event));
+  is_canceled_cb.Reset();  // So the one in task thread runner is the last ref,
+                           // and will be destroyed on task thread.
+
+  test->tracker_->TryCancel(test->task_id_);
+  test->UnblockTaskThread();
+}
+
+TEST_F(CancelableTaskTrackerTest, CheckTrackedTaskIdOnDifferentThread) {
+  RunOnClientAndWait(&CheckTrackedTaskIdOnDifferentThread_Test);
+  EXPECT_EQ(0, test_data_);
+}
+
 }  // namespace