Bug 1290766 - Return the scroll capture information from the PPluginWidget Create method, instead of using a separate asynchronous method which is delivered later and may race with fast shutdown. r=billm
authorBenjamin Smedberg <benjamin@smedbergs.us>
Wed, 05 Oct 2016 11:36:26 -0400
changeset 316825 baabb29c04ef0d00cf76a2fc19ddf74e0e474252
parent 316824 f4e54ec370fa2b06f912b1329fb4b7e879b3b493
child 316826 a7a43428c1932c98785c9fd696a15d57eb86d5bc
push id30783
push userphilringnalda@gmail.com
push dateFri, 07 Oct 2016 02:58:32 +0000
treeherdermozilla-central@a5b04b518afe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs1290766
milestone52.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 1290766 - Return the scroll capture information from the PPluginWidget Create method, instead of using a separate asynchronous method which is delivered later and may race with fast shutdown. r=billm MozReview-Commit-ID: JJA1VaIuDxL
dom/ipc/PPluginWidget.ipdl
dom/plugins/ipc/PluginWidgetChild.cpp
dom/plugins/ipc/PluginWidgetChild.h
dom/plugins/ipc/PluginWidgetParent.cpp
dom/plugins/ipc/PluginWidgetParent.h
widget/PluginWidgetProxy.cpp
widget/moz.build
--- a/dom/ipc/PPluginWidget.ipdl
+++ b/dom/ipc/PPluginWidget.ipdl
@@ -26,38 +26,36 @@ namespace plugins {
  * be torn down first by the tab, followed by the deref'ing of the nsIWidget
  * via layout.
  */
 sync protocol PPluginWidget {
   manager PBrowser;
 
 parent:
   async __delete__();
-  sync Create() returns (nsresult aResult);
+
+  /**
+   * Used to set the ID of a scroll capture container from the parent process,
+   * so that we can create a proxy container in the layer tree.
+   * @param aScrollCaptureId async container ID of the parent container
+   * @param aPluginInstanceId plugin ID on which to set the scroll capture ID
+   */
+  sync Create() returns (nsresult aResult, uint64_t aScrollCaptureId,
+                         uintptr_t aPluginInstanceId);
   async SetFocus(bool aRaise);
 
   /**
    * Returns NS_NATIVE_PLUGIN_PORT and its variants: a sharable native
    * window for plugins. On Linux, this returns an XID for a socket widget
    * embedded in the chrome side native window. On Windows this returns the
    * native HWND of the plugin widget.
    */
   sync GetNativePluginPort() returns (uintptr_t value);
 
   /**
    * Sends an NS_NATIVE_CHILD_WINDOW to be adopted by the widget's native window
    * on the chrome side. This is only currently used on Windows.
    */
   sync SetNativeChildWindow(uintptr_t childWindow);
-
-child:
-  /**
-   * Used to set the ID of a scroll capture container from the parent process,
-   * so that we can create a proxy container in the layer tree.
-   * @param aScrollCaptureId async container ID of the parent container
-   * @param aPluginInstanceId plugin ID on which to set the scroll capture ID
-   */
-  async SetScrollCaptureId(uint64_t aScrollCaptureId,
-                           uintptr_t aPluginInstanceId);
 };
 
 }
 }
--- a/dom/plugins/ipc/PluginWidgetChild.cpp
+++ b/dom/plugins/ipc/PluginWidgetChild.cpp
@@ -30,35 +30,16 @@ PluginWidgetChild::PluginWidgetChild() :
 }
 
 PluginWidgetChild::~PluginWidgetChild()
 {
   PWLOG("PluginWidgetChild::~PluginWidgetChild()\n");
   MOZ_COUNT_DTOR(PluginWidgetChild);
 }
 
