Bug 1356271 - Make it so that it's easier to define repeating reflows for reflow tests. r=florian
authorMike Conley <mconley@mozilla.com>
Mon, 10 Jul 2017 18:07:25 -0400
changeset 419881 8ec898cb33d5d88e82d5258415c9e751ada10579
parent 419880 33172a0a496ae73cb0b14cd9d65637c692577670
child 419882 54b35cdc1f84b630904271858183b78ff763c39a
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersflorian
bugs1356271
milestone56.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 1356271 - Make it so that it's easier to define repeating reflows for reflow tests. r=florian MozReview-Commit-ID: 5ZL5RtItbiL
browser/base/content/test/performance/browser_appmenu_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_windowopen_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
@@ -5,99 +5,79 @@
  * EXPECTED_APPMENU_OPEN_REFLOWS. This is a whitelist that should slowly go
  * away as we improve the performance of the front-end. Instead of adding more
  * reflows to the whitelist, you should be modifying your code to avoid the reflow.
  *
  * See https://developer.mozilla.org/en-US/Firefox/Performance_best_practices_for_Firefox_fe_engineers
  * for tips on how to do that.
  */
 const EXPECTED_APPMENU_OPEN_REFLOWS = [
-  [
-    "openPopup@chrome://global/content/bindings/popup.xml",
-    "show/</<@chrome://browser/content/customizableui/panelUI.js",
-  ],
-
-  [
-    "get_alignmentPosition@chrome://global/content/bindings/popup.xml",
-    "adjustArrowPosition@chrome://global/content/bindings/popup.xml",
-    "onxblpopupshowing@chrome://global/content/bindings/popup.xml",
-    "openPopup@chrome://global/content/bindings/popup.xml",
-    "show/</<@chrome://browser/content/customizableui/panelUI.js",
-  ],
+  {
+    stack: [
+      "openPopup@chrome://global/content/bindings/popup.xml",
+      "show/</<@chrome://browser/content/customizableui/panelUI.js",
+    ],
+  },
 
-  [
-    "get_alignmentPosition@chrome://global/content/bindings/popup.xml",
-    "adjustArrowPosition@chrome://global/content/bindings/popup.xml",
-    "onxblpopupshowing@chrome://global/content/bindings/popup.xml",
-    "openPopup@chrome://global/content/bindings/popup.xml",
-    "show/</<@chrome://browser/content/customizableui/panelUI.js",
-  ],
+  {
+    stack: [
+      "get_alignmentPosition@chrome://global/content/bindings/popup.xml",
+      "adjustArrowPosition@chrome://global/content/bindings/popup.xml",
+      "onxblpopupshowing@chrome://global/content/bindings/popup.xml",
+      "openPopup@chrome://global/content/bindings/popup.xml",
+      "show/</<@chrome://browser/content/customizableui/panelUI.js",
+    ],
 
-  [
-    "get_alignmentPosition@chrome://global/content/bindings/popup.xml",
-    "adjustArrowPosition@chrome://global/content/bindings/popup.xml",
-    "onxblpopuppositioned@chrome://global/content/bindings/popup.xml",
-  ],
+    times: 2, // This number should only ever go down - never up.
+  },
 
-  [
-    "get_alignmentPosition@chrome://global/content/bindings/popup.xml",
-    "handleEvent@resource:///modules/PanelMultiView.jsm",
-    "openPopup@chrome://global/content/bindings/popup.xml",
-  ],
-
-  [
-    "handleEvent@resource:///modules/PanelMultiView.jsm",
-    "openPopup@chrome://global/content/bindings/popup.xml",
-  ],
-
-  [
-    "handleEvent@resource:///modules/PanelMultiView.jsm",
-    "openPopup@chrome://global/content/bindings/popup.xml",
-  ],
+  {
+    stack: [
+      "get_alignmentPosition@chrome://global/content/bindings/popup.xml",
+      "adjustArrowPosition@chrome://global/content/bindings/popup.xml",
+      "onxblpopuppositioned@chrome://global/content/bindings/popup.xml",
+    ],
+  },
 
-  [
-    "handleEvent@resource:///modules/PanelMultiView.jsm",
-    "openPopup@chrome://global/content/bindings/popup.xml",
-  ],
-
-  [
-    "handleEvent@resource:///modules/PanelMultiView.jsm",
-    "openPopup@chrome://global/content/bindings/popup.xml",
-  ],
+  {
+    stack: [
+      "get_alignmentPosition@chrome://global/content/bindings/popup.xml",
+      "handleEvent@resource:///modules/PanelMultiView.jsm",
+      "openPopup@chrome://global/content/bindings/popup.xml",
+    ],
+  },
 
-  [
-    "handleEvent@resource:///modules/PanelMultiView.jsm",
-    "openPopup@chrome://global/content/bindings/popup.xml",
-  ],
+  {
+    stack: [
+      "handleEvent@resource:///modules/PanelMultiView.jsm",
+      "openPopup@chrome://global/content/bindings/popup.xml",
+    ],
 
-  [
-    "handleEvent@resource:///modules/PanelMultiView.jsm",
-    "openPopup@chrome://global/content/bindings/popup.xml",
-  ],
+    times: 6, // This number should only ever go down - never up.
+  },
 ];
 
 const EXPECTED_APPMENU_SUBVIEW_REFLOWS = [
   /**
    * The synced tabs view has labels that are multiline. Because of bugs in
    * XUL layout relating to multiline text in scrollable containers, we need
    * to manually read their height in order to ensure container heights are
    * correct. Unfortunately this requires 2 sync reflows.
    *
    * If we add more views where this is necessary, we may need to duplicate
    * these expected reflows further.
    */
-  [
-    "descriptionHeightWorkaround@resource:///modules/PanelMultiView.jsm",
-    "onTransitionEnd@resource:///modules/PanelMultiView.jsm",
-  ],
+  {
+    stack: [
+      "descriptionHeightWorkaround@resource:///modules/PanelMultiView.jsm",
+      "onTransitionEnd@resource:///modules/PanelMultiView.jsm",
+    ],
 
-  [
-    "descriptionHeightWorkaround@resource:///modules/PanelMultiView.jsm",
-    "onTransitionEnd@resource:///modules/PanelMultiView.jsm",
-  ],
+    times: 2, // This number should only ever go down - never up.
+  },
 
   /**
    * Please don't add anything new!
    */
 ];
 
 add_task(async function() {
   await ensureNoPreloadedBrowser();
--- a/browser/base/content/test/performance/browser_tabopen_reflows.js
+++ b/browser/base/content/test/performance/browser_tabopen_reflows.js
@@ -10,19 +10,21 @@
  * be modifying your code to avoid the reflow.
  *
  * See https://developer.mozilla.org/en-US/Firefox/Performance_best_practices_for_Firefox_fe_engineers
  * for tips on how to do that.
  */
 const EXPECTED_REFLOWS = [
   // selection change notification may cause querying the focused editor content
   // by IME and that will cause reflow.
-  [
-    "select@chrome://global/content/bindings/textbox.xml",
-  ],
+  {
+    stack: [
+      "select@chrome://global/content/bindings/textbox.xml",
+    ],
+  }
 ];
 
 /*
  * This test ensures that there are no unexpected
  * uninterruptible reflows when opening new tabs.
  */
 add_task(async function() {
   await ensureNoPreloadedBrowser();
--- a/browser/base/content/test/performance/browser_tabopen_squeeze_reflows.js
+++ b/browser/base/content/test/performance/browser_tabopen_squeeze_reflows.js
@@ -5,21 +5,23 @@
  * is a whitelist that should slowly go away as we improve the performance of
  * the front-end. Instead of adding more reflows to the whitelist, you should
  * be modifying your code to avoid the reflow.
  *
  * See https://developer.mozilla.org/en-US/Firefox/Performance_best_practices_for_Firefox_fe_engineers
  * for tips on how to do that.
  */
 const EXPECTED_REFLOWS = [
-  [
-    "select@chrome://global/content/bindings/textbox.xml",
-    "focusAndSelectUrlBar@chrome://browser/content/browser.js",
-    "_adjustFocusAfterTabSwitch@chrome://browser/content/tabbrowser.xml",
-  ],
+  {
+    stack: [
+      "select@chrome://global/content/bindings/textbox.xml",
+      "focusAndSelectUrlBar@chrome://browser/content/browser.js",
+      "_adjustFocusAfterTabSwitch@chrome://browser/content/tabbrowser.xml",
+    ],
+  }
 ];
 
 /*
  * This test ensures that there are no unexpected
  * uninterruptible reflows when opening a new tab that will
  * cause the existing tabs to squeeze smaller.
  */
 add_task(async function() {
--- a/browser/base/content/test/performance/browser_windowopen_reflows.js
+++ b/browser/base/content/test/performance/browser_windowopen_reflows.js
@@ -8,124 +8,104 @@
  * is a whitelist that should slowly go away as we improve the performance of
  * the front-end. Instead of adding more reflows to the whitelist, you should
  * be modifying your code to avoid the reflow.
  *
  * See https://developer.mozilla.org/en-US/Firefox/Performance_best_practices_for_Firefox_fe_engineers
  * for tips on how to do that.
  */
 const EXPECTED_REFLOWS = [
-  [
-    "select@chrome://global/content/bindings/textbox.xml",
-    "focusAndSelectUrlBar@chrome://browser/content/browser.js",
-    "_delayedStartup@chrome://browser/content/browser.js",
-  ],
+  {
+    stack: [
+      "select@chrome://global/content/bindings/textbox.xml",
+      "focusAndSelectUrlBar@chrome://browser/content/browser.js",
+      "_delayedStartup@chrome://browser/content/browser.js",
+    ],
+  },
 ];
 
 if (Services.appinfo.OS == "Linux") {
   if (gMultiProcessBrowser) {
-    EXPECTED_REFLOWS.push(
-      [
+    EXPECTED_REFLOWS.push({
+      stack: [
         "handleEvent@chrome://browser/content/tabbrowser.xml",
         "EventListener.handleEvent*tabbrowser-tabs_XBL_Constructor@chrome://browser/content/tabbrowser.xml",
       ],
-    );
+    });
   } else {
-    EXPECTED_REFLOWS.push(
-      [
+    EXPECTED_REFLOWS.push({
+      stack: [
         "handleEvent@chrome://browser/content/tabbrowser.xml",
         "inferFromText@chrome://browser/content/browser.js",
         "handleEvent@chrome://browser/content/browser.js",
       ],
-    );
+    });
   }
 }
 
 if (Services.appinfo.OS == "Darwin") {
-  EXPECTED_REFLOWS.push(
-    [
+  EXPECTED_REFLOWS.push({
+    stack: [
       "handleEvent@chrome://browser/content/tabbrowser.xml",
       "inferFromText@chrome://browser/content/browser.js",
       "handleEvent@chrome://browser/content/browser.js",
     ],
-  );
+  });
 }
 
 if (Services.appinfo.OS == "WINNT") {
   EXPECTED_REFLOWS.push(
-    [
-      "verticalMargins@chrome://browser/content/browser-tabsintitlebar.js",
-      "_update@chrome://browser/content/browser-tabsintitlebar.js",
-      "updateAppearance@chrome://browser/content/browser-tabsintitlebar.js",
-      "handleEvent@chrome://browser/content/tabbrowser.xml",
-    ],
+    {
+      stack: [
+        "verticalMargins@chrome://browser/content/browser-tabsintitlebar.js",
+        "_update@chrome://browser/content/browser-tabsintitlebar.js",
+        "updateAppearance@chrome://browser/content/browser-tabsintitlebar.js",
+        "handleEvent@chrome://browser/content/tabbrowser.xml",
+      ],
+      times: 2, // This number should only ever go down - never up.
+    },
 
-    [
-      "verticalMargins@chrome://browser/content/browser-tabsintitlebar.js",
-      "_update@chrome://browser/content/browser-tabsintitlebar.js",
-      "updateAppearance@chrome://browser/content/browser-tabsintitlebar.js",
-      "handleEvent@chrome://browser/content/tabbrowser.xml",
-    ],
+    {
+      stack: [
+        "handleEvent@chrome://browser/content/tabbrowser.xml",
+        "inferFromText@chrome://browser/content/browser.js",
+        "handleEvent@chrome://browser/content/browser.js",
+      ],
+    },
 
-    [
-      "handleEvent@chrome://browser/content/tabbrowser.xml",
-      "inferFromText@chrome://browser/content/browser.js",
-      "handleEvent@chrome://browser/content/browser.js",
-    ],
-
-    [
-      "handleEvent@chrome://browser/content/tabbrowser.xml",
-      "EventListener.handleEvent*tabbrowser-tabs_XBL_Constructor@chrome://browser/content/tabbrowser.xml",
-    ],
+    {
+      stack: [
+        "handleEvent@chrome://browser/content/tabbrowser.xml",
+        "EventListener.handleEvent*tabbrowser-tabs_XBL_Constructor@chrome://browser/content/tabbrowser.xml",
+      ],
+    }
   );
 }
 
 if (Services.appinfo.OS == "WINNT" || Services.appinfo.OS == "Darwin") {
   EXPECTED_REFLOWS.push(
-    [
-      "rect@chrome://browser/content/browser-tabsintitlebar.js",
-      "_update@chrome://browser/content/browser-tabsintitlebar.js",
-      "updateAppearance@chrome://browser/content/browser-tabsintitlebar.js",
-      "handleEvent@chrome://browser/content/tabbrowser.xml",
-    ],
-
-    [
-      "rect@chrome://browser/content/browser-tabsintitlebar.js",
-      "_update@chrome://browser/content/browser-tabsintitlebar.js",
-      "updateAppearance@chrome://browser/content/browser-tabsintitlebar.js",
-      "handleEvent@chrome://browser/content/tabbrowser.xml",
-    ],
-
-    [
-      "rect@chrome://browser/content/browser-tabsintitlebar.js",
-      "_update@chrome://browser/content/browser-tabsintitlebar.js",
-      "updateAppearance@chrome://browser/content/browser-tabsintitlebar.js",
-      "handleEvent@chrome://browser/content/tabbrowser.xml",
-    ],
+    {
+      stack: [
+        "rect@chrome://browser/content/browser-tabsintitlebar.js",
+        "_update@chrome://browser/content/browser-tabsintitlebar.js",
+        "updateAppearance@chrome://browser/content/browser-tabsintitlebar.js",
+        "handleEvent@chrome://browser/content/tabbrowser.xml",
+      ],
+      times: 4, // This number should only ever go down - never up.
+    },
 
-    [
-      "rect@chrome://browser/content/browser-tabsintitlebar.js",
-      "_update@chrome://browser/content/browser-tabsintitlebar.js",
-      "updateAppearance@chrome://browser/content/browser-tabsintitlebar.js",
-      "handleEvent@chrome://browser/content/tabbrowser.xml",
-    ],
-
-    [
-      "verticalMargins@chrome://browser/content/browser-tabsintitlebar.js",
-      "_update@chrome://browser/content/browser-tabsintitlebar.js",
-      "updateAppearance@chrome://browser/content/browser-tabsintitlebar.js",
-      "handleEvent@chrome://browser/content/tabbrowser.xml",
-    ],
-
-    [
-      "verticalMargins@chrome://browser/content/browser-tabsintitlebar.js",
-      "_update@chrome://browser/content/browser-tabsintitlebar.js",
-      "updateAppearance@chrome://browser/content/browser-tabsintitlebar.js",
-      "handleEvent@chrome://browser/content/tabbrowser.xml",
-    ],
+    {
+      stack: [
+        "verticalMargins@chrome://browser/content/browser-tabsintitlebar.js",
+        "_update@chrome://browser/content/browser-tabsintitlebar.js",
+        "updateAppearance@chrome://browser/content/browser-tabsintitlebar.js",
+        "handleEvent@chrome://browser/content/tabbrowser.xml",
+      ],
+      times: 2, // This number should only ever go down - never up.
+    }
   );
 }
 
 /*
  * This test ensures that there are no unexpected
  * uninterruptible reflows when opening new windows.
  */
 add_task(async function() {
--- a/browser/base/content/test/performance/head.js
+++ b/browser/base/content/test/performance/head.js
@@ -1,66 +1,78 @@
 /**
  * Async utility function for ensuring that no unexpected uninterruptible
  * reflows occur during some period of time in a window.
  *
  * @param testFn (async function)
  *        The async function that will exercise the browser activity that is
  *        being tested for reflows.
- * @param expectedStacks (Array, optional)
- *        An Array of Arrays representing stacks.
+ * @param expectedReflows (Array, optional)
+ *        An Array of Objects representing reflows.
  *
  *        Example:
  *
  *        [
- *          // This reflow is caused by lorem ipsum
- *          [
- *            "select@chrome://global/content/bindings/textbox.xml",
- *            "focusAndSelectUrlBar@chrome://browser/content/browser.js",
- *            "openLinkIn@chrome://browser/content/utilityOverlay.js",
- *            "openUILinkIn@chrome://browser/content/utilityOverlay.js",
- *            "BrowserOpenTab@chrome://browser/content/browser.js",
- *          ],
+ *          {
+ *            // This reflow is caused by lorem ipsum
+ *            stack: [
+ *              "select@chrome://global/content/bindings/textbox.xml",
+ *              "focusAndSelectUrlBar@chrome://browser/content/browser.js",
+ *              "openLinkIn@chrome://browser/content/utilityOverlay.js",
+ *              "openUILinkIn@chrome://browser/content/utilityOverlay.js",
+ *              "BrowserOpenTab@chrome://browser/content/browser.js",
+ *            ],
+ *            // We expect this particular reflow to happen 2 times
+ *            times: 2,
+ *          },
  *
- *          // This reflow is caused by lorem ipsum
- *          [
- *            "get_scrollPosition@chrome://global/content/bindings/scrollbox.xml",
- *            "_fillTrailingGap@chrome://browser/content/tabbrowser.xml",
- *            "_handleNewTab@chrome://browser/content/tabbrowser.xml",
- *            "onxbltransitionend@chrome://browser/content/tabbrowser.xml",
- *          ],
+ *          {
+ *            // This reflow is caused by lorem ipsum. We expect this reflow
+ *            // to only happen once, so we can omit the "times" property.
+ *            stack: [
+ *              "get_scrollPosition@chrome://global/content/bindings/scrollbox.xml",
+ *              "_fillTrailingGap@chrome://browser/content/tabbrowser.xml",
+ *              "_handleNewTab@chrome://browser/content/tabbrowser.xml",
+ *              "onxbltransitionend@chrome://browser/content/tabbrowser.xml",
+ *            ],
+ *          }
  *
  *        ]
  *
  *        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.
  */
-async function withReflowObserver(testFn, expectedStacks = [], win = window) {
+async function withReflowObserver(testFn, expectedReflows = [], 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're going to remove the reflows 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);
+  // While we're at it, for reflows that omit the "times" property, default
+  // it to 1.
+  expectedReflows = expectedReflows.slice(0);
+  expectedReflows.forEach(r => {
+    r.times = r.times || 1;
+  });
 
   let observer = {
     reflow(start, end) {
       // Gather information about the current code path, slicing out the current
       // frame.
       let path = (new Error().stack).split("\n").slice(1).map(line => {
         return line.replace(/:\d+:\d+$/, "");
       }).join("|");
@@ -71,22 +83,24 @@ async function withReflowObserver(testFn
       dirtyFrameFn();
 
       // Stack trace is empty. Reflow was triggered by native code, which
       // we ignore.
       if (path === "") {
         return;
       }
 
-      let index = expectedStacks.findIndex(stack => path.startsWith(stack.join("|")));
+      let index = expectedReflows.findIndex(reflow => path.startsWith(reflow.stack.join("|")));
 
       if (index != -1) {
         Assert.ok(true, "expected uninterruptible reflow: '" +
                   JSON.stringify(pathWithLineNumbers, null, "\t") + "'");
-        expectedStacks.splice(index, 1);
+        if (--expectedReflows[index].times == 0) {
+          expectedReflows.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, but might as well take the
@@ -104,24 +118,24 @@ async function withReflowObserver(testFn
   docShell.addWeakReflowObserver(observer);
 
   els.addListenerForAllEvents(win, dirtyFrameFn, true);
 
   try {
     dirtyFrameFn();
     await testFn();
   } finally {
-    for (let remainder of expectedStacks) {
+    for (let remainder of expectedReflows) {
       Assert.ok(false,
-                `Unused expected reflow: ${JSON.stringify(remainder, null, "\t")}.\n` +
+                `Unused expected reflow: ${JSON.stringify(remainder.stack, null, "\t")}\n` +
+                `This reflow was supposed to be hit ${remainder.times} more time(s).\n` +
                 "This is probably a good thing - just remove it from the " +
                 "expected list.");
     }
 
-
     els.removeListenerForAllEvents(win, dirtyFrameFn, true);
     docShell.removeWeakReflowObserver(observer);
   }
 }
 
 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