From 14dc9ace94843e7a71483d54bdb1b9028fd198e4 Mon Sep 17 00:00:00 2001 From: Bill McCloskey Date: Tue, 24 Mar 2015 13:05:39 -0700 Subject: [PATCH] Bug 1131375 - Fix message ordering for in-process message manager (r=smaug) --- dom/base/SameProcessMessageQueue.cpp | 72 ++++++++++++++++++++++++++ dom/base/SameProcessMessageQueue.h | 57 ++++++++++++++++++++ dom/base/moz.build | 2 + dom/base/nsFrameMessageManager.cpp | 41 ++++----------- dom/base/nsFrameMessageManager.h | 4 +- dom/base/nsInProcessTabChildGlobal.cpp | 32 ++++-------- dom/base/nsInProcessTabChildGlobal.h | 1 - 7 files changed, 154 insertions(+), 55 deletions(-) create mode 100644 dom/base/SameProcessMessageQueue.cpp create mode 100644 dom/base/SameProcessMessageQueue.h diff --git a/dom/base/SameProcessMessageQueue.cpp b/dom/base/SameProcessMessageQueue.cpp new file mode 100644 index 000000000000..0ae24174d3b7 --- /dev/null +++ b/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> 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(); +} diff --git a/dom/base/SameProcessMessageQueue.h b/dom/base/SameProcessMessageQueue.h new file mode 100644 index 000000000000..eff5fb42b29b --- /dev/null +++ b/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> mQueue; + static SameProcessMessageQueue* sSingleton; +}; + +} // namespace dom +} // namespace mozilla + +#endif // mozilla_dom_SameProcessMessageQueue_h diff --git a/dom/base/moz.build b/dom/base/moz.build index ee5f5fb70f0a..eb1bd3489bdf 100644 --- a/dom/base/moz.build +++ b/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', diff --git a/dom/base/nsFrameMessageManager.cpp b/dom/base/nsFrameMessageManager.cpp index 39a572b2c67f..5ecf0f4d94ad 100644 --- a/dom/base/nsFrameMessageManager.cpp +++ b/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 >* 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 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(ppm), ppm); - } + nsFrameMessageManager* ppm = nsFrameMessageManager::sSameProcessParentManager; + ReceiveMessage(static_cast(ppm), ppm); return NS_OK; } - -private: - bool mDelivered; }; /** @@ -1960,15 +1950,9 @@ public: InfallibleTArray* aJSONRetVal, bool aIsSync) override { - nsTArray > asyncMessages; - if (nsFrameMessageManager::sPendingSameProcessAsyncMessages) { - asyncMessages.SwapElements(*nsFrameMessageManager::sPendingSameProcessAsyncMessages); - uint32_t len = asyncMessages.Length(); - for (uint32_t i = 0; i < len; ++i) { - nsCOMPtr async = asyncMessages[i]; - async->Run(); - } - } + SameProcessMessageQueue* queue = SameProcessMessageQueue::Get(); + queue->Flush(); + if (nsFrameMessageManager::sSameProcessParentManager) { SameProcessCpowHolder cpows(js::GetRuntime(aCx), aCpows); nsRefPtr ppm = nsFrameMessageManager::sSameProcessParentManager; @@ -1984,13 +1968,10 @@ public: JS::Handle aCpows, nsIPrincipal* aPrincipal) override { - if (!nsFrameMessageManager::sPendingSameProcessAsyncMessages) { - nsFrameMessageManager::sPendingSameProcessAsyncMessages = new nsTArray >; - } - nsCOMPtr ev = + SameProcessMessageQueue* queue = SameProcessMessageQueue::Get(); + nsRefPtr ev = new nsAsyncMessageToSameProcessParent(aCx, aMessage, aData, aCpows, aPrincipal); - nsFrameMessageManager::sPendingSameProcessAsyncMessages->AppendElement(ev); - NS_DispatchToCurrentThread(ev); + queue->Push(ev); return true; } diff --git a/dom/base/nsFrameMessageManager.h b/dom/base/nsFrameMessageManager.h index bdd1fa8e1590..3a617f6990e1 100644 --- a/dom/base/nsFrameMessageManager.h +++ b/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; diff --git a/dom/base/nsInProcessTabChildGlobal.cpp b/dom/base/nsInProcessTabChildGlobal.cpp index 2738664b6365..64e200b7879d 100644 --- a/dom/base/nsInProcessTabChildGlobal.cpp +++ b/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* aJSONRetVal, bool aIsSync) { - nsTArray > asyncMessages; - asyncMessages.SwapElements(mASyncMessages); - uint32_t len = asyncMessages.Length(); - for (uint32_t i = 0; i < len; ++i) { - nsCOMPtr async = asyncMessages[i]; - async->Run(); - } + SameProcessMessageQueue* queue = SameProcessMessageQueue::Get(); + queue->Flush(); + if (mChromeMessageManager) { SameProcessCpowHolder cpows(js::GetRuntime(aCx), aCpows); nsRefPtr 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 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 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 aCpows, nsIPrincipal* aPrincipal) { - nsCOMPtr ev = + SameProcessMessageQueue* queue = SameProcessMessageQueue::Get(); + nsRefPtr ev = new nsAsyncMessageToParent(aCx, this, aMessage, aData, aCpows, aPrincipal); - mASyncMessages.AppendElement(ev); - NS_DispatchToCurrentThread(ev); + queue->Push(ev); return true; } diff --git a/dom/base/nsInProcessTabChildGlobal.h b/dom/base/nsInProcessTabChildGlobal.h index 650e84dca480..c15b0f8e16a2 100644 --- a/dom/base/nsInProcessTabChildGlobal.h +++ b/dom/base/nsInProcessTabChildGlobal.h @@ -168,7 +168,6 @@ protected: public: nsIContent* mOwner; nsFrameMessageManager* mChromeMessageManager; - nsTArray > mASyncMessages; }; #endif