Reland "[base] Migrate the Mac/iOS MessagePump to DoSomeWork"

This is a reland of de5ec163

Reason for revert: rendering.desktop/frame_times regressions
Fix: SetWorkBatchSize(2) in gpu_main to keep previous behavior.

Original change's description:
> [base] Migrate the Mac/iOS MessagePump to DoSomeWork
>
> Recycling https://chromium-review.googlesource.com/c/chromium/src/+/1470938/13
> This CL also fixes DetachToBrowserTabDragControllerTest.PressEscapeWhileDetached
> to make CQ happy.
>
> Bug: 885371
> Change-Id: I0b54ade1fd3481a9248c89ec2993738b20fce002
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1906337
> Reviewed-by: Allen Bauer <kylixrd@chromium.org>
> Reviewed-by: Mark Mentovai <mark@chromium.org>
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#722094}

Bug: 885371, 1032188
Change-Id: Id1d6916a83e1c72cacc554c100d60fd0c78c6b7b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1992161
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Allen Bauer <kylixrd@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732010}

base/message_loop/message_pump_mac.h

@@ -155,6 +155,10 @@ class BASE_EXPORT MessagePumpCFRunLoopBase : public MessagePump {
   // The maximum number of run loop modes that can be monitored.
   static constexpr int kNumModes = 4;
 
+  // All sources of delayed work scheduling converge to this, using TimeDelta
+  // avoids querying Now() for key callers.
+  void ScheduleDelayedWorkImpl(TimeDelta delta);
+
   // Marking timers as invalid at the right time helps significantly reduce
   // power use (see the comment in RunDelayedWorkTimer()), however there is no
   // public API for doing so. CFRuntime.h states that CFRuntimeBase, upon which
@@ -175,33 +179,32 @@ class BASE_EXPORT MessagePumpCFRunLoopBase : public MessagePump {
   void SetDelayedWorkTimerValid(bool valid);
 
   // Timer callback scheduled by ScheduleDelayedWork.  This does not do any
-  // work, but it signals work_source_ so that delayed work can be performed
+  // work, but it signals |work_source_| so that delayed work can be performed
   // within the appropriate priority constraints.
   static void RunDelayedWorkTimer(CFRunLoopTimerRef timer, void* info);
 
-  // Perform highest-priority work.  This is associated with work_source_
+  // Perform highest-priority work.  This is associated with |work_source_|
   // signalled by ScheduleWork or RunDelayedWorkTimer.  The static method calls
   // the instance method; the instance method returns true if it resignalled
-  // work_source_ to be called again from the loop.
+  // |work_source_| to be called again from the loop.
   static void RunWorkSource(void* info);
   bool RunWork();
 
   // Perform idle-priority work.  This is normally called by PreWaitObserver,
-  // but is also associated with idle_work_source_.  When this function
+  // but is also associated with |idle_work_source_|.  When this function
   // actually does perform idle work, it will resignal that source.  The
-  // static method calls the instance method; the instance method returns
-  // true if idle work was done.
+  // static method calls the instance method.
   static void RunIdleWorkSource(void* info);
-  bool RunIdleWork();
+  void RunIdleWork();
 
   // Perform work that may have been deferred because it was not runnable
   // within a nested run loop.  This is associated with
-  // nesting_deferred_work_source_ and is signalled by
+  // |nesting_deferred_work_source_| and is signalled by
   // MaybeScheduleNestingDeferredWork when returning from a nested loop,
   // so that an outer loop will be able to perform the necessary tasks if it
   // permits nestable tasks.
   static void RunNestingDeferredWorkSource(void* info);
-  bool RunNestingDeferredWork();
+  void RunNestingDeferredWork();
 
   // Schedules possible nesting-deferred work to be processed before the run
   // loop goes to sleep, exits, or begins processing sources at the top of its
@@ -211,24 +214,24 @@ class BASE_EXPORT MessagePumpCFRunLoopBase : public MessagePump {
   void MaybeScheduleNestingDeferredWork();
 
   // Observer callback responsible for performing idle-priority work, before
-  // the run loop goes to sleep.  Associated with idle_work_observer_.
+  // the run loop goes to sleep.  Associated with |pre_wait_observer_|.
   static void PreWaitObserver(CFRunLoopObserverRef observer,
                               CFRunLoopActivity activity, void* info);
 
   // Observer callback called before the run loop processes any sources.
-  // Associated with pre_source_observer_.
+  // Associated with |pre_source_observer_|.
   static void PreSourceObserver(CFRunLoopObserverRef observer,
                                 CFRunLoopActivity activity, void* info);
 
   // Observer callback called when the run loop starts and stops, at the
   // beginning and end of calls to CFRunLoopRun.  This is used to maintain
-  // nesting_level_.  Associated with enter_exit_observer_.
+  // |nesting_level_|.  Associated with |enter_exit_observer_|.
   static void EnterExitObserver(CFRunLoopObserverRef observer,
                                 CFRunLoopActivity activity, void* info);
 
-  // Called by EnterExitObserver after performing maintenance on nesting_level_.
-  // This allows subclasses an opportunity to perform additional processing on
-  // the basis of run loops starting and stopping.
+  // Called by EnterExitObserver after performing maintenance on
+  // |nesting_level_|. This allows subclasses an opportunity to perform
+  // additional processing on the basis of run loops starting and stopping.
   virtual void EnterExitRunLoop(CFRunLoopActivity activity);
 
   // The thread's run loop.
@@ -250,12 +253,6 @@ class BASE_EXPORT MessagePumpCFRunLoopBase : public MessagePump {
   // (weak) Delegate passed as an argument to the innermost Run call.
   Delegate* delegate_;
 
-  // The time that delayed_work_timer_ is scheduled to fire.  This is tracked
-  // independently of CFRunLoopTimerGetNextFireDate(delayed_work_timer_)
-  // to be able to reset the timer properly after waking from system sleep.
-  // See PowerStateNotification.
-  CFAbsoluteTime delayed_work_fire_time_;
-
   base::TimerSlack timer_slack_;
 
   // The recursion depth of the currently-executing CFRunLoopRun loop on the
@@ -263,7 +260,7 @@ class BASE_EXPORT MessagePumpCFRunLoopBase : public MessagePump {
   // the object was created in.
   int nesting_level_;
 
-  // The recursion depth (calculated in the same way as nesting_level_) of the
+  // The recursion depth (calculated in the same way as |nesting_level_|) of the
   // innermost executing CFRunLoopRun loop started by a call to Run.
   int run_nesting_level_;
 
@@ -306,8 +303,8 @@ class BASE_EXPORT MessagePumpCFRunLoop : public MessagePumpCFRunLoopBase {
   void EnterExitRunLoop(CFRunLoopActivity activity) override;
 
   // True if Quit is called to stop the innermost MessagePump
-  // (innermost_quittable_) but some other CFRunLoopRun loop (nesting_level_)
-  // is running inside the MessagePump's innermost Run call.
+  // (|innermost_quittable_|) but some other CFRunLoopRun loop
+  // (|nesting_level_|) is running inside the MessagePump's innermost Run call.
   bool quit_pending_;
 
   DISALLOW_COPY_AND_ASSIGN(MessagePumpCFRunLoop);

base/message_loop/message_pump_mac.mm

@@ -208,9 +208,10 @@ void MessagePumpCFRunLoopBase::ScheduleWork() {
 // Must be called on the run loop thread.
 void MessagePumpCFRunLoopBase::ScheduleDelayedWork(
     const TimeTicks& delayed_work_time) {
-  TimeDelta delta = delayed_work_time - TimeTicks::Now();
-  delayed_work_fire_time_ = CFAbsoluteTimeGetCurrent() + delta.InSecondsF();
+  ScheduleDelayedWorkImpl(delayed_work_time - TimeTicks::Now());
+}
 
+void MessagePumpCFRunLoopBase::ScheduleDelayedWorkImpl(TimeDelta delta) {
   // Flip the timer's validation bit just before setting the new fire time. Do
   // this now because CFRunLoopTimerSetNextFireDate() likely checks the validity
   // of a timer before proceeding to set its fire date. Making the timer valid
@@ -227,7 +228,8 @@ void MessagePumpCFRunLoopBase::ScheduleDelayedWork(
   } else {
     CFRunLoopTimerSetTolerance(delayed_work_timer_, 0);
   }
-  CFRunLoopTimerSetNextFireDate(delayed_work_timer_, delayed_work_fire_time_);
+  CFRunLoopTimerSetNextFireDate(
+      delayed_work_timer_, CFAbsoluteTimeGetCurrent() + delta.InSecondsF());
 }
 
 void MessagePumpCFRunLoopBase::SetTimerSlack(TimerSlack timer_slack) {
@@ -243,7 +245,6 @@ void MessagePumpCFRunLoopBase::Detach() {}
 // Must be called on the run loop thread.
 MessagePumpCFRunLoopBase::MessagePumpCFRunLoopBase(int initial_mode_mask)
     : delegate_(NULL),
-      delayed_work_fire_time_(kCFTimeIntervalMax),
       timer_slack_(base::TIMER_SLACK_NONE),
       nesting_level_(0),
       run_nesting_level_(0),
@@ -427,9 +428,6 @@ void MessagePumpCFRunLoopBase::RunDelayedWorkTimer(CFRunLoopTimerRef timer,
                                                    void* info) {
   MessagePumpCFRunLoopBase* self = static_cast<MessagePumpCFRunLoopBase*>(info);
 
-  // The timer won't fire again until it's reset.
-  self->delayed_work_fire_time_ = kCFTimeIntervalMax;
-
   // The message pump's timer needs to fire at changing and unpredictable
   // intervals. Creating a new timer for each firing time is very expensive, so
   // the message pump instead uses a repeating timer with a very large repeat
@@ -455,11 +453,11 @@ void MessagePumpCFRunLoopBase::RunDelayedWorkTimer(CFRunLoopTimerRef timer,
   // timer's new firing time.
   self->SetDelayedWorkTimerValid(false);
 
-  // CFRunLoopTimers fire outside of the priority scheme for CFRunLoopSources.
-  // In order to establish the proper priority in which work and delayed work
-  // are processed one for one, the timer used to schedule delayed work must
-  // signal a CFRunLoopSource used to dispatch both work and delayed work.
-  CFRunLoopSourceSignal(self->work_source_);
+  // The timer fired, assume we have work and let RunWork() figure out what to
+  // do and what to schedule after.
+  base::mac::CallWithEHFrame(^{
+    self->RunWork();
+  });
 }
 
 // Called from the run loop.
@@ -471,10 +469,10 @@ void MessagePumpCFRunLoopBase::RunWorkSource(void* info) {
   });
 }
 
-// Called by MessagePumpCFRunLoopBase::RunWorkSource.
+// Called by MessagePumpCFRunLoopBase::RunWorkSource and RunDelayedWorkTimer.
 bool MessagePumpCFRunLoopBase::RunWork() {
   if (!delegate_) {
-    // This point can be reached with a NULL delegate_ if Run is not on the
+    // This point can be reached with a nullptr |delegate_| if Run is not on the
     // stack but foreign code is spinning the CFRunLoop.  Arrange to come back
     // here when a delegate is available.
     delegateless_work_ = true;
@@ -490,40 +488,16 @@ bool MessagePumpCFRunLoopBase::RunWork() {
   // released promptly even in the absence of UI events.
   MessagePumpScopedAutoreleasePool autorelease_pool(this);
 
-  // Call DoWork and DoDelayedWork once, and if something was done, arrange to
-  // come back here again as long as the loop is still running.
-  bool did_work = delegate_->DoWork();
-  bool resignal_work_source = did_work;
-
-  TimeTicks next_time;
-  if (keep_running())
-    delegate_->DoDelayedWork(&next_time);
-  if (!did_work) {
-    // Determine whether there's more delayed work, and if so, if it needs to
-    // be done at some point in the future or if it's already time to do it.
-    // Only do these checks if did_work is false. If did_work is true, this
-    // function, and therefore any additional delayed work, will get another
-    // chance to run before the loop goes to sleep.
-    bool more_delayed_work = !next_time.is_null();
-    if (more_delayed_work) {
-      TimeDelta delay = next_time - TimeTicks::Now();
-      if (delay > TimeDelta()) {
-        // There's more delayed work to be done in the future.
-        ScheduleDelayedWork(next_time);
-      } else {
-        // There's more delayed work to be done, and its time is in the past.
-        // Arrange to come back here directly as long as the loop is still
-        // running.
-        resignal_work_source = true;
-      }
-    }
-  }
+  Delegate::NextWorkInfo next_work_info = delegate_->DoSomeWork();
 
-  if (resignal_work_source) {
+  if (next_work_info.is_immediate()) {
     CFRunLoopSourceSignal(work_source_);
+    return true;
   }
 
-  return resignal_work_source;
+  if (!next_work_info.delayed_run_time.is_max())
+    ScheduleDelayedWorkImpl(next_work_info.remaining_delay());
+  return false;
 }
 
 // Called from the run loop.
@@ -536,32 +510,27 @@ void MessagePumpCFRunLoopBase::RunIdleWorkSource(void* info) {
 }
 
 // Called by MessagePumpCFRunLoopBase::RunIdleWorkSource.
-bool MessagePumpCFRunLoopBase::RunIdleWork() {
+void MessagePumpCFRunLoopBase::RunIdleWork() {
   if (!delegate_) {
-    // This point can be reached with a NULL delegate_ if Run is not on the
+    // This point can be reached with a nullptr delegate_ if Run is not on the
     // stack but foreign code is spinning the CFRunLoop.  Arrange to come back
     // here when a delegate is available.
     delegateless_idle_work_ = true;
-    return false;
+    return;
   }
   if (!keep_running())
-    return false;
-
+    return;
   // The NSApplication-based run loop only drains the autorelease pool at each
   // UI event (NSEvent).  The autorelease pool is not drained for each
   // CFRunLoopSource target that's run.  Use a local pool for any autoreleased
   // objects if the app is not currently handling a UI event to ensure they're
   // released promptly even in the absence of UI events.
   MessagePumpScopedAutoreleasePool autorelease_pool(this);
-
   // Call DoIdleWork once, and if something was done, arrange to come back here
   // again as long as the loop is still running.
   bool did_work = delegate_->DoIdleWork();
-  if (did_work) {
+  if (did_work)
     CFRunLoopSourceSignal(idle_work_source_);
-  }
-
-  return did_work;
 }
 
 // Called from the run loop.
@@ -574,27 +543,22 @@ void MessagePumpCFRunLoopBase::RunNestingDeferredWorkSource(void* info) {
 }
 
 // Called by MessagePumpCFRunLoopBase::RunNestingDeferredWorkSource.
-bool MessagePumpCFRunLoopBase::RunNestingDeferredWork() {
+void MessagePumpCFRunLoopBase::RunNestingDeferredWork() {
   if (!delegate_) {
-    // This point can be reached with a NULL delegate_ if Run is not on the
+    // This point can be reached with a nullptr |delegate_| if Run is not on the
     // stack but foreign code is spinning the CFRunLoop.  There's no sense in
     // attempting to do any work or signalling the work sources because
     // without a delegate, work is not possible.
-    return false;
+    return;
   }
 
-  // Immediately try work in priority order.
-  if (!RunWork()) {
-    if (!RunIdleWork()) {
-      return false;
-    }
-  } else {
+  if (RunWork()) {
     // Work was done.  Arrange for the loop to try non-nestable idle work on
     // a subsequent pass.
     CFRunLoopSourceSignal(idle_work_source_);
+  } else {
+    RunIdleWork();
   }
-
-  return true;
 }
 
 // Called before the run loop goes to sleep or exits, or processes sources.

base/message_loop/message_pump_unittest.cc

@@ -35,21 +35,11 @@ namespace {
 bool PumpTypeUsesDoSomeWork(MessagePumpType type) {
   switch (type) {
     case MessagePumpType::DEFAULT:
-#if defined(OS_IOS)
-      // iOS uses a MessagePumpCFRunLoop instead of MessagePumpDefault for
-      // TYPE_DEFAULT. TODO(gab): migrate MessagePumpCFRunLoop too.
-      return false;
-#else
       return true;
-#endif
 
     case MessagePumpType::UI:
-#if defined(OS_IOS)
-      // iOS uses a MessagePumpCFRunLoop for UI in unit tests, ref.
-      // test_support_ios.mm::CreateMessagePumpForUIForTests(). TODO(gab):
-      // migrate MessagePumpCFRunLoop too.
-      return false;
-#elif defined(OS_WIN) || defined(OS_ANDROID) || defined(USE_GLIB)
+#if defined(OS_WIN) || defined(OS_ANDROID) || defined(USE_GLIB) || \
+    defined(OS_MACOSX)
       return true;
 #elif defined(OS_POSIX) && !defined(OS_NACL_SFI)
       // MessagePumpLibevent was migrated (ref. message_pump_for_ui.h and
@@ -62,7 +52,7 @@ bool PumpTypeUsesDoSomeWork(MessagePumpType type) {
 #endif
 
     case MessagePumpType::IO:
-#if defined(OS_WIN) || (defined(OS_MACOSX) && !defined(OS_IOS))
+#if defined(OS_WIN) || defined(OS_MACOSX)
       return true;
 #elif defined(OS_POSIX) && !defined(OS_NACL_SFI)
       // MessagePumpLibevent was migrated (ref. message_pump_for_io.h and

base/task/single_thread_task_executor.cc

@@ -43,4 +43,8 @@ scoped_refptr<SingleThreadTaskRunner> SingleThreadTaskExecutor::task_runner()
   return default_task_queue_->task_runner();
 }
 
+void SingleThreadTaskExecutor::SetWorkBatchSize(size_t work_batch_size) {
+  sequence_manager_->SetWorkBatchSize(work_batch_size);
+}
+
 }  // namespace base

base/task/single_thread_task_executor.h

@@ -44,6 +44,12 @@ class BASE_EXPORT SingleThreadTaskExecutor {
 
   MessagePumpType type() const { return type_; }
 
+  // Sets the number of application tasks executed every time the MessagePump
+  // asks its delegate to DoWork(). Defaults to 1. Can be increased in some
+  // scenarios where the native pump (i.e. not MessagePumpType::DEFAULT) has
+  // high overhead and yielding to native isn't critical.
+  void SetWorkBatchSize(size_t work_batch_size);
+
  private:
   explicit SingleThreadTaskExecutor(MessagePumpType type,
                                     std::unique_ptr<MessagePump> pump);

chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc

@@ -1851,9 +1851,9 @@ namespace {
 void PressEscapeWhileDetachedStep2(const BrowserList* browser_list) {
   ASSERT_EQ(2u, browser_list->size());
   Browser* new_browser = browser_list->get(1);
-  ui_controls::SendKeyPress(
+  ASSERT_TRUE(ui_test_utils::SendKeyPressToWindowSync(
       new_browser->window()->GetNativeWindow(), ui::VKEY_ESCAPE, false, false,
-      false, false);
+      false, false));
 }
 
 }  // namespace

content/gpu/gpu_main.cc

@@ -308,6 +308,10 @@ int GpuMain(const MainFunctionParams& parameters) {
     main_thread_task_executor =
         std::make_unique<base::SingleThreadTaskExecutor>(
             base::MessagePumpType::NS_RUNLOOP);
+    // As part of the migration to DoSomeWork(), this policy is required to keep
+    // previous behavior and avoid regressions.
+    // TODO(crbug.com/1041853): Consider updating the policy.
+    main_thread_task_executor->SetWorkBatchSize(2);
 #else
     main_thread_task_executor =
         std::make_unique<base::SingleThreadTaskExecutor>(