[base] Add a master-toplevel trace event while a ThreadController is active

The trace event will be called "ThreadController active" and be in the
"base" tracing category. It will be activated for each active nested
loop ThreadController is involved in (i.e. all loops but purely native
nested loops -- undetectable).

The implementation uses a stack of active run-levels (replacing some
existing run-levels counters). This is is simpler than the alternatives
as the amount of conditions to enter/exit a run-level are too many to
hope to get manual accounting right:
Enter:
 - DoWork() coming from Run()
 - DoWork() coming from native HandleWorkMessage
 - BeforeDoInternalWork() coming from Run() or native as well
 - DoIdleWork coming from a native nested loop (this one is surprising
   but happens on Mac because the |idle_work_source_| gets invoked
   first when the |work_source_| isn't signaled and a native nested
   loop starts..!)
Exit:
 - DoIdleWork()
 - Quit() (was it already idle or not?)
 - Exit nested loop (was it already idle or not?)
 - Exit native task (was there a native nested loop within that native
   task that we need to "end" now?)

Two tricky cases:
  Nested native task : Native task => native nested loop => native task
 vs
  Subsequent native tasks: Native task A (returns), Native task B

BeforeDoInternalWork() was refactored as a scoped object because we
need to be able to observe when a native task is done to capture
Native Task => Nested native loop => End native task (needs to end
the native loop, we aren't otherwise notified about it). We need to
capture begin/end to be able to differentiate nested native tasks
from subsequent native tasks (e.g. in a nested RunLoop without
application tasks).

The stack-based approach + scoped native work handles all of these
cases across both ThreadController impls.

R=altimin@chromium.org, fdoray@chromium.org

      and regular runtime. Example trace @ goto.google.com/oasrn

Test: Locally with startup trace, nested loops (tab drag-out-of-window),
Bug: 899897, 1074019
Change-Id: I68831ce92a66da8b91a523d931846262182b5de5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2226216
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Oliver Li <olivierli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825082}

base/BUILD.gn

