From a109831d755f688c8337a6dbd4f03cc786743966 Mon Sep 17 00:00:00 2001 From: Phil Ringnalda Date: Tue, 11 Sep 2012 20:46:34 -0700 Subject: [PATCH] Back out df2ddcab7143 (bug 788100), d9e96444da92 (bug 788405), 092a8add22fd (bug 787511) for xpcshell bustage --- browser/base/content/browser-social.js | 2 +- .../content/test/browser_social_chatwindow.js | 11 ++---- .../content/test/browser_social_flyout.js | 3 +- .../content/test/browser_social_isVisible.js | 13 +++---- .../test/browser_social_mozSocial_API.js | 36 ++++++++++++------- .../test/browser_social_shareButton.js | 4 +-- browser/base/content/test/social_worker.js | 2 +- browser/modules/Social.jsm | 35 ++++++------------ toolkit/components/social/FrameWorker.jsm | 7 ---- .../components/social/MessagePortWorker.js | 2 +- toolkit/components/social/MozSocialAPI.jsm | 2 +- toolkit/components/social/SocialService.jsm | 21 +++++++---- toolkit/components/social/WorkerAPI.jsm | 9 ++--- .../test/browser/browser_SocialProvider.js | 11 ++---- .../test/browser/browser_notifications.js | 6 ++-- .../social/test/browser/browser_workerAPI.js | 35 ++++++++++-------- .../social/test/browser/relative_import.js | 5 --- .../social/test/browser/worker_relative.js | 7 +--- .../social/test/browser/worker_social.js | 1 + 19 files changed, 95 insertions(+), 117 deletions(-) diff --git a/browser/base/content/browser-social.js b/browser/base/content/browser-social.js index 727b95346f4c..ec195ecb3bde 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 4ceb3aa79540..8aedeaec084c 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.getWorkerPort(); + let port = Social.provider.port; ok(port, "provider has a port"); port.onmessage = function (e) { let topic = e.data.topic; @@ -43,7 +43,6 @@ 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(); @@ -61,9 +60,8 @@ var tests = { testManyChats: function(next) { // open enough chats to overflow the window, then check // if the menupopup is visible - let port = Social.provider.getWorkerPort(); + let port = Social.provider.port; 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) { @@ -83,7 +81,6 @@ var tests = { chats.selectedChat.close(); } ok(!chats.selectedChat, "chats are all closed"); - port.close(); next(); break; } @@ -95,9 +92,8 @@ var tests = { }, testWorkerChatWindow: function(next) { const chatUrl = "https://example.com/browser/browser/base/content/test/social_chat.html"; - let port = Social.provider.getWorkerPort(); + let port = Social.provider.port; ok(port, "provider has a port"); - port.postMessage({topic: "test-init"}); port.onmessage = function (e) { let topic = e.data.topic; switch (topic) { @@ -108,7 +104,6 @@ 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 a03e32296616..5fedfcc549d7 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.getWorkerPort(); + let port = Social.provider.port; ok(port, "provider has a port"); port.onmessage = function (e) { let topic = e.data.topic; @@ -31,7 +31,6 @@ 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 4287eaeca766..6d5366210984 100644 --- a/browser/base/content/test/browser_social_isVisible.js +++ b/browser/base/content/test/browser_social_isVisible.js @@ -19,46 +19,41 @@ function test() { var tests = { testSidebarMessage: function(next) { - let port = Social.provider.getWorkerPort(); + let port = Social.provider.port; ok(port, "provider has a port"); port.postMessage({topic: "test-init"}); - port.onmessage = function (e) { + Social.provider.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.getWorkerPort(); - port.postMessage({topic: "test-init"}); + let port = Social.provider.port; 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.getWorkerPort(); - port.postMessage({topic: "test-init"}); + let port = Social.provider.port; 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 bb4e64d98e8d..e4b13fc848ef 100644 --- a/browser/base/content/test/browser_social_mozSocial_API.js +++ b/browser/base/content/test/browser_social_mozSocial_API.js @@ -29,23 +29,17 @@ var tests = { function triggerIconPanel() { let statusIcons = document.getElementById("social-status-iconbox"); - waitForCondition(function() statusIcons.firstChild && !statusIcons.firstChild.hidden, - function() { - // Click the button to trigger its contentPanel - let panel = document.getElementById("social-notification-panel"); - EventUtils.synthesizeMouseAtCenter(statusIcons.firstChild, {}); - }, "Status icon didn't become non-hidden"); + ok(!statusIcons.firstChild.hidden, "status icon is visible"); + // Click the button to trigger its contentPanel + let panel = document.getElementById("social-notification-panel"); + EventUtils.synthesizeMouseAtCenter(statusIcons.firstChild, {}); } - let port = Social.provider.getWorkerPort(); + let port = Social.provider.port; ok(port, "provider has a port"); port.onmessage = function (e) { let topic = e.data.topic; switch (topic) { - case "test-init-done": - iconsReady = true; - checkNext(); - break; case "got-panel-message": ok(true, "got panel message"); // Check the panel isn't in our history. @@ -58,7 +52,6 @@ var tests = { panel.hidePopup(); } else if (e.data.result == "hidden") { ok(true, "panel hidden"); - port.close(); next(); } break; @@ -73,5 +66,24 @@ var tests = { } } port.postMessage({topic: "test-init"}); + + // Our worker sets up ambient notification at the same time as it responds to + // the workerAPI initialization. If it's already initialized, we can + // immediately check the icons, otherwise wait for initialization by + // observing the topic sent out by the social service. + if (Social.provider.workerAPI.initialized) { + iconsReady = true; + checkNext(); + } else { + Services.obs.addObserver(function obs() { + Services.obs.removeObserver(obs, "social:ambient-notification-changed"); + // Let the other observers (like the one that updates the UI) run before + // checking the icons. + executeSoon(function () { + iconsReady = true; + checkNext(); + }); + }, "social:ambient-notification-changed", false); + } } } diff --git a/browser/base/content/test/browser_social_shareButton.js b/browser/base/content/test/browser_social_shareButton.js index 06e6d9128c43..66252eb92a4f 100644 --- a/browser/base/content/test/browser_social_shareButton.js +++ b/browser/base/content/test/browser_social_shareButton.js @@ -40,9 +40,7 @@ function tabLoaded() { function testInitial(finishcb) { ok(Social.provider, "Social provider is active"); ok(Social.provider.enabled, "Social provider is enabled"); - let port = Social.provider.getWorkerPort(); - ok(port, "Social provider has a port to its FrameWorker"); - port.close(); + ok(Social.provider.port, "Social provider has a port to its FrameWorker"); let {shareButton, sharePopup} = SocialShareButton; ok(shareButton, "share button exists"); diff --git a/browser/base/content/test/social_worker.js b/browser/base/content/test/social_worker.js index 1e047d996b98..8d4b3541d3c5 100644 --- a/browser/base/content/test/social_worker.js +++ b/browser/base/content/test/social_worker.js @@ -11,7 +11,6 @@ onconnect = function(e) { switch (topic) { case "test-init": testPort = port; - port.postMessage({topic: "test-init-done"}); break; case "sidebar-message": sidebarPort = port; @@ -70,6 +69,7 @@ onconnect = function(e) { case "social.initialize": // This is the workerAPI port, respond and set up a notification icon. apiPort = port; + port.postMessage({topic: "social.initialize-response"}); let profile = { portrait: "https://example.com/portrait.jpg", userName: "trickster", diff --git a/browser/modules/Social.jsm b/browser/modules/Social.jsm index 165203766833..c38276a0cd19 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; + return this.provider && this.provider.enabled && this.provider.port; }, set enabled(val) { @@ -62,6 +62,13 @@ 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(); @@ -78,43 +85,21 @@ 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; - port.postMessage({ + this.sendWorkerMessage({ 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]; - port.postMessage({ + this.sendWorkerMessage({ topic: "social.user-unrecommend", data: { url: url } }); - port.close(); }, _sharedUrls: {} diff --git a/toolkit/components/social/FrameWorker.jsm b/toolkit/components/social/FrameWorker.jsm index eeffc3dcc9b9..13973621bd5e 100644 --- a/toolkit/components/social/FrameWorker.jsm +++ b/toolkit/components/social/FrameWorker.jsm @@ -130,13 +130,6 @@ FrameWorker.prototype = { }; sandbox.navigator = navigator; - // Our importScripts function needs to 'eval' the script code from inside - // a function, but using eval() directly means functions in the script - // don't end up in the global scope. - sandbox._evalInSandbox = function(s) { - Cu.evalInSandbox(s, sandbox); - }; - // and we delegate ononline and onoffline events to the worker. // See http://www.whatwg.org/specs/web-apps/current-work/multipage/workers.html#workerglobalscope this.frame.addEventListener('offline', function fw_onoffline(event) { diff --git a/toolkit/components/social/MessagePortWorker.js b/toolkit/components/social/MessagePortWorker.js index 8570aeab5f6e..d06769cd13e6 100644 --- a/toolkit/components/social/MessagePortWorker.js +++ b/toolkit/components/social/MessagePortWorker.js @@ -32,7 +32,7 @@ function importScripts() { xhr.onreadystatechange = function(aEvt) { if (xhr.readyState == 4) { if (xhr.status == 200 || xhr.status == 0) { - _evalInSandbox(xhr.responseText); + eval(xhr.responseText); } else { throw new Error("Unable to importScripts ["+scriptURL+"], status " + xhr.status) diff --git a/toolkit/components/social/MozSocialAPI.jsm b/toolkit/components/social/MozSocialAPI.jsm index 5335a4c9db0e..f78e334895e3 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 b13b60ace7e6..e79d01314b85 100644 --- a/toolkit/components/social/SocialService.jsm +++ b/toolkit/components/social/SocialService.jsm @@ -191,6 +191,10 @@ 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, @@ -257,9 +261,11 @@ 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() { @@ -270,9 +276,7 @@ SocialProvider.prototype = { Cu.reportError("SocialProvider FrameWorker termination failed: " + e); } } - if (this.workerAPI) { - this.workerAPI.terminate(); - } + this.port = null; this.workerAPI = null; }, @@ -284,9 +288,14 @@ SocialProvider.prototype = { * * @param {DOMWindow} window (optional) */ - getWorkerPort: function getWorkerPort(window) { + _getWorkerPort: function _getWorkerPort(window) { if (!this.workerURL || !this.enabled) return null; - return getFrameWorkerHandle(this.workerURL, window).port; + try { + return getFrameWorkerHandle(this.workerURL, window).port; + } catch (ex) { + Cu.reportError("SocialProvider: retrieving worker port failed:" + ex); + return null; + } } } diff --git a/toolkit/components/social/WorkerAPI.jsm b/toolkit/components/social/WorkerAPI.jsm index 0c2f05f3698d..6137a2d710bc 100644 --- a/toolkit/components/social/WorkerAPI.jsm +++ b/toolkit/components/social/WorkerAPI.jsm @@ -23,6 +23,8 @@ function WorkerAPI(provider, port) { this._port = port; this._port.onmessage = this._handleMessage.bind(this); + this.initialized = false; + // Send an "intro" message so the worker knows this is the port // used for the api. // later we might even include an API version - version 0 for now! @@ -33,10 +35,6 @@ 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]; @@ -52,6 +50,9 @@ WorkerAPI.prototype = { }, handlers: { + "social.initialize-response": function (data) { + this.initialized = true; + }, "social.user-profile": function (data) { this._provider.updateUserProfile(data); }, diff --git a/toolkit/components/social/test/browser/browser_SocialProvider.js b/toolkit/components/social/test/browser/browser_SocialProvider.js index c12887d38dce..f9811a3ff8ce 100644 --- a/toolkit/components/social/test/browser/browser_SocialProvider.js +++ b/toolkit/components/social/test/browser/browser_SocialProvider.js @@ -15,24 +15,19 @@ function test() { SocialService.addProvider(manifest, function (provider) { ok(provider.enabled, "provider is initially enabled"); - let port = provider.getWorkerPort(); - ok(port, "should be able to get a port from enabled provider"); - port.close(); + ok(provider.port, "should be able to get a port from enabled provider"); ok(provider.workerAPI, "should be able to get a workerAPI from enabled provider"); provider.enabled = false; ok(!provider.enabled, "provider is now disabled"); - port = provider.getWorkerPort(); - ok(!port, "shouldn't be able to get a port from disabled provider"); + ok(!provider.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"); - let port = provider.getWorkerPort(); - ok(port, "should be able to get a port from re-enabled provider"); - port.close(); + ok(provider.port, "should be able to get a port from re-enabled provider"); 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 a1c9df268252..3b229789eb44 100644 --- a/toolkit/components/social/test/browser/browser_notifications.js +++ b/toolkit/components/social/test/browser/browser_notifications.js @@ -132,19 +132,17 @@ let tests = { } Services.obs.addObserver(observer, "social-test:notification-alert", false); - let port = provider.getWorkerPort(); - port.onmessage = function(e) { + provider.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(); } } - port.postMessage({topic: "test.initialize"}); + provider.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 5f68d19d6e66..e6e0377a7bb3 100644 --- a/toolkit/components/social/test/browser/browser_workerAPI.js +++ b/toolkit/components/social/test/browser/browser_workerAPI.js @@ -24,6 +24,23 @@ function test() { } let tests = { + testInitializeWorker: function(next) { + ok(provider.workerAPI, "provider has a workerAPI"); + is(provider.workerAPI.initialized, false, "workerAPI is not yet initialized"); + + let port = provider.port; + 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"); + next(); + } + } + port.postMessage({topic: "test-initialization"}); + }, + testProfile: function(next) { let expect = { portrait: "https://example.com/portrait.jpg", @@ -40,15 +57,10 @@ let tests = { is(profile.displayName, expect.displayName, "displayName is set"); is(profile.profileURL, expect.profileURL, "profileURL is set"); - // see below - if not for bug 788368 we could close this earlier. - port.close(); next(); } Services.obs.addObserver(ob, "social:profile-changed", false); - let port = provider.getWorkerPort(); - port.postMessage({topic: "test-profile", data: expect}); - // theoretically we should be able to close the port here, but bug 788368 - // means that if we do, the worker never sees the test-profile message. + provider.port.postMessage({topic: "test-profile", data: expect}); }, testAmbientNotification: function(next) { @@ -64,9 +76,7 @@ let tests = { next(); } Services.obs.addObserver(ob, "social:ambient-notification-changed", false); - let port = provider.getWorkerPort(); - port.postMessage({topic: "test-ambient", data: expect}); - port.close(); + provider.port.postMessage({topic: "test-ambient", data: expect}); }, testProfileCleared: function(next) { @@ -86,22 +96,19 @@ let tests = { }, testCookies: function(next) { - let port = provider.getWorkerPort(); - port.onmessage = function onMessage(event) { + provider.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); - port.postMessage({topic: "test-initialization"}); - port.postMessage({topic: "test.cookies-get"}); + provider.port.postMessage({topic: "test.cookies-get"}); } }; diff --git a/toolkit/components/social/test/browser/relative_import.js b/toolkit/components/social/test/browser/relative_import.js index 5e4f1b6d7a29..f3df9875189d 100644 --- a/toolkit/components/social/test/browser/relative_import.js +++ b/toolkit/components/social/test/browser/relative_import.js @@ -1,6 +1 @@ dump("relative_import file\n"); - -testVar = "oh hai"; -function testFunc() { - return "oh hai"; -} diff --git a/toolkit/components/social/test/browser/worker_relative.js b/toolkit/components/social/test/browser/worker_relative.js index 9b50b132f3e5..f00e255a0656 100644 --- a/toolkit/components/social/test/browser/worker_relative.js +++ b/toolkit/components/social/test/browser/worker_relative.js @@ -4,12 +4,7 @@ onconnect = function(e) { let req; 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"}); - } + port.postMessage({topic: "done", result: "ok"}); } catch(e) { port.postMessage({topic: "done", result: "FAILED to importScripts, " + e.toString() }); return; diff --git a/toolkit/components/social/test/browser/worker_social.js b/toolkit/components/social/test/browser/worker_social.js index efc26240b799..8890dd5391af 100644 --- a/toolkit/components/social/test/browser/worker_social.js +++ b/toolkit/components/social/test/browser/worker_social.js @@ -17,6 +17,7 @@ onconnect = function(e) { switch (topic) { case "social.initialize": apiPort = port; + apiPort.postMessage({topic: "social.initialize-response"}); break; case "test-initialization": testerPort = port;