Bug 1146339 - Do anchor scrolling right before dispatching popstate/hashchange. r=bz, a=lmandel
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Mon, 23 Mar 2015 22:03:40 -0400
changeset 252099 4d306a83ae1b
parent 252098 2592523e1eb0
child 252100 b8c1a399905d
push id700
push userryanvm@gmail.com
push date2015-03-24 12:29 +0000
treeherdermozilla-release@07c827be741f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, lmandel
bugs1146339
milestone37.0
Bug 1146339 - Do anchor scrolling right before dispatching popstate/hashchange. r=bz, a=lmandel
docshell/base/nsDocShell.cpp
docshell/base/nsDocShell.h
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -4570,18 +4570,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);
@@ -9403,18 +9402,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);
@@ -9853,29 +9850,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
@@ -9953,26 +9937,16 @@ nsDocShell::InternalLoad(nsIURI * aURI,
                     mOSHE->SetPostData(postData);
 
                 // 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) {
@@ -9999,41 +9973,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
@@ -878,17 +878,16 @@ protected:
     bool                       mIsOffScreenBrowser;
     bool                       mIsActive;
     bool                       mIsPrerendered;
     bool                       mIsAppTab;
     bool                       mUseGlobalHistory;
     bool                       mInPrivateBrowsing;
     bool                       mUseRemoteTabs;
     bool                       mDeviceSizeIsPageSize;
-    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