Bug 1144988, don't let other pages to load while doing scroll-to-anchor, r=bz, a=RyanVM
☠☠ backed out by 61661f98f3e3 ☠ ☠
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Fri, 20 Mar 2015 00:15:22 +0200
changeset 234485 6d94c4cf9813cd8feac6b3e668439054ae94559c
parent 234484 8c039a4e032de4d22ae98d843e819ca75c9ac520
child 234486 8f257f3baf0c72b9ece851770e877f8ee8c4ba22
push id57157
push useropettay@mozilla.com
push dateThu, 19 Mar 2015 22:28:36 +0000
treeherdermozilla-inbound@6d94c4cf9813 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, RyanVM
bugs1144988
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 1144988, don't let other pages to load while doing scroll-to-anchor, r=bz, a=RyanVM
docshell/base/nsDocShell.cpp
docshell/base/nsDocShell.h
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -4630,17 +4630,18 @@ nsDocShell::IsPrintingOrPP(bool aDisplay
   return mIsPrintingOrPP;
 }
 
 bool
 nsDocShell::IsNavigationAllowed(bool aDisplayPrintErrorDialog,
                                 bool aCheckIfUnloadFired)
 {
   bool isAllowed = !IsPrintingOrPP(aDisplayPrintErrorDialog) &&
-                   (!aCheckIfUnloadFired || !mFiredUnloadEvent);
+                   (!aCheckIfUnloadFired || !mFiredUnloadEvent) &&
+                   !mBlockNavigation;
   if (!isAllowed) {
     return false;
   }
   if (!mContentViewer) {
     return true;
   }
   bool firingBeforeUnload;
   mContentViewer->GetBeforeUnloadFiring(&firingBeforeUnload);
@@ -9994,156 +9995,161 @@ 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);
 
