Bug 1099410 - [e10s] Fix re-entrant plugin loading (r=bsmedberg)
authorBill McCloskey <wmccloskey@mozilla.com>
Fri, 21 Nov 2014 14:00:55 -0800
changeset 216896 982fd37469cf3dd27896e72acdb34a9cda81d638
parent 216895 5d23df58a42fef07aa81169e7cfd5c0c562f2e27
child 216897 5b1b37605f2e93b019984680eeb41dd3cf06d44d
push id52177
push userwmccloskey@mozilla.com
push dateFri, 21 Nov 2014 22:02:03 +0000
treeherdermozilla-inbound@5f50581c9273 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsmedberg
bugs1099410
milestone36.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1099410 - [e10s] Fix re-entrant plugin loading (r=bsmedberg)
dom/plugins/ipc/PluginModuleParent.cpp
dom/plugins/ipc/PluginModuleParent.h
--- a/dom/plugins/ipc/PluginModuleParent.cpp
+++ b/dom/plugins/ipc/PluginModuleParent.cpp
@@ -92,56 +92,108 @@ mozilla::plugins::SetupBridge(uint32_t a
     nsresult rv = host->GetPluginForContentProcess(aPluginId, getter_AddRefs(plugin));
     if (NS_FAILED(rv)) {
         return false;
     }
     PluginModuleParent* chromeParent = static_cast<PluginModuleParent*>(plugin->GetLibrary());
     return PPluginModule::Bridge(aContentParent, chromeParent);
 }
 
-PluginModuleContentParent* PluginModuleContentParent::sSavedModuleParent;
+// A linked list used to fetch newly created PluginModuleContentParent instances
+// for LoadModule. Each pending LoadModule call owns an element in this list.
+// The element's mModule field is filled in when the new
+// PluginModuleContentParent arrives from chrome.
+struct MOZ_STACK_CLASS SavedPluginModule
+{
+    SavedPluginModule() : mModule(nullptr), mNext(sSavedModuleStack)
+    {
+        sSavedModuleStack = this;
+    }
+    ~SavedPluginModule()
+    {
+        MOZ_ASSERT(sSavedModuleStack == this);
+        sSavedModuleStack = mNext;
+    }
+
+    PluginModuleContentParent* GetModule() { return mModule; }
+
+    // LoadModule can be on the stack multiple times since the intr message it
+    // sends will dispatch arbitrary incoming messages from the chrome process,
+    // which can include new HTTP data. This makes it somewhat tricky to match
+    // up the object created in PluginModuleContentParent::Create with the
+    // LoadModule call that asked for it.
+    //
+    // Each invocation of LoadModule pushes a new SavedPluginModule object on
+    // the sSavedModuleStack stack, with the most recent invocation at the
+    // front. LoadModule messages are always processed by the chrome process in
+    // order, and PluginModuleContentParent allocation messages will also be
+    // received in order. Therefore, we need to match up the first received
+    // PluginModuleContentParent creation message with the first sent LoadModule
+    // call. This call will be the last one in the list that doesn't already
+    // have a module attached to it.
+    static void SaveModule(PluginModuleContentParent* module)
+    {
+        SavedPluginModule* saved = sSavedModuleStack;
+        SavedPluginModule* prev = nullptr;
+        while (saved && !saved->mModule) {
+            prev = saved;
+            saved = saved->mNext;
+        }
+        MOZ_ASSERT(prev);
+        MOZ_ASSERT(!prev->mModule);
+        prev->mModule = module;
+    }
+
+private:
+    PluginModuleContentParent* mModule;
+    SavedPluginModule* mNext;
+
+    static SavedPluginModule* sSavedModuleStack;
+};
+
+SavedPluginModule* SavedPluginModule::sSavedModuleStack;
 
 /* static */ PluginLibrary*
 PluginModuleContentParent::LoadModule(uint32_t aPluginId)
 {
-    MOZ_ASSERT(!sSavedModuleParent);
+    SavedPluginModule saved;
+
     MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_Content);
 
     /*
      * We send a LoadPlugin message to the chrome process using an intr
-     * message. Before it sends its response, it sends a message to create
-     * PluginModuleParent instance. That message is handled by
-     * PluginModuleContentParent::Create, which saves the instance in
-     * sSavedModuleParent. We fetch it from there after LoadPlugin finishes.
+     * message. Before it sends its response, it sends a message to create a
+     * PluginModuleContentParent instance. That message is handled by
+     * PluginModuleContentParent::Create. See SavedPluginModule for details
+     * about how we match up the result of PluginModuleContentParent::Create
+     * with the LoadModule call that requested it.
      */
     dom::ContentChild* cp = dom::ContentChild::GetSingleton();
     if (!cp->CallLoadPlugin(aPluginId)) {
         return nullptr;
     }
 
-    PluginModuleContentParent* parent = sSavedModuleParent;
+    PluginModuleContentParent* parent = saved.GetModule();
     MOZ_ASSERT(parent);
-    sSavedModuleParent = nullptr;
 
     return parent;
 }
 
 /* static */ PluginModuleContentParent*
 PluginModuleContentParent::Create(mozilla::ipc::Transport* aTransport,
                                   base::ProcessId aOtherProcess)
 {
     nsAutoPtr<PluginModuleContentParent> parent(new PluginModuleContentParent());
     ProcessHandle handle;
     if (!base::OpenProcessHandle(aOtherProcess, &handle)) {
         // Bug 1090578 - need to kill |aOtherProcess|, it's boned.
         return nullptr;
     }
 
-    MOZ_ASSERT(!sSavedModuleParent);
-    sSavedModuleParent = parent;
+    SavedPluginModule::SaveModule(parent);
 
     DebugOnly<bool> ok = parent->Open(aTransport, handle, XRE_GetIOMessageLoop(),
                                       mozilla::ipc::ParentSide);
     MOZ_ASSERT(ok);
 
     // Request Windows message deferral behavior on our channel. This
     // applies to the top level and all sub plugin protocols since they
     // all share the same channel.
--- a/dom/plugins/ipc/PluginModuleParent.h
+++ b/dom/plugins/ipc/PluginModuleParent.h
@@ -269,18 +269,16 @@ class PluginModuleContentParent : public
                                              base::ProcessId aOtherProcess);
 
   private:
     explicit PluginModuleContentParent();
 
 #ifdef MOZ_CRASHREPORTER_INJECTOR
     void OnCrash(DWORD processID) MOZ_OVERRIDE {}
 #endif
-
-    static PluginModuleContentParent* sSavedModuleParent;
 };
 
 class PluginModuleChromeParent
     : public PluginModuleParent
     , public mozilla::HangMonitor::Annotator
 {
   public:
     /**