Bug 790201 - better cleanup of social worker sandbox. r=felipe
authorMark Hammond <mhammond@skippinet.com.au>
Thu, 18 Oct 2012 16:42:40 +1100
changeset 110636 b872b207397dc5c15ad9db923b4d242e41630893
parent 110635 fdd5c75c14f2e5b79e7505d0232d18612e7edaf3
child 110637 0923556aea0eac9f582e209df5928ca4991faf0f
push id23704
push useremorley@mozilla.com
push dateThu, 18 Oct 2012 17:12:58 +0000
treeherdermozilla-central@3779eb3f036f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfelipe
bugs790201
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 790201 - better cleanup of social worker sandbox. r=felipe
toolkit/components/social/FrameWorker.jsm
toolkit/components/social/test/browser/worker_relative.js
--- a/toolkit/components/social/FrameWorker.jsm
+++ b/toolkit/components/social/FrameWorker.jsm
@@ -64,29 +64,30 @@ function getFrameWorkerHandle(url, clien
  * that has a select set of methods cloned from the URL's domain.
  */
 function FrameWorker(url, name) {
   this.url = url;
   this.name = name || url;
   this.ports = {};
   this.pendingPorts = [];
   this.loaded = false;
+  this.reloading = false;
 
   this.frame = makeHiddenFrame();
   this.load();
 }
 
 FrameWorker.prototype = {
   load: function FrameWorker_loadWorker() {
     var self = this;
     Services.obs.addObserver(function injectController(doc, topic, data) {
       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) {
         Cu.reportError("FrameWorker: failed to create sandbox for " + url + ". " + e);
       }
     }, "document-element-inserted", false);
 
     this.frame.setAttribute("src", this.url);
@@ -95,36 +96,44 @@ 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 = {};
-    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() {
     let workerWindow = this.frame.contentWindow;
     let sandbox = new Cu.Sandbox(workerWindow);
 
     // copy the window apis onto the sandbox namespace only functions or
     // objects that are naturally a part of an iframe, I'm assuming they are
     // safe to import this way
     let workerAPI = ['WebSocket', 'localStorage', 'atob', 'btoa',
                      'clearInterval', 'clearTimeout', 'dump',
                      'setInterval', 'setTimeout', 'XMLHttpRequest',
                      'MozBlobBuilder', 'FileReader', 'Blob',
                      '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");
       }
     });
     // the "navigator" object in a worker is a subset of the full navigator;
     // specifically, just the interfaces 'NavigatorID' and 'NavigatorOnLine'
     let navigator = {
@@ -163,18 +172,19 @@ FrameWorker.prototype = {
 
     sandbox._postMessage = function fw_postMessage(d, o) {
       workerWindow.postMessage(d, o)
     };
     sandbox._addEventListener = function fw_addEventListener(t, l, c) {
       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() {
       workerWindow.removeEventListener("load", loadListener);
       // the iframe has loaded the js file as text - first inject the magic
       // port-handling code into the sandbox.
       try {
         Services.scriptloader.loadSubScript("resource://gre/modules/MessagePortBase.jsm", sandbox);
@@ -214,34 +224,75 @@ FrameWorker.prototype = {
             port._createWorkerAndEntangle(worker);
           }
           catch(e) {
             Cu.reportError("FrameWorker: Failed to create worker port: " + e + "\n" + e.stack);
           }
         }
       }
     });
+
+    // 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);
     }.bind(this), Ci.nsIThread.DISPATCH_NORMAL);
   }
 };
 
 function makeHiddenFrame() {
--- a/toolkit/components/social/test/browser/worker_relative.js
+++ b/toolkit/components/social/test/browser/worker_relative.js
@@ -5,14 +5,15 @@ onconnect = function(e) {
   try {
     importScripts("relative_import.js");
     // the import should have exposed "testVar" and "testFunc" from the module.
     if (testVar == "oh hai" && testFunc() == "oh hai") {
       port.postMessage({topic: "done", result: "ok"});
     } 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;
   }
   port.postMessage({topic: "done", result: "FAILED to importScripts, no exception" });
 }