bug 906212 fix socialapi ui startup regressions, r=markh
authorShane Caraveo <scaraveo@mozilla.com>
Fri, 30 Aug 2013 11:11:20 -0700
changeset 153122 a2de1265566fdb32c71db07ba835fa856fa72a2c
parent 153121 87d25d64fbb48f8b2caf0b0b0c57c5208656c8cc
child 153123 aff6afb77f62802574a28e38fc9315b89c595200
push idunknown
push userunknown
push dateunknown
reviewersmarkh
bugs906212
milestone26.0a1
bug 906212 fix socialapi ui startup regressions, r=markh
browser/base/content/browser-sets.inc
browser/base/content/browser-social.js
browser/base/content/test/social/browser_social_window.js
browser/base/content/test/social/head.js
browser/modules/Social.jsm
toolkit/components/social/SocialService.jsm
--- a/browser/base/content/browser-sets.inc
+++ b/browser/base/content/browser-sets.inc
@@ -106,17 +106,17 @@
      oncommand="Cc['@mozilla.org/browser/browserglue;1'].getService(Ci.nsIBrowserGlue).sanitize(window);"/>
     <command id="Tools:PrivateBrowsing"
       oncommand="OpenBrowserWindow({private: true});"/>
     <command id="History:UndoCloseTab" oncommand="undoCloseTab();"/>
     <command id="History:UndoCloseWindow" oncommand="undoCloseWindow();"/>
     <command id="Browser:ToggleAddonBar" oncommand="toggleAddonBar();"/>
     <command id="Social:TogglePageMark" oncommand="SocialMark.togglePageMark();" disabled="true"/>
     <command id="Social:SharePage" oncommand="SocialShare.sharePage();" disabled="true"/>
-    <command id="Social:ToggleSidebar" oncommand="Social.toggleSidebar();"/>
+    <command id="Social:ToggleSidebar" oncommand="Social.toggleSidebar();" hidden="true"/>
     <command id="Social:ToggleNotifications" oncommand="Social.toggleNotifications();" hidden="true"/>
     <command id="Social:FocusChat" oncommand="SocialChatBar.focus();" hidden="true" disabled="true"/>
     <command id="Social:Toggle" oncommand="Social.toggle();" hidden="true"/>
     <command id="Social:Addons" oncommand="BrowserOpenAddonsMgr('addons://list/service');"/>
   </commandset>
 
   <commandset id="placesCommands">
     <command id="Browser:ShowAllBookmarks"
