diff --git a/browser/base/content/browser-social.js b/browser/base/content/browser-social.js index ec195ecb3bd..727b95346f4 100644 --- a/browser/base/content/browser-social.js +++ b/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") { diff --git a/browser/base/content/test/browser_social_chatwindow.js b/browser/base/content/test/browser_social_chatwindow.js index 8aedeaec084..4ceb3aa7954 100644 --- a/browser/base/content/test/browser_social_chatwindow.js +++ b/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; diff --git a/browser/base/content/test/browser_social_flyout.js b/browser/base/content/test/browser_social_flyout.js index 5fedfcc549d..a03e3229661 100644 --- a/browser/base/content/test/browser_social_flyout.js +++ b/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"); diff --git a/browser/base/content/test/browser_social_isVisible.js b/browser/base/content/test/browser_social_isVisible.js index 6d536621098..4287eaeca76 100644 --- a/browser/base/content/test/browser_social_isVisible.js +++ b/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(); } }; diff --git a/browser/base/content/test/browser_social_mozSocial_API.js b/browser/base/content/test/browser_social_mozSocial_API.js index e4b13fc848e..19b6ee51639 100644 --- a/browser/base/content/test/browser_social_mozSocial_API.js +++ b/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; diff --git a/browser/base/content/test/browser_social_shareButton.js b/browser/base/content/test/browser_social_shareButton.js index de8e1ad09a9..bec0180f3b7 100644 --- a/browser/base/content/test/browser_social_shareButton.js +++ b/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"); diff --git a/browser/modules/Social.jsm b/browser/modules/Social.jsm index c38276a0cd1..16520376683 100644 --- a/browser/modules/Social.jsm +++ b/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: {} diff --git a/toolkit/components/social/MozSocialAPI.jsm b/toolkit/components/social/MozSocialAPI.jsm index f78e334895e..5335a4c9db0 100644 --- a/toolkit/components/social/MozSocialAPI.jsm +++ b/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 diff --git a/toolkit/components/social/SocialService.jsm b/toolkit/components/social/SocialService.jsm index e79d01314b8..b13b60ace7e 100644 --- a/toolkit/components/social/SocialService.jsm +++ b/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; - } catch (ex) { - Cu.reportError("SocialProvider: retrieving worker port failed:" + ex); - return null; - } + return getFrameWorkerHandle(this.workerURL, window).port; } } diff --git a/toolkit/components/social/WorkerAPI.jsm b/toolkit/components/social/WorkerAPI.jsm index 6137a2d710b..48e12b7ff92 100644 --- a/toolkit/components/social/WorkerAPI.jsm +++ b/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]; diff --git a/toolkit/components/social/test/browser/browser_SocialProvider.js b/toolkit/components/social/test/browser/browser_SocialProvider.js index f9811a3ff8c..c12887d38dc 100644 --- a/toolkit/components/social/test/browser/browser_SocialProvider.js +++ b/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); diff --git a/toolkit/components/social/test/browser/browser_notifications.js b/toolkit/components/social/test/browser/browser_notifications.js index 3b229789eb4..a1c9df26825 100644 --- a/toolkit/components/social/test/browser/browser_notifications.js +++ b/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"}); }); } }; diff --git a/toolkit/components/social/test/browser/browser_workerAPI.js b/toolkit/components/social/test/browser/browser_workerAPI.js index e6e0377a7bb..2592af27627 100644 --- a/toolkit/components/social/test/browser/browser_workerAPI.js +++ b/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"}); } }; diff --git a/toolkit/components/social/test/xpcshell/test_SocialService.js b/toolkit/components/social/test/xpcshell/test_SocialService.js index df3ddc0a163..f47dfb05703 100644 --- a/toolkit/components/social/test/xpcshell/test_SocialService.js +++ b/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",