Backed out 2 changesets (bug 1599773, bug 1599413) for browser_frameAttached.js failures CLOSED TREE
authorBogdan Tara <btara@mozilla.com>
Tue, 19 May 2020 12:29:17 +0300
changeset 530769 0f440282ada89ad47b96c81b541e52259bd7eb20
parent 530768 0b304f86c3c8eb9d6b5d358fb88ea0fd031351e7
child 530770 e7dfa75d822d019dda4eaaa51930281a13481f04
push id116358
push userbtara@mozilla.com
push dateTue, 19 May 2020 09:29:42 +0000
treeherderautoland@0f440282ada8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1599773, 1599413
milestone78.0a1
backs out779bc06348ad134826ba47896e54abf9363924a8
6f223ae549d830b35b71f50a42450d35550a40c5
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out 2 changesets (bug 1599773, bug 1599413) for browser_frameAttached.js failures CLOSED TREE Backed out changeset 779bc06348ad (bug 1599773) Backed out changeset 6f223ae549d8 (bug 1599413)
remote/domains/content/Page.jsm
remote/observers/ContextObserver.jsm
remote/test/browser/head.js
remote/test/browser/page/browser.ini
remote/test/browser/page/browser_frameAttached.js
remote/test/browser/page/browser_frameDetached.js
--- a/remote/domains/content/Page.jsm
+++ b/remote/domains/content/Page.jsm
@@ -32,18 +32,16 @@ class Page extends ContentProcessDomain 
     super(session);
 
     this.enabled = false;
     this.lifecycleEnabled = false;
     // script id => { source, worldName }
     this.scriptsToEvaluateOnLoad = new Map();
     this.worldsToEvaluateOnLoad = new Set();
 
