Bug 1532725 - Make TabParent::Recv{MouseEvent, HandleTap, ..} a script boundary and hold a strong reference. r=bzbarsky
authorRyan Hunt <rhunt@eqrion.net>
Wed, 06 Mar 2019 16:54:58 -0600
changeset 521234 8c8e2862fd5e
parent 521233 e8a962d607cd
child 521235 48241ea6a125
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs1532725
milestone67.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 1532725 - Make TabParent::Recv{MouseEvent, HandleTap, ..} a script boundary and hold a strong reference. r=bzbarsky These message handlers are currently marked as MOZ_CAN_RUN_SCRIPT, but are called from PBrowserChild::OnMessageReceived which is not marked to run script. For some reason this is not currently an issue on tip. I suspect it has something to do with unified builds as renaming files caused this issue to happen. I haven't looked into it enough to say for certain. This commit changes the message handlers to be script boundaries so that the analysis is satisified. This seemed like an easier change than modifying IPDL to emit the script boundary around PBrowserChild::OnMessageReceived. Additionally, Nika pointed out that IPDL doesn't hold a strong reference when calling message handlers. If the script enters a nested event loop, it may be possible for the protocol to be freed before control returns to it. This commit adds strong references to prevent this. Differential Revision: https://phabricator.services.mozilla.com/D22450
dom/ipc/TabChild.cpp
dom/ipc/TabChild.h
--- a/dom/ipc/TabChild.cpp
+++ b/dom/ipc/TabChild.cpp
@@ -1241,16 +1241,20 @@ void TabChild::HandleDoubleTap(const CSS
     mApzcTreeManager->ZoomToRect(guid, zoomToRect, DEFAULT_BEHAVIOR);
   }
 }
 
 mozilla::ipc::IPCResult TabChild::RecvHandleTap(
     const GeckoContentController::TapType& aType,
     const LayoutDevicePoint& aPoint, const Modifiers& aModifiers,
     const ScrollableLayerGuid& aGuid, const uint64_t& aInputBlockId) {
+  // IPDL doesn't hold a strong reference to protocols as they're not required
+  // to be refcounted. This function can run script, which may trigger a nested
+  // event loop, which may release this, so we hold a strong reference here.
+  RefPtr<TabChild> kungFuDeathGrip(this);
   nsCOMPtr<nsIPresShell> presShell = GetPresShell();
   if (!presShell) {
     return IPC_OK();
   }
   if (!presShell->GetPresContext()) {
     return IPC_OK();
   }
   CSSToLayoutDeviceScale scale(
@@ -1286,16 +1290,20 @@ mozilla::ipc::IPCResult TabChild::RecvHa
   }
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult TabChild::RecvNormalPriorityHandleTap(
     const GeckoContentController::TapType& aType,
     const LayoutDevicePoint& aPoint, const Modifiers& aModifiers,
     const ScrollableLayerGuid& aGuid, const uint64_t& aInputBlockId) {
+  // IPDL doesn't hold a strong reference to protocols as they're not required
+  // to be refcounted. This function can run script, which may trigger a nested
+  // event loop, which may release this, so we hold a strong reference here.
+  RefPtr<TabChild> kungFuDeathGrip(this);
   return RecvHandleTap(aType, aPoint, aModifiers, aGuid, aInputBlockId);
 }
 
 bool TabChild::NotifyAPZStateChange(
     const ViewID& aViewId,
     const layers::GeckoContentController::APZStateChange& aChange,
     const int& aArg) {
   mAPZEventState->ProcessAPZStateChange(aViewId, aChange, aArg);
@@ -1373,16 +1381,20 @@ mozilla::ipc::IPCResult TabChild::RecvSt
   IMEStateManager::StopIMEStateManagement();
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult TabChild::RecvMouseEvent(
     const nsString& aType, const float& aX, const float& aY,
     const int32_t& aButton, const int32_t& aClickCount,
     const int32_t& aModifiers, const bool& aIgnoreRootScrollFrame) {
+  // IPDL doesn't hold a strong reference to protocols as they're not required
+  // to be refcounted. This function can run script, which may trigger a nested
+  // event loop, which may release this, so we hold a strong reference here.
+  RefPtr<TabChild> kungFuDeathGrip(this);
   APZCCallbackHelper::DispatchMouseEvent(
       GetPresShell(), aType, CSSPoint(aX, aY), aButton, aClickCount, aModifiers,
       aIgnoreRootScrollFrame, MouseEvent_Binding::MOZ_SOURCE_UNKNOWN,
       0 /* Use the default value here. */);
   return IPC_OK();
 }
 
 void TabChild::ProcessPendingCoalescedMouseDataAndDispatchEvents() {
--- a/dom/ipc/TabChild.h
+++ b/dom/ipc/TabChild.h
@@ -306,17 +306,17 @@ class TabChild final : public TabChildBa
       const mozilla::dom::DimensionInfo& aDimensionInfo) override;
   virtual mozilla::ipc::IPCResult RecvSizeModeChanged(
       const nsSizeMode& aSizeMode) override;
 
   mozilla::ipc::IPCResult RecvActivate();
 
   mozilla::ipc::IPCResult RecvDeactivate();
 
-  MOZ_CAN_RUN_SCRIPT
+  MOZ_CAN_RUN_SCRIPT_BOUNDARY
   virtual mozilla::ipc::IPCResult RecvMouseEvent(
       const nsString& aType, const float& aX, const float& aY,
       const int32_t& aButton, const int32_t& aClickCount,
       const int32_t& aModifiers, const bool& aIgnoreRootScrollFrame) override;
 
   virtual mozilla::ipc::IPCResult RecvRealMouseMoveEvent(
       const mozilla::WidgetMouseEvent& aEvent, const ScrollableLayerGuid& aGuid,
       const uint64_t& aInputBlockId) override;
@@ -571,23 +571,23 @@ class TabChild final : public TabChildBa
   // Call RecvShow(nsIntSize(0, 0)) and block future calls to RecvShow().
   void DoFakeShow(const ShowInfo& aShowInfo);
 
   void ContentReceivedInputBlock(const ScrollableLayerGuid& aGuid,
                                  uint64_t aInputBlockId,
                                  bool aPreventDefault) const;
   void SetTargetAPZC(uint64_t aInputBlockId,
                      const nsTArray<ScrollableLayerGuid>& aTargets) const;
-  MOZ_CAN_RUN_SCRIPT
+  MOZ_CAN_RUN_SCRIPT_BOUNDARY
   mozilla::ipc::IPCResult RecvHandleTap(
       const layers::GeckoContentController::TapType& aType,
       const LayoutDevicePoint& aPoint, const Modifiers& aModifiers,
       const ScrollableLayerGuid& aGuid, const uint64_t& aInputBlockId) override;
 
-  MOZ_CAN_RUN_SCRIPT
+  MOZ_CAN_RUN_SCRIPT_BOUNDARY
   mozilla::ipc::IPCResult RecvNormalPriorityHandleTap(
       const layers::GeckoContentController::TapType& aType,
       const LayoutDevicePoint& aPoint, const Modifiers& aModifiers,
       const ScrollableLayerGuid& aGuid, const uint64_t& aInputBlockId) override;
 
   void SetAllowedTouchBehavior(
       uint64_t aInputBlockId, const nsTArray<TouchBehaviorFlags>& aFlags) const;