Bug 1131375 - Fix message ordering for in-process message manager (r=smaug)

dom/base/SameProcessMessageQueue.cpp

@@ -0,0 +1,72 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "SameProcessMessageQueue.h"
+
+using namespace mozilla;
+using namespace mozilla::dom;
+
+SameProcessMessageQueue* SameProcessMessageQueue::sSingleton;
+
+SameProcessMessageQueue::SameProcessMessageQueue()
+{
+}
+
+SameProcessMessageQueue::~SameProcessMessageQueue()
+{
+  // This code should run during shutdown, and we should already have pumped the
+  // event loop. So we should only see messages here if someone is sending
+  // messages pretty late in shutdown.
+  NS_WARN_IF_FALSE(mQueue.IsEmpty(), "Shouldn't send messages during shutdown");
+  sSingleton = nullptr;
+}
+
+void
+SameProcessMessageQueue::Push(Runnable* aRunnable)
+{
+  mQueue.AppendElement(aRunnable);
+  NS_DispatchToCurrentThread(aRunnable);
+}
+
+void
+SameProcessMessageQueue::Flush()
+{
+  nsTArray<nsRefPtr<Runnable>> queue;
+  mQueue.SwapElements(queue);
+  for (size_t i = 0; i < queue.Length(); i++) {
+    queue[i]->Run();
+  }
+}
+
+/* static */ SameProcessMessageQueue*
+SameProcessMessageQueue::Get()
+{
+  if (!sSingleton) {
+    sSingleton = new SameProcessMessageQueue();
+  }
+  return sSingleton;
+}
+
+SameProcessMessageQueue::Runnable::Runnable()
+ : mDispatched(false)
+{
+}
+
+NS_IMPL_ISUPPORTS(SameProcessMessageQueue::Runnable, nsIRunnable)
+
+nsresult
+SameProcessMessageQueue::Runnable::Run()
+{
+  if (mDispatched) {
+    return NS_OK;
+  }
+
+  SameProcessMessageQueue* queue = SameProcessMessageQueue::Get();
+  queue->mQueue.RemoveElement(this);
+
+  mDispatched = true;
+  return HandleMessage();
+}

dom/base/SameProcessMessageQueue.h

@@ -0,0 +1,57 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_dom_SameProcessMessageQueue_h
+#define mozilla_dom_SameProcessMessageQueue_h
+
+#include "nsIRunnable.h"
+#include "nsRefPtr.h"
+#include "nsTArray.h"
+
+namespace mozilla {
+namespace dom {
+
+class CancelableRunnable;
+
+class SameProcessMessageQueue
+{
+public:
+  SameProcessMessageQueue();
+  virtual ~SameProcessMessageQueue();
+
+  class Runnable : public nsIRunnable
+  {
+  public:
+    explicit Runnable();
+
+    NS_DECL_ISUPPORTS
+    NS_DECL_NSIRUNNABLE
+
+    virtual nsresult HandleMessage() = 0;
+
+  protected:
+    virtual ~Runnable() {}
+
+  private:
+    bool mDispatched;
+  };
+
+  void Push(Runnable* aRunnable);
+  void Flush();
+
+  static SameProcessMessageQueue* Get();
+
+private:
+  friend class CancelableRunnable;
+
+  nsTArray<nsRefPtr<Runnable>> mQueue;
+  static SameProcessMessageQueue* sSingleton;
+};
+
+} // namespace dom
+} // namespace mozilla
+
+#endif // mozilla_dom_SameProcessMessageQueue_h

dom/base/moz.build

