Bug 899908 - remove ability for a social worker to reload and reconnect ports. r=mixedpuppy

toolkit/components/social/FrameWorker.jsm

@@ -82,17 +82,6 @@ function _Worker(browserPromise, options) {
 }
 
 _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) {

toolkit/components/social/FrameWorkerContent.js

@@ -45,7 +45,6 @@ function FrameWorker(url, name, origin, exposeLocalStorage) {
   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;
@@ -78,22 +77,6 @@ FrameWorker.prototype = {
     }
   },
 
-  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);
@@ -246,30 +229,14 @@ FrameWorker.prototype = {
     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);
-      }
     });
   },
 };
@@ -285,7 +252,6 @@ const FrameWorkerManager = {
 
     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);
   },
@@ -304,10 +270,6 @@ const FrameWorkerManager = {
       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

toolkit/components/social/WorkerAPI.jsm

@@ -60,11 +60,7 @@ WorkerAPI.prototype = {
       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);

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

@@ -559,33 +559,6 @@ let tests = {
     }
   },
 
-  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) {

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

@@ -128,103 +128,41 @@ let tests = {
     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) {

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

@@ -25,11 +25,6 @@ onconnect = function(e) {
       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;
@@ -40,8 +35,6 @@ onconnect = function(e) {
         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":
@@ -63,9 +56,4 @@ onconnect = function(e) {
         break;
     }
   }
-  // used for "test-reload-worker"
-  if (apiPort && apiPort != port) {
-    port.postMessage({topic: "worker.connected"})
-  }
-
 }