Implement skeleton for responsiveness::Watcher.

This class will eventually be responsible for forwarding task and event metadata
to the Calculator. This class spans the UI and IO threads, so the threading is
tricky. This CL sets up the structure of the class, including all the of
threading details, and reentrancy handling. Future CLs will hook up tasks and
events.

Bug: 859155
Change-Id: I7e8706e82dd07ae54ae9b150a6db7a8113ffdc9b
Reviewed-on: https://chromium-review.googlesource.com/1144226Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577372}

content/browser/BUILD.gn

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

content/browser/scheduler/responsiveness/calculator.h

@@ -25,12 +25,14 @@ class CONTENT_EXPORT Calculator {
   virtual ~Calculator();
 
   // Must be called from the UI thread.
-  void TaskOrEventFinishedOnUIThread(base::TimeTicks schedule_time,
-                                     base::TimeTicks finish_time);
+  // virtual for testing.
+  virtual void TaskOrEventFinishedOnUIThread(base::TimeTicks schedule_time,
+                                             base::TimeTicks finish_time);
 
   // Must be called from the IO thread.
-  void TaskOrEventFinishedOnIOThread(base::TimeTicks schedule_time,
-                                     base::TimeTicks finish_time);
+  // virtual for testing.
+  virtual void TaskOrEventFinishedOnIOThread(base::TimeTicks schedule_time,
+                                             base::TimeTicks finish_time);
 
   // Each janking task/event is fully defined by |start_time| and |end_time|.
   // Note that |duration| = |end_time| - |start_time|.

content/browser/scheduler/responsiveness/calculator_unittest.cc

@@ -13,7 +13,6 @@ namespace {
 // Copied from calculator.cc.
 constexpr int kMeasurementIntervalInMs = 30 * 1000;
 constexpr int kJankThresholdInMs = 100;
-}  // namespace
 
 class FakeCalculator : public Calculator {
  public:
@@ -29,6 +28,8 @@ class FakeCalculator : public Calculator {
   std::vector<int> janky_slices_;
 };
 
+}  // namespace
+
 class ResponsivenessCalculatorTest : public testing::Test {
  public:
   void SetUp() override {

content/browser/scheduler/responsiveness/watcher.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/watcher.h"
+
+#include "base/pending_task.h"
+#include "content/browser/scheduler/responsiveness/calculator.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace responsiveness {
+
+Watcher::Metadata::Metadata(void* identifier) : identifier(identifier) {}
+
+Watcher::Watcher() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+}
+
+void Watcher::SetUp() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  // Destroy() has the corresponding call to Release().
+  // We need this additional reference to make sure the object stays alive
+  // through hops to the IO thread, which are necessary both during construction
+  // and destruction.
+  AddRef();
+
+  calculator_ = MakeCalculator();
+
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::BindOnce(&Watcher::SetUpOnIOThread, this, calculator_.get()));
+}
+
+void Watcher::Destroy() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  DCHECK(!destroy_was_called_);
+  destroy_was_called_ = true;
+
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::BindOnce(&Watcher::TearDownOnIOThread, this));
+}
+
+std::unique_ptr<Calculator> Watcher::MakeCalculator() {
+  return std::make_unique<Calculator>();
+}
+
+Watcher::~Watcher() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(destroy_was_called_);
+}
+
+void Watcher::SetUpOnIOThread(Calculator* calculator) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  // TODO(erikchen): Add MessageLoopObserver to IO thread.
+
+  calculator_io_ = calculator;
+}
+
+void Watcher::TearDownOnIOThread() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  // TODO(erikchen): Remove MessageLoopObserver from IO thread.
+
+  calculator_io_ = nullptr;
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::BindOnce(&Watcher::TearDownOnUIThread, this));
+}
+
+void Watcher::TearDownOnUIThread() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  // Corresponding call to AddRef() is in the constructor.
+  Release();
+}
+
+void Watcher::WillRunTaskOnUIThread(base::PendingTask* task) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  WillRunTask(task, &currently_running_metadata_ui_);
+}
+
+void Watcher::DidRunTaskOnUIThread(base::PendingTask* task) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  // It's safe to use base::Unretained since the callback will be synchronously
+  // invoked.
+  TaskOrEventFinishedCallback callback =
+      base::BindOnce(&Calculator::TaskOrEventFinishedOnUIThread,
+                     base::Unretained(calculator_.get()));
+
+  DidRunTask(task, &currently_running_metadata_ui_,
+             &mismatched_task_identifiers_ui_, std::move(callback));
+}
+
+void Watcher::WillRunTaskOnIOThread(base::PendingTask* task) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  WillRunTask(task, &currently_running_metadata_io_);
+}
+
+void Watcher::DidRunTaskOnIOThread(base::PendingTask* task) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  // It's safe to use base::Unretained since the callback will be synchronously
+  // invoked.
+  TaskOrEventFinishedCallback callback =
+      base::BindOnce(&Calculator::TaskOrEventFinishedOnIOThread,
+                     base::Unretained(calculator_io_));
+  DidRunTask(task, &currently_running_metadata_io_,
+             &mismatched_task_identifiers_io_, std::move(callback));
+}
+
+void Watcher::WillRunTask(base::PendingTask* task,
+                          std::stack<Metadata>* currently_running_metadata) {
+  // Reentrancy should be rare.
+  if (UNLIKELY(!currently_running_metadata->empty())) {
+    currently_running_metadata->top().caused_reentrancy = true;
+  }
+
+  currently_running_metadata->emplace(task);
+
+  // For delayed tasks, record the time right before the task is run.
+  if (!task->delayed_run_time.is_null()) {
+    currently_running_metadata->top().delayed_task_start =
+        base::TimeTicks::Now();
+  }
+}
+
+void Watcher::DidRunTask(base::PendingTask* task,
+                         std::stack<Metadata>* currently_running_metadata,
+                         int* mismatched_task_identifiers,
+                         TaskOrEventFinishedCallback callback) {
+  // Calls to DidRunTask should always be paired with WillRunTask. The only time
+  // 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)) {
+    *mismatched_task_identifiers += 1;
+    DCHECK_LE(*mismatched_task_identifiers, 1);
+    return;
+  }
+
+  bool caused_reentrancy = currently_running_metadata->top().caused_reentrancy;
+  base::TimeTicks delayed_task_start =
+      currently_running_metadata->top().delayed_task_start;
+  currently_running_metadata->pop();
+
+  // Ignore tasks that caused reentrancy, since their execution latency will
+  // be very large, but Chrome was still responsive.
+  if (UNLIKELY(caused_reentrancy))
+    return;
+
+  // 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;
+  } else {
+    schedule_time = delayed_task_start;
+  }
+
+  std::move(callback).Run(schedule_time, base::TimeTicks::Now());
+}
+
+}  // namespace responsiveness