@@ -621,6 +621,7 @@ component("base") {
     "task/sequence_manager/task_time_observer.h",
     "task/sequence_manager/tasks.cc",
     "task/sequence_manager/tasks.h",
+    "task/sequence_manager/thread_controller.cc",
     "task/sequence_manager/thread_controller.h",
     "task/sequence_manager/thread_controller_impl.cc",
     "task/sequence_manager/thread_controller_impl.h",

base/message_loop/message_pump.h

@@ -5,8 +5,11 @@
 #ifndef BASE_MESSAGE_LOOP_MESSAGE_PUMP_H_
 #define BASE_MESSAGE_LOOP_MESSAGE_PUMP_H_
 
+#include <utility>
+
 #include "base/base_export.h"
 #include "base/check_op.h"
+#include "base/compiler_specific.h"
 #include "base/message_loop/message_pump_type.h"
 #include "base/message_loop/timer_slack.h"
 #include "base/sequence_checker.h"
@@ -36,18 +39,6 @@ class BASE_EXPORT MessagePump {
    public:
     virtual ~Delegate() = default;
 
-    // Called before a unit of work internal to the message pump is executed.
-    // This allows reports about individual units of work to be produced. The
-    // unit of work ends when BeforeDoInternalWork() is called again, or when
-    // BeforeWait(), DoWork(), or DoIdleWork() is called.
-    // TODO(crbug.com/851163): Place calls for all platforms.
-    virtual void BeforeDoInternalWork() = 0;
-
-    // Called before the message pump starts waiting for work.
-    // This indicates the end of the current unit of work, which is required
-    // to produce reports about individual units of work.
-    virtual void BeforeWait() = 0;
-
     struct NextWorkInfo {
       // Helper to extract a TimeDelta for pumps that need a
       // timeout-till-next-task.
@@ -85,6 +76,50 @@ class BASE_EXPORT MessagePump {
     // Returns true to indicate that idle work was done. Returning false means
     // the pump will now wait.
     virtual bool DoIdleWork() = 0;
+
+    class ScopedDoNativeWork {
+     public:
+      ~ScopedDoNativeWork() {
+        if (outer_)
+          outer_->OnEndNativeWork();
+      }
+
+      ScopedDoNativeWork(ScopedDoNativeWork&& rhs)
+          : outer_(std::exchange(rhs.outer_, nullptr)) {}
+      ScopedDoNativeWork& operator=(ScopedDoNativeWork&& rhs) {
+        outer_ = std::exchange(rhs.outer_, nullptr);
+        return *this;
+      }
+
+     private:
+      friend Delegate;
+
+      explicit ScopedDoNativeWork(Delegate* outer) : outer_(outer) {
+        outer_->OnBeginNativeWork();
+      }
+
+      Delegate* outer_;
+    };
+
+    // Called before a unit of native work is executed. This allows reports
+    // about individual units of work to be produced. The unit of work ends when
+    // the returned ScopedDoNativeWork goes out of scope.
+    // TODO(crbug.com/851163): Place calls for all platforms. Without this, some
+    // state like the top-level "ThreadController active" trace event will not
+    // be correct when native work is performed.
+    ScopedDoNativeWork BeginNativeWork() WARN_UNUSED_RESULT {
+      return ScopedDoNativeWork(this);
+    }
+
+    // Called before the message pump starts waiting for work. This indicates
+    // that the message pump is idle (out of application work and ideally out of
+    // native work -- if it can tell).
+    virtual void BeforeWait() = 0;
+
+   private:
+    // Called upon entering/exiting a ScopedDoNativeWork.
+    virtual void OnBeginNativeWork() = 0;
+    virtual void OnEndNativeWork() = 0;
   };
 
   MessagePump();

base/message_loop/message_pump_libevent.cc

@@ -213,8 +213,10 @@ void MessagePumpLibevent::Run(Delegate* delegate) {
       break;
 
     // Process native events if any are ready. Do not block waiting for more.
-    delegate->BeforeDoInternalWork();
-    event_base_loop(event_base_, EVLOOP_NONBLOCK);
+    {
+      const auto scoped_do_native_work = delegate->BeginNativeWork();
+      event_base_loop(event_base_, EVLOOP_NONBLOCK);
+    }
 
     bool attempt_more_work = immediate_work_available || processed_io_events_;
     processed_io_events_ = false;

base/message_loop/message_pump_unittest.cc

@@ -19,12 +19,17 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#if defined(OS_WIN)
+#include <windows.h>
+#endif
+
 #if defined(OS_POSIX) && !defined(OS_NACL_SFI)
 #include "base/message_loop/message_pump_libevent.h"
 #endif
 
 using ::testing::_;
 using ::testing::AnyNumber;
+using ::testing::AtMost;
 using ::testing::Invoke;
 using ::testing::Return;
 
@@ -37,11 +42,13 @@ class MockMessagePumpDelegate : public MessagePump::Delegate {
   MockMessagePumpDelegate() = default;
 
   // MessagePump::Delegate:
-  void BeforeDoInternalWork() override {}
   void BeforeWait() override {}
   MOCK_METHOD0(DoWork, MessagePump::Delegate::NextWorkInfo());
   MOCK_METHOD0(DoIdleWork, bool());
 
+  MOCK_METHOD0(OnBeginNativeWork, void(void));
+  MOCK_METHOD0(OnEndNativeWork, void(void));
+
  private:
   DISALLOW_COPY_AND_ASSIGN(MockMessagePumpDelegate);
 };
@@ -51,6 +58,39 @@ class MessagePumpTest : public ::testing::TestWithParam<MessagePumpType> {
   MessagePumpTest() : message_pump_(MessagePump::Create(GetParam())) {}
 
  protected:
+  void AddPreDoWorkExpectations(
+      testing::StrictMock<MockMessagePumpDelegate>& delegate) {
+#if defined(OS_WIN)
+    if (GetParam() == MessagePumpType::UI) {
+      // The Windows MessagePumpForUI may do native work from ::PeekMessage()
+      // and labels itself as such.
+      EXPECT_CALL(delegate, OnBeginNativeWork);
+      EXPECT_CALL(delegate, OnEndNativeWork);
+
+      // If the above event was MessagePumpForUI's own kMsgHaveWork internal
+      // event, it will process another event to replace it (ref.
+      // ProcessPumpReplacementMessage).
+      EXPECT_CALL(delegate, OnBeginNativeWork).Times(AtMost(1));
+      EXPECT_CALL(delegate, OnEndNativeWork).Times(AtMost(1));
+    }
+#endif  // defined(OS_WIN)
+  }
+
+  void AddPostDoWorkExpectations(
+      testing::StrictMock<MockMessagePumpDelegate>& delegate) {
+#if defined(OS_POSIX) && !defined(OS_NACL_SFI)
+    if ((GetParam() == MessagePumpType::UI &&
+         std::is_same<MessagePumpForUI, MessagePumpLibevent>::value) ||
+        (GetParam() == MessagePumpType::IO &&
+         std::is_same<MessagePumpForIO, MessagePumpLibevent>::value)) {
+      // MessagePumpLibEvent checks for native notifications once after
+      // processing a DoWork().
+      EXPECT_CALL(delegate, OnBeginNativeWork);
+      EXPECT_CALL(delegate, OnEndNativeWork);
+    }
+#endif  // defined(OS_POSIX) && !defined(OS_NACL_SFI)
+  }
+
   std::unique_ptr<MessagePump> message_pump_;
 };
 
@@ -60,7 +100,10 @@ TEST_P(MessagePumpTest, QuitStopsWork) {
   testing::InSequence sequence;
   testing::StrictMock<MockMessagePumpDelegate> delegate;
 
-  // Not expecting any calls to DoIdleWork after quitting.
+  AddPreDoWorkExpectations(delegate);
+
+  // Not expecting any calls to DoIdleWork after quitting, nor any of the
+  // PostDoWorkExpectations, quitting should be instantaneous.
   EXPECT_CALL(delegate, DoWork).WillOnce(Invoke([this] {
     message_pump_->Quit();
     return MessagePump::Delegate::NextWorkInfo{TimeTicks::Max()};
@@ -76,6 +119,8 @@ TEST_P(MessagePumpTest, QuitStopsWorkWithNestedRunLoop) {
   testing::StrictMock<MockMessagePumpDelegate> delegate;
   testing::StrictMock<MockMessagePumpDelegate> nested_delegate;
 
+  AddPreDoWorkExpectations(delegate);
+
   // We first schedule a call to DoWork, which runs a nested run loop. After
   // the nested loop exits, we schedule another DoWork which quits the outer
   // (original) run loop. The test verifies that there are no extra calls to
@@ -85,6 +130,8 @@ TEST_P(MessagePumpTest, QuitStopsWorkWithNestedRunLoop) {
     // A null NextWorkInfo indicates immediate follow-up work.
     return MessagePump::Delegate::NextWorkInfo();
   }));
+
+  AddPreDoWorkExpectations(nested_delegate);
   EXPECT_CALL(nested_delegate, DoWork).WillOnce(Invoke([&] {
     // Quit the nested run loop.
     message_pump_->Quit();
@@ -94,11 +141,16 @@ TEST_P(MessagePumpTest, QuitStopsWorkWithNestedRunLoop) {
     return MessagePump::Delegate::NextWorkInfo{TimeTicks::Max()};
   }));
 
+  // PostDoWorkExpectations for the first DoWork.
+  AddPostDoWorkExpectations(delegate);
+
   // The outer pump may or may not trigger idle work at this point.
   // TODO(scheduler-dev): This is unexpected, attempt to remove this legacy
   // allowance.
   EXPECT_CALL(delegate, DoIdleWork()).Times(AnyNumber());
 
+  AddPreDoWorkExpectations(delegate);
+
   EXPECT_CALL(delegate, DoWork).WillOnce(Invoke([this] {
     message_pump_->Quit();
     return MessagePump::Delegate::NextWorkInfo{TimeTicks::Max()};
@@ -124,7 +176,8 @@ class TimerSlackTestDelegate : public MessagePump::Delegate {
     action_.store(NONE);
   }
 
-  void BeforeDoInternalWork() override {}
+  void OnBeginNativeWork() override {}
+  void OnEndNativeWork() override {}
   void BeforeWait() override {}
 
   MessagePump::Delegate::NextWorkInfo DoWork() override {
@@ -197,6 +250,9 @@ TEST_P(MessagePumpTest, TimerSlackWithLongDelays) {
 TEST_P(MessagePumpTest, RunWithoutScheduleWorkInvokesDoWork) {
   testing::InSequence sequence;
   testing::StrictMock<MockMessagePumpDelegate> delegate;
+
+  AddPreDoWorkExpectations(delegate);
+
   EXPECT_CALL(delegate, DoWork).WillOnce(Invoke([this] {
     message_pump_->Quit();
     return MessagePump::Delegate::NextWorkInfo{TimeTicks::Max()};
@@ -210,8 +266,12 @@ TEST_P(MessagePumpTest, RunWithoutScheduleWorkInvokesDoWork) {
 TEST_P(MessagePumpTest, NestedRunWithoutScheduleWorkInvokesDoWork) {
   testing::InSequence sequence;
   testing::StrictMock<MockMessagePumpDelegate> delegate;
+
+  AddPreDoWorkExpectations(delegate);
+
   EXPECT_CALL(delegate, DoWork).WillOnce(Invoke([this] {
     testing::StrictMock<MockMessagePumpDelegate> nested_delegate;
+    AddPreDoWorkExpectations(nested_delegate);
     EXPECT_CALL(nested_delegate, DoWork).WillOnce(Invoke([this] {
       message_pump_->Quit();
       return MessagePump::Delegate::NextWorkInfo{TimeTicks::Max()};

base/message_loop/message_pump_win.cc

@@ -285,21 +285,15 @@ void MessagePumpForUI::WaitForWork(Delegate::NextWorkInfo next_work_info) {
       // MsgWaitForMultipleObjectsEx above when there are no messages for the
       // current thread.
 
+      // As in ProcessNextWindowsMessage().
+      const auto scoped_do_native_work = state_->delegate->BeginNativeWork();
       {
-        // Trace as in ProcessNextWindowsMessage().
         TRACE_EVENT0("base", "MessagePumpForUI::WaitForWork GetQueueStatus");
         if (HIWORD(::GetQueueStatus(QS_SENDMESSAGE)) & QS_SENDMESSAGE)
           return;
       }
-
       {
-        // ::GetQueueStatus() above may racily miss a sent-message and
-        // ::PeekMessage() below may thus process one and/or internal events per
-        // its doc.
-        state_->delegate->BeforeDoInternalWork();
-
         MSG msg;
-        // Trace as in ProcessNextWindowsMessage().
         TRACE_EVENT0("base", "MessagePumpForUI::WaitForWork PeekMessage");
         if (::PeekMessage(&msg, nullptr, 0, 0, PM_NOREMOVE))
           return;
@@ -457,45 +451,54 @@ void MessagePumpForUI::KillNativeTimer() {
 bool MessagePumpForUI::ProcessNextWindowsMessage() {
   DCHECK_CALLED_ON_VALID_THREAD(bound_thread_);
 
-  // If there are sent messages in the queue then PeekMessage internally
-  // dispatches the message and returns false. We return true in this
-  // case to ensure that the message loop peeks again instead of calling
-  // MsgWaitForMultipleObjectsEx.
-  bool more_work_is_plausible = false;
-  {
-    // Individually trace ::GetQueueStatus and ::PeekMessage because sampling
-    // profiler is hinting that we're spending a surprising amount of time with
-    // these on top of the stack. Tracing will be able to tell us whether this
-    // is a bias of sampling profiler (e.g. kernel takes ::GetQueueStatus as an
-    // opportunity to swap threads and is more likely to schedule the sampling
-    // profiler's thread while the sampled thread is swapped out on this frame).
-    TRACE_EVENT0("base",
-                 "MessagePumpForUI::ProcessNextWindowsMessage GetQueueStatus");
-    DWORD queue_status = ::GetQueueStatus(QS_SENDMESSAGE);
-    if (HIWORD(queue_status) & QS_SENDMESSAGE)
-      more_work_is_plausible = true;
-  }
-
   MSG msg;
   bool has_msg = false;
+  bool more_work_is_plausible = false;
   {
-    // ::PeekMessage() may process sent messages (regardless of |had_messages|
-    // as ::GetQueueStatus() is an optimistic check that may racily have missed
-    // an incoming event -- it doesn't hurt to have empty internal units of work
-    // when ::PeekMessage turns out to be a no-op).
-    state_->delegate->BeforeDoInternalWork();
-
-    // PeekMessage can run a message if there are sent messages, trace that and
-    // emit the boolean param to see if it ever janks independently (ref.
-    // comment on GetQueueStatus).
-    TRACE_EVENT(
-        "base", "MessagePumpForUI::ProcessNextWindowsMessage PeekMessage",
-        [&](perfetto::EventContext ctx) {
-          perfetto::protos::pbzero::ChromeMessagePump* msg_pump_data =
-              ctx.event()->set_chrome_message_pump();
-          msg_pump_data->set_sent_messages_in_queue(more_work_is_plausible);
-        });
-    has_msg = ::PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE) != FALSE;
+    // ::PeekMessage() may process sent and/or internal messages (regardless of
+    // |had_messages| as ::GetQueueStatus() is an optimistic check that may
+    // racily have missed an incoming event -- it doesn't hurt to have empty
+    // internal units of work when ::PeekMessage turns out to be a no-op).
+    // Instantiate |scoped_do_native_work| ahead of GetQueueStatus() so that
+    // trace events it emits fully outscope GetQueueStatus' events
+    // (GetQueueStatus() itself not being expected to do work; it's fine to use
+    // only on ScopedDoNativeWork for both calls -- we trace them independently
+    // just in case internal work stalls).
+    const auto scoped_do_native_work = state_->delegate->BeginNativeWork();
+
+    {
+      // Individually trace ::GetQueueStatus and ::PeekMessage because sampling
+      // profiler is hinting that we're spending a surprising amount of time
+      // with these on top of the stack. Tracing will be able to tell us whether
+      // this is a bias of sampling profiler (e.g. kernel takes ::GetQueueStatus
+      // as an opportunity to swap threads and is more likely to schedule the
+      // sampling profiler's thread while the sampled thread is swapped out on
+      // this frame).
+      TRACE_EVENT0(
+          "base", "MessagePumpForUI::ProcessNextWindowsMessage GetQueueStatus");
+      DWORD queue_status = ::GetQueueStatus(QS_SENDMESSAGE);
+
+      // If there are sent messages in the queue then PeekMessage internally
+      // dispatches the message and returns false. We return true in this case
+      // to ensure that the message loop peeks again instead of calling
+      // MsgWaitForMultipleObjectsEx.
+      if (HIWORD(queue_status) & QS_SENDMESSAGE)
+        more_work_is_plausible = true;
+    }
+
+    {
+      // PeekMessage can run a message if there are sent messages, trace that
+      // and emit the boolean param to see if it ever janks independently (ref.
+      // comment on GetQueueStatus).
+      TRACE_EVENT(
+          "base", "MessagePumpForUI::ProcessNextWindowsMessage PeekMessage",
+          [&](perfetto::EventContext ctx) {
+            perfetto::protos::pbzero::ChromeMessagePump* msg_pump_data =
+                ctx.event()->set_chrome_message_pump();
+            msg_pump_data->set_sent_messages_in_queue(more_work_is_plausible);
+          });
+      has_msg = ::PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE) != FALSE;
+    }
   }
   if (has_msg)
     more_work_is_plausible |= ProcessMessageHelper(msg);
@@ -526,7 +529,7 @@ bool MessagePumpForUI::ProcessMessageHelper(const MSG& msg) {
   if (msg.message == kMsgHaveWork && msg.hwnd == message_window_.hwnd())
     return ProcessPumpReplacementMessage();
 
-  state_->delegate->BeforeDoInternalWork();
+  const auto scoped_do_native_work = state_->delegate->BeginNativeWork();
 
   for (Observer& observer : observers_)
     observer.WillDispatchMSG(msg);
@@ -553,8 +556,8 @@ bool MessagePumpForUI::ProcessPumpReplacementMessage() {
   MSG msg;
   bool have_message = false;
   {
-    // Bump the work id since ::PeekMessage may process internal events.
-    state_->delegate->BeforeDoInternalWork();
+    // ::PeekMessage may process internal events. Consider it native work.
+    const auto scoped_do_native_work = state_->delegate->BeginNativeWork();
 
     TRACE_EVENT0("base",
                  "MessagePumpForUI::ProcessPumpReplacementMessage PeekMessage");
@@ -609,14 +612,14 @@ bool MessagePumpForUI::ProcessPumpReplacementMessage() {
              msg.wParam == reinterpret_cast<UINT_PTR>(this)) {
     // This happens when a native nested loop invokes HandleWorkMessage() =>
     // ProcessPumpReplacementMessage() which finds the WM_TIMER message
-    // installed by ScheduleNativeTimer(). That message needs to be handle
+    // installed by ScheduleNativeTimer(). That message needs to be handled
     // directly as handing it off to ProcessMessageHelper() below would cause an
-    // unnecessary BeforeDoInternalWork() which may incorrectly lead the
-    // Delegate's heuristics to conclude that the DoWork() in
-    // HandleTimerMessage() is nested inside a native task. It's also safe to
-    // skip the below ScheduleWork() as it is not mandatory before invoking
-    // DoWork() and HandleTimerMessage() handles re-installing the necessary
-    // followup messages.
+    // unnecessary ScopedDoNativeWork which may incorrectly lead the Delegate's
+    // heuristics to conclude that the DoWork() in HandleTimerMessage() is
+    // nested inside a native task. It's also safe to skip the below
+    // ScheduleWork() as it is not mandatory before invoking DoWork() and
+    // HandleTimerMessage() handles re-installing the necessary followup
+    // messages.
     HandleTimerMessage();
     return true;
   }

base/task/sequence_manager/test/mock_time_message_pump_unittest.cc

@@ -22,7 +22,8 @@ using ::testing::StrictMock;
 
 class MockMessagePumpDelegate : public MessagePump::Delegate {
  public:
-  MOCK_METHOD0(BeforeDoInternalWork, void());
+  MOCK_METHOD0(OnBeginNativeWork, void());
+  MOCK_METHOD0(OnEndNativeWork, void());
   MOCK_METHOD0(BeforeWait, void());
   MOCK_METHOD0(DoWork, NextWorkInfo());
   MOCK_METHOD0(DoIdleWork, bool());

base/task/sequence_manager/thread_controller.cc

+// Copyright 2020 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 "base/task/sequence_manager/thread_controller.h"
+
+#include "base/check.h"
+#include "base/trace_event/base_tracing.h"
+
+namespace base {
+namespace sequence_manager {
+namespace internal {
+
+ThreadController::RunLevelTracker::RunLevelTracker() = default;
+ThreadController::RunLevelTracker::~RunLevelTracker() {
+  // There shouldn't be any remaining |run_levels_| by the time this unwinds.
+  DCHECK(run_levels_.empty());
+}
+
+void ThreadController::RunLevelTracker::OnRunLoopStarted(State initial_state) {
+  run_levels_.emplace(initial_state);
+}
+
+void ThreadController::RunLevelTracker::OnRunLoopEnded() {
+  // Normally this will occur while kIdle or kSelectingNextTask but it can also
+  // occur while kRunningTask in rare situations where the owning
+  // ThreadController is deleted from within a task. Ref.
+  // SequenceManagerWithTaskRunnerTest.DeleteSequenceManagerInsideATask. Thus we
+  // can't assert anything about the current state other than that it must be
+  // exiting an existing RunLevel.
+  DCHECK(!run_levels_.empty());
+  run_levels_.pop();
+}
+
+void ThreadController::RunLevelTracker::OnTaskStarted() {
+  // Ignore tasks outside the main run loop.
+  // The only practical case where this would happen is if a native loop is spun
+  // outside the main runloop (e.g. system dialog during startup). We cannot
+  // support this because we are not guaranteed to be able to observe its exit
+  // (like we would inside an application task which is at least guaranteed to
+  // itself notify us when it ends).
+  if (run_levels_.empty())
+    return;
+
+  // Already running a task?
+  if (run_levels_.top().state() == kRunningTask) {
+    // #task-in-task-implies-nested
+    run_levels_.emplace(kRunningTask);
+  } else {
+    // Simply going from kIdle or kSelectingNextTask to kRunningTask.
+    run_levels_.top().UpdateState(kRunningTask);
+  }
+}
+
+void ThreadController::RunLevelTracker::OnTaskEnded() {
+  if (run_levels_.empty())
+    return;
+
+  // #done-task-while-not-running-implies-done-nested
+  if (run_levels_.top().state() != kRunningTask)
+    run_levels_.pop();
+
+  // Whether we exited a nested run-level or not: the current run-level is now
+  // transitioning from kRunningTask to kSelectingNextTask.
+  DCHECK_EQ(run_levels_.top().state(), kRunningTask);
+  run_levels_.top().UpdateState(kSelectingNextTask);
+}
+
+void ThreadController::RunLevelTracker::OnIdle() {
+  if (run_levels_.empty())
+    return;
+
+  // This is similar to the logic in OnTaskStarted().
+  if (run_levels_.top().state() == kRunningTask) {
+    // #task-in-task-implies-nested
+    // While OnIdle() isn't typically thought of as a "task" it is a way to "do
+    // work" and, on platforms like Mac which uses an |idle_work_source_|,
+    // DoIdleWork() can be invoked without DoWork() being first invoked at this
+    // run-level. We need to create a nested kIdle RunLevel or we break
+    // #done-task-while-not-running-implies-done-nested.
+    run_levels_.emplace(kIdle);
+  } else {
+    // Simply going kIdle at the current run-level.
+    run_levels_.top().UpdateState(kIdle);
+  }
+}
+
+// static
+void ThreadController::RunLevelTracker::SetTraceObserverForTesting(
+    TraceObserverForTesting* trace_observer_for_testing) {
+  DCHECK_NE(!!trace_observer_for_testing_, !!trace_observer_for_testing);
+  trace_observer_for_testing_ = trace_observer_for_testing;
+}
+
+// static
+ThreadController::RunLevelTracker::TraceObserverForTesting*
+    ThreadController::RunLevelTracker::trace_observer_for_testing_ = nullptr;
+
+ThreadController::RunLevelTracker::RunLevel::RunLevel(State initial_state) {
+  UpdateState(initial_state);
+}
+
+ThreadController::RunLevelTracker::RunLevel::~RunLevel() {
+  UpdateState(kIdle);
+}
+
+ThreadController::RunLevelTracker::RunLevel::RunLevel(RunLevel&& other)
+    : state_(std::exchange(other.state_, kIdle)) {}
+ThreadController::RunLevelTracker::RunLevel&
+ThreadController::RunLevelTracker::RunLevel::operator=(RunLevel&& other) {
+  state_ = std::exchange(other.state_, kIdle);
+  return *this;
+}
+
+void ThreadController::RunLevelTracker::RunLevel::UpdateState(State new_state) {
+  // The only state that can be redeclared is idle, anything else should be a
+  // transition.
+  DCHECK(state_ != new_state || new_state == kIdle)
+      << state_ << "," << new_state;
+
+  const bool was_active = state_ != kIdle;
+  const bool is_active = new_state != kIdle;
+
+  state_ = new_state;
+  if (was_active == is_active)
+    return;
+
+  // Change of state.
+  if (is_active)
+    TRACE_EVENT_BEGIN0("base", "ThreadController active");
+  else
+    TRACE_EVENT_END0("base", "ThreadController active");
+
+  if (trace_observer_for_testing_) {
+    if (is_active)
+      trace_observer_for_testing_->OnThreadControllerActiveBegin();
+    else
+      trace_observer_for_testing_->OnThreadControllerActiveEnd();
+  }
+}
+
+}  // namespace internal
+}  // namespace sequence_manager
+}  // namespace base

base/task/sequence_manager/thread_controller.h

@@ -5,6 +5,10 @@
 #ifndef BASE_TASK_SEQUENCE_MANAGER_THREAD_CONTROLLER_H_
 #define BASE_TASK_SEQUENCE_MANAGER_THREAD_CONTROLLER_H_
 
+#include <stack>
+#include <vector>
+
+#include "base/base_export.h"
 #include "base/message_loop/message_pump.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
@@ -119,6 +123,109 @@ class ThreadController {
   virtual void RemoveNestingObserver(RunLoop::NestingObserver* observer) = 0;
   virtual const scoped_refptr<AssociatedThreadId>& GetAssociatedThread()
       const = 0;
+
+ protected:
+  // Tracks the state of each run-level (main and nested ones) in its associated
+  // ThreadController. It does so using two high-level principles:
+  //  1) #task-in-task-implies-nested :
+  //     If the |state_| is kRunningTask and another task starts
+  //     (OnTaskStarted()), it implies this inner-task is running from a nested
+  //     loop and another RunLevel is pushed onto |run_levels_|.
+  //  2) #done-task-while-not-running-implies-done-nested
+  //     If the current task completes (OnTaskEnded()) and |state_| is not
+  //     kRunningTask, the top RunLevel was an (already exited) nested loop and
+  //     will be popped off |run_levels_|.
+  // We need this logic because native nested loops can run from any task
+  // without a RunLoop being involved, see
+  // ThreadControllerWithMessagePumpTest.ThreadControllerActive* tests for
+  // examples. Using these two heuristics is the simplest way, trying to capture
+  // all the ways in which native+application tasks can nest is harder than
+  // reacting as it happens.
+  //
+  // Note 1: "native tasks" are only captured if the MessagePump is
+  // instrumented to see them and shares them with ThreadController (via
+  // MessagePump::Delegate::OnBeginNativeWork). As such it is still possible to
+  // view trace events emanating from native tasks without "ThreadController
+  // active" being active.
+  // Note 2: Non-instrumented native tasks do not break the two high-level
+  // principles above because:
+  //  A) If a non-instrumented task enters a nested loop, either:
+  //     i) No instrumented tasks run within the loop so it's invisible.
+  //     ii) Instrumented tasks run *and* current state is kRunningTask ((A) is
+  //         a task within an instrumented task):
+  //         #task-in-task-implies-nested triggers and the nested loop is
+  //         visible.
+  //     iii) Instrumented tasks run *and* current state is kIdle or
+  //          kSelectingNextTask ((A) is a task run by a native loop):
+  //          #task-in-task-implies-nested doesn't trigger and tasks (iii) look
+  //          like a non-nested continuation of tasks at the current RunLevel.
+  //  B) When task (A) exits its nested loop and completes, either:
+  //     i) The loop was invisible so no RunLevel was created for it and
+  //        #done-task-while-not-running-implies-done-nested doesn't trigger so
+  //        it balances out.
+  //     ii) Instrumented tasks did run in which case |state_| is
+  //         kSelectingNextTask or kIdle. When the task in which (A) runs
+  //         completes #done-task-while-not-running-implies-done-nested triggers
+  //         and everything balances out.
+  //     iii) Same as (ii) but we're back to kSelectingNextTask or kIdle as
+  //          before and (A) was a no-op on the RunLevels.
+  class BASE_EXPORT RunLevelTracker {
+   public:
+    enum State {
+      // Waiting for work.
+      kIdle,
+      // Between two tasks but not idle.
+      kSelectingNextTask,
+      // Running and currently processing a unit of work.
+      kRunningTask,
+    };
+
+    RunLevelTracker();
+    ~RunLevelTracker();
+
+    void OnRunLoopStarted(State initial_state);
+    void OnRunLoopEnded();
+    void OnTaskStarted();
+    void OnTaskEnded();
+    void OnIdle();
+
+    size_t num_run_levels() const { return run_levels_.size(); }
+
+    // Observers changes of state sent as trace-events so they can be tested.
+    class TraceObserverForTesting {
+     public:
+      virtual ~TraceObserverForTesting() = default;
+
+      virtual void OnThreadControllerActiveBegin() = 0;
+      virtual void OnThreadControllerActiveEnd() = 0;
+    };
+
+    static void SetTraceObserverForTesting(
+        TraceObserverForTesting* trace_observer_for_testing);
+
+   private:
+    class RunLevel {
+     public:
+      explicit RunLevel(State initial_state);
+      ~RunLevel();
+
+      // Moveable for STL compat. Marks |other| as idle so it noops on
+      // destruction after handing off its responsibility.
+      RunLevel(RunLevel&& other);
+      RunLevel& operator=(RunLevel&& other);
+
+      void UpdateState(State new_state);
+
+      State state() const { return state_; }
+
+     private:
+      State state_ = kIdle;
+    };
+
+    std::stack<RunLevel, std::vector<RunLevel>> run_levels_;
+
+    static TraceObserverForTesting* trace_observer_for_testing_;
+  };
 };
 
 }  // namespace internal

base/task/sequence_manager/thread_controller_impl.cc

@@ -41,9 +41,19 @@ ThreadControllerImpl::ThreadControllerImpl(
   delayed_do_work_closure_ =
       BindRepeating(&ThreadControllerImpl::DoWork, weak_factory_.GetWeakPtr(),
                     WorkType::kDelayed);
+
+  // Unlike ThreadControllerWithMessagePumpImpl, ThreadControllerImpl isn't
+  // explicitly Run(). Rather, DoWork() will be invoked at some point in the
+  // future when the associated thread begins pumping messages.
+  main_sequence_only().run_level_tracker.OnRunLoopStarted(
+      RunLevelTracker::kIdle);
 }
 
-ThreadControllerImpl::~ThreadControllerImpl() = default;
+ThreadControllerImpl::~ThreadControllerImpl() {
+  // Balances OnRunLoopStarted() in the constructor to satisfy the exit criteria
+  // of ~RunLevelTracker().
+  main_sequence_only().run_level_tracker.OnRunLoopEnded();
+}
 
 ThreadControllerImpl::MainSequenceOnly::MainSequenceOnly() = default;
 
@@ -78,8 +88,9 @@ void ThreadControllerImpl::ScheduleWork() {
                "ThreadControllerImpl::ScheduleWork::PostTask");
 
   if (work_deduplicator_.OnWorkRequested() ==
-      ShouldScheduleWork::kScheduleImmediate)
+      ShouldScheduleWork::kScheduleImmediate) {
     task_runner_->PostTask(FROM_HERE, immediate_do_work_closure_);
+  }
 }
 
 void ThreadControllerImpl::SetNextDelayedDoWork(LazyNow* lazy_now,
@@ -181,26 +192,28 @@ void ThreadControllerImpl::DoWork(WorkType work_type) {
       // Trace events should finish before we call DidRunTask to ensure that
       // SequenceManager trace events do not interfere with them.
       TRACE_TASK_EXECUTION("ThreadControllerImpl::RunTask", *task);
+      DCHECK_GT(main_sequence_only().run_level_tracker.num_run_levels(), 0U);
+      main_sequence_only().run_level_tracker.OnTaskStarted();
       task_annotator_.RunTask("SequenceManager RunTask", task);
+      if (!weak_ptr)
+        return;
+      main_sequence_only().run_level_tracker.OnTaskEnded();
     }
 
-    if (!weak_ptr)
-      return;
-
     sequence_->DidRunTask();
 
     // NOTE: https://crbug.com/828835.
     // When we're running inside a nested RunLoop it may quit anytime, so any
     // outstanding pending tasks must run in the outer RunLoop
     // (see SequenceManagerTestWithMessageLoop.QuitWhileNested test).
-    // Unfortunately, it's MessageLoop who's receving that signal and we can't
+    // Unfortunately, it's MessageLoop who's receiving that signal and we can't
     // know it before we return from DoWork, hence, OnExitNestedRunLoop
     // will be called later. Since we must implement ThreadController and
     // SequenceManager in conformance with MessageLoop task runners, we need
     // to disable this batching optimization while nested.
     // Implementing MessagePump::Delegate ourselves will help to resolve this
     // issue.
-    if (main_sequence_only().nesting_depth > 0)
+    if (main_sequence_only().run_level_tracker.num_run_levels() > 1)
       break;
   }
 
@@ -230,14 +243,17 @@ void ThreadControllerImpl::DoWork(WorkType work_type) {
     return;
   }
 
-  // Check if there's no future work.
+  // No more immediate work.
+  main_sequence_only().run_level_tracker.OnIdle();
+
+  // Any future work?
   if (delay_till_next_task == TimeDelta::Max()) {
     main_sequence_only().next_delayed_do_work = TimeTicks::Max();
     cancelable_delayed_do_work_closure_.Cancel();
     return;
   }
 
-  // Check if we've already requested the required delay.
+  // Already requested next delay?
   TimeTicks next_task_at = lazy_now.Now() + delay_till_next_task;
   if (next_task_at == main_sequence_only().next_delayed_do_work)
     return;
@@ -272,7 +288,8 @@ ThreadControllerImpl::GetAssociatedThread() const {
 }
 
 void ThreadControllerImpl::OnBeginNestedRunLoop() {
-  main_sequence_only().nesting_depth++;
+  main_sequence_only().run_level_tracker.OnRunLoopStarted(
+      RunLevelTracker::kSelectingNextTask);
 
   // Just assume we have a pending task and post a DoWork to make sure we don't
   // grind to a halt while nested.
@@ -284,9 +301,9 @@ void ThreadControllerImpl::OnBeginNestedRunLoop() {
 }
 
 void ThreadControllerImpl::OnExitNestedRunLoop() {
-  main_sequence_only().nesting_depth--;
   if (nesting_observer_)
     nesting_observer_->OnExitNestedRunLoop();
+  main_sequence_only().run_level_tracker.OnRunLoopEnded();
 }
 
 void ThreadControllerImpl::SetWorkBatchSize(int work_batch_size) {

base/task/sequence_manager/thread_controller_impl.h

@@ -96,10 +96,12 @@ class BASE_EXPORT ThreadControllerImpl : public ThreadController,
     MainSequenceOnly();
     ~MainSequenceOnly();
 
-    int nesting_depth = 0;
     int work_batch_size_ = 1;
 
     TimeTicks next_delayed_do_work = TimeTicks::Max();
+
+    // Tracks the number and state of each run-level managed by this instance.
+    RunLevelTracker run_level_tracker;
   };
 
   scoped_refptr<AssociatedThreadId> associated_thread_;

base/task/sequence_manager/thread_controller_with_message_pump_impl.cc

@@ -188,7 +188,9 @@ void ThreadControllerWithMessagePumpImpl::MaybeStartHangWatchScopeEnabled() {
   // Nested runloops are covered by the parent loop hang watch scope.
   // TODO(crbug/1034046): Provide more granular scoping that reuses the parent
   // scope deadline.
-  if (main_thread_only().runloop_count == 1 && base::HangWatcher::IsEnabled()) {
+  // TODO(crbug/1034046): Also track native work outside a top-level RunLoop.
+  if (main_thread_only().run_level_tracker.num_run_levels() == 1 &&
+      base::HangWatcher::IsEnabled()) {
     hang_watch_scope_.emplace(
         base::HangWatchScopeEnabled::kDefaultHangWatchTime);
   }
@@ -225,22 +227,36 @@ ThreadControllerWithMessagePumpImpl::GetAssociatedThread() const {
   return associated_thread_;
 }
 
-void ThreadControllerWithMessagePumpImpl::BeforeDoInternalWork() {
+void ThreadControllerWithMessagePumpImpl::OnBeginNativeWork() {
   MaybeStartHangWatchScopeEnabled();
   work_id_provider_->IncrementWorkId();
+  main_thread_only().run_level_tracker.OnTaskStarted();
 }
 
-void ThreadControllerWithMessagePumpImpl::BeforeWait() {
+void ThreadControllerWithMessagePumpImpl::OnEndNativeWork() {
+  work_id_provider_->IncrementWorkId();
+
   // Nested runloops are covered by the parent loop hang watch scope.
   // TODO(crbug/1034046): Provide more granular scoping that reuses the parent
   // scope deadline.
-  if (main_thread_only().runloop_count == 1) {
-    // Waiting for work cannot be covered by a hang watch scope because that
-    // means the thread can be idle for unbounded time.
+  if (main_thread_only().run_level_tracker.num_run_levels() == 1)
     hang_watch_scope_.reset();
-  }
 
+  main_thread_only().run_level_tracker.OnTaskEnded();
+}
+
+void ThreadControllerWithMessagePumpImpl::BeforeWait() {
   work_id_provider_->IncrementWorkId();
+
+  // Nested runloops are covered by the parent loop hang watch scope.
+  // TODO(crbug/1034046): Provide more granular scoping that reuses the parent
+  // scope deadline.
+  // TODO(crbug/1034046): There should never be an outstanding
+  // |hang_watch_scope_| here, DCHECK?
+  if (main_thread_only().run_level_tracker.num_run_levels() == 1)
+    hang_watch_scope_.reset();
+
+  main_thread_only().run_level_tracker.OnIdle();
 }
 
 MessagePump::Delegate::NextWorkInfo
@@ -296,6 +312,9 @@ TimeDelta ThreadControllerWithMessagePumpImpl::DoWorkImpl(
                "ThreadControllerImpl::DoWork");
 
   if (!main_thread_only().task_execution_allowed) {
+    // Broadcast in a trace event that application tasks were disallowed. This
+    // helps spot nested loops that intentionally starve application tasks.
+    TRACE_EVENT0("base", "ThreadController: application tasks disallowed");
     if (main_thread_only().quit_runloop_after == TimeTicks::Max())
       return TimeDelta::Max();
     return main_thread_only().quit_runloop_after - continuation_lazy_now->Now();
@@ -329,7 +348,9 @@ TimeDelta ThreadControllerWithMessagePumpImpl::DoWorkImpl(
       // Trace events should finish before we call DidRunTask to ensure that
       // SequenceManager trace events do not interfere with them.
       TRACE_TASK_EXECUTION("ThreadControllerImpl::RunTask", *task);
+      main_thread_only().run_level_tracker.OnTaskStarted();
       task_annotator_.RunTask("SequenceManager RunTask", task);
+      main_thread_only().run_level_tracker.OnTaskEnded();
     }
 
 #if DCHECK_IS_ON()
@@ -401,6 +422,8 @@ bool ThreadControllerWithMessagePumpImpl::DoIdleWork() {
     return false;
   }
 
+  main_thread_only().run_level_tracker.OnIdle();
+
   // Check if any runloop timeout has expired.
   if (main_thread_only().quit_runloop_after != TimeTicks::Max() &&
       main_thread_only().quit_runloop_after <= time_source_->NowTicks()) {
@@ -431,11 +454,13 @@ void ThreadControllerWithMessagePumpImpl::Run(bool application_tasks_allowed,
   AutoReset<bool> quit_when_idle_requested(&quit_when_idle_requested_, false);
 #endif
 
+  main_thread_only().run_level_tracker.OnRunLoopStarted(
+      RunLevelTracker::kSelectingNextTask);
+
   // Quit may have been called outside of a Run(), so |quit_pending| might be
   // true here. We can't use InTopLevelDoWork() in Quit() as this call may be
   // outside top-level DoWork but still in Run().
   main_thread_only().quit_pending = false;
-  main_thread_only().runloop_count++;
   if (application_tasks_allowed && !main_thread_only().task_execution_allowed) {
     // Allow nested task execution as explicitly requested.
     DCHECK(RunLoop::IsNestedOnCurrentThread());
@@ -451,12 +476,13 @@ void ThreadControllerWithMessagePumpImpl::Run(bool application_tasks_allowed,
     DVLOG(1) << "ThreadControllerWithMessagePumpImpl::Quit";
 #endif
 
-  main_thread_only().runloop_count--;
+  main_thread_only().run_level_tracker.OnRunLoopEnded();
   main_thread_only().quit_pending = false;
 
   // Reset the hang watch scope upon exiting the outermost loop since the
-  // execution it covers is now completely over.
-  if (main_thread_only().runloop_count == 0)
+  // execution it covers is now completely over. TODO(crbug/1034046): There
+  // should never be an outstanding |hang_watch_scope_| here, DCHECK?
+  if (main_thread_only().run_level_tracker.num_run_levels() == 0)
     hang_watch_scope_.reset();
 }
 
@@ -484,8 +510,9 @@ void ThreadControllerWithMessagePumpImpl::Quit() {
 
 void ThreadControllerWithMessagePumpImpl::EnsureWorkScheduled() {
   if (work_deduplicator_.OnWorkRequested() ==
-      ShouldScheduleWork::kScheduleImmediate)
+      ShouldScheduleWork::kScheduleImmediate) {
     pump_->ScheduleWork();
+  }
 }
 
 void ThreadControllerWithMessagePumpImpl::SetTaskExecutionAllowed(
@@ -528,7 +555,7 @@ void ThreadControllerWithMessagePumpImpl::AttachToMessagePump() {
 #endif
 
 bool ThreadControllerWithMessagePumpImpl::ShouldQuitRunLoopWhenIdle() {
-  if (main_thread_only().runloop_count == 0)
+  if (main_thread_only().run_level_tracker.num_run_levels() == 0)
     return false;
   // It's only safe to call ShouldQuitWhenIdle() when in a RunLoop.
   return ShouldQuitWhenIdle();

base/task/sequence_manager/thread_controller_with_message_pump_impl.h

@@ -89,7 +89,8 @@ class BASE_EXPORT ThreadControllerWithMessagePumpImpl
       const SequenceManager::Settings& settings);
 
   // MessagePump::Delegate implementation.
-  void BeforeDoInternalWork() override;
+  void OnBeginNativeWork() override;
+  void OnEndNativeWork() override;
   void BeforeWait() override;
   MessagePump::Delegate::NextWorkInfo DoWork() override;
   bool DoIdleWork() override;
@@ -117,7 +118,8 @@ class BASE_EXPORT ThreadControllerWithMessagePumpImpl
     // Number of tasks processed in a single DoWork invocation.
     int work_batch_size = 1;
 
-    int runloop_count = 0;
+    // Tracks the number and state of each run-level managed by this instance.
+    RunLevelTracker run_level_tracker;
 
     // When the next scheduled delayed work should run, if any.
     TimeTicks next_delayed_do_work = TimeTicks::Max();