Bug 1146339 - Do anchor scrolling right before dispatching popstate/hashchange. r=bz
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Mon, 23 Mar 2015 21:54:03 -0400
changeset 265521 7ee9c3fce092eafa3181bb4c551990a6e7a5a2f7
parent 265520 c44d46087f5923c1b978ce9fae94d55ea88ba501
child 265522 794e1729fb0deed5c535ae3177f47e14db6384a5
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1146339
milestone39.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 1146339 - Do anchor scrolling right before dispatching popstate/hashchange. r=bz CLOSED TREE
docshell/base/nsDocShell.cpp
docshell/base/nsDocShell.h
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -4646,18 +4646,17 @@ nsDocShell::IsPrintingOrPP(bool aDisplay
   return mIsPrintingOrPP;
 }
 
 bool
 nsDocShell::IsNavigationAllowed(bool aDisplayPrintErrorDialog,
                                 bool aCheckIfUnloadFired)
 {
   bool isAllowed = !IsPrintingOrPP(aDisplayPrintErrorDialog) &&
-                   (!aCheckIfUnloadFired || !mFiredUnloadEvent) &&
-                   !mBlockNavigation;
+                   (!aCheckIfUnloadFired || !mFiredUnloadEvent);
   if (!isAllowed) {
     return false;
   }
   if (!mContentViewer) {
     return true;
   }
   bool firingBeforeUnload;
   mContentViewer->GetBeforeUnloadFiring(&firingBeforeUnload);
@@ -9558,18 +9557,16 @@ nsDocShell::InternalLoad(nsIURI* aURI,
                          nsISHEntry* aSHEntry,
                          bool aFirstParty,
                          const nsAString& aSrcdoc,
                          nsIDocShell* aSourceDocShell,
                          nsIURI* aBaseURI,
                          nsIDocShell** aDocShell,
                          nsIRequest** aRequest)
 {
-  MOZ_RELEASE_ASSERT(!mBlockNavigation);
-
   nsresult rv = NS_OK;
   mOriginalUriString.Truncate();
 
 #ifdef PR_LOGGING
   if (gDocShellLeakLog && PR_LOG_TEST(gDocShellLeakLog, PR_LOG_DEBUG)) {
     nsAutoCString spec;
     if (aURI) {
       aURI->GetSpec(spec);
@@ -10015,29 +10012,16 @@ nsDocShell::InternalLoad(nsIURI* aURI,
        sameExceptHashes && !newHash.IsEmpty());
 
     if (doShortCircuitedLoad) {
       // Save the position of the scrollers.
       nscoord cx = 0, cy = 0;
       GetCurScrollPos(ScrollOrientation_X, &cx);
       GetCurScrollPos(ScrollOrientation_Y, &cy);
 
-      {
-        AutoRestore<bool> scrollingToAnchor(mBlockNavigation);
-        mBlockNavigation = true;
-
-        // ScrollToAnchor doesn't necessarily cause us to scroll the window;
-        // the function decides whether a scroll is appropriate based on the
-        // arguments it receives.  But even if we don't end up scrolling,
-        // ScrollToAnchor performs other important tasks, such as informing
-        // the presShell that we have a new hash.  See bug 680257.
-        rv = ScrollToAnchor(curHash, newHash, aLoadType);
-        NS_ENSURE_SUCCESS(rv, rv);
-      }
-
       // Reset mLoadType to its original value once we exit this block,
       // because this short-circuited load might have started after a
       // normal, network load, and we don't want to clobber its load type.
       // See bug 737307.
       AutoRestore<uint32_t> loadTypeResetter(mLoadType);
 
       // If a non-short-circuit load (i.e., a network load) is pending,
       // make this a replacement load, so that we don't add a SHEntry here
@@ -10117,26 +10101,16 @@ nsDocShell::InternalLoad(nsIURI* aURI,
 
         // Make sure we won't just repost without hitting the
         // cache first
         if (cacheKey) {
           mOSHE->SetCacheKey(cacheKey);
         }
       }
 
-      /* restore previous position of scroller(s), if we're moving
-       * back in history (bug 59774)
-       */
-      if (mOSHE && (aLoadType == LOAD_HISTORY ||
-                    aLoadType == LOAD_RELOAD_NORMAL)) {
-        nscoord bx, by;
-        mOSHE->GetScrollPosition(&bx, &by);
-        SetCurScrollPosEx(bx, by);
-      }
-
       /* Restore the original LSHE if we were loading something
        * while short-circuited load was initiated.
        */
       SetHistoryEntry(&mLSHE, oldLSHE);
       /* Set the title for the SH entry for this target url. so that
        * SH menus in go/back/forward buttons won't be empty for this.
        */
       if (mSessionHistory) {
@@ -10161,41 +10135,61 @@ nsDocShell::InternalLoad(nsIURI* aURI,
 
       // Set the doc's URI according to the new history entry's URI.
       nsCOMPtr<nsIDocument> doc = GetDocument();
       NS_ENSURE_TRUE(doc, NS_ERROR_FAILURE);
       doc->SetDocumentURI(aURI);
 
       SetDocCurrentStateObj(mOSHE);
 
+      // Inform the favicon service that the favicon for oldURI also
+      // applies to aURI.
+      CopyFavicon(currentURI, aURI, mInPrivateBrowsing);
+
+      nsRefPtr<nsGlobalWindow> win = mScriptGlobal ?
+        mScriptGlobal->GetCurrentInnerWindowInternal() : nullptr;
+
+      // ScrollToAnchor doesn't necessarily cause us to scroll the window;
+      // the function decides whether a scroll is appropriate based on the
+      // arguments it receives.  But even if we don't end up scrolling,
+      // ScrollToAnchor performs other important tasks, such as informing
+      // the presShell that we have a new hash.  See bug 680257.
+      rv = ScrollToAnchor(curHash, newHash, aLoadType);
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      /* restore previous position of scroller(s), if we're moving
+       * back in history (bug 59774)
+       */
+      if (mOSHE && (aLoadType == LOAD_HISTORY ||
+                    aLoadType == LOAD_RELOAD_NORMAL)) {
+        nscoord bx, by;
+        mOSHE->GetScrollPosition(&bx, &by);
+        SetCurScrollPosEx(bx, by);
+      }
+
       // Dispatch the popstate and hashchange events, as appropriate.
       //
       // The event dispatch below can cause us to re-enter script and
       // destroy the docshell, nulling out mScriptGlobal. Hold a stack
       // reference to avoid null derefs. See bug 914521.
-      nsRefPtr<nsGlobalWindow> win = mScriptGlobal;
       if (win) {
         // Fire a hashchange event URIs differ, and only in their hashes.
         bool doHashchange = sameExceptHashes && !curHash.Equals(newHash);
 
         if (historyNavBetweenSameDoc || doHashchange) {
           win->DispatchSyncPopState();
         }
 
         if (doHashchange) {
           // Note that currentURI hasn't changed because it's on the
           // stack, so we can just use it directly as the old URI.
           win->DispatchAsyncHashchange(currentURI, aURI);
         }
       }
 
-      // Inform the favicon service that the favicon for oldURI also
-      // applies to aURI.
-      CopyFavicon(currentURI, aURI, mInPrivateBrowsing);
-
       return NS_OK;
     }
   }
 
   // Check if the webbrowser chrome wants the load to proceed; this can be
   // used to cancel attempts to load URIs in the wrong process.
   nsCOMPtr<nsIWebBrowserChrome3> browserChrome3 = do_GetInterface(mTreeOwner);
   if (browserChrome3) {
--- a/docshell/base/nsDocShell.h
+++ b/docshell/base/nsDocShell.h
@@ -893,17 +893,16 @@ protected:
   bool mIsActive;
   bool mIsPrerendered;
   bool mIsAppTab;
   bool mUseGlobalHistory;
   bool mInPrivateBrowsing;
   bool mUseRemoteTabs;
   bool mDeviceSizeIsPageSize;
   bool mWindowDraggingAllowed;
-  bool mBlockNavigation;
 
   // Because scriptability depends on the mAllowJavascript values of our
   // ancestors, we cache the effective scriptability and recompute it when
   // it might have changed;
   bool mCanExecuteScripts;
   void RecomputeCanExecuteScripts();
 
   // This boolean is set to true right before we fire pagehide and generally