Bug 1566783 - Make sure to clear mLastAnchorScrolledTo after calling PresShell::ScrollToAnchor(). r=dholbert, a=RyanVM
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sat, 20 Jul 2019 15:09:07 +0000
changeset 544789 56362f37f5bde34e664a8428a4f12271e70b932e
parent 544788 ed74dcb0d208a1f1c880ac1f23b05b37e81ddd00
child 544790 f30ea3cd87e51258dc48c3cda219f4f21603710e
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, RyanVM
bugs1566783
milestone69.0
Bug 1566783 - Make sure to clear mLastAnchorScrolledTo after calling PresShell::ScrollToAnchor(). r=dholbert, a=RyanVM Seems we can leave this node alive for too long if the user scrolls between domcontentloaded (where GoToAnchor is called) and onload (where ScrollToAnchor() is called). Though it seems we can leave it for too long if we don't end up calling ScrollToAnchor(), the documentation of the method claims that it's cleared unconditionally. Differential Revision: https://phabricator.services.mozilla.com/D38398
layout/base/PresShell.cpp
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -3183,32 +3183,30 @@ nsresult PresShell::GoToAnchor(const nsA
     }
   }
 #endif  // #ifdef ACCESSIBILITY
 
   return rv;
 }
 
 nsresult PresShell::ScrollToAnchor() {
-  if (!mLastAnchorScrolledTo) {
+  nsCOMPtr<nsIContent> lastAnchor = mLastAnchorScrolledTo.forget();
+  if (!lastAnchor) {
     return NS_OK;
   }
+
   NS_ASSERTION(mDidInitialize, "should have done initial reflow by now");
-
   nsIScrollableFrame* rootScroll = GetRootScrollFrameAsScrollable();
   if (!rootScroll ||
       mLastAnchorScrollPositionY != rootScroll->GetScrollPosition().y) {
     return NS_OK;
   }
-  nsCOMPtr<nsIContent> lastAnchorScrollTo = mLastAnchorScrolledTo;
-  nsresult rv = ScrollContentIntoView(
-      lastAnchorScrollTo, ScrollAxis(kScrollToTop, WhenToScroll::Always),
-      ScrollAxis(), ScrollFlags::AnchorScrollFlags);
-  mLastAnchorScrolledTo = nullptr;
-  return rv;
+  return ScrollContentIntoView(lastAnchor,
+                               ScrollAxis(kScrollToTop, WhenToScroll::Always),
+                               ScrollAxis(), ScrollFlags::AnchorScrollFlags);
 }
 
 /*
  * Helper (per-continuation) for ScrollContentIntoView.
  *
  * @param aContainerFrame [in] the frame which aRect is relative to
  * @param aFrame [in] Frame whose bounds should be unioned
  * @param aUseWholeLineHeightForInlines [in] if true, then for inline frames