Bug 1312997 - Store 'aTargetFrame' in 'mCurrentTarget' before doing anything else, then use 'mCurrentTarget' throughout PostHandleEvent. r=smaug a=jcristau
authorMats Palmgren <mats@mozilla.com>
Wed, 30 Nov 2016 01:37:13 +0100
changeset 353043 99348c6bd30ba4d5474012750c753f9efcab1318
parent 353042 3143352cb282da14f176d6895671ff177a1e978e
child 353044 44a934bd6938f1bd2622f216bd20b4d8776ab1a0
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, jcristau
bugs1312997
milestone52.0a2
Bug 1312997 - Store 'aTargetFrame' in 'mCurrentTarget' before doing anything else, then use 'mCurrentTarget' throughout PostHandleEvent. r=smaug a=jcristau This is a safer way of accessing the target frame because 'mCurrentTarget' is a nsWeakFrame which will be nulled out in case the frame is destroyed.
dom/events/EventStateManager.cpp
--- a/dom/events/EventStateManager.cpp
+++ b/dom/events/EventStateManager.cpp
@@ -2903,22 +2903,24 @@ nsresult
 EventStateManager::PostHandleEvent(nsPresContext* aPresContext,
                                    WidgetEvent* aEvent,
                                    nsIFrame* aTargetFrame,
                                    nsEventStatus* aStatus)
 {
   NS_ENSURE_ARG(aPresContext);
   NS_ENSURE_ARG_POINTER(aStatus);
 
-  bool dispatchedToContentProcess = HandleCrossProcessEvent(aEvent,
-                                                            aStatus);
-
   mCurrentTarget = aTargetFrame;
   mCurrentTargetContent = nullptr;
 
+  bool dispatchedToContentProcess = HandleCrossProcessEvent(aEvent, aStatus);
+  // NOTE: the above call may have destroyed aTargetFrame, please use
+  // mCurrentTarget henceforth.  This is to avoid using it accidentally:
+  aTargetFrame = nullptr;
+
   // Most of the events we handle below require a frame.
   // Add special cases here.
   if (!mCurrentTarget && aEvent->mMessage != eMouseUp &&
       aEvent->mMessage != eMouseDown) {
     return NS_OK;
   }
 
   //Keep the prescontext alive, we might need it after event dispatch
@@ -3165,17 +3167,17 @@ EventStateManager::PostHandleEvent(nsPre
     }
     break;
   case eWheelOperationEnd:
     {
       MOZ_ASSERT(aEvent->IsTrusted());
       ScrollbarsForWheel::MayInactivate();
       WidgetWheelEvent* wheelEvent = aEvent->AsWheelEvent();
       nsIScrollableFrame* scrollTarget =
-        do_QueryFrame(ComputeScrollTarget(aTargetFrame, wheelEvent,
+        do_QueryFrame(ComputeScrollTarget(mCurrentTarget, wheelEvent,
                                           COMPUTE_DEFAULT_ACTION_TARGET));
       if (scrollTarget) {
         scrollTarget->ScrollSnap();
       }
     }
     break;
   case eWheel:
   case eWheelOperationStart:
@@ -3188,17 +3190,17 @@ EventStateManager::PostHandleEvent(nsPre
       }
 
       WidgetWheelEvent* wheelEvent = aEvent->AsWheelEvent();
 
       // Check if the frame to scroll before checking the default action
       // because if the scroll target is a plugin, the default action should be
       // chosen by the plugin rather than by our prefs.
       nsIFrame* frameToScroll =
-        ComputeScrollTarget(aTargetFrame, wheelEvent,
+        ComputeScrollTarget(mCurrentTarget, wheelEvent,
                             COMPUTE_DEFAULT_ACTION_TARGET);
       nsPluginFrame* pluginFrame = do_QueryFrame(frameToScroll);
 
       // When APZ is enabled, the actual scroll animation might be handled by
       // the compositor.
       WheelPrefs::Action action;
       if (pluginFrame) {
         MOZ_ASSERT(pluginFrame->WantsToHandleWheelEventAsDefaultAction());
@@ -3208,28 +3210,28 @@ EventStateManager::PostHandleEvent(nsPre
       } else {
         action = WheelPrefs::GetInstance()->ComputeActionFor(wheelEvent);
       }
       switch (action) {
         case WheelPrefs::ACTION_SCROLL: {
           // For scrolling of default action, we should honor the mouse wheel
           // transaction.
 
-          ScrollbarsForWheel::PrepareToScrollText(this, aTargetFrame, wheelEvent);
+          ScrollbarsForWheel::PrepareToScrollText(this, mCurrentTarget, wheelEvent);
 
           if (aEvent->mMessage != eWheel ||
               (!wheelEvent->mDeltaX && !wheelEvent->mDeltaY)) {
             break;
           }
 
           nsIScrollableFrame* scrollTarget = do_QueryFrame(frameToScroll);
           ScrollbarsForWheel::SetActiveScrollTarget(scrollTarget);
 
-          nsIFrame* rootScrollFrame = !aTargetFrame ? nullptr :
-            aTargetFrame->PresContext()->PresShell()->GetRootScrollFrame();
+          nsIFrame* rootScrollFrame = !mCurrentTarget ? nullptr :
+            mCurrentTarget->PresContext()->PresShell()->GetRootScrollFrame();
           nsIScrollableFrame* rootScrollableFrame = nullptr;
           if (rootScrollFrame) {
             rootScrollableFrame = do_QueryFrame(rootScrollFrame);
           }
           if (!scrollTarget || scrollTarget == rootScrollableFrame) {
             wheelEvent->mViewPortIsOverscrolled = true;
           }
           wheelEvent->mOverflowDeltaX = wheelEvent->mDeltaX;
@@ -3256,17 +3258,17 @@ EventStateManager::PostHandleEvent(nsPre
         }
         case WheelPrefs::ACTION_ZOOM: {
           // If this event doesn't cause eLegacyMouseLineOrPageScroll event or
           // the direction is oblique, don't perform zoom in/out.
           int32_t intDelta = wheelEvent->GetPreferredIntDelta();
           if (!intDelta) {
             break;
           }
-          DoScrollZoom(aTargetFrame, intDelta);
+          DoScrollZoom(mCurrentTarget, intDelta);
           break;
         }
         case WheelPrefs::ACTION_SEND_TO_PLUGIN:
           MOZ_ASSERT(pluginFrame);
 
           if (wheelEvent->mMessage != eWheel ||
               (!wheelEvent->mDeltaX && !wheelEvent->mDeltaY)) {
             break;
@@ -3286,17 +3288,17 @@ EventStateManager::PostHandleEvent(nsPre
           bool allDeltaOverflown = false;
           if (wheelEvent->mFlags.mHandledByAPZ) {
             if (wheelEvent->mCanTriggerSwipe) {
               // For events that can trigger swipes, APZ needs to know whether
               // scrolling is possible in the requested direction. It does this
               // by looking at the scroll overflow values on mCanTriggerSwipe
               // events after they have been processed.
               allDeltaOverflown =
-                !ComputeScrollTarget(aTargetFrame, wheelEvent,
+                !ComputeScrollTarget(mCurrentTarget, wheelEvent,
                                      COMPUTE_DEFAULT_ACTION_TARGET);
             }
           } else {
             // The event was processed neither by APZ nor by us, so all of the
             // delta values must be overflown delta values.
             allDeltaOverflown = true;
           }