@@ -191,6 +191,7 @@ EXPORTS.mozilla.dom += [
     'PerformanceResourceTiming.h',
     'ProcessGlobal.h',
     'ResponsiveImageSelector.h',
+    'SameProcessMessageQueue.h',
     'ScreenOrientation.h',
     'ScriptSettings.h',
     'ShadowRoot.h',
@@ -328,6 +329,7 @@ UNIFIED_SOURCES += [
     'PerformanceResourceTiming.cpp',
     'ProcessGlobal.cpp',
     'ResponsiveImageSelector.cpp',
+    'SameProcessMessageQueue.cpp',
     'ScriptSettings.cpp',
     'ShadowRoot.cpp',
     'StyleSheetList.cpp',

dom/base/nsFrameMessageManager.cpp

@@ -35,6 +35,7 @@
 #include "mozilla/dom/nsIContentParent.h"
 #include "mozilla/dom/PermissionMessageUtils.h"
 #include "mozilla/dom/ProcessGlobal.h"
+#include "mozilla/dom/SameProcessMessageQueue.h"
 #include "mozilla/dom/ScriptSettings.h"
 #include "mozilla/dom/StructuredCloneUtils.h"
 #include "mozilla/dom/ipc/BlobChild.h"
@@ -1749,7 +1750,6 @@ NS_IMPL_ISUPPORTS(nsScriptCacheCleaner, nsIObserver)
 nsFrameMessageManager* nsFrameMessageManager::sChildProcessManager = nullptr;
 nsFrameMessageManager* nsFrameMessageManager::sParentProcessManager = nullptr;
 nsFrameMessageManager* nsFrameMessageManager::sSameProcessParentManager = nullptr;
-nsTArray<nsCOMPtr<nsIRunnable> >* nsFrameMessageManager::sPendingSameProcessAsyncMessages = nullptr;
 
 class nsAsyncMessageToSameProcessChild : public nsSameProcessAsyncMessageBase,
                                          public nsRunnable
@@ -1907,7 +1907,7 @@ public:
 
 
 class nsAsyncMessageToSameProcessParent : public nsSameProcessAsyncMessageBase,
-                                          public nsRunnable
+                                          public SameProcessMessageQueue::Runnable
 {
 public:
   nsAsyncMessageToSameProcessParent(JSContext* aCx,
@@ -1916,25 +1916,15 @@ public:
                                     JS::Handle<JSObject *> aCpows,
                                     nsIPrincipal* aPrincipal)
     : nsSameProcessAsyncMessageBase(aCx, aMessage, aData, aCpows, aPrincipal)
-    , mDelivered(false)
   {
   }
 
-  NS_IMETHOD Run()
+  virtual nsresult HandleMessage() override
   {
-    if (nsFrameMessageManager::sPendingSameProcessAsyncMessages) {
-      nsFrameMessageManager::sPendingSameProcessAsyncMessages->RemoveElement(this);
-    }
-    if (!mDelivered) {
-      mDelivered = true;
-      nsFrameMessageManager* ppm = nsFrameMessageManager::sSameProcessParentManager;
-      ReceiveMessage(static_cast<nsIContentFrameMessageManager*>(ppm), ppm);
-    }
+    nsFrameMessageManager* ppm = nsFrameMessageManager::sSameProcessParentManager;
+    ReceiveMessage(static_cast<nsIContentFrameMessageManager*>(ppm), ppm);
     return NS_OK;
   }
-
-private:
-  bool mDelivered;
 };
 
 /**
@@ -1960,15 +1950,9 @@ public:
                                      InfallibleTArray<nsString>* aJSONRetVal,
                                      bool aIsSync) override
   {
-    nsTArray<nsCOMPtr<nsIRunnable> > asyncMessages;
-    if (nsFrameMessageManager::sPendingSameProcessAsyncMessages) {
-      asyncMessages.SwapElements(*nsFrameMessageManager::sPendingSameProcessAsyncMessages);
-      uint32_t len = asyncMessages.Length();
-      for (uint32_t i = 0; i < len; ++i) {
-        nsCOMPtr<nsIRunnable> async = asyncMessages[i];
-        async->Run();
-      }
-    }
+    SameProcessMessageQueue* queue = SameProcessMessageQueue::Get();
+    queue->Flush();
+
     if (nsFrameMessageManager::sSameProcessParentManager) {
       SameProcessCpowHolder cpows(js::GetRuntime(aCx), aCpows);
       nsRefPtr<nsFrameMessageManager> ppm = nsFrameMessageManager::sSameProcessParentManager;
@@ -1984,13 +1968,10 @@ public:
                                   JS::Handle<JSObject *> aCpows,
                                   nsIPrincipal* aPrincipal) override
   {
-    if (!nsFrameMessageManager::sPendingSameProcessAsyncMessages) {
-      nsFrameMessageManager::sPendingSameProcessAsyncMessages = new nsTArray<nsCOMPtr<nsIRunnable> >;
-    }
-    nsCOMPtr<nsIRunnable> ev =
+    SameProcessMessageQueue* queue = SameProcessMessageQueue::Get();
+    nsRefPtr<nsAsyncMessageToSameProcessParent> ev =
       new nsAsyncMessageToSameProcessParent(aCx, aMessage, aData, aCpows, aPrincipal);
-    nsFrameMessageManager::sPendingSameProcessAsyncMessages->AppendElement(ev);
-    NS_DispatchToCurrentThread(ev);
+    queue->Push(ev);
     return true;
   }
 

dom/base/nsFrameMessageManager.h

@@ -27,6 +27,7 @@
 #include "mozilla/Attributes.h"
 #include "js/RootingAPI.h"
 #include "nsTObserverArray.h"
+#include "mozilla/dom/SameProcessMessageQueue.h"
 #include "mozilla/dom/StructuredCloneUtils.h"
 #include "mozilla/jsipc/CpowHolder.h"
 
@@ -203,8 +204,7 @@ private:
       }
       if (this == sChildProcessManager) {
         sChildProcessManager = nullptr;
-        delete sPendingSameProcessAsyncMessages;
-        sPendingSameProcessAsyncMessages = nullptr;
+        delete mozilla::dom::SameProcessMessageQueue::Get();
       }
       if (this == sSameProcessParentManager) {
         sSameProcessParentManager = nullptr;

dom/base/nsInProcessTabChildGlobal.cpp

@@ -19,6 +19,7 @@
 #include "nsIMozBrowserFrame.h"
 #include "nsDOMClassInfoID.h"
 #include "mozilla/EventDispatcher.h"
+#include "mozilla/dom/SameProcessMessageQueue.h"
 #include "mozilla/dom/StructuredCloneUtils.h"
 #include "js/StructuredClone.h"
 
@@ -35,13 +36,9 @@ nsInProcessTabChildGlobal::DoSendBlockingMessage(JSContext* aCx,
                                                  InfallibleTArray<nsString>* aJSONRetVal,
                                                  bool aIsSync)
 {
-  nsTArray<nsCOMPtr<nsIRunnable> > asyncMessages;
-  asyncMessages.SwapElements(mASyncMessages);
-  uint32_t len = asyncMessages.Length();
-  for (uint32_t i = 0; i < len; ++i) {
-    nsCOMPtr<nsIRunnable> async = asyncMessages[i];
-    async->Run();
-  }
+  SameProcessMessageQueue* queue = SameProcessMessageQueue::Get();
+  queue->Flush();
+
   if (mChromeMessageManager) {
     SameProcessCpowHolder cpows(js::GetRuntime(aCx), aCpows);
     nsRefPtr<nsFrameMessageManager> mm = mChromeMessageManager;
@@ -52,7 +49,7 @@ nsInProcessTabChildGlobal::DoSendBlockingMessage(JSContext* aCx,
 }
 
 class nsAsyncMessageToParent : public nsSameProcessAsyncMessageBase,
-                               public nsRunnable
+                               public SameProcessMessageQueue::Runnable
 {
 public:
   nsAsyncMessageToParent(JSContext* aCx,
@@ -62,25 +59,16 @@ public:
                          JS::Handle<JSObject *> aCpows,
                          nsIPrincipal* aPrincipal)
     : nsSameProcessAsyncMessageBase(aCx, aMessage, aData, aCpows, aPrincipal),
-      mTabChild(aTabChild), mRun(false)
+      mTabChild(aTabChild)
   {
   }
 
-  NS_IMETHOD Run()
+  virtual nsresult HandleMessage() override
   {
-    if (mRun) {
-      return NS_OK;
-    }
-
-    mRun = true;
-    mTabChild->mASyncMessages.RemoveElement(this);
     ReceiveMessage(mTabChild->mOwner, mTabChild->mChromeMessageManager);
     return NS_OK;
   }
   nsRefPtr<nsInProcessTabChildGlobal> mTabChild;
-  // True if this runnable has already been called. This can happen if DoSendSyncMessage
-  // is called while waiting for an asynchronous message send.
-  bool mRun;
 };
 
 bool
@@ -90,10 +78,10 @@ nsInProcessTabChildGlobal::DoSendAsyncMessage(JSContext* aCx,
                                               JS::Handle<JSObject *> aCpows,
                                               nsIPrincipal* aPrincipal)
 {
-  nsCOMPtr<nsIRunnable> ev =
+  SameProcessMessageQueue* queue = SameProcessMessageQueue::Get();
+  nsRefPtr<nsAsyncMessageToParent> ev =
     new nsAsyncMessageToParent(aCx, this, aMessage, aData, aCpows, aPrincipal);
-  mASyncMessages.AppendElement(ev);
-  NS_DispatchToCurrentThread(ev);
+  queue->Push(ev);
   return true;
 }
 

dom/base/nsInProcessTabChildGlobal.h

@@ -168,7 +168,6 @@ protected:
 public:
   nsIContent* mOwner;
   nsFrameMessageManager* mChromeMessageManager;
-  nsTArray<nsCOMPtr<nsIRunnable> > mASyncMessages;
 };
 
 #endif