Implement responsiveness::MessageLoopObserver.

This CL adds the class MessageLoopObserver, which forwards events from the UI
and IO thread message loop task runners to the responsiveness Watcher and
Calculator.

Bug: 859155
Change-Id: Ie492f1fbc26940007cb303bdc5cdbdb59d887c19
Reviewed-on: https://chromium-review.googlesource.com/1149042
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579083}

base/message_loop/message_loop.cc

@@ -273,6 +273,11 @@ void MessageLoop::RemoveTaskObserver(TaskObserver* task_observer) {
   task_observers_.RemoveObserver(task_observer);
 }
 
+void MessageLoop::SetAddQueueTimeToTasks(bool enable) {
+  DCHECK_CALLED_ON_VALID_THREAD(bound_thread_checker_);
+  underlying_task_runner_->SetAddQueueTimeToTasks(enable);
+}
+
 bool MessageLoop::IsIdleForTesting() {
   // Have unprocessed tasks? (this reloads the work queue if necessary)
   if (sequenced_task_source_->HasTasks())

base/message_loop/message_loop.h

@@ -182,6 +182,10 @@ class BASE_EXPORT MessageLoop : public MessagePump::Delegate,
   void AddTaskObserver(TaskObserver* task_observer);
   void RemoveTaskObserver(TaskObserver* task_observer);
 
+  // When this functionality is enabled, the queue time will be recorded for
+  // posted tasks.
+  void SetAddQueueTimeToTasks(bool enable);
+
   // Returns true if the message loop is idle (ignoring delayed tasks). This is
   // the same condition which triggers DoWork() to return false: i.e.
   // out of tasks which can be processed at the current run-level -- there might

base/message_loop/message_loop_current.cc

@@ -74,6 +74,11 @@ void MessageLoopCurrent::RemoveTaskObserver(TaskObserver* task_observer) {
   current_->RemoveTaskObserver(task_observer);
 }
 
+void MessageLoopCurrent::SetAddQueueTimeToTasks(bool enable) {
+  DCHECK_CALLED_ON_VALID_THREAD(current_->bound_thread_checker_);
+  current_->SetAddQueueTimeToTasks(enable);
+}
+
 void MessageLoopCurrent::SetNestableTasksAllowed(bool allowed) {
   DCHECK_CALLED_ON_VALID_THREAD(current_->bound_thread_checker_);
   if (allowed) {

base/message_loop/message_loop_current.h

@@ -120,6 +120,10 @@ class BASE_EXPORT MessageLoopCurrent {
   void AddTaskObserver(TaskObserver* task_observer);
   void RemoveTaskObserver(TaskObserver* task_observer);
 
+  // When this functionality is enabled, the queue time will be recorded for
+  // posted tasks.
+  void SetAddQueueTimeToTasks(bool enable);
+
   // Enables or disables the recursive task processing. This happens in the case
   // of recursive message loops. Some unwanted message loops may occur when
   // using common controls or printer functions. By default, recursive task

base/message_loop/message_loop_task_runner.cc

@@ -107,6 +107,11 @@ void MessageLoopTaskRunner::InjectTask(OnceClosure task) {
   DCHECK(success) << "Injected a task in a dead task runner.";
 }
 
+void MessageLoopTaskRunner::SetAddQueueTimeToTasks(bool enable) {
+  DCHECK_NE(add_queue_time_to_tasks_, enable);
+  add_queue_time_to_tasks_ = enable;
+}
+
 MessageLoopTaskRunner::~MessageLoopTaskRunner() = default;
 
 bool MessageLoopTaskRunner::AddToIncomingQueue(const Location& from_here,
@@ -122,6 +127,13 @@ bool MessageLoopTaskRunner::AddToIncomingQueue(const Location& from_here,
 
   PendingTask pending_task(from_here, std::move(task),
                            CalculateDelayedRuntime(from_here, delay), nestable);
+  if (add_queue_time_to_tasks_) {
+    if (pending_task.delayed_run_time.is_null()) {
+      pending_task.queue_time = base::TimeTicks::Now();
+    } else {
+      pending_task.queue_time = pending_task.delayed_run_time - delay;
+    }
+  }
 
 #if defined(OS_WIN)
   // We consider the task needs a high resolution timer if the delay is

base/message_loop/message_loop_task_runner.h

@@ -57,6 +57,10 @@ class BASE_EXPORT MessageLoopTaskRunner : public SingleThreadTaskRunner,
   bool HasTasks() override;
   void InjectTask(OnceClosure task) override;
 
+  // When this functionality is enabled, AddToIncomingQueue() will also add the
+  // queue time to the task.
+  void SetAddQueueTimeToTasks(bool enable);
+
  private:
   friend class RefCountedThreadSafe<MessageLoopTaskRunner>;
   ~MessageLoopTaskRunner() override;
@@ -100,6 +104,9 @@ class BASE_EXPORT MessageLoopTaskRunner : public SingleThreadTaskRunner,
   // The next sequence number to use for delayed tasks.
   int next_sequence_num_ = 0;
 
+  // Whether to add the queue time to tasks.
+  bool add_queue_time_to_tasks_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(MessageLoopTaskRunner);
 };
 

base/pending_task.h

@@ -11,6 +11,7 @@
 #include "base/callback.h"
 #include "base/containers/queue.h"
 #include "base/location.h"
+#include "base/optional.h"
 #include "base/time/time.h"
 
 namespace base {
@@ -44,6 +45,10 @@ struct BASE_EXPORT PendingTask {
   // The time when the task should be run.
   base::TimeTicks delayed_run_time;
 
+  // The time at which the task was queued. Only set if the task was posted to a
+  // MessageLoop with SetAddQueueTimeToTasks(true).
+  Optional<TimeTicks> queue_time;
+
   // Chain of up-to-four symbols of the parent tasks which led to this one being
   // posted.
   std::array<const void*, 4> task_backtrace = {};

content/browser/BUILD.gn

@@ -1518,6 +1518,8 @@ jumbo_source_set("browser") {
     "sandbox_parameters_mac.mm",
     "scheduler/responsiveness/calculator.cc",
     "scheduler/responsiveness/calculator.h",
+    "scheduler/responsiveness/message_loop_observer.cc",
+    "scheduler/responsiveness/message_loop_observer.h",
     "scheduler/responsiveness/watcher.cc",
     "scheduler/responsiveness/watcher.h",
     "scoped_active_url.cc",

content/browser/scheduler/responsiveness/message_loop_observer.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/browser/scheduler/responsiveness/message_loop_observer.h"
+
+namespace responsiveness {
+
+MessageLoopObserver::MessageLoopObserver(TaskCallback will_run_task_callback,
+                                         TaskCallback did_run_task_callback)
+    : will_run_task_callback_(will_run_task_callback),
+      did_run_task_callback_(did_run_task_callback) {
+  base::MessageLoopCurrent::Get()->SetAddQueueTimeToTasks(true);
+  base::MessageLoopCurrent::Get()->AddTaskObserver(this);
+}
+
+MessageLoopObserver::~MessageLoopObserver() {
+  base::MessageLoopCurrent::Get()->RemoveTaskObserver(this);
+  base::MessageLoopCurrent::Get()->SetAddQueueTimeToTasks(false);
+}
+
+void MessageLoopObserver::WillProcessTask(
+    const base::PendingTask& pending_task) {
+  will_run_task_callback_.Run(&pending_task);
+}
+
+void MessageLoopObserver::DidProcessTask(
+    const base::PendingTask& pending_task) {
+  did_run_task_callback_.Run(&pending_task);
+}
+
+}  // namespace responsiveness

content/browser/scheduler/responsiveness/message_loop_observer.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_BROWSER_SCHEDULER_RESPONSIVENESS_MESSAGE_LOOP_OBSERVER_H_
+#define CONTENT_BROWSER_SCHEDULER_RESPONSIVENESS_MESSAGE_LOOP_OBSERVER_H_
+
+#include "base/macros.h"
+#include "base/message_loop/message_loop_current.h"
+#include "content/common/content_export.h"
+
+namespace base {
+struct PendingTask;
+}  // namespace base
+
+namespace responsiveness {
+
+// This object is not thread safe. It must be constructed and destroyed on the
+// same thread. The callbacks will occur synchronously from WillProcessTask()
+// and DidProcessTask().
+class CONTENT_EXPORT MessageLoopObserver
+    : base::MessageLoopCurrent::TaskObserver {
+ public:
+  using TaskCallback =
+      base::RepeatingCallback<void(const base::PendingTask* task)>;
+
+  // The constructor will register the object as an observer of the current
+  // MessageLoop. The destructor will unregister the object.
+  MessageLoopObserver(TaskCallback will_run_task_callback,
+                      TaskCallback did_run_task_callback);
+  ~MessageLoopObserver() override;
+
+ private:
+  void WillProcessTask(const base::PendingTask& pending_task) override;
+  void DidProcessTask(const base::PendingTask& pending_task) override;
+
+  TaskCallback will_run_task_callback_;
+  TaskCallback did_run_task_callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(MessageLoopObserver);
+};
+
+}  // namespace responsiveness
+
+#endif  // CONTENT_BROWSER_SCHEDULER_RESPONSIVENESS_MESSAGE_LOOP_OBSERVER_H_

content/browser/scheduler/responsiveness/watcher.cc

@@ -6,11 +6,12 @@
 
 #include "base/pending_task.h"
 #include "content/browser/scheduler/responsiveness/calculator.h"
+#include "content/browser/scheduler/responsiveness/message_loop_observer.h"
 #include "content/public/browser/browser_thread.h"
 
 namespace responsiveness {
 
-Watcher::Metadata::Metadata(void* identifier) : identifier(identifier) {}
+Watcher::Metadata::Metadata(const void* identifier) : identifier(identifier) {}
 
 Watcher::Watcher() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -27,9 +28,12 @@ void Watcher::SetUp() {
 
   calculator_ = MakeCalculator();
 
+  RegisterMessageLoopObserverUI();
+
   content::BrowserThread::PostTask(
       content::BrowserThread::IO, FROM_HERE,
-      base::BindOnce(&Watcher::SetUpOnIOThread, this, calculator_.get()));
+      base::BindOnce(&Watcher::SetUpOnIOThread, base::Unretained(this),
+                     calculator_.get()));
 }
 
 void Watcher::Destroy() {
@@ -38,9 +42,11 @@ void Watcher::Destroy() {
   DCHECK(!destroy_was_called_);
   destroy_was_called_ = true;
 
+  message_loop_observer_ui_.reset();
+
   content::BrowserThread::PostTask(
       content::BrowserThread::IO, FROM_HERE,
-      base::BindOnce(&Watcher::TearDownOnIOThread, this));
+      base::BindOnce(&Watcher::TearDownOnIOThread, base::Unretained(this)));
 }
 
 std::unique_ptr<Calculator> Watcher::MakeCalculator() {
@@ -52,23 +58,42 @@ Watcher::~Watcher() {
   DCHECK(destroy_was_called_);
 }
 
+void Watcher::RegisterMessageLoopObserverUI() {
+  // We must use base::Unretained(this) to prevent ownership cycle.
+  MessageLoopObserver::TaskCallback will_run_callback = base::BindRepeating(
+      &Watcher::WillRunTaskOnUIThread, base::Unretained(this));
+  MessageLoopObserver::TaskCallback did_run_callback = base::BindRepeating(
+      &Watcher::DidRunTaskOnUIThread, base::Unretained(this));
+  message_loop_observer_ui_.reset(new MessageLoopObserver(
+      std::move(will_run_callback), std::move(did_run_callback)));
+}
+
+void Watcher::RegisterMessageLoopObserverIO() {
+  // We must use base::Unretained(this) to prevent ownership cycle.
+  MessageLoopObserver::TaskCallback will_run_callback = base::BindRepeating(
+      &Watcher::WillRunTaskOnIOThread, base::Unretained(this));
+  MessageLoopObserver::TaskCallback did_run_callback = base::BindRepeating(
+      &Watcher::DidRunTaskOnIOThread, base::Unretained(this));
+  message_loop_observer_io_.reset(new MessageLoopObserver(
+      std::move(will_run_callback), std::move(did_run_callback)));
+}
+
 void Watcher::SetUpOnIOThread(Calculator* calculator) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
-  // TODO(erikchen): Add MessageLoopObserver to IO thread.
-
+  RegisterMessageLoopObserverIO();
   calculator_io_ = calculator;
 }
 
 void Watcher::TearDownOnIOThread() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
-  // TODO(erikchen): Remove MessageLoopObserver from IO thread.
+  message_loop_observer_io_.reset();
 
   calculator_io_ = nullptr;
   content::BrowserThread::PostTask(
       content::BrowserThread::UI, FROM_HERE,
-      base::BindOnce(&Watcher::TearDownOnUIThread, this));
+      base::BindOnce(&Watcher::TearDownOnUIThread, base::Unretained(this)));
 }
 
 void Watcher::TearDownOnUIThread() {
@@ -78,13 +103,13 @@ void Watcher::TearDownOnUIThread() {
   Release();
 }
 
-void Watcher::WillRunTaskOnUIThread(base::PendingTask* task) {
+void Watcher::WillRunTaskOnUIThread(const base::PendingTask* task) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   WillRunTask(task, &currently_running_metadata_ui_);
 }
 
-void Watcher::DidRunTaskOnUIThread(base::PendingTask* task) {
+void Watcher::DidRunTaskOnUIThread(const base::PendingTask* task) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   // It's safe to use base::Unretained since the callback will be synchronously
@@ -97,13 +122,13 @@ void Watcher::DidRunTaskOnUIThread(base::PendingTask* task) {
              &mismatched_task_identifiers_ui_, std::move(callback));
 }
 
-void Watcher::WillRunTaskOnIOThread(base::PendingTask* task) {
+void Watcher::WillRunTaskOnIOThread(const base::PendingTask* task) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
   WillRunTask(task, &currently_running_metadata_io_);
 }
 
-void Watcher::DidRunTaskOnIOThread(base::PendingTask* task) {
+void Watcher::DidRunTaskOnIOThread(const base::PendingTask* task) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
   // It's safe to use base::Unretained since the callback will be synchronously
@@ -115,7 +140,7 @@ void Watcher::DidRunTaskOnIOThread(base::PendingTask* task) {
              &mismatched_task_identifiers_io_, std::move(callback));
 }
 
-void Watcher::WillRunTask(base::PendingTask* task,
+void Watcher::WillRunTask(const base::PendingTask* task,
                           std::stack<Metadata>* currently_running_metadata) {
   // Reentrancy should be rare.
   if (UNLIKELY(!currently_running_metadata->empty())) {
@@ -131,7 +156,7 @@ void Watcher::WillRunTask(base::PendingTask* task,
   }
 }
 
-void Watcher::DidRunTask(base::PendingTask* task,
+void Watcher::DidRunTask(const base::PendingTask* task,
                          std::stack<Metadata>* currently_running_metadata,
                          int* mismatched_task_identifiers,
                          TaskOrEventFinishedCallback callback) {
@@ -139,7 +164,8 @@ void Watcher::DidRunTask(base::PendingTask* task,
   // the identifier should differ is when Watcher is first constructed. The
   // TaskRunner Observers are added while a task is being run, which means that
   // there was no corresponding WillRunTask.
-  if (UNLIKELY(task != currently_running_metadata->top().identifier)) {
+  if (UNLIKELY(currently_running_metadata->empty() ||
+               (task != currently_running_metadata->top().identifier))) {
     *mismatched_task_identifiers += 1;
     DCHECK_LE(*mismatched_task_identifiers, 1);
     return;
@@ -158,9 +184,14 @@ void Watcher::DidRunTask(base::PendingTask* task,
   // For delayed tasks, measure the duration of the task itself, rather than the
   // duration from schedule time to finish time.
   base::TimeTicks schedule_time;
-  if (task->delayed_run_time.is_null()) {
-    // TODO(erikchen): Implement DelayedTask::queue_time.
-    // schedule_time = task->queue_time;
+  if (delayed_task_start.is_null()) {
+    // Tasks which were posted before the MessageLoopObserver was created will
+    // not have a queue_time, and should be ignored. This doesn't affect delayed
+    // tasks.
+    if (UNLIKELY(!task->queue_time))
+      return;
+
+    schedule_time = task->queue_time.value();
   } else {
     schedule_time = delayed_task_start;
   }

content/browser/scheduler/responsiveness/watcher.h

@@ -21,6 +21,7 @@ struct PendingTask;
 namespace responsiveness {
 
 class Calculator;
+class MessageLoopObserver;
 
 // This class watches events and tasks processed on the UI and IO threads of the
 // browser process. It forwards stats on execution latency to Calculator, which
@@ -51,6 +52,8 @@ class CONTENT_EXPORT Watcher : public base::RefCounted<Watcher> {
   // Exposed for tests.
   virtual std::unique_ptr<Calculator> MakeCalculator();
   virtual ~Watcher();
+  virtual void RegisterMessageLoopObserverUI();
+  virtual void RegisterMessageLoopObserverIO();
 
  private:
   friend class base::RefCounted<Watcher>;
@@ -60,10 +63,10 @@ class CONTENT_EXPORT Watcher : public base::RefCounted<Watcher> {
   // Metadata for currently running tasks and events is needed to track whether
   // or not they caused reentrancy.
   struct Metadata {
-    explicit Metadata(void* identifier);
+    explicit Metadata(const void* identifier);
 
     // An opaque identifier for the task or event.
-    void* identifier = nullptr;
+    const void* identifier = nullptr;
 
     // Whether the task or event has caused reentrancy.
     bool caused_reentrancy = false;
@@ -80,32 +83,31 @@ class CONTENT_EXPORT Watcher : public base::RefCounted<Watcher> {
   void TearDownOnIOThread();
   void TearDownOnUIThread();
 
-  // TODO(erikchen): Implement MessageLoopObserver.
   // These methods are called by the MessageLoopObserver of the UI thread to
   // allow Watcher to collect metadata about the tasks being run.
-  void WillRunTaskOnUIThread(base::PendingTask* task);
-  void DidRunTaskOnUIThread(base::PendingTask* task);
+  void WillRunTaskOnUIThread(const base::PendingTask* task);
+  void DidRunTaskOnUIThread(const base::PendingTask* task);
 
-  // TODO(erikchen): Implement MessageLoopObserver.
   // These methods are called by the MessageLoopObserver of the IO thread to
   // allow Watcher to collect metadata about the tasks being run.
-  void WillRunTaskOnIOThread(base::PendingTask* task);
-  void DidRunTaskOnIOThread(base::PendingTask* task);
+  void WillRunTaskOnIOThread(const base::PendingTask* task);
+  void DidRunTaskOnIOThread(const base::PendingTask* task);
 
   // Common implementations for the thread-specific methods.
-  void WillRunTask(base::PendingTask* task,
+  void WillRunTask(const base::PendingTask* task,
                    std::stack<Metadata>* currently_running_metadata);
 
   // |callback| will either be synchronously invoked, or else never invoked.
   using TaskOrEventFinishedCallback =
       base::OnceCallback<void(base::TimeTicks, base::TimeTicks)>;
-  void DidRunTask(base::PendingTask* task,
+  void DidRunTask(const base::PendingTask* task,
                   std::stack<Metadata>* currently_running_metadata,
                   int* mismatched_task_identifiers,
                   TaskOrEventFinishedCallback callback);
 
   // The following members are all affine to the UI thread.
   std::unique_ptr<Calculator> calculator_;
+  std::unique_ptr<MessageLoopObserver> message_loop_observer_ui_;
 
   // Metadata for currently running tasks and events on the UI thread.
   std::stack<Metadata> currently_running_metadata_ui_;
@@ -118,6 +120,7 @@ class CONTENT_EXPORT Watcher : public base::RefCounted<Watcher> {
   // The following members are all affine to the IO thread.
   std::stack<Metadata> currently_running_metadata_io_;
   int mismatched_task_identifiers_io_ = 0;
+  std::unique_ptr<MessageLoopObserver> message_loop_observer_io_;
 
   // The implementation of this class guarantees that |calculator_io_| will be
   // non-nullptr and point to a valid object any time it is used on the IO

content/browser/scheduler/responsiveness/watcher_unittest.cc

@@ -6,7 +6,10 @@
 
 #include "base/location.h"
 #include "base/pending_task.h"
+#include "base/run_loop.h"
+#include "base/synchronization/lock.h"
 #include "content/browser/scheduler/responsiveness/calculator.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -18,20 +21,30 @@ class FakeCalculator : public Calculator {
  public:
   void TaskOrEventFinishedOnUIThread(base::TimeTicks schedule_time,
                                      base::TimeTicks finish_time) override {
-    ++tasks_on_ui_thread_;
+    queue_times_ui_.push_back(schedule_time);
   }
 
   void TaskOrEventFinishedOnIOThread(base::TimeTicks schedule_time,
                                      base::TimeTicks finish_time) override {
-    ++tasks_on_io_thread_;
+    base::AutoLock l(io_thread_lock_);
+    queue_times_io_.push_back(schedule_time);
   }
 
-  int NumTasksOnUIThread() { return tasks_on_ui_thread_; }
-  int NumTasksOnIOThread() { return tasks_on_io_thread_; }
+  int NumTasksOnUIThread() { return static_cast<int>(queue_times_ui_.size()); }
+  std::vector<base::TimeTicks>& QueueTimesUIThread() { return queue_times_ui_; }
+  int NumTasksOnIOThread() {
+    base::AutoLock l(io_thread_lock_);
+    return static_cast<int>(queue_times_io_.size());
+  }
+  std::vector<base::TimeTicks>& QueueTimesIOThread() {
+    base::AutoLock l(io_thread_lock_);
+    return queue_times_io_;
+  }
 
  private:
-  int tasks_on_ui_thread_ = 0;
-  int tasks_on_io_thread_ = 0;
+  std::vector<base::TimeTicks> queue_times_ui_;
+  base::Lock io_thread_lock_;
+  std::vector<base::TimeTicks> queue_times_io_;
 };
 
 class FakeWatcher : public Watcher {
@@ -43,14 +56,32 @@ class FakeWatcher : public Watcher {
     return calculator;
   }
 
-  FakeWatcher() : Watcher() {}
+  void RegisterMessageLoopObserverUI() override {
+    if (register_message_loop_observer_)
+      Watcher::RegisterMessageLoopObserverUI();
+  }
+  void RegisterMessageLoopObserverIO() override {
+    if (register_message_loop_observer_)
+      Watcher::RegisterMessageLoopObserverIO();
+  }
+
+  FakeWatcher(bool register_message_loop_observer)
+      : Watcher(),
+        register_message_loop_observer_(register_message_loop_observer) {}
 
   int NumTasksOnUIThread() { return calculator_->NumTasksOnUIThread(); }
+  std::vector<base::TimeTicks>& QueueTimesUIThread() {
+    return calculator_->QueueTimesUIThread();
+  }
+  std::vector<base::TimeTicks>& QueueTimesIOThread() {
+    return calculator_->QueueTimesIOThread();
+  }
   int NumTasksOnIOThread() { return calculator_->NumTasksOnIOThread(); }
 
  private:
   ~FakeWatcher() override{};
   FakeCalculator* calculator_ = nullptr;
+  bool register_message_loop_observer_ = false;
 };
 
 }  // namespace
@@ -60,7 +91,8 @@ class ResponsivenessWatcherTest : public testing::Test {
   void SetUp() override {
     // Watcher's constructor posts a task to IO thread, which in the unit test
     // is also this thread. Regardless, we need to let those tasks finish.
-    watcher_ = scoped_refptr<FakeWatcher>(new FakeWatcher);
+    watcher_ = scoped_refptr<FakeWatcher>(
+        new FakeWatcher(/*register_message_loop_observer=*/false));
     watcher_->SetUp();
     test_browser_thread_bundle_.RunUntilIdle();
   }
@@ -82,6 +114,7 @@ class ResponsivenessWatcherTest : public testing::Test {
 TEST_F(ResponsivenessWatcherTest, TaskForwarding) {
   for (int i = 0; i < 3; ++i) {
     base::PendingTask task(FROM_HERE, base::OnceClosure());
+    task.queue_time = base::TimeTicks::Now();
     watcher_->WillRunTaskOnUIThread(&task);
     watcher_->DidRunTaskOnUIThread(&task);
   }
@@ -90,6 +123,7 @@ TEST_F(ResponsivenessWatcherTest, TaskForwarding) {
 
   for (int i = 0; i < 4; ++i) {
     base::PendingTask task(FROM_HERE, base::OnceClosure());
+    task.queue_time = base::TimeTicks::Now();
     watcher_->WillRunTaskOnIOThread(&task);
     watcher_->DidRunTaskOnIOThread(&task);
   }
@@ -99,11 +133,14 @@ TEST_F(ResponsivenessWatcherTest, TaskForwarding) {
 
 // Test that nested tasks are not forwarded to the calculator.
 TEST_F(ResponsivenessWatcherTest, TaskNesting) {
-  // TODO(erikchen): Check that the right task is forwarded to the calculator.
-  // Requires implementation of PendingTask::queue_time.
+  base::TimeTicks now = base::TimeTicks::Now();
+
   base::PendingTask task1(FROM_HERE, base::OnceClosure());
+  task1.queue_time = now + base::TimeDelta::FromMilliseconds(1);
   base::PendingTask task2(FROM_HERE, base::OnceClosure());
+  task2.queue_time = now + base::TimeDelta::FromMilliseconds(2);
   base::PendingTask task3(FROM_HERE, base::OnceClosure());
+  task3.queue_time = now + base::TimeDelta::FromMilliseconds(3);
 
   watcher_->WillRunTaskOnUIThread(&task1);
   watcher_->WillRunTaskOnUIThread(&task2);
@@ -112,8 +149,74 @@ TEST_F(ResponsivenessWatcherTest, TaskNesting) {
   watcher_->DidRunTaskOnUIThread(&task2);
   watcher_->DidRunTaskOnUIThread(&task1);
 
-  EXPECT_EQ(1, watcher_->NumTasksOnUIThread());
+  ASSERT_EQ(1, watcher_->NumTasksOnUIThread());
+
+  // The innermost task should be the one that is passed through, as it didn't
+  // cause reentrancy.
+  EXPECT_EQ(now + base::TimeDelta::FromMilliseconds(3),
+            watcher_->QueueTimesUIThread()[0]);
   EXPECT_EQ(0, watcher_->NumTasksOnIOThread());
 }
 
+class ResponsivenessWatcherRealIOThreadTest : public testing::Test {
+ public:
+  ResponsivenessWatcherRealIOThreadTest()
+      : test_browser_thread_bundle_(
+            content::TestBrowserThreadBundle::REAL_IO_THREAD) {}
+
+  void SetUp() override {
+    // Watcher's constructor posts a task to IO thread. We need to let those
+    // tasks finish.
+    watcher_ = scoped_refptr<FakeWatcher>(
+        new FakeWatcher(/*register_message_loop_observer=*/true));
+    watcher_->SetUp();
+    test_browser_thread_bundle_.RunIOThreadUntilIdle();
+  }
+
+  void TearDown() override {
+    watcher_->Destroy();
+    watcher_.reset();
+
+    // Destroy a task onto the IO thread, which posts back to the UI thread
+    // to complete destruction.
+    test_browser_thread_bundle_.RunIOThreadUntilIdle();
+    test_browser_thread_bundle_.RunUntilIdle();
+  }
+
+ protected:
+  // This member sets up BrowserThread::IO and BrowserThread::UI. It must be the
+  // first member, as other members may depend on these abstractions.
+  content::TestBrowserThreadBundle test_browser_thread_bundle_;
+
+  scoped_refptr<FakeWatcher> watcher_;
+};
+
+TEST_F(ResponsivenessWatcherRealIOThreadTest, MessageLoopObserver) {
+  // Post a do-nothing task onto the UI thread.
+  content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
+                                   base::BindOnce([]() {}));
+
+  // Post a do-nothing task onto the IO thread.
+  content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE,
+                                   base::BindOnce([]() {}));
+
+  // Post a task onto the IO thread that hops back to the UI thread. This
+  // guarantees that both of the do-nothing tasks have already been processed.
+  base::RunLoop run_loop;
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::BindOnce(
+          [](base::OnceClosure quit_closure) {
+            content::BrowserThread::PostTask(
+                content::BrowserThread::UI, FROM_HERE, std::move(quit_closure));
+          },
+          run_loop.QuitClosure()));
+  run_loop.Run();
+
+  ASSERT_GE(watcher_->NumTasksOnUIThread(), 1);
+  EXPECT_FALSE(watcher_->QueueTimesUIThread()[0].is_null());
+  ASSERT_GE(watcher_->NumTasksOnIOThread(), 1);
+  EXPECT_FALSE(watcher_->QueueTimesIOThread()[0].is_null());
+}
+
 }  // namespace responsiveness