-    this._onFrameAttached = this._onFrameAttached.bind(this);
-    this._onFrameDetached = this._onFrameDetached.bind(this);
     this._onFrameNavigated = this._onFrameNavigated.bind(this);
     this._onScriptLoaded = this._onScriptLoaded.bind(this);
 
     this.session.contextObserver.on("script-loaded", this._onScriptLoaded);
   }
 
   destructor() {
     this.setLifecycleEventsEnabled({ enabled: false });
@@ -52,18 +50,17 @@ class Page extends ContentProcessDomain 
 
     super.destructor();
   }
 
   // commands
 
   async enable() {
     if (!this.enabled) {
-      this.session.contextObserver.on("frame-attached", this._onFrameAttached);
-      this.session.contextObserver.on("frame-detached", this._onFrameDetached);
+      this.enabled = true;
       this.session.contextObserver.on(
         "frame-navigated",
         this._onFrameNavigated
       );
 
       this.chromeEventHandler.addEventListener("readystatechange", this, {
         mozSystemGroup: true,
         capture: true,
@@ -80,25 +77,21 @@ class Page extends ContentProcessDomain 
       });
       this.chromeEventHandler.addEventListener("load", this, {
         mozSystemGroup: true,
         capture: true,
       });
       this.chromeEventHandler.addEventListener("pageshow", this, {
         mozSystemGroup: true,
       });
-
-      this.enabled = true;
     }
   }
 
   disable() {
     if (this.enabled) {
-      this.session.contextObserver.off("frame-attached", this._onFrameAttached);
-      this.session.contextObserver.off("frame-detached", this._onFrameDetached);
       this.session.contextObserver.off(
         "frame-navigated",
         this._onFrameNavigated
       );
 
       this.chromeEventHandler.removeEventListener("readystatechange", this, {
         mozSystemGroup: true,
         capture: true,
@@ -244,28 +237,16 @@ class Page extends ContentProcessDomain 
 
     this.lifecycleEnabled = enabled;
   }
 
   url() {
     return this.content.location.href;
   }
 
-  _onFrameAttached(name, { frameId, parentFrameId }) {
-    this.emit("Page.frameAttached", {
-      frameId,
-      parentFrameId,
-      stack: null,
-    });
-  }
-
-  _onFrameDetached(name, { frameId }) {
-    this.emit("Page.frameDetached", { frameId });
-  }
-
   _onFrameNavigated(name, { frameId, window }) {
     const url = window.location.href;
     this.emit("Page.frameNavigated", {
       frame: {
         id: frameId,
         // frameNavigated is only emitted for the top level document
         // so that it never has a parent.
         parentId: null,
--- a/remote/observers/ContextObserver.jsm
+++ b/remote/observers/ContextObserver.jsm
@@ -27,18 +27,16 @@ var EXPORTED_SYMBOLS = ["ContextObserver
 
 const { EventEmitter } = ChromeUtils.import(
   "resource://gre/modules/EventEmitter.jsm"
 );
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 const { executeSoon } = ChromeUtils.import("chrome://remote/content/Sync.jsm");
 
-// Temporary flag to not emit frame related events until everything
-// has been completely implemented, and Puppeteer is no longer busted.
 const FRAMES_ENABLED = Services.prefs.getBoolPref(
   "remote.frames.enabled",
   false
 );
 
 class ContextObserver {
   constructor(chromeEventHandler) {
     this.chromeEventHandler = chromeEventHandler;
@@ -52,40 +50,29 @@ class ContextObserver {
     this.chromeEventHandler.addEventListener("pageshow", this, {
       mozSystemGroup: true,
     });
     this.chromeEventHandler.addEventListener("pagehide", this, {
       mozSystemGroup: true,
     });
 
     Services.obs.addObserver(this, "inner-window-destroyed");
-
-    if (FRAMES_ENABLED) {
-      Services.obs.addObserver(this, "webnavigation-create");
-      Services.obs.addObserver(this, "webnavigation-destroy");
-    }
   }
 
   destructor() {
     this.chromeEventHandler.removeEventListener("DOMWindowCreated", this, {
       mozSystemGroup: true,
     });
     this.chromeEventHandler.removeEventListener("pageshow", this, {
       mozSystemGroup: true,
     });
     this.chromeEventHandler.removeEventListener("pagehide", this, {
       mozSystemGroup: true,
     });
-
     Services.obs.removeObserver(this, "inner-window-destroyed");
-
-    if (FRAMES_ENABLED) {
-      Services.obs.removeObserver(this, "webnavigation-create");
-      Services.obs.removeObserver(this, "webnavigation-destroy");
-    }
   }
 
   handleEvent({ type, target, persisted }) {
     const window = target.defaultView;
     const frameId = window.docShell.browsingContext.id.toString();
     const id = window.windowUtils.currentInnerWindowID;
 
     if (window != this.chromeEventHandler.ownerGlobal && !FRAMES_ENABLED) {
@@ -122,42 +109,14 @@ class ContextObserver {
         if (!persisted) {
           return;
         }
         this.emit("context-destroyed", { windowId: id });
         break;
     }
   }
 
+  // "inner-window-destroyed" observer service listener
   observe(subject, topic, data) {
-    switch (topic) {
-      case "inner-window-destroyed":
-        const windowId = subject.QueryInterface(Ci.nsISupportsPRUint64).data;
-        this.emit("context-destroyed", { windowId });
-        break;
-      case "webnavigation-create":
-        subject.QueryInterface(Ci.nsIDocShell);
-        this.onDocShellCreated(subject);
-        break;
-      case "webnavigation-destroy":
-        subject.QueryInterface(Ci.nsIDocShell);
-        this.onDocShellDestroyed(subject);
-        break;
-    }
-  }
-
-  onDocShellCreated(docShell) {
-    const parent = docShell.browsingContext.parent;
-
-    // TODO: Use a unique identifier for frames (bug 1605359)
-    this.emit("frame-attached", {
-      frameId: docShell.browsingContext.id.toString(),
-      parentFrameId: parent ? parent.id.toString() : null,
-    });
-  }
-
-  onDocShellDestroyed(docShell) {
-    // TODO: Use a unique identifier for frames (bug 1605359)
-    this.emit("frame-detached", {
-      frameId: docShell.browsingContext.id.toString(),
-    });
+    const innerWindowID = subject.QueryInterface(Ci.nsISupportsPRUint64).data;
+    this.emit("context-destroyed", { windowId: innerWindowID });
   }
 }
--- a/remote/test/browser/head.js
+++ b/remote/test/browser/head.js
@@ -259,66 +259,24 @@ function toDataURL(src, doctype = "html"
       break;
     default:
       throw new Error("Unexpected doctype: " + doctype);
   }
 
   return `data:${mime},${encodeURIComponent(doc)}`;
 }
 
-function convertArgument(arg) {
-  if (typeof arg === "bigint") {
-    return { unserializableValue: `${arg.toString()}n` };
-  }
-  if (Object.is(arg, -0)) {
-    return { unserializableValue: "-0" };
-  }
-  if (Object.is(arg, Infinity)) {
-    return { unserializableValue: "Infinity" };
-  }
-  if (Object.is(arg, -Infinity)) {
-    return { unserializableValue: "-Infinity" };
-  }
-  if (Object.is(arg, NaN)) {
-    return { unserializableValue: "NaN" };
-  }
-
-  return { value: arg };
-}
-
-async function evaluate(client, contextId, pageFunction, ...args) {
-  const { Runtime } = client;
-
-  if (typeof pageFunction === "string") {
-    return Runtime.evaluate({
-      expression: pageFunction,
-      contextId,
-      returnByValue: true,
-      awaitPromise: true,
-    });
-  } else if (typeof pageFunction === "function") {
-    return Runtime.callFunctionOn({
-      functionDeclaration: pageFunction.toString(),
-      executionContextId: contextId,
-      arguments: args.map(convertArgument),
-      returnByValue: true,
-      awaitPromise: true,
-    });
-  }
-  throw new Error("pageFunction: expected 'string' or 'function'");
-}
-
 /**
  * Load a given URL in the currently selected tab
  */
 async function loadURL(url, expectedURL = undefined) {
   expectedURL = expectedURL || url;
 
   const browser = gBrowser.selectedTab.linkedBrowser;
-  const loaded = BrowserTestUtils.browserLoaded(browser, true, expectedURL);
+  const loaded = BrowserTestUtils.browserLoaded(browser, false, expectedURL);
 
   BrowserTestUtils.loadURI(browser, url);
   await loaded;
 }
 
 /**
  * Enable the Runtime domain
  */
--- a/remote/test/browser/page/browser.ini
+++ b/remote/test/browser/page/browser.ini
@@ -9,18 +9,16 @@ support-files =
   !/remote/test/browser/head.js
   head.js
   doc_empty.html
   sjs_redirect.sjs
 
 [browser_bringToFront.js]
 [browser_captureScreenshot.js]
 [browser_createIsolatedWorld.js]
-[browser_frameAttached.js]
-[browser_frameDetached.js]
 [browser_frameNavigated.js]
 [browser_getFrameTree.js]
 [browser_getLayoutMetrics.js]
 [browser_getNavigationHistory.js]
 [browser_javascriptDialog_alert.js]
 [browser_javascriptDialog_beforeunload.js]
 [browser_javascriptDialog_confirm.js]
 [browser_javascriptDialog_otherTarget.js]
deleted file mode 100644
--- a/remote/test/browser/page/browser_frameAttached.js
+++ /dev/null
@@ -1,137 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
- * http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-const DOC = toDataURL("<div>foo</div>");
-const DOC_IFRAME_MULTI = toDataURL(`
-  <iframe src='data:text/html,foo'></iframe>
-  <iframe src='data:text/html,bar'></iframe>
-`);
-const DOC_IFRAME_NESTED = toDataURL(`
-  <iframe src="data:text/html,<iframe src='data:text/html,foo'></iframe>">
-  </iframe>
-`);
-
-add_task(async function noEventWhenPageDomainDisabled({ client }) {
-  await runFrameAttachedTest(client, 0, async () => {
-    info("Navigate to a page with an iframe");
-    await loadURL(DOC_IFRAME_MULTI);
-  });
-});
-
-add_task(async function noEventAfterPageDomainDisabled({ client }) {
-  const { Page } = client;
-
-  await Page.enable();
-  await Page.disable();
-
-  await runFrameAttachedTest(client, 0, async () => {
-    info("Navigate to a page with an iframe");
-    await loadURL(DOC_IFRAME_MULTI);
-  });
-});
-
-add_task(async function noEventWhenNavigatingWithNoFrames({ client }) {
-  const { Page } = client;
-
-  await Page.enable();
-
-  info("Navigate to a page with iframes");
-  await loadURL(DOC_IFRAME_MULTI);
-
-  await runFrameAttachedTest(client, 0, async () => {
-    info("Navigate to a page without an iframe");
-    await loadURL(DOC);
-  });
-});
-
-add_task(async function eventWhenNavigatingWithFrames({ client }) {
-  const { Page } = client;
-
-  await Page.enable();
-
-  await runFrameAttachedTest(client, 2, async () => {
-    info("Navigate to a page with an iframe");
-    await loadURL(DOC_IFRAME_MULTI);
-  });
-});
-
-add_task(async function eventWhenNavigatingWithNestedFrames({ client }) {
-  const { Page } = client;
-
-  await Page.enable();
-
-  await runFrameAttachedTest(client, 2, async () => {
-    info("Navigate to a page with nested iframes");
-    await loadURL(DOC_IFRAME_NESTED);
-  });
-});
-
-add_task(async function eventWhenAttachingFrame({ client }) {
-  const { Page } = client;
-
-  await Page.enable();
-
-  await runFrameAttachedTest(client, 1, async () => {
-    await SpecialPowers.spawn(gBrowser.selectedBrowser, [], async () => {
-      const frame = content.document.createElement("iframe");
-      frame.src = "data:text/html,frame content";
-
-      const loaded = new Promise(resolve => (frame.onload = resolve));
-      content.document.body.appendChild(frame);
-      await loaded;
-    });
-  });
-});
-
-async function runFrameAttachedTest(client, expectedEventCount, callback) {
-  const { Page } = client;
-
-  const frameAttachedEvents = [];
-  Page.frameAttached(frame => frameAttachedEvents.push(frame));
-
-  const framesBefore = await getFlattendFrameList();
-  await callback();
-  const framesAfter = await getFlattendFrameList();
-
-  if (expectedEventCount == 0) {
-    is(frameAttachedEvents.length, 0, "Got no frame attached event");
-    return;
-  }
-
-  // check how many frames were added or removed
-  const count = Math.abs(framesBefore.size - framesAfter.size);
-
-  is(count, expectedEventCount, "Expected amount of frames added");
-  is(
-    frameAttachedEvents.length,
-    count,
-    "Received the expected amount of frameAttached events"
-  );
-
-  // extract the new or removed frames
-  const framesAll = new Map([...framesBefore, ...framesAfter]);
-  const expectedFrames = new Map(
-    [...framesAll].filter(([key, _value]) => {
-      return !framesBefore.has(key) && framesAfter.has(key);
-    })
-  );
-
-  frameAttachedEvents.forEach(event => {
-    console.log(`Check frame id ${event.frameId}`);
-    const expectedFrame = expectedFrames.get(event.frameId);
-
-    ok(expectedFrame, `Found expected frame with id ${event.frameId}`);
-    is(
-      event.frameId,
-      expectedFrame.id,
-      "Got expected frame id for frameAttached event"
-    );
-    is(
-      event.parentFrameId,
-      expectedFrame.parentId,
-      "Got expected parent frame id for frameAttached event"
-    );
-  });
-}
deleted file mode 100644
--- a/remote/test/browser/page/browser_frameDetached.js
+++ /dev/null
@@ -1,162 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
- * http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-const DOC = toDataURL("<div>foo</div>");
-const DOC_IFRAME_MULTI = toDataURL(`
-  <iframe src='data:text/html,foo'></iframe>
-  <iframe src='data:text/html,bar'></iframe>
-`);
-const DOC_IFRAME_NESTED = toDataURL(`
-  <iframe src="data:text/html,<iframe src='data:text/html,foo'></iframe>">
-  </iframe>
-`);
-
-// Disable bfcache to force documents to be destroyed on navigation
-Services.prefs.setIntPref("browser.sessionhistory.max_total_viewers", 0);
-registerCleanupFunction(() => {
-  Services.prefs.clearUserPref("browser.sessionhistory.max_total_viewers");
-});
-
-add_task(async function noEventWhenPageDomainDisabled({ client }) {
-  await loadURL(DOC_IFRAME_MULTI);
-
-  await runFrameDetachedTest(client, 0, async () => {
-    info("Navigate away from a page with an iframe");
-    await loadURL(DOC);
-  });
-});
-
-add_task(async function noEventAfterPageDomainDisabled({ client }) {
-  const { Page } = client;
-
-  await Page.enable();
-
-  await loadURL(DOC_IFRAME_MULTI);
-
-  await Page.disable();
-
-  await runFrameDetachedTest(client, 0, async () => {
-    info("Navigate away from a page with an iframe");
-    await loadURL(DOC);
-  });
-});
-
-add_task(async function noEventWhenNavigatingWithNoFrames({ client }) {
-  const { Page } = client;
-
-  await Page.enable();
-
-  await loadURL(DOC);
-
-  await runFrameDetachedTest(client, 0, async () => {
-    info("Navigate away from a page with no iframes");
-    await loadURL(DOC);
-  });
-});
-
-add_task(async function eventWhenNavigatingWithFrames({ client }) {
-  const { Page } = client;
-
-  await Page.enable();
-
-  await loadURL(DOC_IFRAME_MULTI);
-
-  await runFrameDetachedTest(client, 2, async () => {
-    info("Navigate away from a page with an iframe");
-    await loadURL(DOC);
-  });
-});
-
-add_task(async function eventWhenNavigatingWithNestedFrames({ client }) {
-  const { Page } = client;
-
-  await Page.enable();
-
-  await loadURL(DOC_IFRAME_NESTED);
-
-  await runFrameDetachedTest(client, 2, async () => {
-    info("Navigate away from a page with nested iframes");
-    await loadURL(DOC);
-  });
-});
-
-add_task(async function eventWhenDetachingFrame({ client }) {
-  const { Page } = client;
-
-  await Page.enable();
-
-  await loadURL(DOC_IFRAME_MULTI);
-
-  await runFrameDetachedTest(client, 1, async () => {
-    // Remove the single frame from the page
-    await SpecialPowers.spawn(gBrowser.selectedBrowser, [], () => {
-      const frame = content.document.getElementsByTagName("iframe")[0];
-      frame.remove();
-    });
-  });
-});
-
-add_task(async function eventWhenDetachingNestedFrames({ client }) {
-  const { Page, Runtime } = client;
-
-  await Page.enable();
-
-  await loadURL(DOC_IFRAME_NESTED);
-
-  await Runtime.enable();
-  const { context } = await Runtime.executionContextCreated();
-
-  await runFrameDetachedTest(client, 2, async () => {
-    // Remove top-frame, which also removes any nested frames
-    await evaluate(client, context.id, async () => {
-      const frame = document.getElementsByTagName("iframe")[0];
-      frame.remove();
-    });
-  });
-});
-
-async function runFrameDetachedTest(client, expectedEventCount, callback) {
-  const { Page } = client;
-
-  const frameDetachedEvents = [];
-  Page.frameDetached(frame => frameDetachedEvents.push(frame));
-
-  const framesBefore = await getFlattendFrameList();
-  await callback();
-  const framesAfter = await getFlattendFrameList();
-
-  // check how many frames were added or removed
-  const count = Math.abs(framesBefore.size - framesAfter.size);
-
-  if (expectedEventCount == 0) {
-    is(frameDetachedEvents.length, 0, "Got no frame detached event");
-    return;
-  }
-
-  is(count, expectedEventCount, "Expected amount of frames removed");
-  is(
-    frameDetachedEvents.length,
-    count,
-    "Received the expected amount of frameDetached events"
-  );
-
-  // extract the new or removed frames
-  const framesAll = new Map([...framesBefore, ...framesAfter]);
-  const expectedFrames = new Map(
-    [...framesAll].filter(([key, _value]) => {
-      return framesBefore.has(key) && !framesAfter.has(key);
-    })
-  );
-
-  frameDetachedEvents.forEach(event => {
-    const expectedFrame = expectedFrames.get(event.frameId);
-
-    is(
-      event.frameId,
-      expectedFrame.id,
-      "Got expected frame id for frameDetached event"
-    );
-  });
-}