bug 906212 fix socialapi ui startup regressions, r=markh
authorShane Caraveo <scaraveo@mozilla.com>
Fri, 30 Aug 2013 11:11:20 -0700
changeset 145138 a2de1265566fdb32c71db07ba835fa856fa72a2c
parent 145137 87d25d64fbb48f8b2caf0b0b0c57c5208656c8cc
child 145139 aff6afb77f62802574a28e38fc9315b89c595200
push id25195
push userryanvm@gmail.com
push dateSat, 31 Aug 2013 01:00:12 +0000
treeherdermozilla-central@4ba8dda1ee31 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh
bugs906212
milestone26.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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.