Bug 811102: fix two bugs with worker reloading, r=gavin/markh, tests by markh/gavin
authorShane Caraveo <mixedpuppy@gmail.com>
Mon, 12 Nov 2012 17:50:51 -0800
changeset 113049 85b3e7667edc32526740f1391709b730d63f6ba5
parent 113048 0d124d4f28b0d3365ae7c4ec0ab4b2a99e399405
child 113050 34e2424d10442b234b713f04b596c884160753d7
push id17909
push usergsharp@mozilla.com
push dateTue, 13 Nov 2012 01:51:52 +0000
treeherdermozilla-inbound@85b3e7667edc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgavin, markh, tests
bugs811102
milestone19.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 811102: fix two bugs with worker reloading, r=gavin/markh, tests by markh/gavin The first bug is that we shouldn't remove the worker from the worker cache when reloading, because it's not actually being removed. terminate() already does this for us in the case where it is needed. The second bug is that we should set .loaded=false as soon as we clear the ports on a worker we're about to reload, because otherwise the creation of a new port can occur between the reload triggering and the re-initialization, and those ports would get lost.
toolkit/components/social/FrameWorker.jsm
toolkit/components/social/test/browser/browser_frameworker.js
toolkit/components/social/test/browser/browser_workerAPI.js
--- a/toolkit/components/social/FrameWorker.jsm
+++ b/toolkit/components/social/FrameWorker.jsm
@@ -97,20 +97,22 @@ FrameWorker.prototype = {
   reload: function FrameWorker_reloadWorker() {
     // push all the ports into pending ports, they will be re-entangled
     // during the call to createSandbox after the document is reloaded
     for (let [portid, port] in Iterator(this.ports)) {
       port._window = null;
       this.pendingPorts.push(port);
     }
     this.ports = {};
+    // Mark the provider as unloaded now, so that any new ports created after
+    // this point but before the unload has fired are properly queued up.
+    this.loaded = false;
     // reset the iframe to about:blank - this will fire the unload event
     // but not remove the iframe from the DOM.  Our unload handler will
-    // (a) set this.loaded to false then (b) see this.reloading is true and
-    // reload for us.
+    // see this.reloading is true and reload for us.
     this.reloading = true;
     this.frame.setAttribute("src", "about:blank");
   },
 
   createSandbox: function createSandbox() {
     let workerWindow = this.frame.contentWindow;
     let sandbox = new Cu.Sandbox(workerWindow);
 
@@ -242,17 +244,16 @@ FrameWorker.prototype = {
       worker.pendingPorts = [];
     });
 
     // the 'unload' listener cleans up the worker and the sandbox.  This
     // will be triggered via either our 'terminate' function or by the
     // window unloading as part of shutdown.
     workerWindow.addEventListener("unload", function unloadListener() {
       workerWindow.removeEventListener("unload", unloadListener);
-      delete workerCache[worker.url];
       // closing the port also removes it from this.ports via port-close
       for (let [portid, port] in Iterator(worker.ports)) {
         // port may have been closed as a side-effect from closing another port
         if (!port)
           continue;
         try {
           port.close();
         } catch (ex) {
--- a/toolkit/components/social/test/browser/browser_frameworker.js
+++ b/toolkit/components/social/test/browser/browser_frameworker.js
@@ -521,9 +521,34 @@ let tests = {
     }
     let worker = getFrameWorkerHandle(makeWorkerUrl(run),
                                       undefined, "testWorkerConnectError");
     worker.port.onmessage = function(e) {
       ok(false, "social:frameworker-error was handled");
       cbnext();
     }
   },
+
+  testReloadAndNewPort: function(cbnext) {
+    let run = function () {
+      onconnect = function(e) {
+        e.ports[0].postMessage({topic: "ready"});
+      }
+    }
+    let doneReload = false;
+    let worker = getFrameWorkerHandle(makeWorkerUrl(run),
+                                      undefined, "testReloadAndNewPort");
+    worker.port.onmessage = function(e) {
+      if (e.data.topic == "ready" && !doneReload) {
+        // do the "reload"
+        doneReload = true;
+        worker._worker.reload();
+        let worker2 = getFrameWorkerHandle(makeWorkerUrl(run),
+                                           undefined, "testReloadAndNewPort");
+        worker2.port.onmessage = function(e) {
+          if (e.data.topic == "ready") {
+            cbnext();
+          }
+        }
+      }
+    }
+  },
 }
--- a/toolkit/components/social/test/browser/browser_workerAPI.js
+++ b/toolkit/components/social/test/browser/browser_workerAPI.js
@@ -132,16 +132,20 @@ let tests = {
     port.onmessage = function (e) {
       let topic = e.data.topic;
       switch (topic) {
         case "test-initialization-complete":
           // tell the worker to send the reload msg
           port.postMessage({topic: "test-reload-init"});
           break;
         case "test-pending-response":
+          // now we've been reloaded, check that we got the pending message
+          // and that our worker is still the same
+          let newWorker = fw.getFrameWorkerHandle(provider.workerURL, undefined, "testWorkerReload");
+          is(worker._worker, newWorker._worker, "worker is the same after reload");
           ok(true, "worker reloaded and testPort was reconnected");
           next();
           break;
       }
     }
     port.postMessage({topic: "test-initialization"});
   }
 };