Allow work that was deferred on account of not being runnable in a nested loop

to be processed when returning to an outer loop.

BUG=11470 13442 13468
TEST=base_unittests, test cases in bugs
Review URL: http://codereview.chromium.org/146006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@19272 0039d316-1c4b-4281-b951-d872f2087c98

base/message_loop_unittest.cc

@@ -593,6 +593,7 @@ enum TaskType {
   QUITMESSAGELOOP,
   ORDERERD,
   PUMPS,
+  SLEEP,
 };
 
 // Saves the order in which the tasks executed.
@@ -623,6 +624,7 @@ std::ostream& operator <<(std::ostream& os, TaskType type) {
   case QUITMESSAGELOOP:   os << "QUITMESSAGELOOP"; break;
   case ORDERERD:          os << "ORDERERD"; break;
   case PUMPS:             os << "PUMPS"; break;
+  case SLEEP:             os << "SLEEP"; break;
   default:
     NOTREACHED();
     os << "Unknown TaskType";
@@ -765,6 +767,22 @@ class QuitTask : public OrderedTasks {
   }
 };
 
+class SleepTask : public OrderedTasks {
+ public:
+  SleepTask(TaskList* order, int cookie, int ms)
+      : OrderedTasks(order, SLEEP, cookie), ms_(ms) {
+  }
+
+  virtual void Run() {
+    RunStart();
+    PlatformThread::Sleep(ms_);
+    RunEnd();
+  }
+
+ private:
+  int ms_;
+};
+
 #if defined(OS_WIN)
 
 class Recursive2Tasks : public Task {
@@ -1027,7 +1045,8 @@ void RunTest_NonNestableWithNoNesting(MessageLoop::Type message_loop_type) {
 }
 
 // Tests that non nestable tasks don't run when there's code in the call stack.
-void RunTest_NonNestableInNestedLoop(MessageLoop::Type message_loop_type) {
+void RunTest_NonNestableInNestedLoop(MessageLoop::Type message_loop_type,
+                                     bool use_delayed) {
   MessageLoop loop(message_loop_type);
 
   TaskList order;
@@ -1035,26 +1054,39 @@ void RunTest_NonNestableInNestedLoop(MessageLoop::Type message_loop_type) {
   MessageLoop::current()->PostTask(FROM_HERE,
                                    new TaskThatPumps(&order, 1));
   Task* task = new OrderedTasks(&order, 2);
-  MessageLoop::current()->PostNonNestableTask(FROM_HERE, task);
+  if (use_delayed) {
+    MessageLoop::current()->PostNonNestableDelayedTask(FROM_HERE, task, 1);
+  } else {
+    MessageLoop::current()->PostNonNestableTask(FROM_HERE, task);
+  }
   MessageLoop::current()->PostTask(FROM_HERE, new OrderedTasks(&order, 3));
-  MessageLoop::current()->PostTask(FROM_HERE, new OrderedTasks(&order, 4));
-  Task* non_nestable_quit = new QuitTask(&order, 5);
-  MessageLoop::current()->PostNonNestableTask(FROM_HERE, non_nestable_quit);
+  MessageLoop::current()->PostTask(FROM_HERE, new SleepTask(&order, 4, 50));
+  MessageLoop::current()->PostTask(FROM_HERE, new OrderedTasks(&order, 5));
+  Task* non_nestable_quit = new QuitTask(&order, 6);
+  if (use_delayed) {
+    MessageLoop::current()->PostNonNestableDelayedTask(FROM_HERE,
+                                                       non_nestable_quit,
+                                                       2);
+  } else {
+    MessageLoop::current()->PostNonNestableTask(FROM_HERE, non_nestable_quit);
+  }
 
   MessageLoop::current()->Run();
 
   // FIFO order.
-  ASSERT_EQ(10U, order.size());
+  ASSERT_EQ(12U, order.size());
   EXPECT_EQ(order[ 0], TaskItem(PUMPS, 1, true));
   EXPECT_EQ(order[ 1], TaskItem(ORDERERD, 3, true));
   EXPECT_EQ(order[ 2], TaskItem(ORDERERD, 3, false));
-  EXPECT_EQ(order[ 3], TaskItem(ORDERERD, 4, true));
-  EXPECT_EQ(order[ 4], TaskItem(ORDERERD, 4, false));
-  EXPECT_EQ(order[ 5], TaskItem(PUMPS, 1, false));
-  EXPECT_EQ(order[ 6], TaskItem(ORDERERD, 2, true));
-  EXPECT_EQ(order[ 7], TaskItem(ORDERERD, 2, false));
-  EXPECT_EQ(order[ 8], TaskItem(QUITMESSAGELOOP, 5, true));
-  EXPECT_EQ(order[ 9], TaskItem(QUITMESSAGELOOP, 5, false));
+  EXPECT_EQ(order[ 3], TaskItem(SLEEP, 4, true));
+  EXPECT_EQ(order[ 4], TaskItem(SLEEP, 4, false));
+  EXPECT_EQ(order[ 5], TaskItem(ORDERERD, 5, true));
+  EXPECT_EQ(order[ 6], TaskItem(ORDERERD, 5, false));
+  EXPECT_EQ(order[ 7], TaskItem(PUMPS, 1, false));
+  EXPECT_EQ(order[ 8], TaskItem(ORDERERD, 2, true));
+  EXPECT_EQ(order[ 9], TaskItem(ORDERERD, 2, false));
+  EXPECT_EQ(order[10], TaskItem(QUITMESSAGELOOP, 6, true));
+  EXPECT_EQ(order[11], TaskItem(QUITMESSAGELOOP, 6, false));
 }
 
 #if defined(OS_WIN)
@@ -1365,9 +1397,15 @@ TEST(MessageLoopTest, NonNestableWithNoNesting) {
 }
 
 TEST(MessageLoopTest, NonNestableInNestedLoop) {
-  RunTest_NonNestableInNestedLoop(MessageLoop::TYPE_DEFAULT);
-  RunTest_NonNestableInNestedLoop(MessageLoop::TYPE_UI);
-  RunTest_NonNestableInNestedLoop(MessageLoop::TYPE_IO);
+  RunTest_NonNestableInNestedLoop(MessageLoop::TYPE_DEFAULT, false);
+  RunTest_NonNestableInNestedLoop(MessageLoop::TYPE_UI, false);
+  RunTest_NonNestableInNestedLoop(MessageLoop::TYPE_IO, false);
+}
+
+TEST(MessageLoopTest, NonNestableDelayedInNestedLoop) {
+  RunTest_NonNestableInNestedLoop(MessageLoop::TYPE_DEFAULT, true);
+  RunTest_NonNestableInNestedLoop(MessageLoop::TYPE_UI, true);
+  RunTest_NonNestableInNestedLoop(MessageLoop::TYPE_IO, true);
 }
 
 #if defined(OS_WIN)

base/message_pump_mac.h

@@ -57,6 +57,11 @@ class MessagePumpCFRunLoopBase : public MessagePump {
   // The thread's run loop.
   CFRunLoopRef run_loop_;
 
+  // The recursion depth of the currently-executing CFRunLoopRun loop on the
+  // run loop's thread.  0 if no run loops are running inside of whatever scope
+  // the object was created in.
+  int nesting_level_;
+
  private:
   // Timer callback scheduled by ScheduleDelayedWork.  This does not do any
   // work, but it signals delayed_work_source_ so that delayed work can be
@@ -64,25 +69,61 @@ class MessagePumpCFRunLoopBase : public MessagePump {
   static void RunDelayedWorkTimer(CFRunLoopTimerRef timer, void* info);
 
   // Perform highest-priority work.  This is associated with work_source_
-  // signalled by ScheduleWork.
-  static void RunWork(void* info);
+  // signalled by ScheduleWork.  The static method calls the instance method;
+  // the instance method returns true if work was done.
+  static void RunWorkSource(void* info);
+  bool RunWork();
 
   // Perform delayed-priority work.  This is associated with
   // delayed_work_source_ signalled by RunDelayedWorkTimer, and is responsible
-  // for calling ScheduleDelayedWork again if appropriate.
-  static void RunDelayedWork(void* info);
+  // for calling ScheduleDelayedWork again if appropriate.  The static method
+  // calls the instance method; the instance method returns true if more
+  // delayed work is available.
+  static void RunDelayedWorkSource(void* info);
+  bool RunDelayedWork();
+
+  // Perform idle-priority work.  This is normally called by PreWaitObserver,
+  // 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 void RunIdleWorkSource(void* info);
+  bool 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 EnterExitObserver when
+  // a run loop exits, so that an outer loop will be able to perform the
+  // necessary tasks.  The static method calls the instance method; the
+  // instance method returns true if anything was done.
+  static void RunNestingDeferredWorkSource(void* info);
+  bool RunNestingDeferredWork();
 
   // Observer callback responsible for performing idle-priority work, before
   // the run loop goes to sleep.  Associated with idle_work_observer_.
-  static void RunIdleWork(CFRunLoopObserverRef observer,
-                          CFRunLoopActivity activity, void* info);
+  static void PreWaitObserver(CFRunLoopObserverRef observer,
+                              CFRunLoopActivity activity, void* info);
 
-  // The timer, sources, and observer are described above alongside their
+  // 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_.
+  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.
+  virtual void EnterExitRunLoop(CFRunLoopActivity activity);
+
+  // The timer, sources, and observers are described above alongside their
   // callbacks.
   CFRunLoopTimerRef delayed_work_timer_;
   CFRunLoopSourceRef work_source_;
   CFRunLoopSourceRef delayed_work_source_;
-  CFRunLoopObserverRef idle_work_observer_;
+  CFRunLoopSourceRef idle_work_source_;
+  CFRunLoopSourceRef nesting_deferred_work_source_;
+  CFRunLoopObserverRef pre_wait_observer_;
+  CFRunLoopObserverRef enter_exit_observer_;
 
   // (weak) Delegate passed as an argument to the innermost Run call.
   Delegate* delegate_;
@@ -93,26 +134,12 @@ class MessagePumpCFRunLoopBase : public MessagePump {
 class MessagePumpCFRunLoop : public MessagePumpCFRunLoopBase {
  public:
   MessagePumpCFRunLoop();
-  virtual ~MessagePumpCFRunLoop();
 
   virtual void DoRun(Delegate* delegate);
   virtual void Quit();
 
  private:
-  // 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_ and to handle deferred loop quits.  Associated with
-  // enter_exit_observer_.
-  static void EnterExitRunLoop(CFRunLoopObserverRef observer,
-                               CFRunLoopActivity activity, void* info);
-
-  // Observer for EnterExitRunLoop.
-  CFRunLoopObserverRef enter_exit_observer_;
-
-  // The recursion depth of the currently-executing CFRunLoopRun loop on the
-  // run loop's thread.  0 if no run loops are running inside of whatever scope
-  // the object was created in.
-  int nesting_level_;
+  virtual void EnterExitRunLoop(CFRunLoopActivity activity);
 
   // The recursion depth (calculated in the same way as nesting_level_) of the
   // innermost executing CFRunLoopRun loop started by a call to Run.

base/message_pump_mac.mm

@@ -22,7 +22,9 @@ namespace base {
 
 // Must be called on the run loop thread.
 MessagePumpCFRunLoopBase::MessagePumpCFRunLoopBase()
-    : delegate_(NULL) {
+    : nesting_level_(0),
+      delegate_(NULL)
+ {
   run_loop_ = CFRunLoopGetCurrent();
   CFRetain(run_loop_);
 
@@ -42,34 +44,69 @@ MessagePumpCFRunLoopBase::MessagePumpCFRunLoopBase()
 
   CFRunLoopSourceContext source_context = CFRunLoopSourceContext();
   source_context.info = this;
-  source_context.perform = RunWork;
+  source_context.perform = RunWorkSource;
   work_source_ = CFRunLoopSourceCreate(NULL,  // allocator
-                                       0,     // priority
+                                       1,     // priority
                                        &source_context);
   CFRunLoopAddSource(run_loop_, work_source_, kCFRunLoopCommonModes);
 
-  source_context.perform = RunDelayedWork;
+  source_context.perform = RunDelayedWorkSource;
   delayed_work_source_ = CFRunLoopSourceCreate(NULL,  // allocator
-                                               1,     // priority
+                                               2,     // priority
                                                &source_context);
   CFRunLoopAddSource(run_loop_, delayed_work_source_, kCFRunLoopCommonModes);
 
+  source_context.perform = RunIdleWorkSource;
+  idle_work_source_ = CFRunLoopSourceCreate(NULL,  // allocator
+                                            3,     // priority
+                                            &source_context);
+  CFRunLoopAddSource(run_loop_, idle_work_source_, kCFRunLoopCommonModes);
+
+  source_context.perform = RunNestingDeferredWorkSource;
+  nesting_deferred_work_source_ = CFRunLoopSourceCreate(NULL,  // allocator
+                                                        0,     // priority
+                                                        &source_context);
+  CFRunLoopAddSource(run_loop_, nesting_deferred_work_source_,
+                     kCFRunLoopCommonModes);
+
   CFRunLoopObserverContext observer_context = CFRunLoopObserverContext();
   observer_context.info = this;
-  idle_work_observer_ = CFRunLoopObserverCreate(NULL,  // allocator
-                                                kCFRunLoopBeforeWaiting,
-                                                true,  // repeat
-                                                0,     // priority
-                                                RunIdleWork,
-                                                &observer_context);
-  CFRunLoopAddObserver(run_loop_, idle_work_observer_, kCFRunLoopCommonModes);
+  pre_wait_observer_ = CFRunLoopObserverCreate(NULL,  // allocator
+                                               kCFRunLoopBeforeWaiting,
+                                               true,  // repeat
+                                               0,     // priority
+                                               PreWaitObserver,
+                                               &observer_context);
+  CFRunLoopAddObserver(run_loop_, pre_wait_observer_, kCFRunLoopCommonModes);
+
+  enter_exit_observer_ = CFRunLoopObserverCreate(NULL,  // allocator
+                                                 kCFRunLoopEntry |
+                                                     kCFRunLoopExit,
+                                                 true,  // repeat
+                                                 0,     // priority
+                                                 EnterExitObserver,
+                                                 &observer_context);
+  CFRunLoopAddObserver(run_loop_, enter_exit_observer_, kCFRunLoopCommonModes);
 }
 
-// Ideally called on the run loop thread.
+// Ideally called on the run loop thread.  If other run loops were running
+// lower on the run loop thread's stack when this object was created, the
+// same number of run loops must be running when this object is destroyed.
 MessagePumpCFRunLoopBase::~MessagePumpCFRunLoopBase() {
-  CFRunLoopRemoveObserver(run_loop_, idle_work_observer_,
+  CFRunLoopRemoveObserver(run_loop_, enter_exit_observer_,
                           kCFRunLoopCommonModes);
-  CFRelease(idle_work_observer_);
+  CFRelease(enter_exit_observer_);
+
+  CFRunLoopRemoveObserver(run_loop_, pre_wait_observer_,
+                          kCFRunLoopCommonModes);
+  CFRelease(pre_wait_observer_);
+
+  CFRunLoopRemoveSource(run_loop_, nesting_deferred_work_source_,
+                        kCFRunLoopCommonModes);
+  CFRelease(nesting_deferred_work_source_);
+
+  CFRunLoopRemoveSource(run_loop_, idle_work_source_, kCFRunLoopCommonModes);
+  CFRelease(idle_work_source_);
 
   CFRunLoopRemoveSource(run_loop_, delayed_work_source_, kCFRunLoopCommonModes);
   CFRelease(delayed_work_source_);
@@ -125,7 +162,7 @@ void MessagePumpCFRunLoopBase::ScheduleDelayedWork(
 // static
 void MessagePumpCFRunLoopBase::RunDelayedWorkTimer(CFRunLoopTimerRef timer,
                                                    void* info) {
-  MessagePumpCFRunLoop* self = static_cast<MessagePumpCFRunLoop*>(info);
+  MessagePumpCFRunLoopBase* self = static_cast<MessagePumpCFRunLoopBase*>(info);
 
   // CFRunLoopTimers fire outside of the priority scheme for CFRunLoopSources.
   // In order to establish the proper priority where delegate_->DoDelayedWork
@@ -137,102 +174,159 @@ void MessagePumpCFRunLoopBase::RunDelayedWorkTimer(CFRunLoopTimerRef timer,
 
 // Called from the run loop.
 // static
-void MessagePumpCFRunLoopBase::RunWork(void* info) {
-  MessagePumpCFRunLoop* self = static_cast<MessagePumpCFRunLoop*>(info);
+void MessagePumpCFRunLoopBase::RunWorkSource(void* info) {
+  MessagePumpCFRunLoopBase* self = static_cast<MessagePumpCFRunLoopBase*>(info);
+  self->RunWork();
+}
 
+// Called by MessagePumpCFRunLoopBase::RunWorkSource.
+bool MessagePumpCFRunLoopBase::RunWork() {
   // If we're on the main event loop, the NSApp runloop won't clean up the
-  // autoreleasepool until there is UI event, so use a local one for any
+  // autorelease pool until there is a UI event, so use a local one for any
   // autoreleased objects to ensure they go away sooner.
   ScopedNSAutoreleasePool autorelease_pool;
 
   // Call DoWork once, and if something was done, arrange to come back here
   // again as long as the loop is still running.
-  if (self->delegate_->DoWork()) {
-    CFRunLoopSourceSignal(self->work_source_);
+  bool did_work = delegate_->DoWork();
+  if (did_work) {
+    CFRunLoopSourceSignal(work_source_);
   }
+
+  return did_work;
 }
 
 // Called from the run loop.
 // static
-void MessagePumpCFRunLoopBase::RunDelayedWork(void* info) {
-  MessagePumpCFRunLoop* self = static_cast<MessagePumpCFRunLoop*>(info);
+void MessagePumpCFRunLoopBase::RunDelayedWorkSource(void* info) {
+  MessagePumpCFRunLoopBase* self = static_cast<MessagePumpCFRunLoopBase*>(info);
+  self->RunDelayedWork();
+}
 
+// Called by MessagePumpCFRunLoopBase::RunDelayedWorkSource.
+bool MessagePumpCFRunLoopBase::RunDelayedWork() {
   // If we're on the main event loop, the NSApp runloop won't clean up the
-  // autoreleasepool until there is UI event, so use a local one for any
+  // autorelease pool until there is a UI event, so use a local one for any
   // autoreleased objects to ensure they go away sooner.
   ScopedNSAutoreleasePool autorelease_pool;
 
   Time next_time;
-  self->delegate_->DoDelayedWork(&next_time);
-  if (!next_time.is_null()) {
+  delegate_->DoDelayedWork(&next_time);
+
+  bool more_work = !next_time.is_null();
+  if (more_work) {
     TimeDelta delay = next_time - Time::Now();
     if (delay > TimeDelta()) {
       // There's more delayed work to be done in the future.
-      self->ScheduleDelayedWork(next_time);
+      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.
-      CFRunLoopSourceSignal(self->delayed_work_source_);
+      CFRunLoopSourceSignal(delayed_work_source_);
     }
   }
+
+  return more_work;
 }
 
 // Called from the run loop.
 // static
-void MessagePumpCFRunLoopBase::RunIdleWork(CFRunLoopObserverRef observer,
-                                           CFRunLoopActivity activity,
-                                           void* info) {
-  MessagePumpCFRunLoop* self = static_cast<MessagePumpCFRunLoop*>(info);
+void MessagePumpCFRunLoopBase::RunIdleWorkSource(void* info) {
+  MessagePumpCFRunLoopBase* self = static_cast<MessagePumpCFRunLoopBase*>(info);
+  self->RunIdleWork();
+}
 
+// Called by MessagePumpCFRunLoopBase::RunIdleWorkSource.
+bool MessagePumpCFRunLoopBase::RunIdleWork() {
   // If we're on the main event loop, the NSApp runloop won't clean up the
-  // autoreleasepool until there is UI event, so use a local one for any
+  // autorelease pool until there is a UI event, so use a local one for any
   // autoreleased objects to ensure they go away sooner.
   ScopedNSAutoreleasePool autorelease_pool;
 
-  // The "self->delegate_ &&" part of the clause is needed for the case of
-  // the temporary modal first run dialog.  The dialog is displayed really
-  // early in the Chrome launch process at which time self->delegate_ is null.
-  // TODO: remove the "self->delegate_ &&" clause from the bellow condition once
-  // we remove the modal first run dialog.
-  if (self->delegate_ && self->delegate_->DoIdleWork()) {
-    // If idle work was done, don't let the loop go to sleep.  More idle work
-    // might be waiting.
-    CFRunLoopWakeUp(self->run_loop_);
+  // 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) {
+    CFRunLoopSourceSignal(idle_work_source_);
   }
+
+  return did_work;
+}
+
+// Called from the run loop.
+// static
+void MessagePumpCFRunLoopBase::RunNestingDeferredWorkSource(void* info) {
+  MessagePumpCFRunLoopBase* self = static_cast<MessagePumpCFRunLoopBase*>(info);
+  self->RunNestingDeferredWork();
+}
+
+// Called by MessagePumpCFRunLoopBase::RunNestingDeferredWorkSource.
+bool MessagePumpCFRunLoopBase::RunNestingDeferredWork() {
+  if (!RunWork()) {
+    if (!RunDelayedWork()) {
+      if (!RunIdleWork()) {
+        return false;
+      }
+    }
+  }
+
+  return true;
+}
+
+// Called from the run loop.
+// static
+void MessagePumpCFRunLoopBase::PreWaitObserver(CFRunLoopObserverRef observer,
+                                               CFRunLoopActivity activity,
+                                               void* info) {
+  MessagePumpCFRunLoopBase* self = static_cast<MessagePumpCFRunLoopBase*>(info);
+
+  // Attempt to do some idle work before going to sleep.
+  self->RunIdleWork();
+}
+
+// Called from the run loop.
+// static
+void MessagePumpCFRunLoopBase::EnterExitObserver(CFRunLoopObserverRef observer,
+                                                 CFRunLoopActivity activity,
+                                                 void* info) {
+  MessagePumpCFRunLoopBase* self = static_cast<MessagePumpCFRunLoopBase*>(info);
+
+  switch (activity) {
+    case kCFRunLoopEntry:
+      ++self->nesting_level_;
+      break;
+    case kCFRunLoopExit:
+      --self->nesting_level_;
+      if (self->nesting_level_) {
+        // It's possible that some work was not performed because it was
+        // inappropriate to do within a nested loop.  When leaving any inner
+        // loop, signal the nesting-deferred work source to ensure that such
+        // work be afforded an opportunity to be processed if appropriate.
+        CFRunLoopSourceSignal(self->nesting_deferred_work_source_);
+      }
+      break;
+    default:
+      break;
+  }
+
+  self->EnterExitRunLoop(activity);
+}
+
+// Called by MessagePumpCFRunLoopBase::EnterExitRunLoop.  The default
+// implementation is a no-op.
+void MessagePumpCFRunLoopBase::EnterExitRunLoop(CFRunLoopActivity activity) {
 }
 
-// Must be called on the run loop thread.
 MessagePumpCFRunLoop::MessagePumpCFRunLoop()
-    : nesting_level_(0),
-      innermost_quittable_(0),
+    : innermost_quittable_(0),
       quit_pending_(false) {
-  CFRunLoopObserverContext observer_context = CFRunLoopObserverContext();
-  observer_context.info = this;
-  enter_exit_observer_ = CFRunLoopObserverCreate(NULL,  // allocator
-                                                 kCFRunLoopEntry |
-                                                     kCFRunLoopExit,
-                                                 true,  // repeat
-                                                 0,     // priority
-                                                 EnterExitRunLoop,
-                                                 &observer_context);
-  CFRunLoopAddObserver(run_loop_, enter_exit_observer_, kCFRunLoopCommonModes);
 }
 
-// Ideally called on the run loop thread.  If other CFRunLoopRun loops were
+// Called by MessagePumpCFRunLoopBase::DoRun.  If other CFRunLoopRun loops were
 // running lower on the run loop thread's stack when this object was created,
-// the same number of CFRunLoopRun loops must be running when this object is
-// destroyed.
-MessagePumpCFRunLoop::~MessagePumpCFRunLoop() {
-  CFRunLoopRemoveObserver(run_loop_, enter_exit_observer_,
-                          kCFRunLoopCommonModes);
-  CFRelease(enter_exit_observer_);
-}
-
-// Called by CFRunLoopBase::DoRun.  If other CFRunLoopRun loops were running
-// lower on the run loop thread's stack when this object was created, the same
-// number of CFRunLoopRun loops must be running for the outermost call to Run.
-// Run/DoRun are reentrant after that point.
+// the same number of CFRunLoopRun loops must be running for the outermost call
+// to Run.  Run/DoRun are reentrant after that point.
 void MessagePumpCFRunLoop::DoRun(Delegate* delegate) {
   // nesting_level_ will be incremented in EnterExitRunLoop, so set
   // innermost_quittable_ accordingly.
@@ -267,32 +361,17 @@ void MessagePumpCFRunLoop::Quit() {
   }
 }
 
-// Called from the run loop.
-// static
-void MessagePumpCFRunLoop::EnterExitRunLoop(CFRunLoopObserverRef observer,
-                                            CFRunLoopActivity activity,
-                                            void* info) {
-  MessagePumpCFRunLoop* self = static_cast<MessagePumpCFRunLoop*>(info);
-
-  switch (activity) {
-    case kCFRunLoopEntry:
-      // If the run loop was entered by a call to Run, this will properly
-      // balance the decrement done in Run before entering the loop.
-      ++self->nesting_level_;
-      break;
-    case kCFRunLoopExit:
-      if (--self->nesting_level_ == self->innermost_quittable_ &&
-          self->quit_pending_) {
-        // Quit was called while loops other than those managed by this object
-        // were running further inside a run loop managed by this object.  Now
-        // that all unmanaged inner run loops are gone, stop the loop running
-        // just inside Run.
-        CFRunLoopStop(self->run_loop_);
-        self->quit_pending_ = false;
-      }
-      break;
-    default:
-      break;
+// Called by MessagePumpCFRunLoopBase::EnterExitObserver.
+void MessagePumpCFRunLoop::EnterExitRunLoop(CFRunLoopActivity activity) {
+  if (activity == kCFRunLoopExit &&
+      nesting_level_ == innermost_quittable_ &&
+      quit_pending_) {
+    // Quit was called while loops other than those managed by this object
+    // were running further inside a run loop managed by this object.  Now
+    // that all unmanaged inner run loops are gone, stop the loop running
+    // just inside Run.
+    CFRunLoopStop(run_loop_);
+    quit_pending_ = false;
   }
 }