bug 862314 fix double install of social provider, r=markh
☠☠ backed out by 46a74e87779c ☠ ☠
authorShane Caraveo <scaraveo@mozilla.com>
Wed, 08 May 2013 17:59:17 -0700
changeset 138115 bf0dfd39ebac8583394be92bcb37d18641f5bc8b
parent 138114 ad46747e8ec467d3e3d7a913b22e0ffd12354466
child 138116 ddc934f971ecca705305d819593e28b97bd4919e
push id3752
push userlsblakk@mozilla.com
push dateMon, 13 May 2013 17:21:10 +0000
treeherdermozilla-aurora@1580544aef0b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh
bugs862314
milestone23.0a1
bug 862314 fix double install of social provider, r=markh
browser/base/content/test/social/browser_social_activation.js
browser/base/content/test/social/social_activate.html
toolkit/components/social/SocialService.jsm
--- a/browser/base/content/test/social/browser_social_activation.js
+++ b/browser/base/content/test/social/browser_social_activation.js
@@ -37,17 +37,19 @@ function postTestCleanup(callback) {
     } catch(ex) {
       // this test didn't add this provider - that's ok.
       cbRemoved();
     }
   }
 }
 
 function addBuiltinManifest(manifest) {
-  setManifestPref("social.manifest."+manifest.origin, manifest);
+  let prefname = getManifestPrefname(manifest);
+  setBuiltinManifestPref(prefname, manifest);
+  return prefname;
 }
 
 function addTab(url, callback) {
   let tab = gBrowser.selectedTab = gBrowser.addTab(url, {skipAnimation: true});
   tab.linkedBrowser.addEventListener("load", function tabLoad(event) {
     tab.linkedBrowser.removeEventListener("load", tabLoad, true);
     tabsToRemove.push(tab);
     executeSoon(function() {callback(tab)});
@@ -75,16 +77,88 @@ function activateProvider(domain, callba
 
 function activateIFrameProvider(domain, callback) {
   let activationURL = domain+"/browser/browser/base/content/test/social/social_activate_iframe.html"
   addTab(activationURL, function(tab) {
     sendActivationEvent(tab, callback, false);
   });
 }
 
+function waitForProviderLoad(cb) {
+  Services.obs.addObserver(function providerSet(subject, topic, data) {
+    Services.obs.removeObserver(providerSet, "social:provider-set");
+    info("social:provider-set observer was notified");
+    waitForCondition(function() {
+      let sbrowser = document.getElementById("social-sidebar-browser");
+      return Social.provider.profile &&
+             Social.provider.profile.displayName &&
+             sbrowser.docShellIsActive;
+    }, function() {
+      // executeSoon to let the browser UI observers run first
+      executeSoon(cb);
+    },
+    "waitForProviderLoad: provider profile was not set");
+  }, "social:provider-set", false);
+}
+
+
+function getAddonItemInList(aId, aList) {
+  var item = aList.firstChild;
+  while (item) {
+    if ("mAddon" in item && item.mAddon.id == aId) {
+      aList.ensureElementIsVisible(item);
+      return item;
+    }
+    item = item.nextSibling;
+  }
+  return null;
+}
+
+function clickAddonRemoveButton(tab, aCallback) {
+  AddonManager.getAddonsByTypes(["service"], function(aAddons) {
+    let addon = aAddons[0];
+
+    let doc = tab.linkedBrowser.contentDocument;
+    let list = doc.getElementById("addon-list");
+
+    let item = getAddonItemInList(addon.id, list);
+    isnot(item, null, "Should have found the add-on in the list");
+
+    var button = doc.getAnonymousElementByAttribute(item, "anonid", "remove-btn");
+    isnot(button, null, "Should have a remove button");
+    ok(!button.disabled, "Button should not be disabled");
+
+    EventUtils.synthesizeMouseAtCenter(button, { }, doc.defaultView);
+
+    // Force XBL to apply
+    item.clientTop;
+
+    is(item.getAttribute("pending"), "uninstall", "Add-on should be uninstalling");
+
+    executeSoon(function() { aCallback(addon); });
+  });
+}
+
+function activateOneProvider(manifest, finishActivation, aCallback) {
+  activateProvider(manifest.origin, function() {
+    waitForProviderLoad(function() {
+      ok(!SocialUI.activationPanel.hidden, "activation panel is showing");
+      is(Social.provider.origin, manifest.origin, "new provider is active");
+      checkSocialUI();
+
+      if (finishActivation)
+        document.getElementById("social-activation-button").click();
+      else
+        document.getElementById("social-undoactivation-button").click();
+
+      executeSoon(aCallback);
+    });
+  });
+}
+
 let gTestDomains = ["https://example.com", "https://test1.example.com", "https://test2.example.com"];
 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#no-profile,no-recommend",
     iconURL: "chrome://branding/content/icon48.png"
@@ -134,105 +208,136 @@ var tests = {
       Services.prefs.clearUserPref("social.whitelist");
       next();
     });
   },
 
   testActivationFirstProvider: function(next) {
     Services.prefs.setCharPref("social.whitelist", gTestDomains.join(","));
     // first up we add a manifest entry for a single provider.
-    activateProvider(gTestDomains[0], function() {
-      ok(!SocialUI.activationPanel.hidden, "activation panel should be showing");
-      is(Social.provider.origin, gTestDomains[0], "new provider is active");
+    activateOneProvider(gProviders[0], false, function() {
+      // we deactivated leaving no providers left, so Social is disabled.
+      ok(!Social.provider, "should be no provider left after disabling");
       checkSocialUI();
-      // hit "undo"
-      document.getElementById("social-undoactivation-button").click();
-      executeSoon(function() {
-        // we deactivated leaving no providers left, so Social is disabled.
-        ok(!Social.provider, "should be no provider left after disabling");
-        checkSocialUI();
-        Services.prefs.clearUserPref("social.whitelist");
-        next();
-      })
+      Services.prefs.clearUserPref("social.whitelist");
+      next();
     });
   },
 
   testActivationBuiltin: function(next) {
-    let prefname = getManifestPrefname(gProviders[0]);
-    setBuiltinManifestPref(prefname, gProviders[0]);
+    let prefname = addBuiltinManifest(gProviders[0]);
     is(SocialService.getOriginActivationType(gTestDomains[0]), "builtin", "manifest is builtin");
     // first up we add a manifest entry for a single provider.
-    activateProvider(gTestDomains[0], function() {
-      ok(!SocialUI.activationPanel.hidden, "activation panel should be showing");
-      is(Social.provider.origin, gTestDomains[0], "new provider is active");
+    activateOneProvider(gProviders[0], false, function() {
+      // we deactivated leaving no providers left, so Social is disabled.
+      ok(!Social.provider, "should be no provider left after disabling");
       checkSocialUI();
-      // hit "undo"
-      document.getElementById("social-undoactivation-button").click();
-      executeSoon(function() {
-        // we deactivated leaving no providers left, so Social is disabled.
-        ok(!Social.provider, "should be no provider left after disabling");
-        checkSocialUI();
-        resetBuiltinManifestPref(prefname);
-        next();
-      })
-    }, true);
+      resetBuiltinManifestPref(prefname);
+      next();
+    });
   },
 
   testActivationMultipleProvider: function(next) {
     // The trick with this test is to make sure that Social.providers[1] is
     // the current provider when doing the undo - this makes sure that the
     // Social code doesn't fallback to Social.providers[0], which it will
     // do in some cases (but those cases do not include what this test does)
     // first enable the 2 providers
     Services.prefs.setCharPref("social.whitelist", gTestDomains.join(","));
     SocialService.addProvider(gProviders[0], function() {
       SocialService.addProvider(gProviders[1], function() {
         Social.provider = Social.providers[1];
         checkSocialUI();
         // activate the last provider.
-        addBuiltinManifest(gProviders[2]);
-        activateProvider(gTestDomains[2], function() {
-          ok(!SocialUI.activationPanel.hidden, "activation panel should be showing");
-          is(Social.provider.origin, gTestDomains[2], "new provider is active");
+        let prefname = addBuiltinManifest(gProviders[2]);
+        activateOneProvider(gProviders[2], false, function() {
+          // we deactivated - the first provider should be enabled.
+          is(Social.provider.origin, Social.providers[1].origin, "original provider should have been reactivated");
           checkSocialUI();
-          // hit "undo"
-          document.getElementById("social-undoactivation-button").click();
-          executeSoon(function() {
-            // we deactivated - the first provider should be enabled.
-            is(Social.provider.origin, Social.providers[1].origin, "original provider should have been reactivated");
-            checkSocialUI();
-            Services.prefs.clearUserPref("social.whitelist");
-            next();
-          });
+          Services.prefs.clearUserPref("social.whitelist");
+          resetBuiltinManifestPref(prefname);
+          next();
         });
       });
     });
   },
 
   testRemoveNonCurrentProvider: function(next) {
     Services.prefs.setCharPref("social.whitelist", gTestDomains.join(","));
     SocialService.addProvider(gProviders[0], function() {
       SocialService.addProvider(gProviders[1], function() {
         Social.provider = Social.providers[1];
         checkSocialUI();
         // activate the last provider.
         addBuiltinManifest(gProviders[2]);
         activateProvider(gTestDomains[2], function() {
-          ok(!SocialUI.activationPanel.hidden, "activation panel should be showing");
-          is(Social.provider.origin, gTestDomains[2], "new provider is active");
-          checkSocialUI();
-          // A bit contrived, but set a new provider current while the
-          // activation ui is up.
-          Social.provider = Social.providers[1];
-          // hit "undo"
-          document.getElementById("social-undoactivation-button").click();
-          executeSoon(function() {
-            // we deactivated - the same provider should be enabled.
-            is(Social.provider.origin, Social.providers[1].origin, "original provider still be active");
+          waitForProviderLoad(function() {
+            ok(!SocialUI.activationPanel.hidden, "activation panel is showing");
+            is(Social.provider.origin, gTestDomains[2], "new provider is active");
             checkSocialUI();
-            Services.prefs.clearUserPref("social.whitelist");
-            next();
+            // A bit contrived, but set a new provider current while the
+            // activation ui is up.
+            Social.provider = Social.providers[1];
+            // hit "undo"
+            document.getElementById("social-undoactivation-button").click();
+            executeSoon(function() {
+              // we deactivated - the same provider should be enabled.
+              is(Social.provider.origin, Social.providers[1].origin, "original provider still be active");
+              checkSocialUI();
+              Services.prefs.clearUserPref("social.whitelist");
+              next();
+            });
           });
         });
       });
     });
   },
+
+  testAddonManagerDoubleInstall: function(next) {
+    Services.prefs.setCharPref("social.whitelist", gTestDomains.join(","));
+    // Create a new tab and load about:addons
+    let blanktab = gBrowser.addTab();
+    gBrowser.selectedTab = blanktab;
+    BrowserOpenAddonsMgr('addons://list/service');
+
+    is(blanktab, gBrowser.selectedTab, "Current tab should be blank tab");
+
+    gBrowser.selectedBrowser.addEventListener("load", function tabLoad() {
+      gBrowser.selectedBrowser.removeEventListener("load", tabLoad, true);
+      let browser = blanktab.linkedBrowser;
+      is(browser.currentURI.spec, "about:addons", "about:addons should load into blank tab.");
+
+      addBuiltinManifest(gProviders[0]);
+      activateOneProvider(gProviders[0], true, function() {
+        gBrowser.removeTab(gBrowser.selectedTab);
+        tabsToRemove.pop();
+        // uninstall the provider
+        clickAddonRemoveButton(blanktab, function(addon) {
+          checkSocialUI();
+          activateOneProvider(gProviders[0], true, function() {
+
+            // after closing the addons tab, verify provider is still installed
+            gBrowser.tabContainer.addEventListener("TabClose", function onTabClose() {
+              gBrowser.tabContainer.removeEventListener("TabClose", onTabClose);
+              AddonManager.getAddonsByTypes(["service"], function(aAddons) {
+                is(aAddons.length, 1, "there can be only one");
+                Services.prefs.clearUserPref("social.whitelist");
+                next();
+              });
+            });
+
+            // verify only one provider in list
+            AddonManager.getAddonsByTypes(["service"], function(aAddons) {
+              is(aAddons.length, 1, "there can be only one");
+
+              let doc = blanktab.linkedBrowser.contentDocument;
+              let list = doc.getElementById("addon-list");
+              is(list.childNodes.length, 1, "only one addon is displayed");
+
+              gBrowser.removeTab(blanktab);
+            });
+
+          });
+        });
+      });
+    }, true);
+  }
 }
--- a/browser/base/content/test/social/social_activate.html
+++ b/browser/base/content/test/social/social_activate.html
@@ -7,18 +7,18 @@
 var data = {
   // currently required
   "name": "Demo Social Service",
   "iconURL": "chrome://branding/content/icon16.png",
   "icon32URL": "chrome://branding/content/favicon32.png",
   "icon64URL": "chrome://branding/content/icon64.png",
 
   // at least one of these must be defined
-  "workerURL": "/browser/browser/base/content/test/social/social_sidebar.html",
-  "sidebarURL": "/browser/browser/base/content/test/social/social_worker.js",
+  "sidebarURL": "/browser/browser/base/content/test/social/social_sidebar.html",
+  "workerURL": "/browser/browser/base/content/test/social/social_worker.js",
 
   // should be available for display purposes
   "description": "A short paragraph about this provider",
   "author": "Shane Caraveo, Mozilla",
 
   // optional
   "version": "1.0"
 }
--- a/toolkit/components/social/SocialService.jsm
+++ b/toolkit/components/social/SocialService.jsm
@@ -496,25 +496,39 @@ this.SocialService = {
 
     let anchor = "servicesInstall-notification-icon";
     let notificationid = "servicesInstall";
     chromeWin.PopupNotifications.show(browser, notificationid, message, anchor,
                                       action, [], {});
   },
 
   installProvider: function(aDOMDocument, data, installCallback) {
-    let sourceURI = aDOMDocument.location.href;
     let installOrigin = aDOMDocument.nodePrincipal.origin;
 
     let id = getAddonIDFromOrigin(installOrigin);
     let version = data && data.version ? data.version : "0";
     if (Services.blocklist.getAddonBlocklistState(id, version) == Ci.nsIBlocklistService.STATE_BLOCKED)
       throw new Error("installProvider: provider with origin [" +
                       installOrigin + "] is blocklisted");
 
+    AddonManager.getAddonByID(id, function(aAddon) {
+      if (aAddon && aAddon.userDisabled) {
+        aAddon.cancelUninstall();
+        aAddon.userDisabled = false;
+      }
+      schedule(function () {
+        this._installProvider(aDOMDocument, data, installCallback);
+      }.bind(this));
+    }.bind(this));
+  },
+
+  _installProvider: function(aDOMDocument, data, installCallback) {
+    let sourceURI = aDOMDocument.location.href;
+    let installOrigin = aDOMDocument.nodePrincipal.origin;
+
     let installType = this.getOriginActivationType(installOrigin);
     let manifest;
     if (data) {
       // if we get data, we MUST have a valid manifest generated from the data
       manifest = this._manifestFromData(installType, data, aDOMDocument.nodePrincipal);
       if (!manifest)
         throw new Error("SocialService.installProvider: service configuration is invalid from " + sourceURI);
     }
@@ -843,33 +857,38 @@ function getAddonIDFromOrigin(origin) {
 function getPrefnameFromOrigin(origin) {
   return "social.manifest." + SocialServiceInternal.getManifestPrefname(origin);
 }
 
 function AddonInstaller(sourceURI, aManifest, installCallback) {
   aManifest.updateDate = Date.now();
   // get the existing manifest for installDate
   let manifest = SocialServiceInternal.getManifestByOrigin(aManifest.origin);
+  let isNewInstall = !manifest;
   if (manifest && manifest.installDate)
     aManifest.installDate = manifest.installDate;
   else
     aManifest.installDate = aManifest.updateDate;
 
   this.sourceURI = sourceURI;
   this.install = function() {
     let addon = this.addon;
-    AddonManagerPrivate.callInstallListeners("onExternalInstall", null, addon, null, false);
-    AddonManagerPrivate.callAddonListeners("onInstalling", addon, false);
+    if (isNewInstall) {
+      AddonManagerPrivate.callInstallListeners("onExternalInstall", null, addon, null, false);
+      AddonManagerPrivate.callAddonListeners("onInstalling", addon, false);
+    }
 
     let string = Cc["@mozilla.org/supports-string;1"].
                  createInstance(Ci.nsISupportsString);
     string.data = JSON.stringify(aManifest);
     Services.prefs.setComplexValue(getPrefnameFromOrigin(aManifest.origin), Ci.nsISupportsString, string);
 
-    AddonManagerPrivate.callAddonListeners("onInstalled", addon);
+    if (isNewInstall) {
+      AddonManagerPrivate.callAddonListeners("onInstalled", addon);
+    }
     installCallback(aManifest);
   };
   this.cancel = function() {
     Services.prefs.clearUserPref(getPrefnameFromOrigin(aManifest.origin))
   },
   this.addon = new AddonWrapper(aManifest);
 };
 
@@ -1095,24 +1114,16 @@ AddonWrapper.prototype = {
         }.bind(this));
       } else {
         SocialAddonProvider.removeAddon(this);
       }
     }
   },
 
   cancelUninstall: function() {
-    let prefName = getPrefnameFromOrigin(this.manifest.origin);
-    if (Services.prefs.prefHasUserValue(prefName))
-      throw new Error(this.manifest.name + " is not marked to be uninstalled");
-    // ensure we're set into prefs
-    let string = Cc["@mozilla.org/supports-string;1"].
-                 createInstance(Ci.nsISupportsString);
-    string.data = JSON.stringify(this.manifest);
-    Services.prefs.setComplexValue(prefName, Ci.nsISupportsString, string);
     this._pending -= AddonManager.PENDING_UNINSTALL;
     AddonManagerPrivate.callAddonListeners("onOperationCancelled", this);
   }
 };
 
 
 AddonManagerPrivate.registerProvider(SocialAddonProvider, [
   new AddonManagerPrivate.AddonType(ADDON_TYPE_SERVICE, URI_EXTENSION_STRINGS,