Bug 1356271 - Make it so that it's easier to define repeating reflows for reflow tests. r?florian draft
authorMike Conley <mconley@mozilla.com>
Mon, 10 Jul 2017 18:07:25 -0400
changeset 618498 264aa5a1bb019c829a2975565c92502a19cf5989
parent 618497 7ad1065d313fc01967bb1e7e586e5900c8f48cd8
child 618499 d61559c8173b1ac41a96a27e414a442d67156182
push id71361
push usermconley@mozilla.com
push dateMon, 31 Jul 2017 15:54:26 +0000
reviewersflorian
bugs1356271
milestone56.0a1
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