Bug 1363361 - Make reflow tests use nsIDOMWindowUtils.ensureDirtyRootFrame to avoid manual frame dirtying hack. r?florian draft
authorMike Conley <mconley@mozilla.com>
Thu, 29 Jun 2017 08:46:18 -0700
changeset 613232 9322082ca1c33126cfe55d1c4429d81913118be6
parent 613231 829509c4a4f0f50a4c452dd75cdd6f6f09366076
child 613233 ab4d2c0d9e951cecfdf0e900487ed57e9bc006e9
push id69760
push usermconley@mozilla.com
push dateFri, 21 Jul 2017 19:18:20 +0000
reviewersflorian
bugs1363361
milestone56.0a1
Bug 1363361 - Make reflow tests use nsIDOMWindowUtils.ensureDirtyRootFrame to avoid manual frame dirtying hack. r?florian MozReview-Commit-ID: E2azpUdgiSn
browser/base/content/test/performance/browser_appmenu_reflows.js
browser/base/content/test/performance/browser_tabclose_grow_reflows.js
browser/base/content/test/performance/browser_tabclose_reflows.js
browser/base/content/test/performance/browser_tabopen_reflows.js
browser/base/content/test/performance/browser_tabopen_squeeze_reflows.js
browser/base/content/test/performance/browser_tabswitch_reflows.js
browser/base/content/test/performance/head.js
--- a/browser/base/content/test/performance/browser_appmenu_reflows.js
+++ b/browser/base/content/test/performance/browser_appmenu_reflows.js
@@ -113,17 +113,17 @@ add_task(async function() {
   });
 
   // First, open the appmenu.
   await withReflowObserver(async function() {
     let popupPositioned =
       BrowserTestUtils.waitForEvent(PanelUI.panel, "popuppositioned");
     await PanelUI.show();
     await popupPositioned;
-  }, EXPECTED_APPMENU_OPEN_REFLOWS, window, PanelUI.panel);
+  }, EXPECTED_APPMENU_OPEN_REFLOWS);
 
   // Now open a series of subviews, and then close the appmenu. We
   // should not reflow during any of this.
   await withReflowObserver(async function() {
     // This recursive function will take the current main or subview,
     // find all of the buttons that navigate to subviews inside it,
     // and click each one individually. Upon entering the new view,
     // we recurse. When the subviews within a view have been
@@ -155,10 +155,10 @@ add_task(async function() {
       }
     }
 
     await openSubViewsRecursively(PanelUI.mainView);
 
     let hidden = BrowserTestUtils.waitForEvent(PanelUI.panel, "popuphidden");
     PanelUI.hide();
     await hidden;
-  }, EXPECTED_APPMENU_SUBVIEW_REFLOWS, window, PanelUI.panel);
+  }, EXPECTED_APPMENU_SUBVIEW_REFLOWS);
 });
--- a/browser/base/content/test/performance/browser_tabclose_grow_reflows.js
+++ b/browser/base/content/test/performance/browser_tabclose_grow_reflows.js
@@ -31,28 +31,22 @@ add_task(async function() {
 
   // Compute the number of tabs we can put into the strip without
   // overflowing. If we remove one of the tabs, we know that the
   // remaining tabs will grow to fill the remaining space in the
   // tabstrip.
   const TAB_COUNT_FOR_GROWTH = computeMaxTabCount();
   await createTabs(TAB_COUNT_FOR_GROWTH);
 
-  // Because the tab strip is a scrollable frame, we can't use the
-  // default dirtying function from withReflowObserver and reliably
-  // get reflows for the strip. Instead, we provide a node that's
-  // already in the scrollable frame to dirty - in this case, the
-  // original tab.
-  let origTab = gBrowser.selectedTab;
   let lastTab = gBrowser.tabs[gBrowser.tabs.length - 1];
   await BrowserTestUtils.switchTab(gBrowser, lastTab);
 
   await withReflowObserver(async function() {
     let switchDone = BrowserTestUtils.waitForEvent(window, "TabSwitchDone");
     let tab = gBrowser.tabs[gBrowser.tabs.length - 1];
     gBrowser.removeTab(tab, { animate: true });
     await BrowserTestUtils.waitForEvent(tab, "transitionend",
       false, e => e.propertyName === "max-width");
     await switchDone;
-  }, EXPECTED_REFLOWS, window, origTab);
+  }, EXPECTED_REFLOWS);
 
   await removeAllButFirstTab();
 });
