Bug 787511 - rename SocialProvider._getWorkerPort to getWorkerPort, remove SocialProvider.port. r=jaws

browser/base/content/browser-social.js

@@ -326,7 +326,7 @@ let SocialShareButton = {
     // whenever we notice the provider has changed - but the concept of
     // "provider changed" will only exist once bug 774520 lands. 
     // get the recommend-prompt info.
-    let port = Social.provider._getWorkerPort();
+    let port = Social.provider.getWorkerPort();
     if (port) {
       port.onmessage = function(evt) {
         if (evt.data.topic == "social.user-recommend-prompt-response") {

browser/base/content/test/browser_social_chatwindow.js

@@ -24,7 +24,7 @@ function test() {
 var tests = {
   testOpenCloseChat: function(next) {
     let chats = document.getElementById("pinnedchats");
-    let port = Social.provider.port;
+    let port = Social.provider.getWorkerPort();
     ok(port, "provider has a port");
     port.onmessage = function (e) {
       let topic = e.data.topic;
@@ -43,6 +43,7 @@ var tests = {
             iframe.addEventListener("unload", function chatUnload() {
               iframe.removeEventListener("unload", chatUnload, true);
               ok(true, "got chatbox unload on close");
+              port.close();
               next();
             }, true);
             chats.selectedChat.close();
@@ -60,8 +61,9 @@ var tests = {
   testManyChats: function(next) {
     // open enough chats to overflow the window, then check
     // if the menupopup is visible
-    let port = Social.provider.port;
+    let port = Social.provider.getWorkerPort();
     ok(port, "provider has a port");
+    port.postMessage({topic: "test-init"});
     let width = document.documentElement.boxObject.width;
     let numToOpen = (width / 200) + 1;
     port.onmessage = function (e) {
@@ -81,6 +83,7 @@ var tests = {
             chats.selectedChat.close();
           }
           ok(!chats.selectedChat, "chats are all closed");
+          port.close();
           next();
           break;
       }
@@ -92,8 +95,9 @@ var tests = {
   },
   testWorkerChatWindow: function(next) {
     const chatUrl = "https://example.com/browser/browser/base/content/test/social_chat.html";
-    let port = Social.provider.port;
+    let port = Social.provider.getWorkerPort();
     ok(port, "provider has a port");
+    port.postMessage({topic: "test-init"});
     port.onmessage = function (e) {
       let topic = e.data.topic;
       switch (topic) {
@@ -104,6 +108,7 @@ var tests = {
             chats.selectedChat.close();
           }
           ok(!chats.selectedChat, "chats are all closed");
+          port.close();
           ensureSocialUrlNotRemembered(chatUrl);
           next();
           break;

browser/base/content/test/browser_social_flyout.js

@@ -20,7 +20,7 @@ function test() {
 var tests = {
   testOpenCloseFlyout: function(next) {
     let panel = document.getElementById("social-flyout-panel");
-    let port = Social.provider.port;
+    let port = Social.provider.getWorkerPort();
     ok(port, "provider has a port");
     port.onmessage = function (e) {
       let topic = e.data.topic;
@@ -31,6 +31,7 @@ var tests = {
         case "got-flyout-visibility":
           if (e.data.result == "hidden") {
             ok(true, "flyout visibility is 'hidden'");
+            port.close();
             next();
           } else if (e.data.result == "shown") {
             ok(true, "flyout visibility is 'shown");

browser/base/content/test/browser_social_isVisible.js

@@ -19,41 +19,46 @@ function test() {
 
 var tests = {
   testSidebarMessage: function(next) {
-    let port = Social.provider.port;
+    let port = Social.provider.getWorkerPort();
     ok(port, "provider has a port");
     port.postMessage({topic: "test-init"});
-    Social.provider.port.onmessage = function (e) {
+    port.onmessage = function (e) {
       let topic = e.data.topic;
       switch (topic) {
         case "got-sidebar-message":
           // The sidebar message will always come first, since it loads by default
           ok(true, "got sidebar message");
+          port.close();
           next();
           break;
       }
     };
   },
   testIsVisible: function(next) {
-    let port = Social.provider.port;
+    let port = Social.provider.getWorkerPort();
+    port.postMessage({topic: "test-init"});
     port.onmessage = function (e) {
       let topic = e.data.topic;
       switch (topic) {
         case "got-isVisible-response":
           is(e.data.result, true, "Sidebar should be visible by default");
           Social.toggleSidebar();
+          port.close();
           next();
       }
     };
     port.postMessage({topic: "test-isVisible"});
   },
   testIsNotVisible: function(next) {
-    let port = Social.provider.port;
+    let port = Social.provider.getWorkerPort();
+    port.postMessage({topic: "test-init"});
     port.onmessage = function (e) {
       let topic = e.data.topic;
       switch (topic) {
         case "got-isVisible-response":
           is(e.data.result, false, "Sidebar should be hidden");
           Services.prefs.clearUserPref("social.sidebar.open");
+          port.close();
           next();
       }
     };

browser/base/content/test/browser_social_mozSocial_API.js

@@ -35,7 +35,7 @@ var tests = {
       EventUtils.synthesizeMouseAtCenter(statusIcons.firstChild, {});
     }
 
-    let port = Social.provider.port;
+    let port = Social.provider.getWorkerPort();
     ok(port, "provider has a port");
     port.onmessage = function (e) {
       let topic = e.data.topic;
@@ -52,6 +52,7 @@ var tests = {
             panel.hidePopup();
           } else if (e.data.result == "hidden") {
             ok(true, "panel hidden");
+            port.close();
             next();
           }
           break;

browser/base/content/test/browser_social_shareButton.js

@@ -40,7 +40,9 @@ function tabLoaded() {
 function testInitial(finishcb) {
   ok(Social.provider, "Social provider is active");
   ok(Social.provider.enabled, "Social provider is enabled");
-  ok(Social.provider.port, "Social provider has a port to its FrameWorker");
+  let port = Social.provider.getWorkerPort();
+  ok(port, "Social provider has a port to its FrameWorker");
+  port.close();
 
   let {shareButton, sharePopup} = SocialShareButton;
   ok(shareButton, "share button exists");

browser/modules/Social.jsm

@@ -35,7 +35,7 @@ let Social = {
   },
 
   get uiVisible() {
-    return this.provider && this.provider.enabled && this.provider.port;
+    return this.provider && this.provider.enabled;
   },
 
   set enabled(val) {
@@ -62,13 +62,6 @@ let Social = {
     Services.prefs.setBoolPref("social.sidebar.open", !prefValue);
   },
 
-  sendWorkerMessage: function Social_sendWorkerMessage(message) {
-    // Responses aren't handled yet because there is no actions to perform
-    // based on the response from the provider at this point.
-    if (this.provider && this.provider.port)
-      this.provider.port.postMessage(message);
-  },
-
   // Sharing functionality
   _getShareablePageUrl: function Social_getShareablePageUrl(aURI) {
     let uri = aURI.clone();
@@ -85,21 +78,43 @@ let Social = {
   },
 
   sharePage: function Social_sharePage(aURI) {
+    // this should not be called if this.provider or the port is null
+    if (!this.provider) {
+      Cu.reportError("Can't share a page when no provider is current");
+      return;
+    }
+    let port = this.provider.getWorkerPort();
+    if (!port) {
+      Cu.reportError("Can't share page as no provider port is available");
+      return;
+    }
     let url = this._getShareablePageUrl(aURI);
     this._sharedUrls[url] = true;
-    this.sendWorkerMessage({
+    port.postMessage({
       topic: "social.user-recommend",
       data: { url: url }
     });
+    port.close();
   },
 
   unsharePage: function Social_unsharePage(aURI) {
+    // this should not be called if this.provider or the port is null
+    if (!this.provider) {
+      Cu.reportError("Can't unshare a page when no provider is current");
+      return;
+    }
+    let port = this.provider.getWorkerPort();
+    if (!port) {
+      Cu.reportError("Can't unshare page as no provider port is available");
+      return;
+    }
     let url = this._getShareablePageUrl(aURI);
     delete this._sharedUrls[url];
-    this.sendWorkerMessage({
+    port.postMessage({
       topic: "social.user-unrecommend",
       data: { url: url }
     });
+    port.close();
   },
 
   _sharedUrls: {}

toolkit/components/social/MozSocialAPI.jsm

@@ -75,7 +75,7 @@ function attachToWindow(provider, targetWindow) {
     throw new Error("MozSocialAPI: cannot attach " + origin + " to " + targetDocURI.spec);
   }
 
-  var port = provider._getWorkerPort(targetWindow);
+  var port = provider.getWorkerPort(targetWindow);
 
   let mozSocialObj = {
     // Use a method for backwards compat with existing providers, but we

toolkit/components/social/SocialService.jsm

@@ -191,10 +191,6 @@ SocialProvider.prototype = {
     }
   },
 
-  // Active port to the provider's FrameWorker. Null if the provider has no
-  // FrameWorker, or is disabled.
-  port: null,
-
   // Reference to a workerAPI object for this provider. Null if the provider has
   // no FrameWorker, or is disabled.
   workerAPI: null,
@@ -261,11 +257,9 @@ SocialProvider.prototype = {
   _activate: function _activate() {
     // Initialize the workerAPI and its port first, so that its initialization
     // occurs before any other messages are processed by other ports.
-    let workerAPIPort = this._getWorkerPort();
+    let workerAPIPort = this.getWorkerPort();
     if (workerAPIPort)
       this.workerAPI = new WorkerAPI(this, workerAPIPort);
-
-    this.port = this._getWorkerPort();
   },
 
   _terminate: function _terminate() {
@@ -276,7 +270,9 @@ SocialProvider.prototype = {
         Cu.reportError("SocialProvider FrameWorker termination failed: " + e);
       }
     }
-    this.port = null;
+    if (this.workerAPI) {
+      this.workerAPI.terminate();
+    }
     this.workerAPI = null;
   },
 
@@ -288,14 +284,9 @@ SocialProvider.prototype = {
    *
    * @param {DOMWindow} window (optional)
    */
-  _getWorkerPort: function _getWorkerPort(window) {
+  getWorkerPort: function getWorkerPort(window) {
     if (!this.workerURL || !this.enabled)
       return null;
-    try {
+    return getFrameWorkerHandle(this.workerURL, window).port;
-      return getFrameWorkerHandle(this.workerURL, window).port;
-    } catch (ex) {
-      Cu.reportError("SocialProvider: retrieving worker port failed:" + ex);
-      return null;
-    }
   }
 }

toolkit/components/social/WorkerAPI.jsm

@@ -35,6 +35,10 @@ function WorkerAPI(provider, port) {
 }
 
 WorkerAPI.prototype = {
+  terminate: function terminate() {
+    this._port.close();
+  },
+
   _handleMessage: function _handleMessage(event) {
     let {topic, data} = event.data;
     let handler = this.handlers[topic];

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

@@ -15,19 +15,24 @@ function test() {
 
   SocialService.addProvider(manifest, function (provider) {
     ok(provider.enabled, "provider is initially enabled");
-    ok(provider.port, "should be able to get a port from enabled provider");
+    let port = provider.getWorkerPort();
+    ok(port, "should be able to get a port from enabled provider");
+    port.close();
     ok(provider.workerAPI, "should be able to get a workerAPI from enabled provider");
 
     provider.enabled = false;
 
     ok(!provider.enabled, "provider is now disabled");
-    ok(!provider.port, "shouldn't be able to get a port from disabled provider");
+    port = provider.getWorkerPort();
+    ok(!port, "shouldn't be able to get a port from disabled provider");
     ok(!provider.workerAPI, "shouldn't be able to get a workerAPI from disabled provider");
 
     provider.enabled = true;
 
     ok(provider.enabled, "provider is re-enabled");
-    ok(provider.port, "should be able to get a port from re-enabled provider");
+    let port = provider.getWorkerPort();
+    ok(port, "should be able to get a port from re-enabled provider");
+    port.close();
     ok(provider.workerAPI, "should be able to get a workerAPI from re-enabled provider");
 
     SocialService.removeProvider(provider.origin, finish);

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

@@ -132,17 +132,19 @@ let tests = {
       }
       Services.obs.addObserver(observer, "social-test:notification-alert", false);
 
-      provider.port.onmessage = function(e) {
+      let port = provider.getWorkerPort();
+      port.onmessage = function(e) {
         if (e.data.topic == "test.done") {
           ok(e.data.data, "check the test worked");
           ok(observer.observedData, "test observer fired");
           is(observer.observedData.text, "test notification", "check the alert text is correct");
           is(observer.observedData.title, "Example Provider", "check the alert title is correct");
           is(observer.observedData.textClickable, true, "check the alert is clickable");
+          port.close();
           cbnext();
         }
       }
-      provider.port.postMessage({topic: "test.initialize"});
+      port.postMessage({topic: "test.initialize"});
     });
   }
 };

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

@@ -28,13 +28,14 @@ let tests = {
     ok(provider.workerAPI, "provider has a workerAPI");
     is(provider.workerAPI.initialized, false, "workerAPI is not yet initialized");
   
-    let port = provider.port;
+    let port = provider.getWorkerPort();
     ok(port, "should be able to get a port from the provider");
-  
+
     port.onmessage = function onMessage(event) {
       let topic = event.data.topic;
       if (topic == "test-initialization-complete") {
         is(provider.workerAPI.initialized, true, "workerAPI is now initialized");
+        port.close();
         next();
       }
     }
@@ -60,7 +61,9 @@ let tests = {
       next();
     }
     Services.obs.addObserver(ob, "social:profile-changed", false);
-    provider.port.postMessage({topic: "test-profile", data: expect});
+    let port = provider.getWorkerPort();
+    port.postMessage({topic: "test-profile", data: expect});
+    port.close();
   },
 
   testAmbientNotification: function(next) {
@@ -76,7 +79,9 @@ let tests = {
       next();
     }
     Services.obs.addObserver(ob, "social:ambient-notification-changed", false);
-    provider.port.postMessage({topic: "test-ambient", data: expect});
+    let port = provider.getWorkerPort();
+    port.postMessage({topic: "test-ambient", data: expect});
+    port.close();
   },
 
   testProfileCleared: function(next) {
@@ -96,19 +101,22 @@ let tests = {
   },
 
   testCookies: function(next) {
-    provider.port.onmessage = function onMessage(event) {
+    let port = provider.getWorkerPort();
+    port.onmessage = function onMessage(event) {
       let {topic, data} = event.data;
       if (topic == "test.cookies-get-response") {
         is(data.length, 1, "got one cookie");
         is(data[0].name, "cheez", "cookie has the correct name");
         is(data[0].value, "burger", "cookie has the correct value");
         Services.cookies.remove('.example.com', '/', 'cheez', false);
+        port.close();
         next();
       }
     }
     var MAX_EXPIRY = Math.pow(2, 62);
     Services.cookies.add('.example.com', '/', 'cheez', 'burger', false, false, true, MAX_EXPIRY);
-    provider.port.postMessage({topic: "test.cookies-get"});
+    port.postMessage({topic: "test-initialization"});
+    port.postMessage({topic: "test.cookies-get"});
   }
 
 };

toolkit/components/social/test/xpcshell/test_SocialService.js

@@ -5,11 +5,12 @@
 Cu.import("resource://gre/modules/Services.jsm");
 
 function run_test() {
+  // NOTE: none of the manifests here can have a workerURL set, or we attempt
+  // to create a FrameWorker and that fails under xpcshell...
   let manifests = [
     { // normal provider
       name: "provider 1",
       origin: "https://example1.com",
-      workerURL: "https://example1.com/worker.js"
     },
     { // provider without workerURL
       name: "provider 2",