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, a=ritu
authorBenjamin Smedberg <benjamin@smedbergs.us>
Wed, 05 Oct 2016 11:36:26 -0400
changeset 356080 604ee41918125b1287978e3193a93bc99791ff47
parent 356079 121e2e591c8814bc719b34d77945581b02fbd3d5
child 356081 3fc50786827c0ce826b1bcce33f247b91bf8f141
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm, ritu
bugs1290766
milestone51.0a2
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, a=ritu 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']