--- a/browser/base/content/test/performance/browser_tabclose_reflows.js
+++ b/browser/base/content/test/performance/browser_tabclose_reflows.js
@@ -17,28 +17,21 @@ const EXPECTED_REFLOWS = [
 
 /*
  * This test ensures that there are no unexpected
  * uninterruptible reflows when closing new tabs.
  */
 add_task(async function() {
   await ensureNoPreloadedBrowser();
 
-  // Because the tab strip is a scrollable frame, we can't use the
-  // default dirtying function from withReflowObserver and reliably
-  // get reflows for the strip. Instead, we provide a node that's
-  // already in the scrollable frame to dirty - in this case, the
-  // original tab.
-  let origTab = gBrowser.selectedTab;
-
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser);
   await BrowserTestUtils.waitForCondition(() => tab._fullyOpen);
 
   // Add a reflow observer and open a new tab.
   await withReflowObserver(async function() {
     let switchDone = BrowserTestUtils.waitForEvent(window, "TabSwitchDone");
     gBrowser.removeTab(tab, { animate: true });
     await BrowserTestUtils.waitForEvent(tab, "transitionend",
         false, e => e.propertyName === "max-width");
     await switchDone;
-  }, EXPECTED_REFLOWS, window, origTab);
+  }, EXPECTED_REFLOWS);
   is(EXPECTED_REFLOWS.length, 0, "No reflows are expected when closing a tab");
 });
--- a/browser/base/content/test/performance/browser_tabopen_reflows.js
+++ b/browser/base/content/test/performance/browser_tabopen_reflows.js
@@ -22,28 +22,21 @@ const EXPECTED_REFLOWS = [
 
 /*
  * This test ensures that there are no unexpected
  * uninterruptible reflows when opening new tabs.
  */
 add_task(async function() {
   await ensureNoPreloadedBrowser();
 
-  // Because the tab strip is a scrollable frame, we can't use the
-  // default dirtying function from withReflowObserver and reliably
-  // get reflows for the strip. Instead, we provide a node that's
-  // already in the scrollable frame to dirty - in this case, the
-  // original tab.
-  let origTab = gBrowser.selectedTab;
-
   // Add a reflow observer and open a new tab.
   await withReflowObserver(async function() {
     let switchDone = BrowserTestUtils.waitForEvent(window, "TabSwitchDone");
     BrowserOpenTab();
     await BrowserTestUtils.waitForEvent(gBrowser.selectedTab, "transitionend",
         false, e => e.propertyName === "max-width");
     await switchDone;
-  }, EXPECTED_REFLOWS, window, origTab);
+  }, EXPECTED_REFLOWS);
 
   let switchDone = BrowserTestUtils.waitForEvent(window, "TabSwitchDone");
   await BrowserTestUtils.removeTab(gBrowser.selectedTab);
   await switchDone;
 });
--- a/browser/base/content/test/performance/browser_tabopen_squeeze_reflows.js
+++ b/browser/base/content/test/performance/browser_tabopen_squeeze_reflows.js
@@ -28,25 +28,18 @@ add_task(async function() {
   // Compute the number of tabs we can put into the strip without
   // overflowing, and remove one, so that we can create
   // TAB_COUNT_FOR_SQUEEE tabs, and then one more, which should
   // cause the tab to squeeze to a smaller size rather than overflow.
   const TAB_COUNT_FOR_SQUEEZE = computeMaxTabCount() - 1;
 
   await createTabs(TAB_COUNT_FOR_SQUEEZE);
 
-  // Because the tab strip is a scrollable frame, we can't use the
-  // default dirtying function from withReflowObserver and reliably
-  // get reflows for the strip. Instead, we provide a node that's
-  // already in the scrollable frame to dirty - in this case, the
-  // original tab.
-  let origTab = gBrowser.selectedTab;
-
   await withReflowObserver(async function() {
     let switchDone = BrowserTestUtils.waitForEvent(window, "TabSwitchDone");
     BrowserOpenTab();
     await BrowserTestUtils.waitForEvent(gBrowser.selectedTab, "transitionend",
       false, e => e.propertyName === "max-width");
     await switchDone;
-  }, EXPECTED_REFLOWS, window, origTab);
+  }, EXPECTED_REFLOWS);
 
   await removeAllButFirstTab();
 });
