Bug 1497545 - remove client and cleanup from attachURL; r=ochameau
authoryulia <ystartsev@mozilla.com>
Mon, 15 Oct 2018 16:08:53 +0000
changeset 499934 229ac4221d11345be4ee0fc82b4877dca3dcd524
parent 499933 2ed2e01ecdef6ac55541754551b1dc706ec370af
child 499935 fb835f0cd5b620eddc4d754c17fcf2d6036e2249
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersochameau
bugs1497545
milestone64.0a1
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
Bug 1497545 - remove client and cleanup from attachURL; r=ochameau Depends on D8372 Differential Revision: https://phabricator.services.mozilla.com/D8541
devtools/server/tests/mochitest/inspector-helpers.js
devtools/server/tests/mochitest/test_inspector-mutations-childlist.html
devtools/server/tests/mochitest/test_inspector-mutations-frameload.html
devtools/server/tests/mochitest/test_inspector-pseudoclass-lock.html
devtools/server/tests/mochitest/test_inspector-release.html
devtools/server/tests/mochitest/test_inspector-remove.html
devtools/server/tests/mochitest/test_inspector-traversal.html
--- a/devtools/server/tests/mochitest/inspector-helpers.js
+++ b/devtools/server/tests/mochitest/inspector-helpers.js
@@ -58,35 +58,33 @@ async function attachURL(url) {
   const gBrowser = Services.wm.getMostRecentWindow("navigator:browser").gBrowser;
 
   // open the url in a new tab, save a reference to the new inner window global object
   // and wait for it to load. The tests rely on this window object to send a "ready"
   // event to its opener (the test page). This window reference is used within
   // the test tab, to reference the webpage being tested against, which is in another
   // tab.
   const windowOpened = BrowserTestUtils.waitForNewTab(gBrowser, url);
-  let win = window.open(url, "_blank");
+  const win = window.open(url, "_blank");
   await windowOpened;
 
   const target = await getTargetForSelectedTab(gBrowser);
-  const client = target.client;
   await target.attach();
 
   const cleanup = async function() {
-    if (client) {
-      await client.close();
+    if (target.client) {
+      await target.client.close();
     }
     if (win) {
       win.close();
-      win = null;
     }
   };
 
   gAttachCleanups.push(cleanup);
-  return { client, target, doc: win.document, cleanup };
+  return { target, doc: win.document };
 }
 
 function promiseOnce(target, event) {
   return new Promise(resolve => {
     target.on(event, (...args) => {
       if (args.length === 1) {
         resolve(args[0]);
       } else {
@@ -163,34 +161,34 @@ function assertOwnershipTrees(walker) {
   const clientTree = clientOwnershipTree(walker);
   is(JSON.stringify(clientTree, null, " "), JSON.stringify(serverTree, null, " "),
      "Server and client ownership trees should match.");
 
   return ownershipTreeSize(clientTree.root);
 }
 
 // Verify that an actorID is inaccessible both from the client library and the server.
-function checkMissing(client, actorID) {
+function checkMissing({client}, actorID) {
   return new Promise(resolve => {
     const front = client.getActor(actorID);
     ok(!front, "Front shouldn't be accessible from the client for actorID: " + actorID);
 
     client.request({
       to: actorID,
       type: "request",
     }, response => {
       is(response.error, "noSuchActor",
         "node list actor should no longer be contactable.");
       resolve(undefined);
     });
   });
 }
 
 // Verify that an actorID is accessible both from the client library and the server.
-function checkAvailable(client, actorID) {
+function checkAvailable({client}, actorID) {
   return new Promise(resolve => {
     const front = client.getActor(actorID);
     ok(front, "Front should be accessible from the client for actorID: " + actorID);
 
     client.request({
       to: actorID,
       type: "garbageAvailableTest",
     }, response => {
--- a/devtools/server/tests/mochitest/test_inspector-mutations-childlist.html
+++ b/devtools/server/tests/mochitest/test_inspector-mutations-childlist.html
@@ -15,36 +15,30 @@ https://bugzilla.mozilla.org/show_bug.cg
 
 window.onload = function() {
   SimpleTest.waitForExplicitFinish();
   runNextTest();
 };
 
 let gInspectee = null;
 let gWalker = null;
-let gCleanupConnection = null;
 
 async function setup(callback) {
   const url = document.getElementById("inspectorContent").href;
-  const { target, doc, cleanup } = await attachURL(url);
+  const { target, doc } = await attachURL(url);
   gInspectee = doc;
   const inspector = target.getInspector();
   gWalker = await inspector.getWalker();
   ok(gWalker, "getWalker() should return an actor.");
-  gCleanupConnection = cleanup;
   callback();
 }
 
 function teardown() {
   gWalker = null;
   gInspectee = null;
-  if (gCleanupConnection) {
-    gCleanupConnection();
-    gCleanupConnection = null;
-  }
 }
 
 function assertOwnership() {
   return assertOwnershipTrees(gWalker);
 }
 
 function setParent(nodeSelector, newParentSelector) {
   const node = gInspectee.querySelector(nodeSelector);
--- a/devtools/server/tests/mochitest/test_inspector-mutations-frameload.html
+++ b/devtools/server/tests/mochitest/test_inspector-mutations-frameload.html
@@ -15,38 +15,31 @@ https://bugzilla.mozilla.org/show_bug.cg
 
 window.onload = function() {
   SimpleTest.waitForExplicitFinish();
   runNextTest();
 };
 
 let gInspectee = null;
 let gWalker = null;
-let gClient = null;
-let gCleanupConnection = null;
+let gTarget = null;
 
 async function setup(callback) {
   const url = document.getElementById("inspectorContent").href;
-  const { client, target, doc, cleanup } = await attachURL(url);
+  const { target, doc } = await attachURL(url);
   gInspectee = doc;
-  gClient = client;
+  gTarget = target;
   const inspector = target.getInspector();
   gWalker = await inspector.getWalker();
-  gCleanupConnection = cleanup;
   await callback();
 }
 
 function teardown() {
   gWalker = null;
-  gClient = null;
   gInspectee = null;
-  if (gCleanupConnection) {
-    gCleanupConnection();
-    gCleanupConnection = null;
-  }
 }
 
 function assertOwnership() {
   return assertOwnershipTrees(gWalker);
 }
 
 function loadChildSelector(selector) {
   return gWalker.querySelector(gWalker.rootNode, "#childFrame").then(frame => {
@@ -82,17 +75,17 @@ addTest(function loadNewChild() {
       mutations = assertUnload(mutations);
       mutations = assertFrameLoad(mutations);
       mutations = assertChildList(mutations);
 
       is(mutations.length, 0, "Got the expected mutations.");
 
       assertOwnership();
 
-      return checkMissing(gClient, unloaded);
+      return checkMissing(gTarget, unloaded);
     }).then(() => {
       teardown();
     }).then(runNextTest));
   });
 });
 
 addTest(function loadNewChildTwice() {
   setup(() => {
@@ -145,17 +138,17 @@ addTest(function loadNewChildTwiceAndCar
       mutations = assertUnload(mutations);
       mutations = assertFrameLoad(mutations);
       mutations = assertChildList(mutations);
 
       is(mutations.length, 0, "Got the expected mutations.");
 
       assertOwnership();
 
-      return checkMissing(gClient, unloaded);
+      return checkMissing(gTarget, unloaded);
     }).then(() => {
       teardown();
     }).then(runNextTest));
   });
 });
 
 addTest(function testBack() {
   setup(() => {
@@ -178,17 +171,17 @@ addTest(function testBack() {
       mutations = assertSrcChange(mutations);
       mutations = assertUnload(mutations);
       mutations = assertFrameLoad(mutations);
       mutations = assertChildList(mutations);
       is(mutations.length, 0, "Got the expected mutations.");
 
       assertOwnership();
 
-      return checkMissing(gClient, unloaded);
+      return checkMissing(gTarget, unloaded);
     }).then(() => {
       teardown();
     }).then(runNextTest));
   });
 });
 
   </script>
 </head>
--- a/devtools/server/tests/mochitest/test_inspector-pseudoclass-lock.html
+++ b/devtools/server/tests/mochitest/test_inspector-pseudoclass-lock.html
@@ -19,37 +19,31 @@ window.onload = function() {
   SimpleTest.waitForExplicitFinish();
   runNextTest();
 };
 
 const InspectorUtils = require("InspectorUtils");
 
 let gInspectee = null;
 let gWalker = null;
-let gCleanupConnection = null;
 
 async function setup(callback) {
   const url = document.getElementById("inspectorContent").href;
-  const { target, doc, cleanup } = await attachURL(url);
+  const { target, doc } = await attachURL(url);
   gInspectee = doc;
   const inspector = target.getInspector();
   const walker = await inspector.getWalker();
   ok(walker, "getWalker() should return an actor.");
   gWalker = walker;
-  gCleanupConnection = cleanup;
   runNextTest();
 }
 
 function teardown() {
   gWalker = null;
   gInspectee = null;
-  if (gCleanupConnection) {
-    gCleanupConnection();
-    gCleanupConnection = null;
-  }
 }
 
 function checkChange(change, expectation) {
   is(change.type, "pseudoClassLock", "Expect a pseudoclass lock change.");
   const target = change.target;
   if (expectation.id) {
     is(target.id, expectation.id, "Expect a change on node id " + expectation.id);
   }
--- a/devtools/server/tests/mochitest/test_inspector-release.html
+++ b/devtools/server/tests/mochitest/test_inspector-release.html
@@ -14,27 +14,27 @@ https://bugzilla.mozilla.org/show_bug.cg
 "use strict";
 
 window.onload = function() {
   SimpleTest.waitForExplicitFinish();
   runNextTest();
 };
 
 let gWalker = null;
-let gClient = null;
+let gTarget = null;
 
 function assertOwnership() {
   return assertOwnershipTrees(gWalker);
 }
 
 addTest(async function setup() {
   const url = document.getElementById("inspectorContent").href;
-  const { client, target } = await attachURL(url);
+  const { target } = await attachURL(url);
   const inspector = target.getInspector();
-  gClient = client;
+  gTarget = target;
   gWalker = await inspector.getWalker();
   ok(gWalker, "getWalker() should return an actor.");
   runNextTest();
 });
 
 addTest(function testReleaseSubtree() {
   let originalOwnershipSize = 0;
   let longlist = null;
@@ -68,25 +68,25 @@ addTest(function testReleaseSubtree() {
     return gWalker.releaseNode(node);
   }).then(() => {
     // Our ownership size should now be 53 fewer
     // (we forgot about #longlist + 26 children + 26 singleTextChild nodes)
     const newOwnershipSize = assertOwnership();
     is(newOwnershipSize, originalOwnershipSize - 53,
       "Ownership tree should be lower");
     // Now verify that some nodes have gone away
-    return checkMissing(gClient, longlist);
+    return checkMissing(gTarget, longlist);
   }).then(() => {
-    return checkMissing(gClient, firstChild);
+    return checkMissing(gTarget, firstChild);
   }).then(runNextTest));
 });
 
 addTest(function cleanup() {
   gWalker = null;
-  gClient = null;
+  gTarget = null;
   runNextTest();
 });
   </script>
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=">Mozilla Bug </a>
 <a id="inspectorContent" target="_blank" href="inspector-traversal-data.html">Test Document</a>
 <p id="display"></p>
--- a/devtools/server/tests/mochitest/test_inspector-remove.html
+++ b/devtools/server/tests/mochitest/test_inspector-remove.html
@@ -14,37 +14,37 @@ https://bugzilla.mozilla.org/show_bug.cg
 "use strict";
 
 window.onload = function() {
   SimpleTest.waitForExplicitFinish();
   runNextTest();
 };
 
 let gWalker = null;
-let gClient = null;
+let gTarget = null;
 let gInspectee = null;
 
 function assertOwnership() {
   return assertOwnershipTrees(gWalker);
 }
 
 function ignoreNode(node) {
   // Duplicate the walker logic to skip blank nodes...
   return node.nodeType === Node.TEXT_NODE &&
     !/[^\s]/.test(node.nodeValue);
 }
 
 addTest(async function setup() {
   const url = document.getElementById("inspectorContent").href;
-  const { client, target, doc } = await attachURL(url);
+  const { target, doc } = await attachURL(url);
   gInspectee = doc;
   const inspector = target.getInspector();
   gWalker = await inspector.getWalker();
   ok(gWalker, "getWalker() should return an actor.");
-  gClient = client;
+  gTarget = target;
   runNextTest();
 });
 
 addTest(function testRemoveSubtree() {
   let originalOwnershipSize = 0;
   let longlist = null;
   let longlistID = null;
 
@@ -86,23 +86,23 @@ addTest(function testRemoveSubtree() {
   }).then(() => {
     // Our ownership size should now be 51 fewer (we forgot about #longlist + 26
     // children + 26 singleTextChild nodes, but learned about #longlist's
     // prev/next sibling)
     const newOwnershipSize = assertOwnership();
     is(newOwnershipSize, originalOwnershipSize - 51,
       "Ownership tree should be lower");
     // Now verify that some nodes have gone away
-    return checkMissing(gClient, longlistID);
+    return checkMissing(gTarget, longlistID);
   }).then(runNextTest));
 });
 
 addTest(function cleanup() {
   gWalker = null;
-  gClient = null;
+  gTarget = null;
   gInspectee = null;
   runNextTest();
 });
   </script>
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=">Mozilla Bug </a>
 <a id="inspectorContent" target="_blank" href="inspector-traversal-data.html">Test Document</a>
--- a/devtools/server/tests/mochitest/test_inspector-traversal.html
+++ b/devtools/server/tests/mochitest/test_inspector-traversal.html
@@ -14,29 +14,29 @@ https://bugzilla.mozilla.org/show_bug.cg
 "use strict";
 
 window.onload = function() {
   SimpleTest.waitForExplicitFinish();
   runNextTest();
 };
 
 let gInspectee = null;
-let gClient = null;
+let gTarget = null;
 let gWalker = null;
 const checkActorIDs = [];
 
 function assertOwnership() {
   assertOwnershipTrees(gWalker);
 }
 addTest(async function setup() {
   const url = document.getElementById("inspectorContent").href;
-  const { client, target, doc } = await attachURL(url);
+  const { target, doc } = await attachURL(url);
   const inspector = target.getInspector();
   gInspectee = doc;
-  gClient = client;
+  gTarget = target;
   gWalker = await inspector.getWalker();
   ok(gWalker, "getWalker() should return an actor.");
   runNextTest();
 });
 
 addTest(function testWalkerRoot() {
   // Make sure that refetching the root document of the walker returns the same
   // actor as the getWalker returned.
@@ -129,17 +129,17 @@ addTest(function testQuerySelectors() {
     checkActorIDs.push(nodes[0].actorID);
     // Save the node list ID so we can ensure it was destroyed.
     nodeListID = nodeList.actorID;
     assertOwnership();
     return nodeList.release();
   }).then(() => {
     ok(!nodeList.actorID, "Actor should have been destroyed.");
     assertOwnership();
-    return checkMissing(gClient, nodeListID);
+    return checkMissing(gTarget, nodeListID);
   }).then(runNextTest));
 });
 
 // Helper to check the response of requests that return hasFirst/hasLast/nodes
 // node lists (like `children` and `siblings`)
 function nodeArrayChecker(first, last, ids) {
   return function(response) {
     is(response.hasFirst, first,
@@ -305,25 +305,25 @@ addTest(function testShortValue() {
        "Full node value should match the string from the document.");
   }).then(runNextTest));
 });
 
 addTest(function testReleaseWalker() {
   checkActorIDs.push(gWalker.actorID);
 
   promiseDone(gWalker.release().then(() => {
-    const promises = Array.from(checkActorIDs, (id) => checkMissing(gClient, id));
+    const promises = Array.from(checkActorIDs, (id) => checkMissing(gTarget, id));
     return Promise.all(promises);
   }).then(runNextTest));
 });
 
 addTest(function cleanup() {
   gWalker = null;
   gInspectee = null;
-  gClient = null;
+  gTarget = null;
   runNextTest();
 });
   </script>
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=">Mozilla Bug </a>
 <a id="inspectorContent" target="_blank" href="inspector-traversal-data.html">Test Document</a>
 <p id="display"></p>