-bool
-PluginWidgetChild::RecvSetScrollCaptureId(const uint64_t& aScrollCaptureId,
-                                          const uintptr_t& aPluginInstanceId)
-{
-#if defined(XP_WIN)
-  PluginInstanceParent* instance =
-    PluginInstanceParent::LookupPluginInstanceByID(aPluginInstanceId);
-  if (instance) {
-    Unused << NS_WARN_IF(NS_FAILED(instance->SetScrollCaptureId(aScrollCaptureId)));
-  }
-
-  return true;
-#else
-  MOZ_ASSERT_UNREACHABLE(
-    "PluginWidgetChild::RecvSetScrollCaptureId calls not expected.");
-  return false;
-#endif
-}
-
 // Called by the proxy widget when it is destroyed by layout. Only gets
 // called once.
 void
 PluginWidgetChild::ProxyShutdown()
 {
   PWLOG("PluginWidgetChild::ProxyShutdown()\n");
   if (mWidget) {
     mWidget = nullptr;
--- a/dom/plugins/ipc/PluginWidgetChild.h
+++ b/dom/plugins/ipc/PluginWidgetChild.h
@@ -14,19 +14,16 @@ class PluginWidgetProxy;
 namespace plugins {
 
 class PluginWidgetChild : public PPluginWidgetChild
 {
 public:
   PluginWidgetChild();
   virtual ~PluginWidgetChild();
 
-  bool RecvSetScrollCaptureId(const uint64_t& aScrollCaptureId,
-                              const uintptr_t& aPluginInstanceId) override;
-
   virtual void ActorDestroy(ActorDestroyReason aWhy) override;
 
   void SetWidget(mozilla::widget::PluginWidgetProxy* aWidget) {
     mWidget = aWidget;
   }
   void ProxyShutdown();
 
 private:
--- a/dom/plugins/ipc/PluginWidgetParent.cpp
+++ b/dom/plugins/ipc/PluginWidgetParent.cpp
@@ -79,20 +79,24 @@ PluginWidgetParent::SetParent(nsIWidget*
 }
 
 // When plugins run in chrome, nsPluginNativeWindow(Plat) implements platform
 // specific functionality that wraps plugin widgets. With e10s we currently
 // bypass this code on Window, and reuse a bit of it on Linux. Content still
 // makes use of some of the utility functions as well.
 
 bool
-PluginWidgetParent::RecvCreate(nsresult* aResult)
+PluginWidgetParent::RecvCreate(nsresult* aResult, uint64_t* aScrollCaptureId,
+                               uintptr_t* aPluginInstanceId)
 {
   PWLOG("PluginWidgetParent::RecvCreate()\n");
 
+  *aScrollCaptureId = 0;
+  *aPluginInstanceId = 0;
+
   mWidget = do_CreateInstance(kWidgetCID, aResult);
   NS_ASSERTION(NS_SUCCEEDED(*aResult), "widget create failure");
 
 #if defined(MOZ_WIDGET_GTK)
   // We need this currently just for GTK in setting up a socket widget
   // we can send over to content -> plugin.
   PLUG_NewPluginNativeWindow((nsPluginNativeWindow**)&mWrapper);
   if (!mWrapper) {
@@ -137,20 +141,19 @@ PluginWidgetParent::RecvCreate(nsresult*
   PWLOG("Plugin XID=%p\n", (void*)mWrapper->window);
 #elif defined(XP_WIN)
   DebugOnly<DWORD> winres =
     ::SetPropW((HWND)mWidget->GetNativeData(NS_NATIVE_WINDOW),
                mozilla::dom::kPluginWidgetContentParentProperty,
                GetTabParent()->Manager()->AsContentParent());
   NS_ASSERTION(winres, "SetPropW call failure");
 
-  uint64_t scrollCaptureId = mWidget->CreateScrollCaptureContainer();
-  uintptr_t pluginId =
+  *aScrollCaptureId = mWidget->CreateScrollCaptureContainer();
+  *aPluginInstanceId =
     reinterpret_cast<uintptr_t>(mWidget->GetNativeData(NS_NATIVE_PLUGIN_ID));
-  Unused << SendSetScrollCaptureId(scrollCaptureId, pluginId);
 #endif
 
   // This is a special call we make to nsBaseWidget to register this
   // window as a remote plugin window which is expected to receive
   // visibility updates from the compositor, which ships this data
   // over with corresponding layer updates.
   mWidget->RegisterPluginWindowForRemoteUpdates();
 
--- a/dom/plugins/ipc/PluginWidgetParent.h
+++ b/dom/plugins/ipc/PluginWidgetParent.h
@@ -24,17 +24,18 @@ namespace plugins {
 
 class PluginWidgetParent : public PPluginWidgetParent
 {
 public:
   PluginWidgetParent();
   virtual ~PluginWidgetParent();
 
   virtual void ActorDestroy(ActorDestroyReason aWhy) override;
-  virtual bool RecvCreate(nsresult* aResult) override;
+  virtual bool RecvCreate(nsresult* aResult, uint64_t* aScrollCaptureId,
+                          uintptr_t* aPluginInstanceId) override;
   virtual bool RecvSetFocus(const bool& aRaise) override;
   virtual bool RecvGetNativePluginPort(uintptr_t* value) override;
   bool RecvSetNativeChildWindow(const uintptr_t& aChildWindow) override;
 
   // Helper for compositor checks on the channel
   bool ActorDestroyed() { return !mWidget; }
 
   // Called by PBrowser when it receives a Destroy() call from the child.
--- a/widget/PluginWidgetProxy.cpp
+++ b/widget/PluginWidgetProxy.cpp
@@ -1,15 +1,16 @@
 /* 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 "PluginWidgetProxy.h"
 #include "mozilla/dom/TabChild.h"
 #include "mozilla/plugins/PluginWidgetChild.h"
+#include "mozilla/plugins/PluginInstanceParent.h"
 #include "nsDebug.h"
 
 #define PWLOG(...)
 // #define PWLOG(...) printf_stderr(__VA_ARGS__)
 
 /* static */
 already_AddRefed<nsIWidget>
 nsIWidget::CreatePluginProxyWidget(TabChild* aTabChild,
@@ -18,16 +19,18 @@ nsIWidget::CreatePluginProxyWidget(TabCh
   nsCOMPtr<nsIWidget> widget =
     new mozilla::widget::PluginWidgetProxy(aTabChild, aActor);
   return widget.forget();
 }
 
 namespace mozilla {
 namespace widget {
 
+using mozilla::plugins::PluginInstanceParent;
+
 NS_IMPL_ISUPPORTS_INHERITED(PluginWidgetProxy, PuppetWidget, nsIWidget)
 
 #define ENSURE_CHANNEL do {                                   \
   if (!mActor) {                                              \
     NS_WARNING("called on an invalid channel.");              \
     return NS_ERROR_FAILURE;                                  \
   }                                                           \
 } while (0)
@@ -52,29 +55,39 @@ PluginWidgetProxy::Create(nsIWidget* aPa
                           nsNativeWidget aNativeParent,
                           const LayoutDeviceIntRect& aRect,
                           nsWidgetInitData* aInitData)
 {
   ENSURE_CHANNEL;
   PWLOG("PluginWidgetProxy::Create()\n");
 
   nsresult rv = NS_ERROR_UNEXPECTED;
-  mActor->SendCreate(&rv);
+  uint64_t scrollCaptureId;
+  uintptr_t pluginInstanceId;
+  mActor->SendCreate(&rv, &scrollCaptureId, &pluginInstanceId);
   if (NS_FAILED(rv)) {
     NS_WARNING("failed to create chrome widget, plugins won't paint.");
     return rv;
   }
 
   BaseCreate(aParent, aInitData);
   mParent = aParent;
 
   mBounds = aRect;
   mEnabled = true;
   mVisible = true;
 
+#if defined(XP_WIN)
+  PluginInstanceParent* instance =
+    PluginInstanceParent::LookupPluginInstanceByID(pluginInstanceId);
+  if (instance) {
+    Unused << NS_WARN_IF(NS_FAILED(instance->SetScrollCaptureId(scrollCaptureId)));
+  }
+#endif
+
   return NS_OK;
 }
 
 NS_IMETHODIMP
 PluginWidgetProxy::SetParent(nsIWidget* aNewParent)
 {
   nsCOMPtr<nsIWidget> kungFuDeathGrip(this);
   nsIWidget* parent = GetParent();
--- a/widget/moz.build
+++ b/widget/moz.build
@@ -157,17 +157,16 @@ UNIFIED_SOURCES += [
     'nsHTMLFormatConverter.cpp',
     'nsIdleService.cpp',
     'nsIWidgetListener.cpp',
     'nsPrimitiveHelpers.cpp',
     'nsPrintSettingsImpl.cpp',
     'nsScreenManagerProxy.cpp',
     'nsTransferable.cpp',
     'nsXPLookAndFeel.cpp',
-    'PluginWidgetProxy.cpp',
     'PuppetBidiKeyboard.cpp',
     'PuppetWidget.cpp',
     'ScreenProxy.cpp',
     'SharedWidgetUtils.cpp',
     'TextEventDispatcher.cpp',
     'VsyncDispatcher.cpp',
     'WidgetEventImpl.cpp',
     'WidgetUtils.cpp',
@@ -189,19 +188,22 @@ if CONFIG['MOZ_XUL'] and CONFIG['NS_PRIN
     UNIFIED_SOURCES += [
         'nsDeviceContextSpecProxy.cpp',
         'nsPrintOptionsImpl.cpp',
         'nsPrintSession.cpp',
     ]
 
 # nsBaseWidget.cpp needs to be built separately because of name clashes in the OS X headers
 # nsBaseDragService.cpp moved out of UNIFIED to fix xgill crash (bug 1259850) after moving widget/ContentHelper -> apz/util/TouchActionHelper
+# PluginWidgetProxy includes MacOS system headers which define a Point struct
+# that conflicts with mozilla::gfx::Point
 SOURCES += [
     'nsBaseDragService.cpp',
-    'nsBaseWidget.cpp'
+    'nsBaseWidget.cpp',
+    'PluginWidgetProxy.cpp',
 ]
 
 if CONFIG['MOZ_INSTRUMENT_EVENT_LOOP']:
     EXPORTS.mozilla += [
         'WidgetTraceEvent.h',
     ]
 
 EXPORTS.ipc = ['nsGUIEventIPC.h']