Bug 1354194 - Add utility method for more easily writing reflow tests. r?florian draft
authorMike Conley <mconley@mozilla.com>
Tue, 02 May 2017 17:30:18 -0400
changeset 580458 132da170a7aa79182a8607960f15d5b475f96017
parent 579653 85e5d15c31691c89b82d6068c26260416493071f
child 580459 6065e638e09dc16916977189717543329e8d3dd4
child 580476 5fcbed77fd7f3b45b478a634b16bdb31aebea615
push id59561
push userbmo:mconley@mozilla.com
push dateThu, 18 May 2017 15:10:56 +0000
reviewersflorian
bugs1354194
milestone55.0a1
Bug 1354194 - Add utility method for more easily writing reflow tests. r?florian This also consolidates the pre-existing style and layout flush regression tests into the same performance folder. MozReview-Commit-ID: IA3FqroG75O
browser/base/content/test/general/browser.ini
browser/base/content/test/general/browser_tabopen_reflows.js
browser/base/content/test/general/browser_windowopen_reflows.js
browser/base/content/test/performance/.eslintrc.js
browser/base/content/test/performance/browser.ini
browser/base/content/test/performance/browser_tabopen_reflows.js
browser/base/content/test/performance/browser_toolbariconcolor_restyles.js
browser/base/content/test/performance/browser_windowopen_reflows.js
browser/base/content/test/performance/head.js
browser/base/content/test/windows/.eslintrc.js
browser/base/content/test/windows/browser.ini
browser/base/content/test/windows/browser_toolbariconcolor_restyles.js
browser/base/moz.build
--- a/browser/base/content/test/general/browser.ini
+++ b/browser/base/content/test/general/browser.ini
@@ -501,18 +501,16 @@ skip-if = buildapp == 'mulet' || (e10s &
 skip-if = os == "linux" || os == "mac" # No tabs in titlebar on linux
                                        # Disabled on OS X because of bug 967917
 # DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
 [browser_tabfocus.js]
 # DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
 [browser_tabkeynavigation.js]
 skip-if = (os == "mac" && !e10s) # Bug 1237713 - OSX eats keypresses for some reason
 # DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
-[browser_tabopen_reflows.js]
-# DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
 [browser_tabs_close_beforeunload.js]
 support-files =
   close_beforeunload_opens_second_tab.html
   close_beforeunload.html
 # DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
 [browser_tabs_isActive.js]
 # DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
 [browser_tabs_owner.js]
@@ -579,19 +577,16 @@ skip-if = true # Bug 1005420 - fails int
 # DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
 [browser_visibleTabs_contextMenu.js]
 # DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
 [browser_visibleTabs_tabPreview.js]
 skip-if = (os == "win" && !debug)
 # DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
 [browser_web_channel.js]
 # DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
-[browser_windowopen_reflows.js]
-skip-if = os == "mac" # bug 1339317
-# DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
 [browser_zbug569342.js]
 skip-if = e10s || debug # Bug 1094240 - has findbar-related failures
 # DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
 [browser_registerProtocolHandler_notification.js]
 # DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
 [browser_addCertException.js]
 # DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
 [browser_e10s_about_page_triggeringprincipal.js]
rename from browser/base/content/test/windows/.eslintrc.js
rename to browser/base/content/test/performance/.eslintrc.js
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/performance/browser.ini
@@ -0,0 +1,7 @@
+[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
rename from browser/base/content/test/general/browser_tabopen_reflows.js
rename to browser/base/content/test/performance/browser_tabopen_reflows.js
rename from browser/base/content/test/windows/browser_toolbariconcolor_restyles.js
rename to browser/base/content/test/performance/browser_toolbariconcolor_restyles.js
rename from browser/base/content/test/general/browser_windowopen_reflows.js
rename to browser/base/content/test/performance/browser_windowopen_reflows.js
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/performance/head.js
@@ -0,0 +1,155 @@
+/**
+ * Async utility function for ensuring that no unexpected uninterruptible
+ * reflows occur during some period of time in a window.
+ *
+ * The helper works by running a JS function before each event is
+ * dispatched that attempts to dirty the layout tree - the idea being
+ * that this puts us in the "worst case scenario" so that any JS
+ * that attempts to query for layout or style information will cause
+ * a reflow to fire. We also dirty the layout tree after each reflow
+ * occurs, for good measure.
+ *
+ * This sounds good in theory, but it's trickier in practice due to
+ * various optimizations in our Layout engine. The default function
+ * for dirtying the layout tree adds a margin to the first element
+ * child it finds in the window to a maximum of 3px, and then goes
+ * back to 0px again and loops.
+ *
+ * This is not sufficient for reflows that we expect to happen within
+ * scrollable frames, as Gecko is able to side-step reflowing the
+ * contents of a scrollable frame if outer frames are dirtied. Because
+ * of this, it's currently possible to override the default node to
+ * dirty with one more appropriate for the test.
+ *
+ * It is also theoretically possible for enough events to fire between
+ * reflows such that the before and after state of the layout tree is
+ * exactly the same, meaning that no reflow is required, which opens
+ * us up to missing expected reflows. This seems to be possible in
+ * theory, but hasn't yet shown up in practice - it's just something
+ * to be aware of.
+ *
+ * Bug 1363361 has been filed for a more reliable way of dirtying layout.
+ *
+ * @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.
+ *
+ *        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
+ *          [
+ *            "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.
+ * @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";
+  };
+
+  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);
+
+  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("|");
+
+      let pathWithLineNumbers = (new Error().stack).split("\n").slice(1);
+
+      // Just in case, dirty the frame now that we've reflowed.
+      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("|")));
+
+      if (index != -1) {
+        Assert.ok(true, "expected uninterruptible reflow: '" +
+                  JSON.stringify(pathWithLineNumbers, null, "\t") + "'");
+        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.
+    },
+
+    QueryInterface: XPCOMUtils.generateQI([Ci.nsIReflowObserver,
+                                           Ci.nsISupportsWeakReference])
+  };
+
+  let docShell = win.QueryInterface(Ci.nsIInterfaceRequestor)
+                    .getInterface(Ci.nsIWebNavigation)
+                    .QueryInterface(Ci.nsIDocShell);
+  docShell.addWeakReflowObserver(observer);
+
+  els.addListenerForAllEvents(win, dirtyFrameFn, true);
+
+  try {
+    dirtyFrameFn();
+    await testFn();
+  } finally {
+    for (let remainder of expectedStacks) {
+      Assert.ok(false,
+                `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 = "";
+  }
+}
+
deleted file mode 100644
--- a/browser/base/content/test/windows/browser.ini
+++ /dev/null
@@ -1,2 +0,0 @@
-[DEFAULT]
-[browser_toolbariconcolor_restyles.js]
--- a/browser/base/moz.build
+++ b/browser/base/moz.build
@@ -17,33 +17,33 @@ MOCHITEST_CHROME_MANIFESTS += [
 BROWSER_CHROME_MANIFESTS += [
     'content/test/alerts/browser.ini',
     'content/test/captivePortal/browser.ini',
     'content/test/contextMenu/browser.ini',
     'content/test/forms/browser.ini',
     'content/test/general/browser.ini',
     'content/test/newtab/browser.ini',
     'content/test/pageinfo/browser.ini',
+    'content/test/performance/browser.ini',
     'content/test/permissions/browser.ini',
     'content/test/plugins/browser.ini',
     'content/test/popupNotifications/browser.ini',
     'content/test/popups/browser.ini',
     'content/test/referrer/browser.ini',
     'content/test/sidebar/browser.ini',
     'content/test/siteIdentity/browser.ini',
     'content/test/social/browser.ini',
     'content/test/static/browser.ini',
     'content/test/sync/browser.ini',
     'content/test/tabcrashed/browser.ini',
     'content/test/tabPrompts/browser.ini',
     'content/test/tabs/browser.ini',
     'content/test/urlbar/browser.ini',
     'content/test/webextensions/browser.ini',
     'content/test/webrtc/browser.ini',
-    'content/test/windows/browser.ini',
 ]
 
 if CONFIG['MOZ_UPDATER']:
     BROWSER_CHROME_MANIFESTS += ['content/test/appUpdate/browser.ini']
 
 DEFINES['MOZ_APP_VERSION'] = CONFIG['MOZ_APP_VERSION']
 DEFINES['MOZ_APP_VERSION_DISPLAY'] = CONFIG['MOZ_APP_VERSION_DISPLAY']