Bug 588216: Avoid race between IO-thread loop->PostTask() and main-thread loop->SetNestableTasksAllowed() that led to Tasks being ignored. r=bent

ipc/chromium/src/base/message_loop.cc

@@ -315,7 +315,11 @@ void MessageLoop::SetNestableTasksAllowed(bool allowed) {
     if (!nestable_tasks_allowed_)
       return;
     // Start the native pump if we are not already pumping.
+#ifndef CHROMIUM_MOZILLA_BUILD
     pump_->ScheduleWork();
+#else
+    pump_->ScheduleWorkForNestedLoop();
+#endif
   }
 }
 

ipc/chromium/src/base/message_pump.h

@@ -111,6 +111,16 @@ class MessagePump : public RefCountedThreadSafe<MessagePump> {
   // until it returns a value of false.
   virtual void ScheduleWork() = 0;
 
+#if defined(CHROMIUM_MOZILLA_BUILD)
+  // This method may only called from the thread that called Run.
+  //
+  // Ensure that DoWork will be called if a nested loop is entered.
+  // If a MessagePump can already guarantee that DoWork will be called
+  // "reasonably soon", this method can be a no-op to avoid expensive
+  // atomic tests and/or syscalls required for ScheduleWork().
+  virtual void ScheduleWorkForNestedLoop() { ScheduleWork(); };
+#endif  // defined(CHROMIUM_MOZILLA_BUILD)
+
   // Schedule a DoDelayedWork callback to happen at the specified time,
   // cancelling any pending DoDelayedWork callback.  This method may only be
   // used on the thread that called Run.

ipc/glue/MessagePump.cpp

@@ -52,12 +52,6 @@ using mozilla::ipc::MessagePump;
 using mozilla::ipc::MessagePumpForChildProcess;
 using base::Time;
 
-namespace {
-
-bool gRunningSetNestableTasksAllowed = false;
-
-} /* anonymous namespace */
-
 NS_IMPL_THREADSAFE_ISUPPORTS2(DoWorkRunnable, nsIRunnable, nsITimerCallback)
 
 NS_IMETHODIMP
@@ -68,15 +62,13 @@ DoWorkRunnable::Run()
   if (loop) {
     bool nestableTasksAllowed = loop->NestableTasksAllowed();
 
-    gRunningSetNestableTasksAllowed = true;
+    // MessageLoop::RunTask() disallows nesting, but our Frankenvent
+    // will always dispatch DoWork() below from what looks to
+    // MessageLoop like a nested context.  So we unconditionally allow
+    // nesting here.
     loop->SetNestableTasksAllowed(true);
-    gRunningSetNestableTasksAllowed = false;
-
     loop->DoWork();
-
-    gRunningSetNestableTasksAllowed = true;
     loop->SetNestableTasksAllowed(nestableTasksAllowed);
-    gRunningSetNestableTasksAllowed = false;
   }
   return NS_OK;
 }
@@ -150,10 +142,6 @@ MessagePump::Run(MessagePump::Delegate* aDelegate)
 void
 MessagePump::ScheduleWork()
 {
-  if (gRunningSetNestableTasksAllowed) {
-    return;
-  }
-
   // Make sure the event loop wakes up.
   if (mThread) {
     mThread->Dispatch(mDoWorkEvent, NS_DISPATCH_NORMAL);
@@ -166,6 +154,15 @@ MessagePump::ScheduleWork()
   event_.Signal();
 }
 
+void
+MessagePump::ScheduleWorkForNestedLoop()
+{
+  // This method is called when our MessageLoop has just allowed
+  // nested tasks.  In our setup, whenever that happens we know that
+  // DoWork() will be called "soon", so there's no need to pay the
+  // cost of what will be a no-op nsThread::Dispatch(mDoWorkEvent).
+}
+
 void
 MessagePump::ScheduleDelayedWork(const base::Time& aDelayedTime)
 {

ipc/glue/MessagePump.h

@@ -76,6 +76,7 @@ public:
 
   virtual void Run(base::MessagePump::Delegate* aDelegate);
   virtual void ScheduleWork();
+  virtual void ScheduleWorkForNestedLoop();
   virtual void ScheduleDelayedWork(const base::Time& delayed_work_time);
 
   void DoDelayedWork(base::MessagePump::Delegate* aDelegate);