Bug 1501761 - Add reflow tests for tab detaching. r=florian
authorFelipe Gomes <felipc@gmail.com>
Wed, 24 Oct 2018 17:59:58 -0300
changeset 491141 bcc643fef1568b22e72e51501608ac4498c6d0be
parent 491140 0a2a9b94714f0f16a3af50a444e7555ddf6a4e38
child 491142 279926d68bfc9008f082bb001ad4bc65d2f15363
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersflorian
bugs1501761
milestone65.0a1
Bug 1501761 - Add reflow tests for tab detaching. r=florian
browser/base/content/test/performance/browser.ini
browser/base/content/test/performance/browser_tabdetach.js
browser/base/content/test/performance/head.js
--- a/browser/base/content/test/performance/browser.ini
+++ b/browser/base/content/test/performance/browser.ini
@@ -19,16 +19,18 @@ skip-if = !debug
 [browser_startup.js]
 [browser_startup_content.js]
 skip-if = !e10s
 [browser_startup_flicker.js]
 run-if = debug || devedition || nightly_build # Requires startupRecorder.js, which isn't shipped everywhere by default
 [browser_tabclose_grow.js]
 [browser_tabclose.js]
 skip-if = (os == 'win' && bits == 32) # Bug 1488537
+[browser_tabdetach.js]
+skip-if = debug # Bug 1501789
 [browser_tabopen.js]
 skip-if = (verify && (os == 'mac'))
 [browser_tabopen_squeeze.js]
 [browser_tabstrip_overflow_underflow.js]
 skip-if = (verify && !debug && (os == 'win'))
 [browser_tabswitch.js]
 [browser_toolbariconcolor_restyles.js]
 [browser_urlbar_keyed_search.js]
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/performance/browser_tabdetach.js
@@ -0,0 +1,106 @@
+"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 = [
+  {
+    stack: [
+      "clientX@chrome://browser/content/tabbrowser.xml",
+      "onxbldragstart@chrome://browser/content/tabbrowser.xml",
+      "synthesizeMouseAtPoint@chrome://mochikit/content/tests/SimpleTest/EventUtils.js",
+      "synthesizeMouse@chrome://mochikit/content/tests/SimpleTest/EventUtils.js",
+      "synthesizePlainDragAndDrop@chrome://mochikit/content/tests/SimpleTest/EventUtils.js",
+    ],
+    maxCount: 2,
+  },
+
+  {
+    stack: [
+      "onxbldragstart@chrome://browser/content/tabbrowser.xml",
+      "synthesizeMouseAtPoint@chrome://mochikit/content/tests/SimpleTest/EventUtils.js",
+      "synthesizeMouse@chrome://mochikit/content/tests/SimpleTest/EventUtils.js",
+      "synthesizePlainDragAndDrop@chrome://mochikit/content/tests/SimpleTest/EventUtils.js",
+    ],
+  },
+];
+
+/**
+ * This test ensures that there are no unexpected uninterruptible reflows when
+ * detaching a tab via drag and drop. The first testcase tests a non-overflowed
+ * tab strip, and the second tests an overflowed one.
+ */
+
+add_task(async function test_detach_not_overflowed() {
+  await ensureNoPreloadedBrowser();
+  await createTabs(1);
+
+  // Make sure we didn't overflow, as expected
+  await BrowserTestUtils.waitForCondition(() => {
+    return !gBrowser.tabContainer.hasAttribute("overflow");
+  });
+
+  let win;
+  await withPerfObserver(async function() {
+    win = await detachTab(gBrowser.tabs[1]);
+  }, {
+    expectedReflows: EXPECTED_REFLOWS,
+    // we are opening a whole new window, so there's no point in tracking
+    // rects being painted
+    frames: { filter: rects => [] },
+  });
+
+  await BrowserTestUtils.closeWindow(win);
+  win = null;
+});
+
+add_task(async function test_detach_overflowed() {
+  const TAB_COUNT_FOR_OVERFLOW = computeMaxTabCount();
+  await createTabs(TAB_COUNT_FOR_OVERFLOW + 1);
+
+  // Make sure we overflowed, as expected
+  await BrowserTestUtils.waitForCondition(() => {
+    return gBrowser.tabContainer.hasAttribute("overflow");
+  });
+
+  let win;
+  await withPerfObserver(async function() {
+    win = await detachTab(gBrowser.tabs[Math.floor(TAB_COUNT_FOR_OVERFLOW / 2)]);
+  }, {
+    expectedReflows: EXPECTED_REFLOWS,
+    // we are opening a whole new window, so there's no point in tracking
+    // rects being painted
+    frames: { filter: rects => [] },
+  });
+
+  await BrowserTestUtils.closeWindow(win);
+  win = null;
+
+  await removeAllButFirstTab();
+});
+
+async function detachTab(tab) {
+  let newWindowPromise = BrowserTestUtils.waitForNewWindow();
+
+  await EventUtils.synthesizePlainDragAndDrop({
+    srcElement: tab,
+
+    // destElement is null because tab detaching happens due
+    // to a drag'n'drop on an invalid drop target.
+    destElement: null,
+
+    // don't move horizontally because that could cause a tab move
+    // animation, and there's code to prevent a tab detaching if
+    // the dragged tab is released while the animation is running.
+    stepX: 0,
+    stepY: 100,
+  });
+
+  return newWindowPromise;
+}
--- a/browser/base/content/test/performance/head.js
+++ b/browser/base/content/test/performance/head.js
@@ -125,31 +125,39 @@ async function recordReflows(testPromise
  */
 function reportUnexpectedReflows(reflows, expectedReflows = []) {
   let knownReflows = expectedReflows.map(r => {
     return {stack: r.stack, path: r.stack.join("|"),
             count: 0, maxCount: r.maxCount || 1,
             actualStacks: new Map()};
   });
   let unexpectedReflows = new Map();
+
+  if (knownReflows.some(r => r.path.includes("*"))) {
+    Assert.ok(false,
+              "Do not include async frames in the stack, as " +
+              "that feature is not available on all trees.");
+  }
+
   for (let stack of reflows) {
     let path =
       stack.split("\n").slice(1) // the first frame which is our test code.
            .map(line => line.replace(/:\d+:\d+$/, "")) // strip line numbers.
            .join("|");
 
     // Stack trace is empty. Reflow was triggered by native code, which
     // we ignore.
     if (path === "") {
       continue;
     }
 
-    // synthesizeKey from EventUtils.js causes us to reflow. That's the test
+    // Functions from EventUtils.js calculate coordinates and
+    // dimensions, causing us to reflow. That's the test
     // harness and we don't care about that, so we'll filter that out.
-    if (path.startsWith("synthesizeKey@chrome://mochikit/content/tests/SimpleTest/EventUtils.js")) {
+    if (/^(synthesize|send|createDragEventObject).*?@chrome:\/\/mochikit.*?EventUtils\.js/.test(path)) {
       continue;
     }
 
     let index = knownReflows.findIndex(reflow => path.startsWith(reflow.path));
     if (index != -1) {
       let reflow = knownReflows[index];
       ++reflow.count;
       reflow.actualStacks.set(stack, (reflow.actualStacks.get(stack) || 0) + 1);