-      // 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
-      // and the network load goes into the SHEntry it expects to.
-      if (JustStartedNetworkLoad() && (aLoadType & LOAD_CMD_NORMAL)) {
-        mLoadType = LOAD_NORMAL_REPLACE;
-      } else {
-        mLoadType = aLoadType;
-      }
-
-      mURIResultedInDocument = true;
-
-      nsCOMPtr<nsISHEntry> oldLSHE = mLSHE;
-
-      /* we need to assign mLSHE to aSHEntry right here, so that on History
-       * loads, SetCurrentURI() called from OnNewURI() will send proper
-       * onLocationChange() notifications to the browser to update
-       * back/forward buttons.
-       */
-      SetHistoryEntry(&mLSHE, aSHEntry);
-
-      /* This is a anchor traversal with in the same page.
-       * call OnNewURI() so that, this traversal will be
-       * recorded in session and global history.
-       */
-      nsCOMPtr<nsISupports> owner;
-      if (mOSHE) {
-        mOSHE->GetOwner(getter_AddRefs(owner));
-      }
-      // Pass true for aCloneSHChildren, since we're not
-      // changing documents here, so all of our subframes are
-      // still relevant to the new session history entry.
-      //
-      // It also makes OnNewURI(...) set LOCATION_CHANGE_SAME_DOCUMENT
-      // flag on firing onLocationChange(...).
-      // Anyway, aCloneSHChildren param is simply reflecting
-      // doShortCircuitedLoad in this scope.
-      OnNewURI(aURI, nullptr, owner, mLoadType, true, true, true);
-
-      nsCOMPtr<nsIInputStream> postData;
-      nsCOMPtr<nsISupports> cacheKey;
-
-      if (mOSHE) {
-        /* save current position of scroller(s) (bug 59774) */
-        mOSHE->SetScrollPosition(cx, cy);
-        // Get the postdata and page ident from the current page, if
-        // the new load is being done via normal means.  Note that
-        // "normal means" can be checked for just by checking for
-        // LOAD_CMD_NORMAL, given the loadType and allowScroll check
-        // above -- it filters out some LOAD_CMD_NORMAL cases that we
-        // wouldn't want here.
-        if (aLoadType & LOAD_CMD_NORMAL) {
-          mOSHE->GetPostData(getter_AddRefs(postData));
-          mOSHE->GetCacheKey(getter_AddRefs(cacheKey));
-
-          // Link our new SHEntry to the old SHEntry's back/forward
-          // cache data, since the two SHEntries correspond to the
-          // same document.
-          if (mLSHE) {
-            mLSHE->AdoptBFCacheEntry(mOSHE);
+      {
+        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
+        // and the network load goes into the SHEntry it expects to.
+        if (JustStartedNetworkLoad() && (aLoadType & LOAD_CMD_NORMAL)) {
+          mLoadType = LOAD_NORMAL_REPLACE;
+        } else {
+          mLoadType = aLoadType;
+        }
+
+        mURIResultedInDocument = true;
+
+        nsCOMPtr<nsISHEntry> oldLSHE = mLSHE;
+
+        /* we need to assign mLSHE to aSHEntry right here, so that on History
+         * loads, SetCurrentURI() called from OnNewURI() will send proper
+         * onLocationChange() notifications to the browser to update
+         * back/forward buttons.
+         */
+        SetHistoryEntry(&mLSHE, aSHEntry);
+
+        /* This is a anchor traversal with in the same page.
+         * call OnNewURI() so that, this traversal will be
+         * recorded in session and global history.
+         */
+        nsCOMPtr<nsISupports> owner;
+        if (mOSHE) {
+          mOSHE->GetOwner(getter_AddRefs(owner));
+        }
+        // Pass true for aCloneSHChildren, since we're not
+        // changing documents here, so all of our subframes are
+        // still relevant to the new session history entry.
+        //
+        // It also makes OnNewURI(...) set LOCATION_CHANGE_SAME_DOCUMENT
+        // flag on firing onLocationChange(...).
+        // Anyway, aCloneSHChildren param is simply reflecting
+        // doShortCircuitedLoad in this scope.
+        OnNewURI(aURI, nullptr, owner, mLoadType, true, true, true);
+
+        nsCOMPtr<nsIInputStream> postData;
+        nsCOMPtr<nsISupports> cacheKey;
+
+        if (mOSHE) {
+          /* save current position of scroller(s) (bug 59774) */
+          mOSHE->SetScrollPosition(cx, cy);
+          // Get the postdata and page ident from the current page, if
+          // the new load is being done via normal means.  Note that
+          // "normal means" can be checked for just by checking for
+          // LOAD_CMD_NORMAL, given the loadType and allowScroll check
+          // above -- it filters out some LOAD_CMD_NORMAL cases that we
+          // wouldn't want here.
+          if (aLoadType & LOAD_CMD_NORMAL) {
+            mOSHE->GetPostData(getter_AddRefs(postData));
+            mOSHE->GetCacheKey(getter_AddRefs(cacheKey));
+
+            // Link our new SHEntry to the old SHEntry's back/forward
+            // cache data, since the two SHEntries correspond to the
+            // same document.
+            if (mLSHE) {
+              mLSHE->AdoptBFCacheEntry(mOSHE);
+            }
           }
         }
-      }
-
-      /* Assign mOSHE to mLSHE. This will either be a new entry created
-       * by OnNewURI() for normal loads or aSHEntry for history loads.
-       */
-      if (mLSHE) {
-        SetHistoryEntry(&mOSHE, mLSHE);
-        // Save the postData obtained from the previous page
-        // in to the session history entry created for the
-        // anchor page, so that any history load of the anchor
-        // page will restore the appropriate postData.
-        if (postData) {
-          mOSHE->SetPostData(postData);
+
+        /* Assign mOSHE to mLSHE. This will either be a new entry created
+         * by OnNewURI() for normal loads or aSHEntry for history loads.
+         */
+        if (mLSHE) {
+          SetHistoryEntry(&mOSHE, mLSHE);
+          // Save the postData obtained from the previous page
+          // in to the session history entry created for the
+          // anchor page, so that any history load of the anchor
+          // page will restore the appropriate postData.
+          if (postData) {
+            mOSHE->SetPostData(postData);
+          }
+
+          // Make sure we won't just repost without hitting the
+          // cache first
+          if (cacheKey) {
+            mOSHE->SetCacheKey(cacheKey);
+          }
         }
 
-        // 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 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) {
-        int32_t index = -1;
-        mSessionHistory->GetIndex(&index);
-        nsCOMPtr<nsISHEntry> shEntry;
-        mSessionHistory->GetEntryAtIndex(index, false, getter_AddRefs(shEntry));
-        NS_ENSURE_TRUE(shEntry, NS_ERROR_FAILURE);
-        shEntry->SetTitle(mTitle);
-      }
-
-      /* Set the title for the Global History entry for this anchor url.
-       */
-      if (mUseGlobalHistory && !mInPrivateBrowsing) {
-        nsCOMPtr<IHistory> history = services::GetHistoryService();
-        if (history) {
-          history->SetURITitle(aURI, mTitle);
-        } else if (mGlobalHistory) {
-          mGlobalHistory->SetPageTitle(aURI, mTitle);
+
+        /* 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) {
+          int32_t index = -1;
+          mSessionHistory->GetIndex(&index);
+          nsCOMPtr<nsISHEntry> shEntry;
+          mSessionHistory->GetEntryAtIndex(index, false, getter_AddRefs(shEntry));
+          NS_ENSURE_TRUE(shEntry, NS_ERROR_FAILURE);
+          shEntry->SetTitle(mTitle);
         }
-      }
-
-      // 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);
+
+        /* Set the title for the Global History entry for this anchor url.
+         */
+        if (mUseGlobalHistory && !mInPrivateBrowsing) {
+          nsCOMPtr<IHistory> history = services::GetHistoryService();
+          if (history) {
+            history->SetURITitle(aURI, mTitle);
+          } else if (mGlobalHistory) {
+            mGlobalHistory->SetPageTitle(aURI, mTitle);
+          }
+        }
+
+        // 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);
+      }
 
       // 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) {
--- a/docshell/base/nsDocShell.h
+++ b/docshell/base/nsDocShell.h
@@ -892,16 +892,17 @@ 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