--- a/browser/base/content/browser-social.js
+++ b/browser/base/content/browser-social.js
@@ -40,19 +40,19 @@ SocialUI = {
 
     Services.prefs.addObserver("social.sidebar.open", this, false);
     Services.prefs.addObserver("social.toast-notifications.enabled", this, false);
 
     gBrowser.addEventListener("ActivateSocialFeature", this._activationEventHandler.bind(this), true, true);
 
     if (!Social.initialized) {
       Social.init();
-    } else if (Social.enabled) {
-      // social was previously initialized, so it's not going to notify us of
-      // anything, so handle that now.
+    } else if (Social.providers.length > 0) {
+      // Social was initialized during startup in a previous window. If we have
+      // providers enabled initialize the UI for this window.
       this.observe(null, "social:providers-changed", null);
       this.observe(null, "social:provider-set", Social.provider ? Social.provider.origin : null);
     }
   },
 
   // Called on window unload
   uninit: function SocialUI_uninit() {
     Services.obs.removeObserver(this, "social:ambient-notification-changed");
--- a/browser/base/content/test/social/browser_social_window.js
+++ b/browser/base/content/test/social/browser_social_window.js
@@ -1,23 +1,24 @@
 // 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/.
 
 // Test the top-level window UI for social.
 
+let SocialService = Cu.import("resource://gre/modules/SocialService.jsm", {}).SocialService;
+
 // This function should "reset" Social such that the next time Social.init()
 // is called (eg, when a new window is opened), it re-performs all
 // initialization.
 function resetSocial() {
   Social.initialized = false;
   Social._provider = null;
   Social.providers = [];
   // *sob* - listeners keep getting added...
-  let SocialService = Cu.import("resource://gre/modules/SocialService.jsm", {}).SocialService;
   SocialService._providerListeners.clear();
 }
 
 let createdWindows = [];
 
 function openWindowAndWaitForInit(callback) {
   // this notification tells us SocialUI.init() has been run...
   let topic = "browser-delayed-startup-finished";
@@ -47,37 +48,39 @@ let manifest = { // normal provider
 };
 
 function test() {
   waitForExplicitFinish();
   runSocialTests(tests, undefined, postTestCleanup);
 }
 
 let tests = {
-  // check when social is totally disabled at startup (ie, no providers)
+  // check when social is totally disabled at startup (ie, no providers enabled)
   testInactiveStartup: function(cbnext) {
     is(Social.providers.length, 0, "needs zero providers to start this test.");
+    ok(!SocialService.hasEnabledProviders, "no providers are enabled");
     resetSocial();
     openWindowAndWaitForInit(function(w1) {
       checkSocialUI(w1);
       // Now social is (re-)initialized, open a secondary window and check that.
       openWindowAndWaitForInit(function(w2) {
         checkSocialUI(w2);
         checkSocialUI(w1);
         cbnext();
       });
     });
   },
 
-  // Check when providers exist and social is turned on at startup.
+  // Check when providers are enabled and social is turned on at startup.
   testEnabledStartup: function(cbnext) {
     runSocialTestWithProvider(manifest, function (finishcb) {
       resetSocial();
       openWindowAndWaitForInit(function(w1) {
         ok(Social.enabled, "social is enabled");
+        ok(SocialService.hasEnabledProviders, "providers are enabled");
         checkSocialUI(w1);
         // now init is complete, open a second window
         openWindowAndWaitForInit(function(w2) {
           checkSocialUI(w2);
           checkSocialUI(w1);
           // disable social and re-check
           Services.prefs.setBoolPref("social.enabled", false);
           executeSoon(function() { // let all the UI observers run...
@@ -86,59 +89,61 @@ let tests = {
             checkSocialUI(w1);
             finishcb();
           });
         });
       });
     }, cbnext);
   },
 
-  // Check when providers exist but social is turned off at startup.
+  // Check when providers are enabled but social is turned off at startup.
   testDisabledStartup: function(cbnext) {
-    runSocialTestWithProvider(manifest, function (finishcb) {
+    setManifestPref("social.manifest.test", manifest);
+    SocialService.addProvider(manifest, function (provider) {
       Services.prefs.setBoolPref("social.enabled", false);
       resetSocial();
+      ok(SocialService.hasEnabledProviders, "providers are enabled");
       openWindowAndWaitForInit(function(w1) {
         ok(!Social.enabled, "social is disabled");
         checkSocialUI(w1);
         // now init is complete, open a second window
         openWindowAndWaitForInit(function(w2) {
           checkSocialUI(w2);
           checkSocialUI(w1);
           // enable social and re-check
           Services.prefs.setBoolPref("social.enabled", true);
           executeSoon(function() { // let all the UI observers run...
             ok(Social.enabled, "social is enabled");
             checkSocialUI(w2);
             checkSocialUI(w1);
-            finishcb();
+            SocialService.removeProvider(manifest.origin, function() {
+              Services.prefs.clearUserPref("social.manifest.test");
+              cbnext();
+            });
           });
         });
       });
     }, cbnext);
   },
 
-  // Check when the last provider is removed.
+  // Check when the last provider is disabled.
   testRemoveProvider: function(cbnext) {
-    runSocialTestWithProvider(manifest, function (finishcb) {
+    setManifestPref("social.manifest.test", manifest);
+    SocialService.addProvider(manifest, function (provider) {
       openWindowAndWaitForInit(function(w1) {
         checkSocialUI(w1);
         // now init is complete, open a second window
         openWindowAndWaitForInit(function(w2) {
           checkSocialUI(w2);
-          // remove the current provider.
-          let SocialService = Cu.import("resource://gre/modules/SocialService.jsm", {}).SocialService;
+          // disable the current provider.
           SocialService.removeProvider(manifest.origin, function() {
             ok(!Social.enabled, "social is disabled");
             is(Social.providers.length, 0, "no providers");
             checkSocialUI(w2);
             checkSocialUI(w1);
-            // *sob* - runSocialTestWithProvider's cleanup fails when it can't
-            // remove the provider, so re-add it.
-            SocialService.addProvider(manifest, function() {
-              finishcb();
-            });
+            Services.prefs.clearUserPref("social.manifest.test");
+            cbnext();
           });
         });
       });
     }, cbnext);
   },
 }
--- a/browser/base/content/test/social/head.js
+++ b/browser/base/content/test/social/head.js
@@ -189,23 +189,32 @@ function runSocialTests(tests, cbPreTest
     });
   }
   runNextTest();
 }
 
 // A fairly large hammer which checks all aspects of the SocialUI for
 // internal consistency.
 function checkSocialUI(win) {
+  let SocialService = Cu.import("resource://gre/modules/SocialService.jsm", {}).SocialService;
   win = win || window;
   let doc = win.document;
   let provider = Social.provider;
   let enabled = win.SocialUI.enabled;
   let active = Social.providers.length > 0 && !win.SocialUI._chromeless &&
                !PrivateBrowsingUtils.isWindowPrivate(win);
 
+  // if we have enabled providers, we should also have instances of those
+  // providers
+  if (SocialService.hasEnabledProviders) {
+    ok(Social.providers.length > 0, "providers are enabled");
+  } else {
+    is(Social.providers.length, 0, "providers are not enabled");
+  }
+
   // some local helpers to avoid log-spew for the many checks made here.
   let numGoodTests = 0, numTests = 0;
   function _ok(what, msg) {
     numTests++;
     if (!ok)
       ok(what, msg)
     else
       ++numGoodTests;
@@ -241,16 +250,17 @@ function checkSocialUI(win) {
   // the menus should always have the provider name
   if (provider) {
     for (let id of ["menu_socialSidebar", "menu_socialAmbientMenu"])
       _is(document.getElementById(id).getAttribute("label"), Social.provider.name, "element has the provider name");
   }
 
   // and for good measure, check all the social commands.
   isbool(!doc.getElementById("Social:Toggle").hidden, active, "Social:Toggle visible?");
+  isbool(!doc.getElementById("Social:ToggleSidebar").hidden, enabled, "Social:ToggleSidebar visible?");
   isbool(!doc.getElementById("Social:ToggleNotifications").hidden, enabled, "Social:ToggleNotifications visible?");
   isbool(!doc.getElementById("Social:FocusChat").hidden, enabled, "Social:FocusChat visible?");
   isbool(doc.getElementById("Social:FocusChat").getAttribute("disabled"), enabled ? "false" : "true", "Social:FocusChat disabled?");
   _is(doc.getElementById("Social:TogglePageMark").getAttribute("disabled"), canMark ? "false" : "true", "Social:TogglePageMark enabled?");
 
   // broadcasters.
   isbool(!doc.getElementById("socialActiveBroadcaster").hidden, active, "socialActiveBroadcaster hidden?");
   // and report on overall success of failure of the various checks here.
--- a/browser/modules/Social.jsm
+++ b/browser/modules/Social.jsm
@@ -153,18 +153,19 @@ this.Social = {
 
   init: function Social_init() {
     this._disabledForSafeMode = Services.appinfo.inSafeMode && this.enabled;
 
     if (this.initialized) {
       return;
     }
     this.initialized = true;
-
-    if (SocialService.enabled) {
+    // if SocialService.hasEnabledProviders, retreive the providers so the
+    // front-end can generate UI
+    if (SocialService.hasEnabledProviders) {
       // Retrieve the current set of providers, and set the current provider.
       SocialService.getOrderedProviderList(function (providers) {
         Social._updateProviderCache(providers);
         Social._updateWorkerState(true);
       });
     }
 
     // Register an observer for changes to the provider list
--- a/toolkit/components/social/SocialService.jsm
+++ b/toolkit/components/social/SocialService.jsm
@@ -345,16 +345,26 @@ function initService() {
 }
 
 function schedule(callback) {
   Services.tm.mainThread.dispatch(callback, Ci.nsIThread.DISPATCH_NORMAL);
 }
 
 // Public API
 this.SocialService = {
+  get hasEnabledProviders() {
+    // used as an optimization during startup, can be used to check if further
+    // initialization should be done (e.g. creating the instances of
+    // SocialProvider and turning on UI). ActiveProviders may have changed and
+    // not yet flushed so we check the active providers array
+    for (let p in ActiveProviders._providers) {
+      return true;
+    };
+    return false;
+  },
   get enabled() {
     return SocialServiceInternal.enabled;
   },
   set enabled(val) {
     let enable = !!val;
 
     // Allow setting to the same value when in safe mode so the
     // feature can be force enabled.