Bug 1379620 - Disable stop-reload animation while a tab is opening or closing. r?dao draft
authorJared Wein <jwein@mozilla.com>
Wed, 26 Jul 2017 11:34:22 -0400
changeset 620431 84ee48d19880621abcac32b8f785756353023c65
parent 619581 52285ea5e54c73d3ed824544cef2ee3f195f05e6
child 640691 265dc7b4f28dc9cd4f0d14ab3bd4fea6613fe7f0
push id72031
push userbmo:jaws@mozilla.com
push dateThu, 03 Aug 2017 12:35:40 +0000
reviewersdao
bugs1379620
milestone57.0a1
Bug 1379620 - Disable stop-reload animation while a tab is opening or closing. r?dao MozReview-Commit-ID: 4rwBQaH7jTR
browser/base/content/browser.js
browser/base/content/tabbrowser.xml
browser/base/content/test/about/browser.ini
browser/base/content/test/about/browser_aboutStopReload.js
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -5048,16 +5048,17 @@ var CombinedStopReload = {
   switchToStop(aRequest, aWebProgress) {
     if (!this._initialized || !this._shouldSwitch(aRequest))
       return;
 
     let shouldAnimate = AppConstants.MOZ_PHOTON_ANIMATIONS &&
                         aRequest instanceof Ci.nsIRequest &&
                         aWebProgress.isTopLevel &&
                         aWebProgress.isLoadingDocument &&
+                        !gBrowser.tabAnimationsInProgress &&
                         this.animate;
 
     this._cancelTransition();
     if (shouldAnimate) {
       BrowserUtils.setToolbarButtonHeightProperty(this.stopReloadContainer);
       this.stopReloadContainer.setAttribute("animate", "true");
     } else {
       this.stopReloadContainer.removeAttribute("animate");
@@ -5069,16 +5070,17 @@ var CombinedStopReload = {
     if (!this._initialized || !this._shouldSwitch(aRequest) ||
         !this.reload.hasAttribute("displaystop"))
       return;
 
     let shouldAnimate = AppConstants.MOZ_PHOTON_ANIMATIONS &&
                         aRequest instanceof Ci.nsIRequest &&
                         aWebProgress.isTopLevel &&
                         !aWebProgress.isLoadingDocument &&
+                        !gBrowser.tabAnimationsInProgress &&
                         this.animate;
 
     if (shouldAnimate) {
       BrowserUtils.setToolbarButtonHeightProperty(this.stopReloadContainer);
       this.stopReloadContainer.setAttribute("animate", "true");
     } else {
       this.stopReloadContainer.removeAttribute("animate");
     }
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -121,16 +121,20 @@
       <field name="_lastFindValue">
         ""
       </field>
 
       <field name="_contentWaitingCount">
         0
       </field>
 
+      <field name="tabAnimationsInProgress">
+        0
+      </field>
+
       <property name="_numPinnedTabs" readonly="true">
         <getter><![CDATA[
           for (var i = 0; i < this.tabs.length; i++) {
             if (!this.tabs[i].pinned)
               break;
           }
           return i;
         ]]></getter>
@@ -2676,16 +2680,20 @@
               if (this._lastRelatedTab)
                 this._lastRelatedTab.owner = null;
               else
                 t.owner = this.selectedTab;
               this.moveTabTo(t, newTabPos);
               this._lastRelatedTab = t;
             }
 
+            // This field is updated regardless if we actually animate
+            // since it's important that we keep this count correct in all cases.
+            this.tabAnimationsInProgress++;
+
             if (animate) {
               requestAnimationFrame(function() {
                 // kick the animation off
                 t.setAttribute("fadein", "true");
               });
             }
 
             return t;
@@ -2967,16 +2975,24 @@
                 // cancels the operation.  We are finished here in both cases.
                 this._windowIsClosing = window.closeWindow(true, window.warnAboutClosingWindow);
                 return false;
               }
 
               newTab = true;
             }
 
+            if (!aTab._fullyOpen) {
+              // If the opening tab animation hasn't finished before we start closing the
+              // tab, decrement the animation count since _handleNewTab will not get called.
+              this.tabAnimationsInProgress--;
+            }
+
+            this.tabAnimationsInProgress++;
+
             // Mute audio immediately to improve perceived speed of tab closure.
             if (!aAdoptedByTab && aTab.hasAttribute("soundplaying")) {
               // Don't persist the muted state as this wasn't a user action.
               // This lets undo-close-tab return it to an unmuted state.
               aTab.linkedBrowser.mute(true);
             }
 
             aTab.closing = true;
@@ -3049,16 +3065,18 @@
             var [aCloseWindow, aNewTab] = aTab._endRemoveArgs;
             aTab._endRemoveArgs = null;
 
             if (this._windowIsClosing) {
               aCloseWindow = false;
               aNewTab = false;
             }
 
+            this.tabAnimationsInProgress--;
+
             this._lastRelatedTab = null;
 
             // update the UI early for responsiveness
             aTab.collapsed = true;
             this._blurTab(aTab);
 
             this._removingTabs.splice(this._removingTabs.indexOf(aTab), 1);
 
@@ -6786,16 +6804,17 @@
       </method>
 
       <method name="_handleNewTab">
         <parameter name="tab"/>
         <body><![CDATA[
           if (tab.parentNode != this)
             return;
           tab._fullyOpen = true;
+          this.tabbrowser.tabAnimationsInProgress--;
 
           this.adjustTabstrip();
 
           if (tab.getAttribute("selected") == "true") {
             this._handleTabSelect();
           } else if (!tab.hasAttribute("skipbackgroundnotify")) {
             this._notifyBackgroundTab(tab);
           }
--- a/browser/base/content/test/about/browser.ini
+++ b/browser/base/content/test/about/browser.ini
@@ -6,17 +6,16 @@ support-files =
   healthreport_testRemoteCommands.html
   print_postdata.sjs
   searchSuggestionEngine.sjs
   searchSuggestionEngine.xml
   test_bug959531.html
   POSTSearchEngine.xml
 
 [browser_aboutCertError.js]
-[browser_aboutStopReload.js]
-run-if = nightly_build # Photon only
-[browser_aboutNetError.js]
-[browser_aboutSupport.js]
-[browser_aboutSupport_newtab_security_state.js]
 [browser_aboutHealthReport.js]
 skip-if = os == "linux" # Bug 924307
 [browser_aboutHome.js]
 [browser_aboutHome_wrapsCorrectly.js]
+[browser_aboutNetError.js]
+[browser_aboutStopReload.js]
+[browser_aboutSupport.js]
+[browser_aboutSupport_newtab_security_state.js]
--- a/browser/base/content/test/about/browser_aboutStopReload.js
+++ b/browser/base/content/test/about/browser_aboutStopReload.js
@@ -55,36 +55,60 @@ add_task(async function checkDontShowSto
   await BrowserTestUtils.removeTab(tab);
 
   Assert.ok(true, "Test finished: stop-reload does not animate when navigating to local URI from non-local URI");
   stopReloadContainerObserver.disconnect();
 });
 
 add_task(async function checkDoShowStopOnNewTab() {
   let stopReloadContainer = document.getElementById("stop-reload-button");
-  let animatePromise = getAnimatePromise(stopReloadContainer);
+  let reloadButton = document.getElementById("reload-button");
+  let stopPromise = BrowserTestUtils.waitForAttribute("displaystop", reloadButton);
 
   await waitForNoAnimation(stopReloadContainer);
+
   let tab = await BrowserTestUtils.openNewForegroundTab({gBrowser,
                                                         opening: "https://example.com",
                                                         waitForStateStop: true});
-  await animatePromise;
+  await stopPromise;
   await waitForNoAnimation(stopReloadContainer);
   await BrowserTestUtils.removeTab(tab);
 
-  info("Test finished: stop-reload animates when navigating to non-local URI on new tab");
+  info("Test finished: stop-reload shows stop when navigating to non-local URI during tab opening");
+});
+
+add_task(async function checkAnimateStopOnTabAfterTabFinishesOpening() {
+  let stopReloadContainer = document.getElementById("stop-reload-button");
+
+  await waitForNoAnimation(stopReloadContainer);
+  let tab = await BrowserTestUtils.openNewForegroundTab({gBrowser,
+                                                        waitForStateStop: true});
+  await BrowserTestUtils.waitForCondition(() => {
+    info("Waiting for tabAnimationsInProgress to equal 0, currently " + gBrowser.tabAnimationsInProgress);
+    return !gBrowser.tabAnimationsInProgress;
+  });
+  let animatePromise = getAnimatePromise(stopReloadContainer);
+  await BrowserTestUtils.loadURI(tab.linkedBrowser, "https://example.com");
+  await animatePromise;
+  await BrowserTestUtils.removeTab(tab);
+
+  info("Test finished: stop-reload animates when navigating to non-local URI on new tab after tab has opened");
 });
 
 add_task(async function checkDoShowStopFromLocalURI() {
   let stopReloadContainer = document.getElementById("stop-reload-button");
 
   await waitForNoAnimation(stopReloadContainer);
   let tab = await BrowserTestUtils.openNewForegroundTab({gBrowser,
                                                         opening: "about:robots",
                                                         waitForStateStop: true});
+  await BrowserTestUtils.waitForCondition(() => {
+    info("Waiting for tabAnimationsInProgress to equal 0, currently " + gBrowser.tabAnimationsInProgress);
+    return !gBrowser.tabAnimationsInProgress;
+  });
   let animatePromise = getAnimatePromise(stopReloadContainer);
   await BrowserTestUtils.loadURI(tab.linkedBrowser, "https://example.com");
   await animatePromise;
   await waitForNoAnimation(stopReloadContainer);
   await BrowserTestUtils.removeTab(tab);
 
   info("Test finished: stop-reload animates when navigating to non-local URI from local URI");
 });