Bug 790201 - better cleanup of social worker sandbox. r=felipe

toolkit/components/social/FrameWorker.jsm

@@ -69,6 +69,7 @@ function FrameWorker(url, name) {
   this.ports = {};
   this.pendingPorts = [];
   this.loaded = false;
+  this.reloading = false;
 
   this.frame = makeHiddenFrame();
   this.load();
@@ -81,7 +82,7 @@ FrameWorker.prototype = {
       if (!doc.defaultView || doc.defaultView != self.frame.contentWindow) {
         return;
       }
-      Services.obs.removeObserver(injectController, "document-element-inserted", false);
+      Services.obs.removeObserver(injectController, "document-element-inserted");
       try {
         self.createSandbox();
       } catch (e) {
@@ -100,8 +101,12 @@ FrameWorker.prototype = {
       this.pendingPorts.push(port);
     }
     this.ports = {};
-    this.loaded = false;
-    this.load();
+    // 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.
+    this.reloading = true;
+    this.frame.setAttribute("src", "about:blank");
   },
 
   createSandbox: function createSandbox() {
@@ -118,8 +123,12 @@ FrameWorker.prototype = {
                      'location'];
     workerAPI.forEach(function(fn) {
       try {
-        // XXX Need to unwrap for this to work - find out why!
-        sandbox[fn] = XPCNativeWrapper.unwrap(workerWindow)[fn];
+        // Bug 798660 - XHR and WebSocket have issues in a sandbox and need
+        // to be unwrapped to work
+        if (fn == "XMLHttpRequest" || fn == "WebSocket")
+          sandbox[fn] = XPCNativeWrapper.unwrap(workerWindow)[fn];
+        else
+          sandbox[fn] = workerWindow[fn];
       }
       catch(e) {
         Cu.reportError("FrameWorker: failed to import API "+fn+"\n"+e+"\n");
@@ -168,8 +177,9 @@ FrameWorker.prototype = {
       workerWindow.addEventListener(t, l, c)
     };
 
-    this.sandbox = sandbox;
-
+    // Note we don't need to stash |sandbox| in |this| as the unload handler
+    // has a reference in its closure, so it can't die until that handler is
+    // removed - at which time we've explicitly killed it anyway.
     let worker = this;
 
     workerWindow.addEventListener("load", function loadListener() {
@@ -219,24 +229,65 @@ FrameWorker.prototype = {
         }
       }
     });
+
+    // 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) {
+          Cu.reportError("FrameWorker: failed to close port. " + ex);
+        }
+      }
+      // Must reset this to an array incase we are being reloaded.
+      worker.ports = [];
+      // The worker window may not have fired a load event yet, so pendingPorts
+      // might still have items in it - close them too.
+      worker.loaded = false;
+      // If the worker is reloading, when we don't actually close the pending
+      // ports as they are the ports which need to be re-entangled.
+      if (!worker.reloading) {
+        for (let port of worker.pendingPorts) {
+          try {
+            port.close();
+          } catch (ex) {
+            Cu.reportError("FrameWorker: failed to close pending port. " + ex);
+          }
+        }
+        worker.pendingPorts = [];
+      }
+
+      if (sandbox) {
+        Cu.nukeSandbox(sandbox);
+        sandbox = null;
+      }
+      if (worker.reloading) {
+        Services.tm.mainThread.dispatch(function doReload() {
+          worker.reloading = false;
+          worker.load();
+        }, Ci.nsIThread.DISPATCH_NORMAL);
+      }
+    });
   },
 
   terminate: function terminate() {
-    // closing the port also removes it from this.ports via port-close
-    for (let [portid, port] in Iterator(this.ports)) {
-      // port may have been closed as a side-effect from closing another port
-      if (!port)
-        continue;
-      try {
-        port.close();
-      } catch (ex) {
-        Cu.reportError("FrameWorker: failed to close port. " + ex);
-      }
+    if (!(this.url in workerCache)) {
+      // terminating an already terminated worker - ignore it
+      return;
     }
-
+    // we want to "forget" about this worker now even though the termination
+    // may not be complete for a little while...
     delete workerCache[this.url];
-
-    // let pending events get delivered before actually removing the frame
+    // let pending events get delivered before actually removing the frame,
+    // then we perform the actual cleanup in the unload handler.
     Services.tm.mainThread.dispatch(function deleteWorkerFrame() {
       // now nuke the iframe itself and forget everything about this worker.
       this.frame.parentNode.removeChild(this.frame);

toolkit/components/social/test/browser/worker_relative.js

@@ -10,6 +10,7 @@ onconnect = function(e) {
     } else {
       port.postMessage({topic: "done", result: "import worked but global is not available"});
     }
+    return;
   } catch(e) {
     port.postMessage({topic: "done", result: "FAILED to importScripts, " + e.toString() });
     return;