--- a/browser/base/content/test/performance/browser_tabswitch_reflows.js
+++ b/browser/base/content/test/performance/browser_tabswitch_reflows.js
@@ -27,27 +27,21 @@ add_task(async function() {
   await ensureNoPreloadedBrowser();
 
   // At the time of writing, there are no reflows on simple tab switching.
   // Mochitest will fail if we have no assertions, so we add one here
   // to make sure nobody adds any new ones.
   Assert.equal(EXPECTED_REFLOWS.length, 0,
     "We shouldn't have added any new expected reflows.");
 
-  // Because the tab strip is a scrollable frame, we can't use the
-  // default dirtying function from withReflowObserver and reliably
-  // get reflows for the strip. Instead, we provide a node that's
-  // already in the scrollable frame to dirty - in this case, the
-  // original tab.
   let origTab = gBrowser.selectedTab;
-
   let firstSwitchDone = BrowserTestUtils.waitForEvent(window, "TabSwitchDone");
   let otherTab = await BrowserTestUtils.openNewForegroundTab(gBrowser);
   await firstSwitchDone;
 
   await withReflowObserver(async function() {
     let switchDone = BrowserTestUtils.waitForEvent(window, "TabSwitchDone");
     gBrowser.selectedTab = origTab;
     await switchDone;
-  }, EXPECTED_REFLOWS, window, origTab);
+  }, EXPECTED_REFLOWS);
 
   await BrowserTestUtils.removeTab(otherTab);
 });
--- a/browser/base/content/test/performance/head.js
+++ b/browser/base/content/test/performance/head.js
@@ -60,30 +60,27 @@
  *
  *        Note that line numbers are not included in the stacks.
  *
  *        Order of the reflows doesn't matter. Expected reflows that aren't seen
  *        will cause an assertion failure. When this argument is not passed,
  *        it defaults to the empty Array, meaning no reflows are expected.
  * @param window (browser window, optional)
  *        The browser window to monitor. Defaults to the current window.
- * @param elemToDirty (DOM node, optional)
- *        Callers can provide a custom DOM node to change some layout style
- *        on in the event that the action being tested is occurring within
- *        a scrollable frame. Otherwise, withReflowObserver defaults to dirtying
- *        the first element child of the window.
  */
-async function withReflowObserver(testFn, expectedStacks = [], win = window, elemToDirty) {
-  if (!elemToDirty) {
-    elemToDirty = win.document.firstElementChild;
-  }
-
-  let i = 0;
-  let dirtyFrameFn = (e) => {
-    elemToDirty.style.margin = (++i % 4) + "px";
+async function withReflowObserver(testFn, expectedStacks = [], win = window) {
+  let dwu = win.QueryInterface(Ci.nsIInterfaceRequestor)
+               .getInterface(Ci.nsIDOMWindowUtils);
+  let dirtyFrameFn = () => {
+    try {
+      dwu.ensureDirtyRootFrame();
+    } catch (e) {
+      // If this fails, we should probably make note of it, but it's not fatal.
+      info("Note: ensureDirtyRootFrame threw an exception.");
+    }
   };
 
   let els = Cc["@mozilla.org/eventlistenerservice;1"]
               .getService(Ci.nsIEventListenerService);
 
   // We're going to remove the stacks one by one as we see them so that
   // we can check for expected, unseen reflows, so let's clone the array.
   expectedStacks = expectedStacks.slice(0);
@@ -115,17 +112,19 @@ async function withReflowObserver(testFn
         expectedStacks.splice(index, 1);
       } else {
         Assert.ok(false, "unexpected uninterruptible reflow \n" +
                          JSON.stringify(pathWithLineNumbers, null, "\t") + "\n");
       }
     },
 
     reflowInterruptible(start, end) {
-      // We're not interested in interruptible reflows.
+      // We're not interested in interruptible reflows, but might as well take the
+      // opportuntiy to dirty the root frame.
+      dirtyFrameFn();
     },
 
     QueryInterface: XPCOMUtils.generateQI([Ci.nsIReflowObserver,
                                            Ci.nsISupportsWeakReference])
   };
 
   let docShell = win.QueryInterface(Ci.nsIInterfaceRequestor)
                     .getInterface(Ci.nsIWebNavigation)
@@ -143,18 +142,16 @@ async function withReflowObserver(testFn
                 `Unused expected reflow: ${JSON.stringify(remainder, null, "\t")}.\n` +
                 "This is probably a good thing - just remove it from the " +
                 "expected list.");
     }
 
 
     els.removeListenerForAllEvents(win, dirtyFrameFn, true);
     docShell.removeWeakReflowObserver(observer);
-
-    elemToDirty.style.margin = "";
   }
 }
 
 async function ensureNoPreloadedBrowser() {
   // If we've got a preloaded browser, get rid of it so that it
   // doesn't interfere with the test if it's loading. We have to
   // do this before we disable preloading or changing the new tab
   // URL, otherwise _getPreloadedBrowser will return null, despite