Bug 1178726 - Simplify how we deal with freezing/thawing workers;r=jlongster,khuey

devtools/client/debugger/debugger-controller.js

@@ -425,10 +425,8 @@ var DebuggerController = {
 };
 
 function Workers() {
-  this._workerClients = new Map();
+  this._workerForms = Object.create(null);
   this._onWorkerListChanged = this._onWorkerListChanged.bind(this);
-  this._onWorkerFreeze = this._onWorkerFreeze.bind(this);
-  this._onWorkerThaw = this._onWorkerThaw.bind(this);
   this._onWorkerSelect = this._onWorkerSelect.bind(this);
 }
 
@@ -456,29 +454,23 @@ Workers.prototype = {
     }
 
     this._tabClient.listWorkers((response) => {
-      let workerActors = new Set();
+      let workerForms = Object.create(null);
       for (let worker of response.workers) {
-        workerActors.add(worker.actor);
+        workerForms[worker.actor] = worker;
       }
 
-      for (let [workerActor, workerClient] of this._workerClients) {
-        if (!workerActors.has(workerActor)) {
-          workerClient.removeListener("freeze", this._onWorkerFreeze);
-          workerClient.removeListener("thaw", this._onWorkerThaw);
-          this._workerClients.delete(workerActor);
+      for (let workerActor in this._workerForms) {
+        if (!(workerActor in workerForms)) {
+          delete this._workerForms[workerActor];
           DebuggerView.Workers.removeWorker(workerActor);
         }
       }
 
-      for (let actor of workerActors) {
-        let workerActor = actor
-        if (!this._workerClients.has(workerActor)) {
-          this._tabClient.attachWorker(workerActor, (response, workerClient) => {
-            workerClient.addListener("freeze", this._onWorkerFreeze);
-            workerClient.addListener("thaw", this._onWorkerThaw);
-            this._workerClients.set(workerActor, workerClient);
-            DebuggerView.Workers.addWorker(workerActor, workerClient.url);
-          });
+      for (let workerActor in workerForms) {
+        if (!(workerActor in this._workerForms)) {
+          let workerForm = workerForms[workerActor];
+          this._workerForms[workerActor] = workerForm;
+          DebuggerView.Workers.addWorker(workerActor, workerForm.url);
         }
       }
     });
@@ -488,20 +480,11 @@ Workers.prototype = {
     this._updateWorkerList();
   },
 
-  _onWorkerFreeze: function (type, packet) {
-    DebuggerView.Workers.removeWorker(packet.from);
-  },
-
-  _onWorkerThaw: function (type, packet) {
-    let workerClient = this._workerClients.get(packet.from);
-    DebuggerView.Workers.addWorker(packet.from, workerClient.url);
-  },
-
-  _onWorkerSelect: function (workerActor) {
-    let workerClient = this._workerClients.get(workerActor);
-    gDevTools.showToolbox(TargetFactory.forWorker(workerClient),
-                          "jsdebugger",
-                          Toolbox.HostType.WINDOW);
+  _onWorkerSelect: function (type, workerActor) {
+    DebuggerController.client.attachWorker(workerActor, (response, workerClient) => {
+      gDevTools.showToolbox(devtools.TargetFactory.forWorker(workerClient),
+                            "jsdebugger", devtools.Toolbox.HostType.WINDOW);
+    });
   }
 };
 

devtools/client/debugger/test/mochitest/browser_dbg_WorkerActor.attach.js

@@ -30,30 +30,30 @@ function test() {
     let { workers } = yield listWorkers(tabClient);
     let [, workerClient1] = yield attachWorker(tabClient,
                                                findWorker(workers, WORKER1_URL));
-    is(workerClient1.isFrozen, false);
+    is(workerClient1.isClosed, false, "worker in tab 1 should not be closed");
 
     executeSoon(() => {
       tab.linkedBrowser.loadURI(TAB2_URL);
     });
-    yield waitForWorkerFreeze(workerClient1);
-    is(workerClient1.isFrozen, true, "worker should be frozen");
+    yield waitForWorkerClose(workerClient1);
+    is(workerClient1.isClosed, true, "worker in tab 1 should be closed");
 
     yield createWorkerInTab(tab, WORKER2_URL);
     ({ workers } = yield listWorkers(tabClient));
     let [, workerClient2] = yield attachWorker(tabClient,
                                                findWorker(workers, WORKER2_URL));
-    is(workerClient2.isFrozen, false);
+    is(workerClient2.isClosed, false, "worker in tab 2 should not be closed");
 
     executeSoon(() => {
       tab.linkedBrowser.contentWindow.history.back();
     });
-    yield Promise.all([
-      waitForWorkerFreeze(workerClient2),
-      waitForWorkerThaw(workerClient1)
-    ]);
+    yield waitForWorkerClose(workerClient2);
+    is(workerClient2.isClosed, true, "worker in tab 2 should be closed");
 
-    terminateWorkerInTab(tab, WORKER1_URL);
-    yield waitForWorkerClose(workerClient1);
+    ({ workers } = yield listWorkers(tabClient));
+    [, workerClient1] = yield attachWorker(tabClient,
+                                           findWorker(workers, WORKER1_URL));
+    is(workerClient1.isClosed, false, "worker in tab 1 should not be closed");
 
     yield close(client);
     SpecialPowers.setIntPref(MAX_TOTAL_VIEWERS, oldMaxTotalViewers);

devtools/client/debugger/test/mochitest/head.js

@@ -1124,24 +1124,6 @@ function waitForWorkerClose(workerClient) {
   });
 }
 
-function waitForWorkerFreeze(workerClient) {
-  info("Waiting for worker to freeze.");
-  return new Promise(function (resolve) {
-    workerClient.addOneTimeListener("freeze", function () {
-      resolve();
-    });
-  });
-}
-
-function waitForWorkerThaw(workerClient) {
-  info("Waiting for worker to thaw.");
-  return new Promise(function (resolve) {
-    workerClient.addOneTimeListener("thaw", function () {
-      resolve();
-    });
-  });
-}
-
 function resume(threadClient) {
   info("Resuming thread.");
   return rdpInvoke(threadClient, threadClient.resume);

devtools/client/framework/target.js

@@ -713,17 +713,13 @@ function WorkerTarget(workerClient) {
  * for remote tabs (from which a TabClient can then be lazily obtained),
  * WorkerTarget is constructed with a WorkerClient directly.
  *
- * The reason for this is that in order to get notifications when a worker
- * closes/freezes/thaws, the UI needs to attach to each worker anyway, so by
- * the time a WorkerTarget for a given worker is created, a WorkerClient for
- * that worker will already be available. Consequently, there is no need to
- * obtain a WorkerClient lazily.
- *
  * WorkerClient is designed to mimic the interface of TabClient as closely as
  * possible. This allows us to debug workers as if they were ordinary tabs,
  * requiring only minimal changes to the rest of the frontend.
  */
 WorkerTarget.prototype = {
+  destroy: function () {},
+
   get isRemote() {
     return true;
   },
@@ -737,12 +733,7 @@ WorkerTarget.prototype = {
   },
 
   get form() {
-    return {
-      from: this._workerClient.actor,
-      type: "attached",
-      isFrozen: this._workerClient.isFrozen,
-      url: this._workerClient.url
-    };
+    return {};
   },
 
   get activeTab() {
@@ -755,7 +746,7 @@ WorkerTarget.prototype = {
 
   destroy: function() {},
 
-  hasActor: function() {
+  hasActor: function (name) {
     return false;
   },
 
@@ -763,5 +754,7 @@ WorkerTarget.prototype = {
     return undefined;
   },
 
-  makeRemote: function() {}
+  makeRemote: function () {
+    return Promise.resolve();
+  }
 };

devtools/server/actors/worker.js

@@ -59,7 +59,6 @@ WorkerActor.prototype = {
 
     return {
       type: "attached",
-      isFrozen: this._dbg.isFrozen,
       url: this._dbg.url
     };
   },
@@ -113,14 +112,6 @@ WorkerActor.prototype = {
     reportError("ERROR:" + filename + ":" + lineno + ":" + message + "\n");
   },
 
-  onFreeze: function () {
-    this.conn.sendActorEvent(this.actorID, "freeze");
-  },
-
-  onThaw: function () {
-    this.conn.sendActorEvent(this.actorID, "thaw");
-  },
-
   _detach: function () {
     if (this._threadActor !== null) {
       this._transport.close();

devtools/shared/client/main.js

@@ -443,7 +443,6 @@ DebuggerClient.prototype = {
       DevToolsUtils.executeSoon(() => aOnResponse({
         from: workerClient.actor,
         type: "attached",
-        isFrozen: workerClient.isFrozen,
         url: workerClient.url
       }, workerClient));
       return;
@@ -1302,16 +1301,11 @@ function WorkerClient(aClient, aForm) {
   this.client = aClient;
   this._actor = aForm.from;
   this._isClosed = false;
-  this._isFrozen = aForm.isFrozen;
   this._url = aForm.url;
 
   this._onClose = this._onClose.bind(this);
-  this._onFreeze = this._onFreeze.bind(this);
-  this._onThaw = this._onThaw.bind(this);
 
   this.addListener("close", this._onClose);
-  this.addListener("freeze", this._onFreeze);
-  this.addListener("thaw", this._onThaw);
 
   this.traits = {};
 }
@@ -1337,10 +1331,6 @@ WorkerClient.prototype = {
     return this._isClosed;
   },
 
-  get isFrozen() {
-    return this._isFrozen;
-  },
-
   detach: DebuggerClient.requester({ type: "detach" }, {
     after: function (aResponse) {
       this.client.unregisterClient(this);
@@ -1374,26 +1364,16 @@ WorkerClient.prototype = {
 
   _onClose: function () {
     this.removeListener("close", this._onClose);
-    this.removeListener("freeze", this._onFreeze);
-    this.removeListener("thaw", this._onThaw);
 
     this.client.unregisterClient(this);
-    this._closed = true;
-  },
-
-  _onFreeze: function () {
-    this._isFrozen = true;
-  },
-
-  _onThaw: function () {
-    this._isFrozen = false;
+    this._isClosed = true;
   },
 
   reconfigure: function () {
     return Promise.resolve();
   },
 
-  events: ["close", "freeze", "thaw"]
+  events: ["close"]
 };
 
 eventSource(WorkerClient.prototype);

dom/workers/WorkerPrivate.cpp

@@ -2278,6 +2278,8 @@ WorkerPrivateParent<Derived>::DisableDebugger()
   if (NS_FAILED(UnregisterWorkerDebugger(self->mDebugger))) {
     NS_WARNING("Failed to unregister worker debugger!");
   }
+
+  self->mDebugger = nullptr;
 }
 
 template <class Derived>
@@ -2525,6 +2527,8 @@ WorkerPrivateParent<Derived>::Freeze(JSContext* aCx, nsPIDOMWindow* aWindow)
     }
   }
 
+  DisableDebugger();
+
   nsRefPtr<FreezeRunnable> runnable =
     new FreezeRunnable(ParentAsWorkerPrivate());
   if (!runnable->Dispatch(aCx)) {
@@ -2591,6 +2595,8 @@ WorkerPrivateParent<Derived>::Thaw(JSContext* aCx, nsPIDOMWindow* aWindow)
     }
   }
 
+  EnableDebugger();
+
   // Execute queued runnables before waking up the worker, otherwise the worker
   // could post new messages before we run those that have been queued.
   if (!IsSuspended() && !mQueuedRunnables.IsEmpty()) {
@@ -3494,8 +3500,7 @@ WorkerDebugger::WorkerDebugger(WorkerPrivate* aWorkerPrivate)
   mCondVar(mMutex, "WorkerDebugger::mCondVar"),
   mWorkerPrivate(aWorkerPrivate),
   mIsEnabled(false),
-  mIsInitialized(false),
-  mIsFrozen(false)
+  mIsInitialized(false)
 {
   mWorkerPrivate->AssertIsOnParentThread();
 }
@@ -3549,21 +3554,6 @@ WorkerDebugger::GetIsChrome(bool* aResult)
   return NS_OK;
 }
 
-NS_IMETHODIMP
-WorkerDebugger::GetIsFrozen(bool* aResult)
-{
-  AssertIsOnMainThread();
-
-  MutexAutoLock lock(mMutex);
-
-  if (!mWorkerPrivate) {
-    return NS_ERROR_UNEXPECTED;
-  }
-
-  *aResult = mIsFrozen;
-  return NS_OK;
-}
-
 NS_IMETHODIMP
 WorkerDebugger::GetIsInitialized(bool* aResult)
 {
@@ -3778,52 +3768,6 @@ WorkerDebugger::Disable()
   NotifyIsEnabled(false);
 }
 
-void
-WorkerDebugger::Freeze()
-{
-  mWorkerPrivate->AssertIsOnWorkerThread();
-
-  nsCOMPtr<nsIRunnable> runnable =
-    NS_NewRunnableMethod(this, &WorkerDebugger::FreezeOnMainThread);
-  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(
-    NS_DispatchToMainThread(runnable, NS_DISPATCH_NORMAL)));
-}
-
-void
-WorkerDebugger::FreezeOnMainThread()
-{
-  AssertIsOnMainThread();
-
-  mIsFrozen = true;
-
-  for (size_t index = 0; index < mListeners.Length(); ++index) {
-    mListeners[index]->OnFreeze();
-  }
-}
-
-void
-WorkerDebugger::Thaw()
-{
-  mWorkerPrivate->AssertIsOnWorkerThread();
-
-  nsCOMPtr<nsIRunnable> runnable =
-    NS_NewRunnableMethod(this, &WorkerDebugger::ThawOnMainThread);
-  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(
-    NS_DispatchToMainThread(runnable, NS_DISPATCH_NORMAL)));
-}
-
-void
-WorkerDebugger::ThawOnMainThread()
-{
-  AssertIsOnMainThread();
-
-  mIsFrozen = false;
-
-  for (size_t index = 0; index < mListeners.Length(); ++index) {
-    mListeners[index]->OnThaw();
-  }
-}
-
 void
 WorkerDebugger::PostMessageToDebugger(const nsAString& aMessage)
 {
@@ -5014,7 +4958,6 @@ WorkerPrivate::FreezeInternal(JSContext* aCx)
   NS_ASSERTION(!mFrozen, "Already frozen!");
 
   mFrozen = true;
-  mDebugger->Freeze();
   return true;
 }
 
@@ -5026,7 +4969,6 @@ WorkerPrivate::ThawInternal(JSContext* aCx)
   NS_ASSERTION(mFrozen, "Not yet frozen!");
 
   mFrozen = false;
-  mDebugger->Thaw();
   return true;
 }
 

dom/workers/WorkerPrivate.h

@@ -825,7 +825,6 @@ class WorkerDebugger : public nsIWorkerDebugger {
 
   // Only touched on the main thread.
   bool mIsInitialized;
-  bool mIsFrozen;
   nsTArray<nsCOMPtr<nsIWorkerDebuggerListener>> mListeners;
 
 public:
@@ -846,12 +845,6 @@ public:
   void
   Disable();
 
-  void
-  Freeze();
-
-  void
-  Thaw();
-
   void
   PostMessageToDebugger(const nsAString& aMessage);
 
@@ -866,12 +859,6 @@ private:
   void
   NotifyIsEnabled(bool aIsEnabled);
 
-  void
-  FreezeOnMainThread();
-
-  void
-  ThawOnMainThread();
-
   void
   PostMessageToDebuggerOnMainThread(const nsAString& aMessage);
 

dom/workers/nsIWorkerDebugger.idl

@@ -2,7 +2,7 @@
 
 interface nsIDOMWindow;
 
-[scriptable, uuid(530db841-1b2c-485a-beeb-f2b1acb9714e)]
+[scriptable, uuid(9cf3b48e-361d-486a-8917-55cf8d00bb41)]
 interface nsIWorkerDebuggerListener : nsISupports
 {
   void onClose();
@@ -10,14 +10,10 @@ interface nsIWorkerDebuggerListener : nsISupports
   void onError(in DOMString filename, in unsigned long lineno,
                in DOMString message);
 
-  void onFreeze();
-
   void onMessage(in DOMString message);
-
-  void onThaw();
 };
 
-[scriptable, builtinclass, uuid(bdcaf96f-916a-4b24-bb53-165c1785668b)]
+[scriptable, builtinclass, uuid(2b8d801c-973d-425b-a6d5-1a2505dd8b78)]
 interface nsIWorkerDebugger : nsISupports
 {
   const unsigned long TYPE_DEDICATED = 0;
@@ -28,8 +24,6 @@ interface nsIWorkerDebugger : nsISupports
 
   readonly attribute bool isChrome;
 
-  readonly attribute bool isFrozen;
-
   readonly attribute bool isInitialized;
 
   readonly attribute nsIWorkerDebugger parent;

dom/workers/test/WorkerDebugger_frozen_iframe1.html

@@ -3,7 +3,7 @@
   <head>
     <meta charset="utf-8">
     <script>
-      var worker = new Worker("WorkerDebugger.isFrozen_worker1.js");
+      var worker = new Worker("WorkerDebugger_frozen_worker1.js");
       worker.onmessage = function () {
         parent.postMessage("ready", "*");
       };

dom/workers/test/WorkerDebugger_frozen_iframe2.html

@@ -3,7 +3,7 @@
   <head>
     <meta charset="utf-8">
     <script>
-      var worker = new Worker("WorkerDebugger.isFrozen_worker2.js");
+      var worker = new Worker("WorkerDebugger_frozen_worker2.js");
       worker.onmessage = function () {
         parent.postMessage("ready", "*");
       };

dom/workers/test/chrome.ini

@@ -4,13 +4,13 @@ support-files =
   WorkerDebugger.initialize_childWorker.js
   WorkerDebugger.initialize_debugger.js
   WorkerDebugger.initialize_worker.js
-  WorkerDebugger.isFrozen_iframe1.html
-  WorkerDebugger.isFrozen_iframe2.html
-  WorkerDebugger.isFrozen_worker1.js
-  WorkerDebugger.isFrozen_worker2.js
   WorkerDebugger.postMessage_childWorker.js
   WorkerDebugger.postMessage_debugger.js
   WorkerDebugger.postMessage_worker.js
+  WorkerDebugger_frozen_iframe1.html
+  WorkerDebugger_frozen_iframe2.html
+  WorkerDebugger_frozen_worker1.js
+  WorkerDebugger_frozen_worker2.js
   WorkerDebuggerGlobalScope.createSandbox_debugger.js
   WorkerDebuggerGlobalScope.createSandbox_sandbox.js
   WorkerDebuggerGlobalScope.createSandbox_worker.js
@@ -54,15 +54,15 @@ support-files =
   sharedWorker_privateBrowsing.js
   test_bug883784.jsm
 
-[test_WorkerDebugger.xul]
 [test_WorkerDebugger.initialize.xul]
-[test_WorkerDebugger.isFrozen.xul]
 [test_WorkerDebugger.postMessage.xul]
+[test_WorkerDebugger.xul]
 [test_WorkerDebuggerGlobalScope.createSandbox.xul]
 [test_WorkerDebuggerGlobalScope.enterEventLoop.xul]
 [test_WorkerDebuggerGlobalScope.reportError.xul]
 [test_WorkerDebuggerGlobalScope.setImmediate.xul]
 [test_WorkerDebuggerManager.xul]
+[test_WorkerDebugger_frozen.xul]
 [test_WorkerDebugger_suspended.xul]
 [test_bug883784.xul]
 [test_chromeWorker.xul]

dom/workers/test/dom_worker_helper.js

@@ -62,6 +62,7 @@ function waitForRegister(url, dbgUrl) {
   return new Promise(function (resolve) {
     wdm.addListener({
       onRegister: function (dbg) {
+        dump("FAK " + dbg.url + "\n");
         if (dbg.url !== url) {
           return;
         }
@@ -130,28 +131,6 @@ function waitForDebuggerMessage(dbg, message) {
   });
 }
 
-function waitForDebuggerFreeze(dbg) {
-  return new Promise(function (resolve) {
-    dbg.addListener({
-      onFreeze: function () {
-        dbg.removeListener(this);
-        resolve();
-      }
-    });
-  });
-}
-
-function waitForDebuggerThaw(dbg) {
-  return new Promise(function (resolve) {
-    dbg.addListener({
-      onThaw: function () {
-        dbg.removeListener(this);
-        resolve();
-      }
-    });
-  });
-}
-
 function waitForWindowMessage(window, message) {
   return new Promise(function (resolve) {
     let onmessage = function (event) {

dom/workers/test/test_WorkerDebugger_frozen.xul

@@ -3,7 +3,7 @@
   Any copyright is dedicated to the Public Domain.
   http://creativecommons.org/publicdomain/zero/1.0/
 -->
-<window title="Test for WorkerDebugger.isFrozen"
+<window title="Test for WorkerDebugger with frozen workers"
         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
         onload="test();">
 
@@ -19,11 +19,11 @@
     const CACHE_SUBFRAMES = "browser.sessionhistory.cache_subframes";
     const MAX_TOTAL_VIEWERS = "browser.sessionhistory.max_total_viewers";
 
-    const IFRAME1_URL = "WorkerDebugger.isFrozen_iframe1.html";
-    const IFRAME2_URL = "WorkerDebugger.isFrozen_iframe2.html";
+    const IFRAME1_URL = "WorkerDebugger_frozen_iframe1.html";
+    const IFRAME2_URL = "WorkerDebugger_frozen_iframe2.html";
 
-    const WORKER1_URL = "WorkerDebugger.isFrozen_worker1.js";
-    const WORKER2_URL = "WorkerDebugger.isFrozen_worker2.js";
+    const WORKER1_URL = "WorkerDebugger_frozen_worker1.js";
+    const WORKER2_URL = "WorkerDebugger_frozen_worker2.js";
 
     function test() {
       Task.spawn(function* () {
@@ -44,39 +44,31 @@
         let [dbg1] = yield promise;
         is(dbg1.isClosed, false,
            "debugger for worker on page 1 should not be closed");
-        is(dbg1.isFrozen, false,
-           "debugger for worker on page 1 should not be frozen");
 
         promise = waitForMultiple([
-          waitForDebuggerFreeze(dbg1),
+          waitForUnregister(WORKER1_URL),
+          waitForDebuggerClose(dbg1),
           waitForRegister(WORKER2_URL),
           waitForWindowMessage(window, "ready"),
         ]);
         iframe.src = IFRAME2_URL;
-        let [_, dbg2] = yield promise;
-        is(dbg1.isClosed, false,
-           "debugger for worker on page 1 should not be closed");
-        is(dbg1.isFrozen, true,
-           "debugger for worker on page 1 should be frozen");
+        let [,, dbg2] = yield promise;
+        is(dbg1.isClosed, true,
+           "debugger for worker on page 1 should be closed");
         is(dbg2.isClosed, false,
            "debugger for worker on page 2 should not be closed");
-        is(dbg2.isFrozen, false,
-           "debugger for worker on page 2 should not be frozen");
 
         promise = Promise.all([
-          waitForDebuggerFreeze(dbg2),
-          waitForDebuggerThaw(dbg1),
+          waitForUnregister(WORKER2_URL),
+          waitForDebuggerClose(dbg2),
+          waitForRegister(WORKER1_URL)
         ]);
         iframe.contentWindow.history.back();
-        yield promise;
+        [,, dbg1] = yield promise;
         is(dbg1.isClosed, false,
-           "debugger for worker on page 1 should not be closed")
-        is(dbg1.isFrozen, false,
-           "debugger for worker on page 1 should not be frozen");
-        is(dbg2.isClosed, false,
-           "debugger for worker on page 2 should not be closed");
-        is(dbg2.isFrozen, true,
-           "debugger for worker on page 2 should be frozen");
+           "debugger for worker on page 1 should not be closed");
+        is(dbg2.isClosed, true,
+           "debugger for worker on page 2 should be closed");
 
         SpecialPowers.clearUserPref(CACHE_SUBFRAMES);
         SpecialPowers.setIntPref(MAX_TOTAL_VIEWERS, oldMaxTotalViewers);