content/browser/scheduler/responsiveness/watcher.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_WATCHER_H_
+#define CONTENT_BROWSER_SCHEDULER_RESPONSIVENESS_WATCHER_H_
+
+#include <memory>
+#include <stack>
+
+#include "base/callback.h"
+#include "base/gtest_prod_util.h"
+#include "base/macros.h"
+#include "base/time/time.h"
+#include "content/common/content_export.h"
+
+namespace base {
+struct PendingTask;
+}  // namespace base
+
+namespace responsiveness {
+
+class Calculator;
+
+// 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
+// emits UMA metrics.
+//
+// This class must only be constructed/destroyed on the UI thread. It has some
+// private members that are affine to the IO thread. It takes care of deleting
+// them appropriately.
+//
+// TODO(erikchen): Once the browser scheduler is implemented, this entire class
+// should become simpler, as either BrowserUIThreadScheduler or
+// BrowserIOThreadScheduler should implement much of the same functionality.
+class CONTENT_EXPORT Watcher : public base::RefCounted<Watcher> {
+ public:
+  Watcher();
+
+  // Must be called immediately after the constructor. This cannot be called
+  // from the constructor because subclasses [for tests] need to be able to
+  // override functions.
+  void SetUp();
+
+  // Destruction requires a thread-hop to the IO thread, so cannot be completed
+  // synchronously. Owners of this class should call this method, and then
+  // release their reference.
+  void Destroy();
+
+ protected:
+  // Exposed for tests.
+  virtual std::unique_ptr<Calculator> MakeCalculator();
+  virtual ~Watcher();
+
+ private:
+  friend class base::RefCounted<Watcher>;
+  FRIEND_TEST_ALL_PREFIXES(ResponsivenessWatcherTest, TaskForwarding);
+  FRIEND_TEST_ALL_PREFIXES(ResponsivenessWatcherTest, TaskNesting);
+
+  // Metadata for currently running tasks and events is needed to track whether
+  // or not they caused reentrancy.
+  struct Metadata {
+    explicit Metadata(void* identifier);
+
+    // An opaque identifier for the task or event.
+    void* identifier = nullptr;
+
+    // Whether the task or event has caused reentrancy.
+    bool caused_reentrancy = false;
+
+    // For delayed tasks, the time at which the event is scheduled to run
+    // is only loosely coupled to the time that the task actually runs. The
+    // difference between these is not interesting for computing responsiveness.
+    // Instead of measuring the duration between |queue_time| and |finish_time|,
+    // we measure the duration of execution itself.
+    base::TimeTicks delayed_task_start;
+  };
+
+  void SetUpOnIOThread(Calculator*);
+  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);
+
+  // 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);
+
+  // Common implementations for the thread-specific methods.
+  void WillRunTask(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,
+                  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_;
+
+  // Metadata for currently running tasks and events on the UI thread.
+  std::stack<Metadata> currently_running_metadata_ui_;
+
+  // Task identifiers should only be mismatched once, since the Watcher
+  // registers itself during a Task execution, and thus doesn't capture the
+  // initial WillRunTask() callback.
+  int mismatched_task_identifiers_ui_ = 0;
+
+  // The following members are all affine to the IO thread.
+  std::stack<Metadata> currently_running_metadata_io_;
+  int mismatched_task_identifiers_io_ = 0;
+
+  // 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
+  // thread. To ensure this, the first task that this class posts onto the IO
+  // thread sets |calculator_io_|. On destruction, this class first tears down
+  // all consumers of |calculator_io_|, and then clears the member and destroys
+  // Calculator.
+  Calculator* calculator_io_ = nullptr;
+
+  bool destroy_was_called_ = false;
+
+  DISALLOW_COPY_AND_ASSIGN(Watcher);
+};
+
+}  // namespace responsiveness
+
+#endif  // CONTENT_BROWSER_SCHEDULER_RESPONSIVENESS_WATCHER_H_

