Bug 1497545 - remove client and cleanup from attachURL; r=ochameau
authoryulia <ystartsev@mozilla.com>
Mon, 15 Oct 2018 16:08:53 +0000
changeset 489853 229ac4221d11345be4ee0fc82b4877dca3dcd524
parent 489852 2ed2e01ecdef6ac55541754551b1dc706ec370af
child 489854 fb835f0cd5b620eddc4d754c17fcf2d6036e2249
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersochameau
bugs1497545
milestone64.0a1
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>