Bug 1343795 - Hold a strong ref on the Selection while calling ScrollIntoView with SCROLL_DO_FLUSH. r=smaug
☠☠ backed out by c9812b88b9ed ☠ ☠
authorMats Palmgren <mats@mozilla.com>
Mon, 20 Mar 2017 18:21:00 -0400
changeset 348481 b4e95d147909540f65d41a921e57c510bdc22bc5
parent 348480 c2f5bc87d8cd577a13a96a8a4c50cd482600a6cf
child 348482 5ef67e15fd88a16025632e31ec1bb6519d2f4441
push id88243
push userryanvm@gmail.com
push dateMon, 20 Mar 2017 22:28:57 +0000
treeherdermozilla-inbound@5ef67e15fd88 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1343795
milestone55.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 1343795 - Hold a strong ref on the Selection while calling ScrollIntoView with SCROLL_DO_FLUSH. r=smaug
layout/generic/Selection.h
layout/generic/nsSelection.cpp
--- a/layout/generic/Selection.h
+++ b/layout/generic/Selection.h
@@ -96,22 +96,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
@@ -1969,20 +1969,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;
@@ -6166,16 +6165,17 @@ NS_IMETHODIMP
 Selection::ScrollSelectionIntoViewEvent::Run()
 {
   if (!mSelection)
     return NS_OK;  // event revoked
 
   int32_t flags = Selection::SCROLL_DO_FLUSH |
                   Selection::SCROLL_SYNCHRONOUS;
 
+  RefPtr<Selection> kungFuDeathGrip = mSelection;
   mSelection->mScrollEvent.Forget();
   mSelection->ScrollIntoView(mRegion, mVerticalScroll,
                              mHorizontalScroll, mFlags | flags);
   return NS_OK;
 }
 
 nsresult
 Selection::PostScrollSelectionIntoViewEvent(