content/browser/scheduler/responsiveness/watcher_unittest.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/watcher.h"
+
+#include "base/location.h"
+#include "base/pending_task.h"
+#include "content/browser/scheduler/responsiveness/calculator.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace responsiveness {
+
+namespace {
+
+class FakeCalculator : public Calculator {
+ public:
+  void TaskOrEventFinishedOnUIThread(base::TimeTicks schedule_time,
+                                     base::TimeTicks finish_time) override {
+    ++tasks_on_ui_thread_;
+  }
+
+  void TaskOrEventFinishedOnIOThread(base::TimeTicks schedule_time,
+                                     base::TimeTicks finish_time) override {
+    ++tasks_on_io_thread_;
+  }
+
+  int NumTasksOnUIThread() { return tasks_on_ui_thread_; }
+  int NumTasksOnIOThread() { return tasks_on_io_thread_; }
+
+ private:
+  int tasks_on_ui_thread_ = 0;
+  int tasks_on_io_thread_ = 0;
+};
+
+class FakeWatcher : public Watcher {
+ public:
+  std::unique_ptr<Calculator> MakeCalculator() override {
+    std::unique_ptr<FakeCalculator> calculator =
+        std::make_unique<FakeCalculator>();
+    calculator_ = calculator.get();
+    return calculator;
+  }
+
+  FakeWatcher() : Watcher() {}
+
+  int NumTasksOnUIThread() { return calculator_->NumTasksOnUIThread(); }
+  int NumTasksOnIOThread() { return calculator_->NumTasksOnIOThread(); }
+
+ private:
+  ~FakeWatcher() override{};
+  FakeCalculator* calculator_ = nullptr;
+};
+
+}  // namespace
+
+class ResponsivenessWatcherTest : public testing::Test {
+ public:
+  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_->SetUp();
+    test_browser_thread_bundle_.RunUntilIdle();
+  }
+
+  void TearDown() override {
+    watcher_->Destroy();
+    watcher_.reset();
+  }
+
+ 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 that tasks are forwarded to calculator.
+TEST_F(ResponsivenessWatcherTest, TaskForwarding) {
+  for (int i = 0; i < 3; ++i) {
+    base::PendingTask task(FROM_HERE, base::OnceClosure());
+    watcher_->WillRunTaskOnUIThread(&task);
+    watcher_->DidRunTaskOnUIThread(&task);
+  }
+  EXPECT_EQ(3, watcher_->NumTasksOnUIThread());
+  EXPECT_EQ(0, watcher_->NumTasksOnIOThread());
+
+  for (int i = 0; i < 4; ++i) {
+    base::PendingTask task(FROM_HERE, base::OnceClosure());
+    watcher_->WillRunTaskOnIOThread(&task);
+    watcher_->DidRunTaskOnIOThread(&task);
+  }
+  EXPECT_EQ(3, watcher_->NumTasksOnUIThread());
+  EXPECT_EQ(4, watcher_->NumTasksOnIOThread());
+}
+
+// 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::PendingTask task1(FROM_HERE, base::OnceClosure());
+  base::PendingTask task2(FROM_HERE, base::OnceClosure());
+  base::PendingTask task3(FROM_HERE, base::OnceClosure());
+
+  watcher_->WillRunTaskOnUIThread(&task1);
+  watcher_->WillRunTaskOnUIThread(&task2);
+  watcher_->WillRunTaskOnUIThread(&task3);
+  watcher_->DidRunTaskOnUIThread(&task3);
+  watcher_->DidRunTaskOnUIThread(&task2);
+  watcher_->DidRunTaskOnUIThread(&task1);
+
+  EXPECT_EQ(1, watcher_->NumTasksOnUIThread());
+  EXPECT_EQ(0, watcher_->NumTasksOnIOThread());
+}
+
+}  // namespace responsiveness

content/test/BUILD.gn

@@ -1510,6 +1510,7 @@ test("content_unittests") {
     "../browser/renderer_host/text_input_client_mac_unittest.mm",
     "../browser/resolve_proxy_msg_helper_unittest.cc",
     "../browser/scheduler/responsiveness/calculator_unittest.cc",
+    "../browser/scheduler/responsiveness/watcher_unittest.cc",
     "../browser/screen_orientation/screen_orientation_provider_unittest.cc",
     "../browser/service_worker/embedded_worker_instance_unittest.cc",
     "../browser/service_worker/service_worker_cache_writer_unittest.cc",