bug 891216 multiple workers enabled, r=markh

browser/base/content/aboutSocialError.xhtml

@@ -112,10 +112,7 @@
     }
 
     function reloadProvider() {
-      Social.enabled = false;
-      Services.tm.mainThread.dispatch(function() {
-        Social.enabled = true;
-      }, Components.interfaces.nsIThread.DISPATCH_NORMAL);
+      Social.provider.reload();
     }
 
     parseQueryString();

browser/base/content/browser-social.js

@@ -36,6 +36,7 @@ SocialUI = {
     Services.obs.addObserver(this, "social:frameworker-error", false);
     Services.obs.addObserver(this, "social:provider-set", false);
     Services.obs.addObserver(this, "social:providers-changed", false);
+    Services.obs.addObserver(this, "social:provider-reload", false);
 
     Services.prefs.addObserver("social.sidebar.open", this, false);
     Services.prefs.addObserver("social.toast-notifications.enabled", this, false);
@@ -60,6 +61,7 @@ SocialUI = {
     Services.obs.removeObserver(this, "social:frameworker-error");
     Services.obs.removeObserver(this, "social:provider-set");
     Services.obs.removeObserver(this, "social:providers-changed");
+    Services.obs.removeObserver(this, "social:provider-reload");
 
     Services.prefs.removeObserver("social.sidebar.open", this);
     Services.prefs.removeObserver("social.toast-notifications.enabled", this);
@@ -74,6 +76,16 @@ SocialUI = {
     // manually :(
     try {
       switch (topic) {
+        case "social:provider-reload":
+          // if the reloaded provider is our current provider, fall through
+          // to social:provider-set so the ui will be reset
+          if (!Social.provider || Social.provider.origin != data)
+            return;
+          // be sure to unload the sidebar as it will not reload if the origin
+          // has not changed, it will be loaded in provider-set below. Other
+          // panels will be unloaded or handle reload.
+          SocialSidebar.unloadSidebar();
+          // fall through to social:provider-set
         case "social:provider-set":
           // Social.provider has changed (possibly to null), update any state
           // which depends on it.
@@ -142,7 +154,7 @@ SocialUI = {
 
   // Miscellaneous helpers
   showProfile: function SocialUI_showProfile() {
-    if (Social.haveLoggedInUser())
+    if (Social.provider.haveLoggedInUser())
       openUILinkIn(Social.provider.profile.profileURL, "tab");
     else {
       // XXX Bug 789585 will implement an API for provider-specified login pages.
@@ -976,22 +988,20 @@ SocialToolbar = {
     let toggleNotificationsCommand = document.getElementById("Social:ToggleNotifications");
     toggleNotificationsCommand.setAttribute("hidden", !socialEnabled);
 
-    if (!Social.haveLoggedInUser() || !socialEnabled) {
-      let parent = document.getElementById("social-notification-panel");
-      while (parent.hasChildNodes()) {
-        let frame = parent.firstChild;
-        SharedFrame.forgetGroup(frame.id);
-        parent.removeChild(frame);
-      }
+    let parent = document.getElementById("social-notification-panel");
+    while (parent.hasChildNodes()) {
+      let frame = parent.firstChild;
+      SharedFrame.forgetGroup(frame.id);
+      parent.removeChild(frame);
+    }
 
-      let tbi = document.getElementById("social-toolbar-item");
-      if (tbi) {
-        // SocialMark is the last button allways
-        let next = SocialMark.button.previousSibling;
-        while (next != this.button) {
-          tbi.removeChild(next);
-          next = SocialMark.button.previousSibling;
-        }
+    let tbi = document.getElementById("social-toolbar-item");
+    if (tbi) {
+      // SocialMark is the last button allways
+      let next = SocialMark.button.previousSibling;
+      while (next != this.button) {
+        tbi.removeChild(next);
+        next = SocialMark.button.previousSibling;
       }
     }
   },
@@ -1035,7 +1045,7 @@ SocialToolbar = {
     // provider.profile == undefined means no response yet from the provider
     // to tell us whether the user is logged in or not.
     if (!SocialUI.enabled ||
-        (!Social.haveLoggedInUser() && Social.provider.profile !== undefined)) {
+        (!Social.provider.haveLoggedInUser() && Social.provider.profile !== undefined)) {
       // Either no enabled provider, or there is a provider and it has
       // responded with a profile and the user isn't loggedin.  The icons
       // etc have already been removed by updateButtonHiddenState, so we want

browser/base/content/nsContextMenu.js

@@ -905,10 +905,7 @@ nsContextMenu.prototype = {
   reload: function(event) {
     if (this.onSocial) {
       // full reload of social provider
-      Social.enabled = false;
-      Services.tm.mainThread.dispatch(function() {
-        Social.enabled = true;
-      }, Components.interfaces.nsIThread.DISPATCH_NORMAL);
+      Social.provider.reload();
     } else {
       BrowserReloadOrDuplicate(event);
     }

browser/base/content/test/social/Makefile.in

@@ -30,6 +30,7 @@ MOCHITEST_BROWSER_FILES = \
                  browser_social_chatwindow_resize.js \
                  browser_social_chatwindowfocus.js \
                  browser_social_multiprovider.js \
+                 browser_social_multiworker.js \
                  browser_social_errorPage.js \
                  browser_social_window.js \
                  social_activate.html \

browser/base/content/test/social/browser_social_multiworker.js

@@ -0,0 +1,101 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+function test() {
+  waitForExplicitFinish();
+
+  Services.prefs.setBoolPref("social.allowMultipleWorkers", true);
+  runSocialTestWithProvider(gProviders, function (finishcb) {
+    runSocialTests(tests, undefined, undefined, function() {
+      Services.prefs.clearUserPref("social.allowMultipleWorkers");
+      finishcb();
+    });
+  });
+}
+
+let gProviders = [
+  {
+    name: "provider 1",
+    origin: "https://example.com",
+    sidebarURL: "https://example.com/browser/browser/base/content/test/social/social_sidebar.html?provider1",
+    workerURL: "https://example.com/browser/browser/base/content/test/social/social_worker.js",
+    iconURL: "chrome://branding/content/icon48.png"
+  },
+  {
+    name: "provider 2",
+    origin: "https://test1.example.com",
+    sidebarURL: "https://test1.example.com/browser/browser/base/content/test/social/social_sidebar.html?provider2",
+    workerURL: "https://test1.example.com/browser/browser/base/content/test/social/social_worker.js",
+    iconURL: "chrome://branding/content/icon48.png"
+  }
+];
+
+var tests = {
+  testWorkersAlive: function(next) {
+    // verify we can get a message from all providers that are enabled
+    let messageReceived = 0;
+    function oneWorkerTest(provider) {
+      let port = provider.getWorkerPort();
+      port.onmessage = function (e) {
+        let topic = e.data.topic;
+        switch (topic) {
+          case "test-init-done":
+            ok(true, "got message from provider " + provider.name);
+            port.close();
+            messageReceived++;
+            break;
+        }
+      };
+      port.postMessage({topic: "test-init"});
+    }
+
+    for (let p of Social.providers) {
+      oneWorkerTest(p);
+    }
+
+    waitForCondition(function() messageReceived == Social.providers.length,
+                     next, "received messages from all workers");
+  },
+  testWorkerDisabling: function(next) {
+    Social.enabled = false;
+    is(Social.providers.length, gProviders.length, "providers still available");
+    for (let p of Social.providers) {
+      ok(!p.enabled, "provider disabled");
+      ok(!p.getWorkerPort(), "worker disabled");
+    }
+    next();
+  },
+
+  testSingleWorkerEnabling: function(next) {
+    // test that only one worker is enabled when we limit workers
+    Services.prefs.setBoolPref("social.allowMultipleWorkers", false);
+    Social.enabled = true;
+    for (let p of Social.providers) {
+      if (p == Social.provider) {
+        ok(p.enabled, "primary provider enabled");
+        let port = p.getWorkerPort();
+        ok(port, "primary worker enabled");
+        port.close();
+      } else {
+        ok(!p.enabled, "secondary provider is not enabled");
+        ok(!p.getWorkerPort(), "secondary worker disabled");
+      }
+    }
+    next();
+  },
+
+  testMultipleWorkerEnabling: function(next) {
+    // test that all workers are enabled when we allow multiple workers
+    Social.enabled = false;
+    Services.prefs.setBoolPref("social.allowMultipleWorkers", true);
+    Social.enabled = true;
+    for (let p of Social.providers) {
+      ok(p.enabled, "provider enabled");
+      let port = p.getWorkerPort();
+      ok(port, "worker enabled");
+      port.close();
+    }
+    next();
+  }
+}

browser/modules/Social.jsm

@@ -91,6 +91,11 @@ this.Social = {
   providers: [],
   _disabledForSafeMode: false,
 
+  get allowMultipleWorkers() {
+    return Services.prefs.prefHasUserValue("social.allowMultipleWorkers") &&
+           Services.prefs.getBoolPref("social.allowMultipleWorkers");
+  },
+
   get _currentProviderPref() {
     try {
       return Services.prefs.getComplexValue("social.provider.current",
@@ -114,15 +119,14 @@ this.Social = {
     this._setProvider(val);
   },
 
-  // Sets the current provider and enables it. Also disables the
-  // previously set provider, and notifies observers of the change.
+  // Sets the current provider and notifies observers of the change.
   _setProvider: function (provider) {
     if (this._provider == provider)
       return;
 
-    // Disable the previous provider, if any, since we want only one provider to
-    // be enabled at once.
-    if (this._provider)
+    // Disable the previous provider, if we are not allowing multiple workers,
+    // since we want only one provider to be enabled at once.
+    if (this._provider && !Social.allowMultipleWorkers)
       this._provider.enabled = false;
 
     this._provider = provider;
@@ -134,7 +138,6 @@ this.Social = {
     let enabled = !!provider;
     if (enabled != SocialService.enabled) {
       SocialService.enabled = enabled;
-      Services.prefs.setBoolPref("social.enabled", enabled);
     }
 
     let origin = this._provider && this._provider.origin;
@@ -159,31 +162,40 @@ this.Social = {
     if (SocialService.enabled) {
       // Retrieve the current set of providers, and set the current provider.
       SocialService.getOrderedProviderList(function (providers) {
-        this._updateProviderCache(providers);
-      }.bind(this));
+        Social._updateProviderCache(providers);
+        Social._updateWorkerState(true);
+      });
     }
 
     // Register an observer for changes to the provider list
     SocialService.registerProviderListener(function providerListener(topic, data) {
-      // An engine change caused by adding/removing a provider should notify
+      // An engine change caused by adding/removing a provider should notify.
+      // any providers we receive are enabled in the AddonsManager
       if (topic == "provider-added" || topic == "provider-removed") {
-        this._updateProviderCache(data);
+        Social._updateProviderCache(data);
+        Social._updateWorkerState(true);
         Services.obs.notifyObservers(null, "social:providers-changed", null);
         return;
       }
       if (topic == "provider-update") {
-        // a provider has self-updated its manifest, we need to update our
-        // cache and possibly reload if it was the current provider.
+        // a provider has self-updated its manifest, we need to update our cache
+        // and reload the provider.
         let provider = data;
-        // if we need a reload, do it now
-        if (provider.enabled) {
-          Social.enabled = false;
-          Services.tm.mainThread.dispatch(function() {
-            Social.enabled = true;
-          }, Components.interfaces.nsIThread.DISPATCH_NORMAL);
-        }
+        SocialService.getOrderedProviderList(function(providers) {
+          Social._updateProviderCache(providers);
+          provider.reload();
+          Services.obs.notifyObservers(null, "social:providers-changed", null);
+        });
       }
-    }.bind(this));
+    });
+  },
+
+  _updateWorkerState: function(enable) {
+    // ensure that our providers are all disabled, and enabled if we allow
+    // multiple workers
+    if (enable && !Social.allowMultipleWorkers)
+      return;
+    [p.enabled = enable for (p of Social.providers) if (p.enabled != enable)];
   },
 
   // Called to update our cache of providers and set the current provider
@@ -203,6 +215,9 @@ this.Social = {
   set enabled(val) {
     // Setting .enabled is just a shortcut for setting the provider to either
     // the default provider or null...
+
+    this._updateWorkerState(val);
+
     if (val) {
       if (!this.provider)
         this.provider = this.defaultProvider;
@@ -210,6 +225,7 @@ this.Social = {
       this.provider = null;
     }
   },
+
   get enabled() {
     return this.provider != null;
   },
@@ -229,10 +245,6 @@ this.Social = {
     Services.prefs.setBoolPref("social.toast-notifications.enabled", !prefValue);
   },
 
-  haveLoggedInUser: function () {
-    return !!(this.provider && this.provider.profile && this.provider.profile.userName);
-  },
-
   setProviderByOrigin: function (origin) {
     this.provider = this._getProviderFromOrigin(origin);
   },

toolkit/components/social/MozSocialAPI.jsm

@@ -49,7 +49,7 @@ function injectController(doc, topic, data) {
       return;
     }
 
-    var containingBrowser = window.QueryInterface(Ci.nsIInterfaceRequestor)
+    let containingBrowser = window.QueryInterface(Ci.nsIInterfaceRequestor)
                                   .getInterface(Ci.nsIWebNavigation)
                                   .QueryInterface(Ci.nsIDocShell)
                                   .chromeEventHandler;
@@ -67,7 +67,7 @@ function injectController(doc, topic, data) {
     }
 
     SocialService.getProvider(doc.nodePrincipal.origin, function(provider) {
-      if (provider && provider.workerURL && provider.enabled) {
+      if (provider && provider.enabled) {
         attachToWindow(provider, window);
       }
     });
@@ -88,7 +88,7 @@ function attachToWindow(provider, targetWindow) {
     return;
   }
 
-  var port = provider.getWorkerPort(targetWindow);
+  let port = provider.workerURL ? provider.getWorkerPort(targetWindow) : null;
 
   let mozSocialObj = {
     // Use a method for backwards compat with existing providers, but we
@@ -206,12 +206,14 @@ function attachToWindow(provider, targetWindow) {
     return targetWindow.navigator.wrappedJSObject.mozSocial = contentObj;
   });
 
-  targetWindow.addEventListener("unload", function () {
-    // We want to close the port, but also want the target window to be
-    // able to use the port during an unload event they setup - so we
-    // set a timer which will fire after the unload events have all fired.
-    schedule(function () { port.close(); });
-  });
+  if (port) {
+    targetWindow.addEventListener("unload", function () {
+      // We want to close the port, but also want the target window to be
+      // able to use the port during an unload event they setup - so we
+      // set a timer which will fire after the unload events have all fired.
+      schedule(function () { port.close(); });
+    });
+  }
 
   // We allow window.close() to close the panel, so add an event handler for
   // this, then cancel the event (so the window itself doesn't die) and

toolkit/components/social/SocialService.jsm

@@ -34,7 +34,13 @@ XPCOMUtils.defineLazyServiceGetter(this, "etld",
 
 // Internal helper methods and state
 let SocialServiceInternal = {
-  enabled: Services.prefs.getBoolPref("social.enabled"),
+  _enabled: Services.prefs.getBoolPref("social.enabled"),
+  get enabled() this._enabled,
+  set enabled(val) {
+    this._enabled = !!val;
+    Services.prefs.setBoolPref("social.enabled", !!val);
+  },
+
   get providerArray() {
     return [p for ([, p] of Iterator(this.providers))];
   },
@@ -362,7 +368,6 @@ this.SocialService = {
 
     SocialServiceInternal.enabled = enable;
     MozSocialAPI.enabled = enable;
-    Services.obs.notifyObservers(null, "social:pref-changed", enable ? "enabled" : "disabled");
     Services.telemetry.getHistogramById("SOCIAL_TOGGLED").add(enable);
   },
 
@@ -723,6 +728,12 @@ function SocialProvider(input) {
 }
 
 SocialProvider.prototype = {
+  reload: function() {
+    this._terminate();
+    this._activate();
+    Services.obs.notifyObservers(null, "social:provider-reload", this.origin);
+  },
+
   // Provider enabled/disabled state. Disabled providers do not have active
   // connections to their FrameWorkers.
   _enabled: false,
@@ -868,6 +879,10 @@ SocialProvider.prototype = {
       closeAllChatWindows(this);
   },
 
+  haveLoggedInUser: function () {
+    return !!(this.profile && this.profile.userName);
+  },
+
   // Called by the workerAPI to add/update a notification icon.
   setAmbientNotification: function(notification) {
     if (!this.profile.userName)

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

@@ -2,6 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+let provider;
+
 function test() {
   waitForExplicitFinish();
 
@@ -13,10 +15,22 @@ function test() {
 
   ensureSocialEnabled();
 
-  SocialService.addProvider(manifest, function (provider) {
-    // enable the provider
-    provider.enabled = true;
+  SocialService.addProvider(manifest, function (p) {
+    provider = p;
+    runTests(tests, undefined, undefined, function () {
+      SocialService.removeProvider(p.origin, function() {
+        ok(!provider.enabled, "removing an enabled provider should have disabled the provider");
+        let port = provider.getWorkerPort();
+        ok(!port, "should not be able to get a port after removing the provider");
+        provider = null;
+        finish();
+      });
+    });
+  });
+}
 
+let tests = {
+  testSingleProvider: function(next) {
     ok(provider.enabled, "provider is initially enabled");
     let port = provider.getWorkerPort();
     ok(port, "should be able to get a port from enabled provider");
@@ -37,12 +51,38 @@ function test() {
     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, function() {
-      ok(!provider.enabled, "removing an enabled provider should have disabled the provider");
+    next();
+  },
+  testTwoProviders: function(next) {
+    // add another provider, test both workers
+    let manifest = {
+      origin: 'http://test2.example.com',
+      name: "Example Provider 2",
+      workerURL: "http://test2.example.com/browser/toolkit/components/social/test/browser/worker_social.js"
+    };
+    SocialService.addProvider(manifest, function (provider2) {
+      ok(provider.enabled, "provider is initially enabled");
+      ok(!provider2.enabled, "provider2 is not initially enabled");
+      provider2.enabled = true;
       let port = provider.getWorkerPort();
-      ok(!port, "should not be able to get a port after removing the provider");
-      finish();
+      let port2 = provider2.getWorkerPort();
+      ok(port, "have port for provider");
+      ok(port2, "have port for provider2");
+      port.onmessage = function(e) {
+        if (e.data.topic == "test-initialization-complete") {
+          ok(true, "first provider initialized");
+          port2.postMessage({topic: "test-initialization"});
+        }
+      }
+      port2.onmessage = function(e) {
+        if (e.data.topic == "test-initialization-complete") {
+          ok(true, "second provider initialized");
+          SocialService.removeProvider(provider2.origin, function() {
+            next();
+          });
+        }
+      }
+      port.postMessage({topic: "test-initialization"});
     });
-  });
+  }
 }

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

@@ -78,38 +78,44 @@ function testGetProviderList(manifests, next) {
   }
 }
 
+
 function testEnabled(manifests, next) {
   // Check that providers are disabled by default
   let providers = yield SocialService.getProviderList(next);
   do_check_true(providers.length >= manifests.length);
-  do_check_true(SocialService.enabled);
+  do_check_true(SocialService.enabled, "social enabled at test start");
   providers.forEach(function (provider) {
     do_check_false(provider.enabled);
   });
 
-  let notificationDisabledCorrect = false;
-  Services.obs.addObserver(function obs1(subj, topic, data) {
-    Services.obs.removeObserver(obs1, "social:pref-changed");
-    notificationDisabledCorrect = data == "disabled";
-  }, "social:pref-changed", false);
+  do_test_pending();
+  function waitForEnableObserver(cb) {
+    Services.prefs.addObserver("social.enabled", function prefObserver(subj, topic, data) {
+      Services.prefs.removeObserver("social.enabled", prefObserver);
+      cb();
+    }, false);
+  }
 
   // enable one of the added providers
   providers[providers.length-1].enabled = true;
 
   // now disable the service and check that it disabled that provider (and all others for good measure)
+  waitForEnableObserver(function() {
+    do_check_true(!SocialService.enabled);
+    providers.forEach(function (provider) {
+      do_check_false(provider.enabled);
+    });
+    waitForEnableObserver(function() {
+      do_check_true(SocialService.enabled);
+      // Enabling the service should not enable providers
+      providers.forEach(function (provider) {
+        do_check_false(provider.enabled);
+      });
+      do_test_finished();
+    });
+    SocialService.enabled = true;
+  });
   SocialService.enabled = false;
-  do_check_true(notificationDisabledCorrect);
-  do_check_true(!SocialService.enabled);
-  providers.forEach(function (provider) {
-    do_check_false(provider.enabled);
-  });
-
-  SocialService.enabled = true;
-  do_check_true(SocialService.enabled);
-  // Enabling the service should not enable providers
-  providers.forEach(function (provider) {
-    do_check_false(provider.enabled);
-  });
 }
 
 function testAddRemoveProvider(manifests, next) {