[Sync] Add support for enabling/disabling an extension before it's installed

Make Enable/DisableExtension handle non-installed extensions gracefully.

Make DisableExtension handle terminated extensions.

Make ProcessSyncData enable/disable an extension regardless of whether
it's already installed.

BUG=72659
TEST=


Review URL: http://codereview.chromium.org/6903127

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@83571 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 59 insertions, 38 deletions

diff --git a/chrome/browser/extensions/extension_pref_value_map.cc b/chrome/browser/extensions/extension_pref_value_map.cc
index 02ebcdf..7032be9 100644
--- a/chrome/browser/extensions/extension_pref_value_map.cc
+++ b/chrome/browser/extensions/extension_pref_value_map.cc
@@ -101,7 +101,10 @@ void ExtensionPrefValueMap::UnregisterExtension(const std::string& ext_id) {
 void ExtensionPrefValueMap::SetExtensionState(const std::string& ext_id,
                                               bool is_enabled) {
   ExtensionEntryMap::const_iterator i = entries_.find(ext_id);
-  CHECK(i != entries_.end());
+  // This may happen when sync sets the extension state for an
+  // extension that is not installed.
+  if (i == entries_.end())
+    return;
   if (i->second->enabled == is_enabled)
     return;
   std::set<std::string> keys;  // keys set by this extension
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 0e43ded..45e796e 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -749,13 +749,18 @@ bool ExtensionService::IsExternalExtensionUninstalled(
 void ExtensionService::EnableExtension(const std::string& extension_id) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
+  if (IsExtensionEnabled(extension_id))
+    return;
+
+  extension_prefs_->SetExtensionState(extension_id, Extension::ENABLED);
+
   const Extension* extension =
       GetExtensionByIdInternal(extension_id, false, true, false);
+  // This can happen if sync enables an extension that is not
+  // installed yet.
   if (!extension)
     return;
 
-  extension_prefs_->SetExtensionState(extension_id, Extension::ENABLED);
-
   // Move it over to the enabled list.
   extensions_.push_back(make_scoped_refptr(extension));
   ExtensionList::iterator iter = std::find(disabled_extensions_.begin(),
@@ -772,23 +777,35 @@ void ExtensionService::EnableExtension(const std::string& extension_id) {
 void ExtensionService::DisableExtension(const std::string& extension_id) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  const Extension* extension =
-      GetExtensionByIdInternal(extension_id, true, false, false);
   // The extension may have been disabled already.
-  if (!extension)
+  if (!IsExtensionEnabled(extension_id))
     return;
 
-  if (!Extension::UserMayDisable(extension->location()))
+  const Extension* extension = GetInstalledExtension(extension_id);
+  // |extension| can be NULL if sync disables an extension that is not
+  // installed yet.
+  if (extension && !Extension::UserMayDisable(extension->location()))
     return;
 
   extension_prefs_->SetExtensionState(extension_id, Extension::DISABLED);
 
+  extension = GetExtensionByIdInternal(extension_id, true, false, true);
+  if (!extension)
+    return;
+
   // Move it over to the disabled list.
   disabled_extensions_.push_back(make_scoped_refptr(extension));
   ExtensionList::iterator iter = std::find(extensions_.begin(),
                                            extensions_.end(),
                                            extension);
-  extensions_.erase(iter);
+  if (iter != extensions_.end()) {
+    extensions_.erase(iter);
+  } else {
+    iter = std::find(terminated_extensions_.begin(),
+                     terminated_extensions_.end(),
+                     extension);
+    terminated_extensions_.erase(iter);
+  }
 
   NotifyExtensionUnloaded(extension, UnloadedExtensionInfo::DISABLE);
 }
@@ -1352,21 +1369,17 @@ void ExtensionService::ProcessSyncData(
     return;
   }
 
+  // Set user settings.
+  if (extension_sync_data.enabled) {
+    EnableExtension(id);
+  } else {
+    DisableExtension(id);
+  }
   SetIsIncognitoEnabled(id, extension_sync_data.incognito_enabled);
 
-  const Extension* extension =
-      GetExtensionByIdInternal(id, true, true, false);
-  // TODO(akalin): Figure out what to do with terminated extensions.
-
-  // Handle already-installed extensions.
+  const Extension* extension = GetInstalledExtension(id);
   if (extension) {
-    // TODO(akalin): Make it so we can enable/disable an extension
-    // even if it's not installed yet.
-    if (extension_sync_data.enabled) {
-      EnableExtension(id);
-    } else {
-      DisableExtension(id);
-    }
+    // If the extension is already installed, check if it's outdated.
     int result = extension->version()->CompareTo(extension_sync_data.version);
     if (result < 0) {
       // Extension is outdated.
@@ -1378,22 +1391,22 @@ void ExtensionService::ProcessSyncData(
       // TODO(akalin): Move that code here.
     }
     return;
+  } else {
+    // TODO(akalin): Remove need to pass the enabled flag.
+    //
+    // TODO(akalin): Replace silent update with a list of enabled
+    // permissions.
+    if (!pending_extension_manager()->AddFromSync(
+            id,
+            extension_sync_data.update_url,
+            filter,
+            true,  // install_silently
+            extension_sync_data.enabled)) {
+      LOG(WARNING) << "Could not add pending extension for " << id;
+      return;
+    }
+    CheckForUpdatesSoon();
   }
-
-  // Handle not-yet-installed extensions.
-  //
-  // TODO(akalin): Replace silent update with a list of enabled
-  // permissions.
-  if (!pending_extension_manager()->AddFromSync(
-          id,
-          extension_sync_data.update_url,
-          filter,
-          true,  // install_silently
-          extension_sync_data.enabled)) {
-    LOG(WARNING) << "Could not add pending extension for " << id;
-    return;
-  }
-  CheckForUpdatesSoon();
 }
 
 bool ExtensionService::IsIncognitoEnabled(
diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h
index 94f15a9..a18aab5 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -276,9 +276,12 @@ class ExtensionService
   virtual bool IsExternalExtensionUninstalled(
       const std::string& extension_id) const OVERRIDE;
 
-  // Enable or disable an extension. No action if the extension is already
-  // enabled/disabled.
+  // Enables the extension.  If the extension is already enabled, does
+  // nothing.
   virtual void EnableExtension(const std::string& extension_id);
+
+  // Disables the extension.  If the extension is already disabled, or
+  // cannot be disabled, does nothing.
   virtual void DisableExtension(const std::string& extension_id);
 
   // Updates the |extension|'s granted permissions lists to include all
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index 1e67410..55f1f47 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -3598,16 +3598,18 @@ TEST_F(ExtensionServiceTest, ProcessSyncDataNotInstalled) {
   ExtensionSyncData extension_sync_data;
   extension_sync_data.id = good_crx;
   extension_sync_data.update_url = GURL("http://www.google.com");
-  extension_sync_data.enabled = true;
+  extension_sync_data.enabled = false;
   extension_sync_data.incognito_enabled = true;
   {
     scoped_ptr<Version> version(Version::GetVersionFromString("1.2.3.4"));
     extension_sync_data.version = *version;
   }
 
+  EXPECT_TRUE(service_->IsExtensionEnabled(good_crx));
   EXPECT_FALSE(service_->IsIncognitoEnabled(good_crx));
   service_->ProcessSyncData(extension_sync_data, &AllExtensions);
   EXPECT_TRUE(service_->updater()->WillCheckSoon());
+  EXPECT_FALSE(service_->IsExtensionEnabled(good_crx));
   EXPECT_TRUE(service_->IsIncognitoEnabled(good_crx));
 
   PendingExtensionInfo info;