Bug 1354194 - Make browser_windowopen_reflows.js use the reflow test helper and make the whitelist reflect reality. r?florian draft
authorMike Conley <mconley@mozilla.com>
Wed, 03 May 2017 13:32:36 -0400
changeset 580477 47b1f5e860b842bbb0bfa8148a175be54060c9b8
parent 580476 5fcbed77fd7f3b45b478a634b16bdb31aebea615
child 580478 0ac5b83ebd2542f6991d4a8d40dbbab07f5314e6
push id59571
push userbmo:mconley@mozilla.com
push dateThu, 18 May 2017 15:54:00 +0000
reviewersflorian
bugs1354194
milestone55.0a1
Bug 1354194 - Make browser_windowopen_reflows.js use the reflow test helper and make the whitelist reflect reality. r?florian MozReview-Commit-ID: 1mknVjSVIKe
browser/base/content/test/performance/browser.ini
browser/base/content/test/performance/browser_windowopen_reflows.js
--- a/browser/base/content/test/performance/browser.ini
+++ b/browser/base/content/test/performance/browser.ini
@@ -1,7 +1,6 @@
 [DEFAULT]
 support-files =
   head.js
 [browser_tabopen_reflows.js]
 [browser_toolbariconcolor_restyles.js]
 [browser_windowopen_reflows.js]
-skip-if = os == "mac" # bug 1339317
\ No newline at end of file
--- a/browser/base/content/test/performance/browser_windowopen_reflows.js
+++ b/browser/base/content/test/performance/browser_windowopen_reflows.js
@@ -1,117 +1,73 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
+/**
+ * WHOA THERE: We should never be adding new things to EXPECTED_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_REFLOWS = [
-  // handleEvent flushes layout to get the tabstrip width after a resize.
-  "handleEvent@chrome://browser/content/tabbrowser.xml|",
-
-  // Loading a tab causes a reflow.
-  "loadTabs@chrome://browser/content/tabbrowser.xml|" +
-    "loadOneOrMoreURIs@chrome://browser/content/browser.js|" +
-    "_delayedStartup@chrome://browser/content/browser.js|",
+  // Selecting the address bar causes two reflows, unfortunately.
+  [
+    "select@chrome://global/content/bindings/textbox.xml",
+    "focusAndSelectUrlBar@chrome://browser/content/browser.js",
+    "_delayedStartup@chrome://browser/content/browser.js",
+  ],
 
-  // Selecting the address bar causes a reflow.
-  "select@chrome://global/content/bindings/textbox.xml|" +
-    "focusAndSelectUrlBar@chrome://browser/content/browser.js|" +
-    "_delayedStartup@chrome://browser/content/browser.js|",
+  // Selecting the address bar causes two reflows, unfortunately.
+  [
+    "select@chrome://global/content/bindings/textbox.xml",
+    "focusAndSelectUrlBar@chrome://browser/content/browser.js",
+    "_delayedStartup@chrome://browser/content/browser.js",
+  ],
+];
 
-  // Focusing the content area causes a reflow.
-  "_delayedStartup@chrome://browser/content/browser.js|",
-];
+if (Services.appinfo.OS == "Darwin") {
+  // TabsInTitlebar._update causes a reflow on OS X trying to do calculations
+  // since layout info is already dirty. This doesn't seem to happen before
+  // MozAfterPaint on Linux.
+  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",
+    ],
+  );
+}
 
 if (Services.appinfo.OS == "WINNT" || Services.appinfo.OS == "Darwin") {
-  // TabsInTitlebar._update causes a reflow on OS X and Windows trying to do calculations
-  // since layout info is already dirty. This doesn't seem to happen before
-  // MozAfterPaint on Linux.
-  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|");
-}
-
-if (Services.appinfo.OS == "Darwin") {
-  // _onOverflow causes a reflow getting widths.
-  EXPECTED_REFLOWS.push("_onOverflow@resource:///modules/CustomizableUI.jsm|" +
-                        "init@resource:///modules/CustomizableUI.jsm|" +
-                        "observe@resource:///modules/CustomizableUI.jsm|" +
-                        "_delayedStartup@chrome://browser/content/browser.js|");
-  // Same as above since in packaged builds there are no function names and the resource URI includes "app"
-  EXPECTED_REFLOWS.push("@resource://app/modules/CustomizableUI.jsm|" +
-                          "@resource://app/modules/CustomizableUI.jsm|" +
-                          "@resource://app/modules/CustomizableUI.jsm|" +
-                          "_delayedStartup@chrome://browser/content/browser.js|");
+  EXPECTED_REFLOWS.push(
+    [
+      "handleEvent@chrome://browser/content/tabbrowser.xml",
+      "inferFromText@chrome://browser/content/browser.js",
+      "handleEvent@chrome://browser/content/browser.js",
+    ],
+  );
 }
 
 /*
  * This test ensures that there are no unexpected
  * uninterruptible reflows when opening new windows.
  */
