Bug 1531704 - Remove isClosed flag from WorkerTargetFront r=ochameau
authorJulian Descottes <jdescottes@mozilla.com>
Thu, 21 Mar 2019 07:54:03 +0000
changeset 465407 95c5a6730ef6cd919c976b807f2619966cb6026c
parent 465406 c3c26233073718833bd55d9bf2930e2a8096c25e
child 465408 342c28406c954c7757d32cd533271e9b8556f0d5
push id81050
push userjdescottes@mozilla.com
push dateThu, 21 Mar 2019 11:03:42 +0000
treeherderautoland@95c5a6730ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersochameau
bugs1531704
milestone68.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 1531704 - Remove isClosed flag from WorkerTargetFront r=ochameau Depends on D21673 The test is already relying on events triggered by the destroy method of the front to progress I don't think having a flag brings anything here. Differential Revision: https://phabricator.services.mozilla.com/D21675
devtools/client/shared/test/browser_dbg_WorkerTargetActor.attach.js
devtools/shared/fronts/targets/worker.js
--- a/devtools/client/shared/test/browser_dbg_WorkerTargetActor.attach.js
+++ b/devtools/client/shared/test/browser_dbg_WorkerTargetActor.attach.js
@@ -16,57 +16,55 @@ Services.scriptloader.loadSubScript(
 
 var MAX_TOTAL_VIEWERS = "browser.sessionhistory.max_total_viewers";
 
 var TAB1_URL = EXAMPLE_URL + "doc_WorkerTargetActor.attach-tab1.html";
 var TAB2_URL = EXAMPLE_URL + "doc_WorkerTargetActor.attach-tab2.html";
 var WORKER1_URL = "code_WorkerTargetActor.attach-worker1.js";
 var WORKER2_URL = "code_WorkerTargetActor.attach-worker2.js";
 
-function test() {
-  Task.spawn(function* () {
-    const oldMaxTotalViewers = SpecialPowers.getIntPref(MAX_TOTAL_VIEWERS);
-    SpecialPowers.setIntPref(MAX_TOTAL_VIEWERS, 10);
+add_task(async function() {
+  const oldMaxTotalViewers = SpecialPowers.getIntPref(MAX_TOTAL_VIEWERS);
+  SpecialPowers.setIntPref(MAX_TOTAL_VIEWERS, 10);
 
-    const tab = yield addTab(TAB1_URL);
-    const target = yield TargetFactory.forTab(tab);
-    yield target.attach();
-    yield listWorkers(target);
+  const tab = await addTab(TAB1_URL);
+  const target = await TargetFactory.forTab(tab);
+  await target.attach();
+  await listWorkers(target);
 
-    // If a page still has pending network requests, it will not be moved into
-    // the bfcache. Consequently, we cannot use waitForWorkerListChanged here,
-    // because the worker is not guaranteed to have finished loading when it is
-    // registered. Instead, we have to wait for the promise returned by
-    // createWorker in the tab to be resolved.
-    yield createWorkerInTab(tab, WORKER1_URL);
-    let { workers } = yield listWorkers(target);
-    let workerTargetFront1 = findWorker(workers, WORKER1_URL);
-    yield workerTargetFront1.attach();
-    is(workerTargetFront1.isClosed, false, "worker in tab 1 should not be closed");
+  // If a page still has pending network requests, it will not be moved into
+  // the bfcache. Consequently, we cannot use waitForWorkerListChanged here,
+  // because the worker is not guaranteed to have finished loading when it is
+  // registered. Instead, we have to wait for the promise returned by
+  // createWorker in the tab to be resolved.
+  await createWorkerInTab(tab, WORKER1_URL);
+  let { workers } = await listWorkers(target);
+  let workerTargetFront1 = findWorker(workers, WORKER1_URL);
+  await workerTargetFront1.attach();
+  ok(workerTargetFront1.actorID, "front for worker in tab 1 has been attached");
 
-    executeSoon(() => {
-      BrowserTestUtils.loadURI(tab.linkedBrowser, TAB2_URL);
-    });
-    yield waitForWorkerClose(workerTargetFront1);
-    is(workerTargetFront1.isClosed, true, "worker in tab 1 should be closed");
+  executeSoon(() => {
+    BrowserTestUtils.loadURI(tab.linkedBrowser, TAB2_URL);
+  });
+  await waitForWorkerClose(workerTargetFront1);
+  ok(!!workerTargetFront1.actorID, "front for worker in tab 1 has been closed");
 
-    yield createWorkerInTab(tab, WORKER2_URL);
-    ({ workers } = yield listWorkers(target));
-    const workerTargetFront2 = findWorker(workers, WORKER2_URL);
-    yield workerTargetFront2.attach();
-    is(workerTargetFront2.isClosed, false, "worker in tab 2 should not be closed");
+  await createWorkerInTab(tab, WORKER2_URL);
+  ({ workers } = await listWorkers(target));
+  const workerTargetFront2 = findWorker(workers, WORKER2_URL);
+  await workerTargetFront2.attach();
+  ok(workerTargetFront2.actorID, "front for worker in tab 2 has been attached");
 
-    executeSoon(() => {
-      tab.linkedBrowser.goBack();
-    });
-    yield waitForWorkerClose(workerTargetFront2);
-    is(workerTargetFront2.isClosed, true, "worker in tab 2 should be closed");
+  executeSoon(() => {
+    tab.linkedBrowser.goBack();
+  });
+  await waitForWorkerClose(workerTargetFront2);
+  ok(!!workerTargetFront2.actorID, "front for worker in tab 2 has been closed");
 
-    ({ workers } = yield listWorkers(target));
-    workerTargetFront1 = findWorker(workers, WORKER1_URL);
-    yield workerTargetFront1.attach();
-    is(workerTargetFront1.isClosed, false, "worker in tab 1 should not be closed");
+  ({ workers } = await listWorkers(target));
+  workerTargetFront1 = findWorker(workers, WORKER1_URL);
+  await workerTargetFront1.attach();
+  ok(workerTargetFront1.actorID, "front for worker in tab 1 has been attached");
 
-    yield target.destroy();
-    SpecialPowers.setIntPref(MAX_TOTAL_VIEWERS, oldMaxTotalViewers);
-    finish();
-  });
-}
+  await target.destroy();
+  SpecialPowers.setIntPref(MAX_TOTAL_VIEWERS, oldMaxTotalViewers);
+  finish();
+});
--- a/devtools/shared/fronts/targets/worker.js
+++ b/devtools/shared/fronts/targets/worker.js
@@ -9,18 +9,16 @@ const { TargetMixin } = require("./targe
 
 class WorkerTargetFront extends
   TargetMixin(FrontClassWithSpec(workerTargetSpec)) {
   constructor(client) {
     super(client);
 
     this.traits = {};
 
-    this._isClosed = false;
-
     // The actor sends a "close" event, which is translated to "worker-close" by
     // the specification in order to not conflict with Target's "close" event.
     // This event is similar to tabDetached and means that the worker is destroyed.
     // So that we should destroy the target in order to significate that the target
     // is no longer debuggable.
     this.once("worker-close", this.destroy.bind(this));
   }
 
@@ -31,26 +29,16 @@ class WorkerTargetFront extends
     // Do not use `form` name to avoid colliding with protocol.js's `form` method
     this.targetForm = json;
     this._url = json.url;
     this.type = json.type;
     this.scope = json.scope;
     this.fetch = json.fetch;
   }
 
-  get isClosed() {
-    return this._isClosed;
-  }
-
-  destroy() {
-    this._isClosed = true;
-
-    super.destroy();
-  }
-
   async attach() {
     if (this._attach) {
       return this._attach;
     }
     this._attach = (async () => {
       const response = await super.attach();
 
       this._url = response.url;