Bug 1343795 - Hold a strong ref on the Selection while calling ScrollIntoView with SCROLL_DO_FLUSH. r=smaug a=gchang
authorMats Palmgren <mats@mozilla.com>
Tue, 21 Mar 2017 02:12:41 +0100
changeset 379176 208797caef2fda1028fdd12169273a7eb3e34f15
parent 379175 3d0b8766a2a5f501dc03db93ece6146a18350ab7
child 379177 60a2d0da9096a1132f267d68c47a1facd2ae19a8
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, gchang
bugs1343795
milestone53.0
Bug 1343795 - Hold a strong ref on the Selection while calling ScrollIntoView with SCROLL_DO_FLUSH. r=smaug a=gchang MozReview-Commit-ID: 5C10dmT0bI9
layout/generic/Selection.h
layout/generic/nsSelection.cpp
--- a/layout/generic/Selection.h
+++ b/layout/generic/Selection.h
@@ -91,22 +91,25 @@ public:
   nsresult      PostScrollSelectionIntoViewEvent(
                                         SelectionRegion aRegion,
                                         int32_t aFlags,
                                         nsIPresShell::ScrollAxis aVertical,
                                         nsIPresShell::ScrollAxis aHorizontal);
   enum {
     SCROLL_SYNCHRONOUS = 1<<1,
     SCROLL_FIRST_ANCESTOR_ONLY = 1<<2,
-    SCROLL_DO_FLUSH = 1<<3,
+    SCROLL_DO_FLUSH = 1<<3,  // only matters if SCROLL_SYNCHRONOUS is passed too
     SCROLL_OVERFLOW_HIDDEN = 1<<5,
     SCROLL_FOR_CARET_MOVE = 1<<6
   };
-  // aDoFlush only matters if aIsSynchronous is true.  If not, we'll just flush
-  // when the scroll event fires so we make sure to scroll to the right place.
+  // If aFlags doesn't contain SCROLL_SYNCHRONOUS, then we'll flush when
+  // the scroll event fires so we make sure to scroll to the right place.
+  // Otherwise, if SCROLL_DO_FLUSH is also in aFlags, then this method will
+  // flush layout and you MUST hold a strong ref on 'this' for the duration
+  // of this call.  This might destroy arbitrary layout objects.
   nsresult      ScrollIntoView(SelectionRegion aRegion,
                                nsIPresShell::ScrollAxis aVertical =
                                  nsIPresShell::ScrollAxis(),
                                nsIPresShell::ScrollAxis aHorizontal =
                                  nsIPresShell::ScrollAxis(),
                                int32_t aFlags = 0);
   nsresult      SubtractRange(RangeData* aRange, nsRange* aSubtract,
                               nsTArray<RangeData>* aOutput);
--- a/layout/generic/nsSelection.cpp
+++ b/layout/generic/nsSelection.cpp
@@ -1959,20 +1959,19 @@ nsFrameSelection::ScrollSelectionIntoVie
       nsIPresShell::SCROLL_CENTER, nsIPresShell::SCROLL_IF_NOT_FULLY_VISIBLE);
   }
   if (aFlags & nsISelectionController::SCROLL_FOR_CARET_MOVE) {
     flags |= Selection::SCROLL_FOR_CARET_MOVE;
   }
 
   // After ScrollSelectionIntoView(), the pending notifications might be
   // flushed and PresShell/PresContext/Frames may be dead. See bug 418470.
-  return mDomSelections[index]->ScrollIntoView(aRegion,
-                                               verticalScroll,
-                                               nsIPresShell::ScrollAxis(),
-                                               flags);
+  RefPtr<Selection> sel = mDomSelections[index];
+  return sel->ScrollIntoView(aRegion, verticalScroll,
+                             nsIPresShell::ScrollAxis(), flags);
 }
 
 nsresult
 nsFrameSelection::RepaintSelection(SelectionType aSelectionType)
 {
   int8_t index = GetIndexFromSelectionType(aSelectionType);
   if (index < 0)
     return NS_ERROR_INVALID_ARG;
@@ -6056,16 +6055,18 @@ NS_IMETHODIMP
 Selection::ScrollSelectionIntoViewEvent::Run()
 {
   if (!mSelection)
     return NS_OK;  // event revoked
 
   int32_t flags = Selection::SCROLL_DO_FLUSH |
                   Selection::SCROLL_SYNCHRONOUS;
 
+  Selection* sel = mSelection; // workaround to satisfy static analysis
+  RefPtr<Selection> kungFuDeathGrip(sel);
   mSelection->mScrollEvent.Forget();
   mSelection->ScrollIntoView(mRegion, mVerticalScroll,
                              mHorizontalScroll, mFlags | flags);
   return NS_OK;
 }
 
 nsresult
 Selection::PostScrollSelectionIntoViewEvent(