-function test() {
-  waitForExplicitFinish();
-
-  // Add a reflow observer and open a new window
+add_task(async function() {
   let win = OpenBrowserWindow();
-  let docShell = win.QueryInterface(Ci.nsIInterfaceRequestor)
-                    .getInterface(Ci.nsIWebNavigation)
-                    .QueryInterface(Ci.nsIDocShell);
-  docShell.addWeakReflowObserver(observer);
-
-  // Wait until the mozafterpaint event occurs.
-  waitForMozAfterPaint(win, function paintListener() {
-    // Remove reflow observer and clean up.
-    docShell.removeWeakReflowObserver(observer);
-    win.close();
-
-    finish();
-  });
-}
-
-var observer = {
-  reflow(start, end) {
-    // Gather information about the current code path.
-    let stack = new Error().stack;
-    let path = stack.split("\n").slice(1).map(line => {
-      return line.replace(/:\d+:\d+$/, "");
-    }).join("|");
-    let pathWithLineNumbers = (new Error().stack).split("\n").slice(1).join("|");
-
-    // Stack trace is empty. Reflow was triggered by native code.
-    if (path === "") {
-      return;
-    }
 
-    // Check if this is an expected reflow.
-    for (let expectedStack of EXPECTED_REFLOWS) {
-      if (path.startsWith(expectedStack) ||
-          // Accept an empty function name for gBrowserInit._delayedStartup or TabsInTitlebar._update to workaround bug 906578.
-          path.startsWith(expectedStack.replace(/(^|\|)(gBrowserInit\._delayedStartup|TabsInTitlebar\._update)@/, "$1@"))) {
-        ok(true, "expected uninterruptible reflow '" + expectedStack + "'");
-        return;
-      }
-    }
-
-    ok(false, "unexpected uninterruptible reflow '" + pathWithLineNumbers + "'");
-  },
-
-  reflowInterruptible(start, end) {
-    // We're not interested in interruptible reflows.
-  },
+  await withReflowObserver(async function() {
+    let resizeEvent = BrowserTestUtils.waitForEvent(win, "resize");
+    let delayedStartup =
+      TestUtils.topicObserved("browser-delayed-startup-finished",
+                              subject => subject == win);
+    await resizeEvent;
+    await delayedStartup;
+  }, EXPECTED_REFLOWS, win);
 
-  QueryInterface: XPCOMUtils.generateQI([Ci.nsIReflowObserver,
-                                         Ci.nsISupportsWeakReference])
-};
+  await BrowserTestUtils.closeWindow(win);
+});
 
-function waitForMozAfterPaint(win, callback) {
-  let dwu = win.QueryInterface(Ci.nsIInterfaceRequestor)
-               .getInterface(Ci.nsIDOMWindowUtils);
-  let lastTransactionId = dwu.lastTransactionId;
-
-  win.addEventListener("MozAfterPaint", function onEnd(event) {
-    if (event.target != win || event.transactionId <= lastTransactionId)
-      return;
-    win.removeEventListener("MozAfterPaint", onEnd);
-    executeSoon(callback);
-  });
-}