Bug 1494974 - Ensure the reftest canvas always maintains the same size, r=ato
authorJames Graham <james@hoppipolla.co.uk>
Tue, 11 Sep 2018 18:58:35 +0100
changeset 490228 d7c8b670ac687fc3e7b98820a5a4fecc046f51b0
parent 490227 4dfb6e690645619a71da734ee66ea658a1ef253a
child 490229 d4e0fd62e8e0e31392256787b2597d3988642806
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersato
bugs1494974
milestone64.0a1
Bug 1494974 - Ensure the reftest canvas always maintains the same size, r=ato Differential Revision: https://phabricator.services.mozilla.com/D7197
testing/marionette/listener.js
testing/marionette/reftest.js
--- a/testing/marionette/listener.js
+++ b/testing/marionette/listener.js
@@ -1642,32 +1642,30 @@ async function reftestWait(url, remote) 
             observer.disconnect();
             logger.debug("reftest-wait removed");
             win.setTimeout(resolve, 0);
           }
         });
         observer.observe(root, {attributes: true});
       });
     }
+  }
 
-    logger.debug("Waiting for rendering");
+  logger.debug("Waiting for rendering");
 
-    await new Promise(resolve => {
-      let maybeResolve = () => {
-        if (flushRendering()) {
-          win.addEventListener("MozAfterPaint", maybeResolve, {once: true});
-        } else {
-          win.setTimeout(resolve, 0);
-        }
-      };
-      maybeResolve();
-    });
-  } else {
-    flushRendering();
-  }
+  await new Promise(resolve => {
+    let maybeResolve = () => {
+      if (flushRendering()) {
+        win.addEventListener("MozAfterPaint", maybeResolve, {once: true});
+      } else {
+        win.setTimeout(resolve, 0);
+      }
+    };
+    maybeResolve();
+  });
 
   if (remote) {
     windowUtils.updateLayerTree();
   }
 }
 
 function domAddEventListener(msg) {
   eventObservers.add(msg.json.type);
--- a/testing/marionette/reftest.js
+++ b/testing/marionette/reftest.js
@@ -27,16 +27,19 @@ const SCREENSHOT_MODE = {
 
 const STATUS = {
   PASS: "PASS",
   FAIL: "FAIL",
   ERROR: "ERROR",
   TIMEOUT: "TIMEOUT",
 };
 
+const REFTEST_WIDTH = 600;
+const REFTEST_HEIGHT = 600;
+
 /**
  * Implements an fast runner for web-platform-tests format reftests
  * c.f. http://web-platform-tests.org/writing-tests/reftests.html.
  *
  * @namespace
  */
 this.reftest = {};
 
@@ -92,17 +95,17 @@ reftest.Runner = class {
     this.reftestWin = reftestWin;
     return reftestWin;
   }
 
   async openWindow() {
     let reftestWin = this.parentWindow.open(
         "chrome://marionette/content/reftest.xul",
         "reftest",
-        "chrome,height=600,width=600");
+        `chrome,height=${REFTEST_WIDTH},width=${REFTEST_HEIGHT}`);
 
     await new Promise(resolve => {
       reftestWin.addEventListener("load", resolve, {once: true});
     });
 
     let browser = reftestWin.document.createElementNS(XUL_NS, "xul:browser");
     browser.permanentKey = {};
     browser.setAttribute("id", "browser");
@@ -112,17 +115,18 @@ reftest.Runner = class {
 
     if (this.remote) {
       browser.setAttribute("remote", "true");
       browser.setAttribute("remoteType", "web");
     }
     // Make sure the browser element is exactly 600x600, no matter
     // what size our window is
     const windowStyle = `padding: 0px; margin: 0px; border:none;
-min-width: 600px; min-height: 600px; max-width: 600px; max-height: 600px`;
+min-width: ${REFTEST_WIDTH}px; min-height: ${REFTEST_HEIGHT}px;
+max-width: ${REFTEST_WIDTH}px; max-height: ${REFTEST_HEIGHT}px`;
     browser.setAttribute("style", windowStyle);
 
     let doc = reftestWin.document.documentElement;
     while (doc.firstChild) {
       doc.firstChild.remove();
     }
     doc.appendChild(browser);
     reftestWin.gBrowser = browser;
@@ -216,41 +220,53 @@ min-width: 600px; min-height: 600px; max
   async runTest(testUrl, references, expected, timeout) {
     let win = await this.ensureWindow();
 
     function toBase64(screenshot) {
       let dataURL = screenshot.canvas.toDataURL();
       return dataURL.split(",")[1];
     }
 
-    win.innerWidth = 600;
-    win.innerHeight = 600;
-
-    let message = "";
+    let result = {
+      status: STATUS.FAIL,
+      message: "",
+      stack: null,
+      extra: {},
+    };
 
     let screenshotData = [];
 
     let stack = [];
     for (let i = references.length - 1; i >= 0; i--) {
       let item = references[i];
       stack.push([testUrl, item[0], item[1], item[2]]);
     }
 
-    let status = STATUS.FAIL;
+    let done = false;
 
-    while (stack.length) {
+    while (stack.length && !done) {
       let [lhsUrl, rhsUrl, references, relation] = stack.pop();
-      message += `Testing ${lhsUrl} ${relation} ${rhsUrl}\n`;
+      result.message += `Testing ${lhsUrl} ${relation} ${rhsUrl}\n`;
 
-      let comparison = await this.compareUrls(
-          win, lhsUrl, rhsUrl, relation, timeout);
+      let comparison;
+      try {
+        comparison = await this.compareUrls(
+            win, lhsUrl, rhsUrl, relation, timeout);
+      } catch (e) {
+        comparison = {lhs: null, rhs: null, passed: false, error: e};
+      }
+      if (comparison.error !== null) {
+        result.status = STATUS.ERROR;
+        result.message = String(comparison.error);
+        result.stack = comparison.error.stack;
+      }
 
       function recordScreenshot() {
-        let encodedLHS = toBase64(comparison.lhs);
-        let encodedRHS = toBase64(comparison.rhs);
+        let encodedLHS = comparison.lhs ? toBase64(comparison.lhs) : "";
+        let encodedRHS = comparison.rhs ? toBase64(comparison.rhs) : "";
         screenshotData.push([{url: lhsUrl, screenshot: encodedLHS},
           relation,
           {url: rhsUrl, screenshot: encodedRHS}]);
       }
 
       if (this.screenshotMode === SCREENSHOT_MODE.always) {
         recordScreenshot();
       }
@@ -258,44 +274,44 @@ min-width: 600px; min-height: 600px; max
       if (comparison.passed) {
         if (references.length) {
           for (let i = references.length - 1; i >= 0; i--) {
             let item = references[i];
             stack.push([rhsUrl, item[0], item[1], item[2]]);
           }
         } else {
           // Reached a leaf node so all of one reference chain passed
-          status = STATUS.PASS;
+          result.status = STATUS.PASS;
           if (this.screenshotMode <= SCREENSHOT_MODE.fail &&
-              expected != status) {
+              expected != result.status) {
             recordScreenshot();
           }
-          break;
+          done = true;
         }
-      } else if (!stack.length) {
+      } else if (!stack.length || result.status == STATUS.ERROR) {
         // If we don't have any alternatives to try then this will be
         // the last iteration, so save the failing screenshots if required.
         let isFail = this.screenshotMode === SCREENSHOT_MODE.fail;
         let isUnexpected = this.screenshotMode === SCREENSHOT_MODE.unexpected;
-        if (isFail || (isUnexpected && expected != status)) {
+        if (isFail || (isUnexpected && expected != result.status)) {
           recordScreenshot();
         }
       }
 
       // Return any reusable canvases to the pool
       let canvasPool = this.canvasCache.get(null);
       [comparison.lhs, comparison.rhs].map(screenshot => {
-        if (screenshot.reuseCanvas) {
+        if (screenshot !== null && screenshot.reuseCanvas) {
           canvasPool.push(screenshot.canvas);
         }
       });
       logger.debug(`Canvas pool is of length ${canvasPool.length}`);
+
     }
 
-    let result = {status, message, extra: {}};
     if (screenshotData.length) {
       // For now the tbpl formatter only accepts one screenshot, so just
       // return the last one we took.
       let lastScreenshot = screenshotData[screenshotData.length - 1];
       // eslint-disable-next-line camelcase
       result.extra.reftest_screenshots = lastScreenshot;
     }
 
@@ -307,41 +323,60 @@ min-width: 600px; min-height: 600px; max
 
     // Take the reference screenshot first so that if we pause
     // we see the test rendering
     let rhs = await this.screenshot(win, rhsUrl, timeout);
     let lhs = await this.screenshot(win, lhsUrl, timeout);
 
     let maxDifferences = {};
 
-    let differences = this.windowUtils.compareCanvases(
-        lhs.canvas, rhs.canvas, maxDifferences);
+    logger.debug(`lhs canvas size ${lhs.canvas.width}x${lhs.canvas.height}`);
+    logger.debug(`rhs canvas size ${rhs.canvas.width}x${rhs.canvas.height}`);
 
+    let error = null;
     let passed;
-    switch (relation) {
-      case "==":
-        passed = differences === 0;
-        if (!passed) {
-          logger.info(`Found ${differences} pixels different, ` +
-              `maximum difference per channel ${maxDifferences.value}`);
-        }
-        break;
 
-      case "!=":
-        passed = differences !== 0;
-        break;
-
-      default:
-        throw new InvalidArgumentError("Reftest operator should be '==' or '!='");
+    let differences;
+    try {
+      differences = this.windowUtils.compareCanvases(
+          lhs.canvas, rhs.canvas, maxDifferences);
+    } catch (e) {
+      differences = null;
+      passed = false;
+      error = e;
     }
 
-    return {lhs, rhs, passed};
+    if (error === null) {
+      switch (relation) {
+        case "==":
+          passed = differences === 0;
+          if (!passed) {
+            logger.info(`Found ${differences} pixels different, ` +
+                        `maximum difference per channel ${maxDifferences.value}`);
+          }
+          break;
+        case "!=":
+          passed = differences !== 0;
+          break;
+        default:
+          throw new InvalidArgumentError("Reftest operator should be '==' or '!='");
+      }
+    }
+
+    return {lhs, rhs, passed, error};
   }
 
   async screenshot(win, url, timeout) {
+    win.innerWidth = REFTEST_WIDTH;
+    win.innerHeight = REFTEST_HEIGHT;
+
+    // On windows the above doesn't *actually* set the window to be the
+    // reftest size; but *does* set the content area to be the right size;
+    // the window is given some extra borders that aren't explicable from CSS
+    let browserRect = win.gBrowser.getBoundingClientRect();
     let canvas = null;
     let remainingCount = this.urlCount.get(url) || 1;
     let cache = remainingCount > 1;
     logger.debug(`screenshot ${url} remainingCount: ` +
         `${remainingCount} cache: ${cache}`);
     let reuseCanvas = false;
     if (this.canvasCache.has(url)) {
       logger.debug(`screenshot ${url} taken from cache`);
@@ -380,19 +415,24 @@ min-width: 600px; min-height: 600px; max
 
       this.driver.curBrowser.contentBrowser.focus();
       await this.driver.listener.reftestWait(url, this.remote);
 
       canvas = capture.canvas(
           win,
           0, // left
           0, // top
-          win.innerWidth,
-          win.innerHeight,
+          browserRect.width,
+          browserRect.height,
           {canvas, flags});
     }
+    if (canvas.width !== REFTEST_WIDTH || canvas.height !== REFTEST_HEIGHT) {
+      logger.warn(`Canvas dimensions changed to ${canvas.width}x${canvas.height}`);
+      reuseCanvas = false;
+      cache = false;
+    }
     if (cache) {
       this.canvasCache.set(url, canvas);
     }
     this.urlCount.set(url, remainingCount - 1);
     return {canvas, reuseCanvas};
   }
 };