Bug 899908 - remove ability for a social worker to reload and reconnect ports. r=mixedpuppy
authorMark Hammond <mhammond@skippinet.com.au>
Wed, 11 Sep 2013 16:10:34 +1000
changeset 146518 88a9034541fd6191d2ec741e09fc2b37fb7ce1df
parent 146517 dd627cf60b9e24ad08d2ef026e571b1559b013bf
child 146519 5417e5da2cebc0d34ccc05b16be513b5d8a01e55
child 146656 481195916b0ebdade2e3ad67b63b7a9064ab9e23
push id25263
push usercbook@mozilla.com
push dateWed, 11 Sep 2013 11:38:18 +0000
treeherdermozilla-central@5417e5da2ceb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy
bugs899908
milestone26.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 899908 - remove ability for a social worker to reload and reconnect ports. r=mixedpuppy
toolkit/components/social/FrameWorker.jsm
toolkit/components/social/FrameWorkerContent.js
toolkit/components/social/WorkerAPI.jsm
toolkit/components/social/test/browser/browser_frameworker.js
toolkit/components/social/test/browser/browser_workerAPI.js
toolkit/components/social/test/browser/worker_social.js
--- a/toolkit/components/social/FrameWorker.jsm
+++ b/toolkit/components/social/FrameWorker.jsm
@@ -77,27 +77,16 @@ function _Worker(browserPromise, options
     mm.loadFrameScript("resource://gre/modules/FrameWorkerContent.js", true);
     mm.sendAsyncMessage("frameworker:init", this.options);
     mm.addMessageListener("frameworker:port-message", this);
     mm.addMessageListener("frameworker:notify-worker-error", this);
   });
 }
 
 _Worker.prototype = {
-  reload: function() {
-    // In the future, it would be nice to just throw away the browser element
-    // and re-create from scratch.  The complication there would be the need
-    // to reconnect existing ports - but even that might be managable.
-    // However, bug 899908 calls for 'reload' to be dropped, so let's do that
-    // instead!
-    this.browserPromise.then(browser => {
-      browser.messageManager.sendAsyncMessage("frameworker:reload");
-    });
-  },
-
   // Message handler.
   receiveMessage: function(msg) {
     switch (msg.name) {
       case "frameworker:port-message":
         let port = this.ports.get(msg.data.portId);
         port._onmessage(msg.data.data);
         break;
       case "frameworker:notify-worker-error":
--- a/toolkit/components/social/FrameWorkerContent.js
+++ b/toolkit/components/social/FrameWorkerContent.js
@@ -40,17 +40,16 @@ function navigate(url) {
  * instead run in a sandbox that has a select set of methods cloned from the
  * URL's domain.
  */
 function FrameWorker(url, name, origin, exposeLocalStorage) {
   this.url = url;
   this.name = name || url;
   this.ports = new Map(); // all unclosed ports, including ones yet to be entangled
   this.loaded = false;
-  this.reloading = false;
   this.origin = origin;
   this._injectController = null;
   this.exposeLocalStorage = exposeLocalStorage;
 
   this.load();
 }
 
 FrameWorker.prototype = {
@@ -73,32 +72,16 @@ FrameWorker.prototype = {
 
   _maybeRemoveInjectController: function() {
     if (this._injectController) {
       Services.obs.removeObserver(this._injectController, "document-element-inserted");
       this._injectController = null;
     }
   },
 
-  reload: function FrameWorker_reloadWorker() {
-    // reset all ports as not entangled; they will then be re-entangled during
-    // the call to createSandbox after the document is reloaded
-    for (let [, port] of this.ports) {
-      port._entangled = false;
-    }
-    // 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 content to about:blank - this will fire the unload event
-    // but not remove our browser from the DOM.  Our unload handler will
-    // see this.reloading is true and reload for us.
-    this.reloading = true;
-    navigate("about:blank");
-  },
-
   createSandbox: function createSandbox() {
     let workerWindow = content;
     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', 'atob', 'btoa',
@@ -241,56 +224,39 @@ FrameWorker.prototype = {
       }
     });
 
     // the 'unload' listener cleans up the worker and the sandbox.  This
     // will be triggered by the window unloading as part of shutdown or reload.
     workerWindow.addEventListener("unload", function unloadListener() {
       workerWindow.removeEventListener("unload", unloadListener);
       worker.loaded = false;
-      // If the worker is reloading, when we don't actually close any ports as
-      // they need to be re-entangled.
-      // If content is already null, we can't send a close message, so skip it.
-      if (!worker.reloading && content) {
-        for (let [, port] of worker.ports) {
-          try {
-            port.close();
-          } catch (ex) {
-            Cu.reportError("FrameWorker: failed to close port. " + ex);
-          }
-        }
-        worker.ports.clear();
-      }
-
+      // No need to close ports - the worker probably wont see a
+      // social.port-closing message and certainly isn't going to have time to
+      // do anything if it did see it.
+      worker.ports.clear();
       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);
-      }
     });
   },
 };
 
 const FrameWorkerManager = {
   init: function() {
     // first, setup the docShell to disable some types of content
     docShell.allowAuth = false;
     docShell.allowPlugins = false;
     docShell.allowImages = false;
     docShell.allowMedia = false;
     docShell.allowWindowControl = false;
 
     addMessageListener("frameworker:init", this._onInit);
     addMessageListener("frameworker:connect", this._onConnect);
-    addMessageListener("frameworker:reload", this._onReload);
     addMessageListener("frameworker:port-message", this._onPortMessage);
     addMessageListener("frameworker:cookie-get", this._onCookieGet);
   },
 
   // This new frameworker is being created.  This should only be called once.
   _onInit: function(msg) {
     let {url, name, origin, exposeLocalStorage} = msg.data;
     frameworker = new FrameWorker(url, name, origin, exposeLocalStorage);
@@ -299,20 +265,16 @@ const FrameWorkerManager = {
   // A new port is being established for this frameworker.
   _onConnect: function(msg) {
     let port = new ClientPort(msg.data.portId);
     frameworker.ports.set(msg.data.portId, port);
     if (frameworker.loaded && !frameworker.reloading)
       port._createWorkerAndEntangle(frameworker);
   },
 
-  _onReload: function(msg) {
-    frameworker.reload();
-  },
-
   // A message related to a port.
   _onPortMessage: function(msg) {
     // find the "client" port for this message and have it post it into
     // the worker.
     let port = frameworker.ports.get(msg.data.portId);
     port._dopost(msg.data);
   },
 
--- a/toolkit/components/social/WorkerAPI.jsm
+++ b/toolkit/components/social/WorkerAPI.jsm
@@ -55,21 +55,17 @@ WorkerAPI.prototype = {
     },
     "social.manifest-set": function(data) {
       // the provider will get reloaded as a result of this call
       let SocialService = Cu.import("resource://gre/modules/SocialService.jsm", {}).SocialService;
       let origin = this._provider.origin;
       SocialService.updateProvider(origin, data);
     },
     "social.reload-worker": function(data) {
-      getFrameWorkerHandle(this._provider.workerURL, null)._worker.reload();
-      // the frameworker is going to be reloaded, send the initialization
-      // so it can have the same startup sequence as if it were loaded
-      // the first time.  This will be queued until the frameworker is ready.
-      this._port.postMessage({topic: "social.initialize"});
+      this._provider.reload();
     },
     "social.user-profile": function (data) {
       this._provider.updateUserProfile(data);
     },
     "social.ambient-notification": function (data) {
       this._provider.setAmbientNotification(data);
     },
     "social.cookies-get": function(data) {
--- a/toolkit/components/social/test/browser/browser_frameworker.js
+++ b/toolkit/components/social/test/browser/browser_frameworker.js
@@ -554,43 +554,16 @@ let tests = {
       cbnext();
     }, 'social:frameworker-error', false);
     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") {
-            // "worker" and "worker2" are handles to the same worker
-            worker2.terminate();
-            cbnext();
-          }
-        }
-      }
-    }
-  },
-
   // This will create the worker, then send a message to the port, then close
   // the port - all before the worker has actually initialized.
   testCloseFirstSend: function(cbnext) {
     let run = function() {
       let numPings = 0, numCloses = 0;
       onconnect = function(e) {
         let port = e.ports[0];
         port.onmessage = function(e) {
--- a/toolkit/components/social/test/browser/browser_workerAPI.js
+++ b/toolkit/components/social/test/browser/browser_workerAPI.js
@@ -123,113 +123,51 @@ let tests = {
     port.postMessage({topic: "test-initialization"});
     port.postMessage({topic: "test.cookies-get"});
   },
 
   testWorkerReload: function(next) {
     let fw = {};
     Cu.import("resource://gre/modules/FrameWorker.jsm", fw);
 
-    // get a real handle to the worker so we can watch the unload event
-    // we watch for the unload of the worker to know it is infact being
-    // unloaded, after that if we get worker.connected we know that
-    // the worker was loaded again and ports reconnected.
-
-    // Given our <browser remote="true"> model, we have to some of this in a
-    // content script.  We turn this function into a data: URL...
-    // (and note that this code only makes sense in the context of
-    //  FrameWorkerContent.js...)
-    let contentScript = function() {
-      // the content script may be executed while there are pending messages,
-      // such as ports from the previous test being closed, which screws us up.
-      // By only doing this work on a message, we ensure previous messages have
-      // all been delivered.
-      addMessageListener("frameworker-test:ready", function onready() {
-        removeMessageListener("frameworker-test:ready", onready);
-        // first, find our test's port - it will be the last one.
-        let port, id
-        for ([id, port] of frameworker.ports) {
-          ; // nothing to do - we just want to get the last...
-        }
-        let unloadHasFired = false,
-            haveNoPendingMessagesBefore = true,
-            havePendingMessagesAfter = false;
-        content.addEventListener("unload", function workerUnload(e) {
-          content.removeEventListener("unload", workerUnload);
-          // There should be no "pending" messages with one subtle exception -
-          // there *might* be a social.initialize message - the reload process
-          // posts a message to do the reload, then posts the init message - it
-          // may or maynot have arrived here by now - but there certainly should
-          // be no other messages.
-          for (let [temp_id, temp_port] of frameworker.ports) {
-            if (temp_port._pendingMessagesOutgoing.length == 0)
-              continue;
-            if (temp_port._pendingMessagesOutgoing.length == 1 &&
-                temp_port._pendingMessagesOutgoing[0].data &&
-                JSON.parse(temp_port._pendingMessagesOutgoing[0].data).topic == "social.initialize")
-              continue;
-            // we found something we aren't expecting...
-            haveNoPendingMessagesBefore = false;
-          }
-          unloadHasFired = true; // at the end, so errors above aren't masked.
-        });
-        addEventListener("DOMWindowCreated", function workerLoaded(e) {
-          removeEventListener("DOMWindowCreated", workerLoaded);
-          // send a message which should end up pending
-          port.postMessage({topic: "test-pending-msg"});
-          for (let [temp_id, temp_port] of frameworker.ports) {
-            if (temp_port._pendingMessagesOutgoing.length >= 0) {
-              havePendingMessagesAfter = true;
-            }
-          }
-          let ok = unloadHasFired && haveNoPendingMessagesBefore && havePendingMessagesAfter;
-          sendAsyncMessage("test-result", {ok: ok});
-        });
-      });
-    };
-
-    let reloading = false;
     let worker = fw.getFrameWorkerHandle(provider.workerURL, undefined, "testWorkerReload");
     let port = provider.getWorkerPort();
-    worker._worker.browserPromise.then(browser => {
-      browser.messageManager.loadFrameScript("data:," + encodeURI(contentScript.toSource()) + "();", false);
-      browser.messageManager.sendAsyncMessage("frameworker-test:ready");
-      let seenTestResult = false;
-      browser.messageManager.addMessageListener("test-result", function _onTestResult(msg) {
-        browser.messageManager.removeMessageListener("test-result", _onTestResult);
-        ok(msg.json.ok, "test result from content-script is ok");
-        seenTestResult = true;
-      });
-
-      port.onmessage = function (e) {
+    // this observer will be fired when the worker reloads - it ensures the
+    // old port was closed and the new worker is functioning.
+    Services.obs.addObserver(function reloadObserver() {
+      Services.obs.removeObserver(reloadObserver, "social:provider-reload");
+      ok(port._closed, "old port was closed by the reload");
+      let newWorker = fw.getFrameWorkerHandle(provider.workerURL, undefined, "testWorkerReload - worker2");
+      let newPort = provider.getWorkerPort();
+      newPort.onmessage = function (e) {
         let topic = e.data.topic;
         switch (topic) {
           case "test-initialization-complete":
-            // tell the worker to send the reload msg - that will trigger the
-            // frameworker to unload and for our content script's unload
-            // handler to post a "test-pending" message, which in-turn causes
-            // the worker to send the test-pending-response.
-            // All of which goes to prove that if a message is delivered while
-            // the frameworker is unloaded due to a reload, that message still
-            // gets delivered.
-            port.postMessage({topic: "test-reload-init"});
-            break;
-          case "test-pending-response":
-            ok(e.data.data.seenInit, "worker has seen the social.initialize message");
-            // now we've been reloaded, check 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");
-            ok(seenTestResult, "saw result message from content");
+            // and we are done.
+            newPort.close();
             next();
             break;
         }
       }
-      port.postMessage({topic: "test-initialization"});
-    });
+      newPort.postMessage({topic: "test-initialization"});
+    }, "social:provider-reload", false);
+
+    port.onmessage = function (e) {
+      let topic = e.data.topic;
+      switch (topic) {
+        case "test-initialization-complete":
+          // tell the worker to send the reload msg - that will trigger the
+          // frameworker to unload and for our content script's unload
+          // handler to post a "test-result" message.
+          port.postMessage({topic: "test-reload-init"});
+          // and the "social:provider-reload" observer should fire...
+          break;
+      }
+    }
+    port.postMessage({topic: "test-initialization"});
   },
 
   testNotificationLinks: function(next) {
     let port = provider.getWorkerPort();
     let data = {
       id: 'an id',
       body: 'the text',
       action: 'link',
--- a/toolkit/components/social/test/browser/worker_social.js
+++ b/toolkit/components/social/test/browser/worker_social.js
@@ -20,33 +20,26 @@ onconnect = function(e) {
         break;
       case "test-initialization":
         testerPort = port;
         port.postMessage({topic: "test-initialization-complete"});
         break;
       case "test-profile":
         apiPort.postMessage({topic: "social.user-profile", data: data});
         break;
-      case "test-pending-msg":
-        // we also want to check we have seen a social.initialize message before
-        // this one, so send that back in the response.
-        port.postMessage({topic: "test-pending-response", data: {seenInit: !!apiPort}});
-        break;
       case "test-ambient":
         apiPort.postMessage({topic: "social.ambient-notification", data: data});
         break;
       case "test.cookies-get":
         apiPort.postMessage({topic: "social.cookies-get"});
         break;
       case "social.cookies-get-response":
         testerPort.postMessage({topic: "test.cookies-get-response", data: data});
         break;
       case "test-reload-init":
-        // browser_social_sidebar.js started test, tell the sidebar to
-        // start
         apiPort.postMessage({topic: 'social.reload-worker'});
         break;
       case "test-notification-create":
         apiPort.postMessage({topic: 'social.notification-create',
                              data: data});
         testerPort.postMessage({topic: 'did-notification-create'});
         break;
       case "test-indexeddb-create":
@@ -58,14 +51,9 @@ onconnect = function(e) {
           // Do something with request.result!
           var db = request.result;
           db.close();
           port.postMessage({topic: 'social.indexeddb-result', data: { result: "ok" }});
         };
         break;
     }
   }
-  // used for "test-reload-worker"
-  if (apiPort && apiPort != port) {
-    port.postMessage({topic: "worker.connected"})
-  }
-
 }