[ios] Adds MessagePumpUIApplication::Detach() and calls it on shutdown.

Adds Attach/Detach as virtual methods on MessagePumpCFRunLoopBase
so that they can be called unconditionally from SequenceManagerImpl.

BUG=1002087

Change-Id: I8263188aa388a449c799b7bde17e7dac8b44d3be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1826517
Commit-Queue: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Alex Clarke <alexclarke@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718316}

base/message_loop/message_pump_mac.h

@@ -88,6 +88,15 @@ class BASE_EXPORT MessagePumpCFRunLoopBase : public MessagePump {
   void ScheduleDelayedWork(const TimeTicks& delayed_work_time) override;
   void SetTimerSlack(TimerSlack timer_slack) override;
 
+#if defined(OS_IOS)
+  // Some iOS message pumps do not support calling |Run()| to spin the main
+  // message loop directly.  Instead, call |Attach()| to set up a delegate, then
+  // |Detach()| before destroying the message pump.  These methods do nothing if
+  // the message pump supports calling |Run()| and |Quit()|.
+  virtual void Attach(Delegate* delegate);
+  virtual void Detach();
+#endif  // OS_IOS
+
  protected:
   // Needs access to CreateAutoreleasePool.
   friend class MessagePumpScopedAutoreleasePool;
@@ -332,9 +341,12 @@ class MessagePumpUIApplication : public MessagePumpCFRunLoopBase {
   void DoRun(Delegate* delegate) override;
   bool DoQuit() override;
 
-  // This message pump can not spin the main message loop directly.  Instead,
-  // call |Attach()| to set up a delegate.  It is an error to call |Run()|.
-  virtual void Attach(Delegate* delegate);
+  // MessagePumpCFRunLoopBase.
+  // MessagePumpUIApplication can not spin the main message loop directly.
+  // Instead, call |Attach()| to set up a delegate.  It is an error to call
+  // |Run()|.
+  void Attach(Delegate* delegate) override;
+  void Detach() override;
 
  private:
   RunLoop* run_loop_;

base/message_loop/message_pump_mac.mm

@@ -234,6 +234,12 @@ void MessagePumpCFRunLoopBase::SetTimerSlack(TimerSlack timer_slack) {
   timer_slack_ = timer_slack;
 }
 
+#if defined(OS_IOS)
+void MessagePumpCFRunLoopBase::Attach(Delegate* delegate) {}
+
+void MessagePumpCFRunLoopBase::Detach() {}
+#endif  // OS_IOS
+
 // Must be called on the run loop thread.
 MessagePumpCFRunLoopBase::MessagePumpCFRunLoopBase(int initial_mode_mask)
     : delegate_(NULL),
@@ -797,6 +803,13 @@ void MessagePumpUIApplication::Attach(Delegate* delegate) {
   SetDelegate(delegate);
 }
 
+void MessagePumpUIApplication::Detach() {
+  DCHECK(run_loop_);
+  run_loop_->AfterRun();
+  SetDelegate(nullptr);
+  run_loop_ = nullptr;
+}
+
 #else
 
 ScopedPumpMessagesInPrivateModes::ScopedPumpMessagesInPrivateModes() {

base/message_loop/message_pump_unittest.cc

@@ -46,9 +46,10 @@ bool PumpTypeUsesDoSomeWork(MessagePumpType type) {
 
     case MessagePumpType::UI:
 #if defined(OS_IOS)
-      // iOS uses a MessagePumpDefault for UI in unit tests, ref.
-      // test_support_ios.mm::CreateMessagePumpForUIForTests().
-      return true;
+      // 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)
       return true;
 #elif defined(OS_POSIX) && !defined(OS_NACL_SFI)

base/task/sequence_manager/sequence_manager_impl.cc

@@ -215,6 +215,13 @@ SequenceManagerImpl::~SequenceManagerImpl() {
   TRACE_EVENT_OBJECT_DELETED_WITH_ID(
       TRACE_DISABLED_BY_DEFAULT("sequence_manager"), "SequenceManager", this);
 
+#if defined(OS_IOS)
+  if (settings_.message_loop_type == MessagePumpType::UI &&
+      associated_thread_->IsBound()) {
+    controller_->DetachFromMessagePump();
+  }
+#endif
+
   // Make sure no Task is running as given that RunLoop does not support the
   // Delegate being destroyed from a Task and
   // ThreadControllerWithMessagePumpImpl does not support being destroyed from a
@@ -313,6 +320,13 @@ void SequenceManagerImpl::BindToMessagePump(std::unique_ptr<MessagePump> pump) {
     controller_->AttachToMessagePump();
   }
 #endif
+
+  // On iOS attach to the native loop when there is one.
+#if defined(OS_IOS)
+  if (settings_.message_loop_type == MessagePumpType::UI) {
+    controller_->AttachToMessagePump();
+  }
+#endif
 }
 
 void SequenceManagerImpl::BindToCurrentThread() {

base/task/sequence_manager/thread_controller.h

@@ -96,6 +96,12 @@ class ThreadController {
   virtual void AttachToMessagePump() = 0;
 #endif
 
+#if defined(OS_IOS)
+  // Detaches this ThreadController from the message pump, allowing the
+  // controller to be shut down cleanly.
+  virtual void DetachFromMessagePump() = 0;
+#endif
+
   // TODO(altimin): Get rid of the methods below.
   // These methods exist due to current integration of SequenceManager
   // with MessageLoop.

base/task/sequence_manager/thread_controller_impl.cc

@@ -322,7 +322,13 @@ MessagePump* ThreadControllerImpl::GetBoundMessagePump() const {
 void ThreadControllerImpl::AttachToMessagePump() {
   NOTREACHED();
 }
-#endif
+#endif  // OS_IOS || OS_ANDROID
+
+#if defined(OS_IOS)
+void ThreadControllerImpl::DetachFromMessagePump() {
+  NOTREACHED();
+}
+#endif  // OS_IOS
 
 }  // namespace internal
 }  // namespace sequence_manager

base/task/sequence_manager/thread_controller_impl.h

@@ -66,6 +66,9 @@ class BASE_EXPORT ThreadControllerImpl : public ThreadController,
   MessagePump* GetBoundMessagePump() const override;
 #if defined(OS_IOS) || defined(OS_ANDROID)
   void AttachToMessagePump() override;
+#endif
+#if defined(OS_IOS)
+  void DetachFromMessagePump() override;
 #endif
   bool ShouldQuitRunLoopWhenIdle() override;
 

base/task/sequence_manager/thread_controller_with_message_pump_impl.cc

@@ -527,7 +527,11 @@ MessagePump* ThreadControllerWithMessagePumpImpl::GetBoundMessagePump() const {
 
 #if defined(OS_IOS)
 void ThreadControllerWithMessagePumpImpl::AttachToMessagePump() {
-  static_cast<MessagePumpUIApplication*>(pump_.get())->Attach(this);
+  static_cast<MessagePumpCFRunLoopBase*>(pump_.get())->Attach(this);
+}
+
+void ThreadControllerWithMessagePumpImpl::DetachFromMessagePump() {
+  static_cast<MessagePumpCFRunLoopBase*>(pump_.get())->Detach();
 }
 #elif defined(OS_ANDROID)
 void ThreadControllerWithMessagePumpImpl::AttachToMessagePump() {

base/task/sequence_manager/thread_controller_with_message_pump_impl.h

@@ -67,6 +67,9 @@ class BASE_EXPORT ThreadControllerWithMessagePumpImpl
   MessagePump* GetBoundMessagePump() const override;
 #if defined(OS_IOS) || defined(OS_ANDROID)
   void AttachToMessagePump() override;
+#endif
+#if defined(OS_IOS)
+  void DetachFromMessagePump() override;
 #endif
   bool ShouldQuitRunLoopWhenIdle() override;
 

base/task/single_thread_task_executor.cc

@@ -21,14 +21,6 @@ SingleThreadTaskExecutor::SingleThreadTaskExecutor(MessagePumpType type)
       simple_task_executor_(sequence_manager_.get(), task_runner()) {
   sequence_manager_->SetDefaultTaskRunner(default_task_queue_->task_runner());
   sequence_manager_->BindToMessagePump(MessagePump::Create(type));
-
-#if defined(OS_IOS)
-  if (type == MessagePumpType::UI) {
-    static_cast<sequence_manager::internal::SequenceManagerImpl*>(
-        sequence_manager_.get())
-        ->AttachToMessagePump();
-  }
-#endif
 }
 
 SingleThreadTaskExecutor::~SingleThreadTaskExecutor() = default;

base/test/test_support_ios.mm

@@ -11,7 +11,7 @@
 #include "base/logging.h"
 #include "base/mac/scoped_nsobject.h"
 #include "base/message_loop/message_pump.h"
-#include "base/message_loop/message_pump_default.h"
+#include "base/message_loop/message_pump_mac.h"
 #import "base/test/ios/google_test_runner_delegate.h"
 #include "base/test/test_suite.h"
 #include "base/test/test_switches.h"
@@ -199,8 +199,8 @@ static char** g_argv;
 namespace {
 
 std::unique_ptr<base::MessagePump> CreateMessagePumpForUIForTests() {
-  // A default MessagePump will do quite nicely in tests.
-  return std::unique_ptr<base::MessagePump>(new base::MessagePumpDefault());
+  // A basic MessagePump will do quite nicely in tests.
+  return std::unique_ptr<base::MessagePump>(new base::MessagePumpCFRunLoop());
 }
 
 }  // namespace