Added checks to handle unsyncable extensions.

BUG=49346,46516
TEST=manual, unit tests

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

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

2 files changed, 62 insertions, 10 deletions

diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index cc168d5..c371aea 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -35,6 +35,7 @@
 #include "chrome/browser/pref_service.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/search_engines/template_url_model.h"
+#include "chrome/browser/sync/glue/extension_util.h"
 #include "chrome/common/child_process_logging.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
@@ -935,12 +936,15 @@ void ExtensionsService::OnExtensionInstalled(Extension* extension,
   PendingExtensionMap::iterator it =
       pending_extensions_.find(extension->id());
   if (it != pending_extensions_.end()) {
+    PendingExtensionInfo pending_extension_info = it->second;
+    pending_extensions_.erase(it);
+    it = pending_extensions_.end();
     // Set initial state from pending extension data.
-    if (it->second.is_theme != extension->is_theme()) {
+    if (pending_extension_info.is_theme != extension->is_theme()) {
       LOG(WARNING)
           << "Not installing pending extension " << extension->id()
           << " with is_theme = " << extension->is_theme()
-          << "; expected is_theme = " << it->second.is_theme;
+          << "; expected is_theme = " << pending_extension_info.is_theme;
       // Delete the extension directory since we're not going to
       // load it.
       ChromeThread::PostTask(
@@ -948,20 +952,42 @@ void ExtensionsService::OnExtensionInstalled(Extension* extension,
           NewRunnableFunction(&DeleteFileHelper, extension->path(), true));
       return;
     }
-    if (it->second.is_theme) {
-      DCHECK(it->second.enable_on_install);
+    if (!extension->is_theme() &&
+        !browser_sync::IsExtensionSyncable(*extension)) {
+      // We're an extension installed via sync that is unsyncable,
+      // i.e. we may have been syncable previously.  We block these
+      // installs.  We'll have to update the clause above if we decide
+      // to sync other extension-like things, like apps or user
+      // scripts.
+      //
+      // Note that this creates a small window where a user who tries
+      // to download/install an extension that is simultaneously
+      // installed via sync (and blocked) will find his download
+      // blocked.
+      //
+      // TODO(akalin): Remove this check once we've put in UI to
+      // approve synced extensions.
+      LOG(WARNING)
+          << "Not installing non-syncable extension " << extension->id();
+      // Delete the extension directory since we're not going to
+      // load it.
+      ChromeThread::PostTask(
+          ChromeThread::FILE, FROM_HERE,
+          NewRunnableFunction(&DeleteFileHelper, extension->path(), true));
+      return;
+    }
+    if (pending_extension_info.is_theme) {
+      DCHECK(pending_extension_info.enable_on_install);
       initial_state = Extension::ENABLED;
-      DCHECK(!it->second.enable_incognito_on_install);
+      DCHECK(!pending_extension_info.enable_incognito_on_install);
       initial_enable_incognito = false;
     } else {
       initial_state =
-          it->second.enable_on_install ?
+          pending_extension_info.enable_on_install ?
           Extension::ENABLED : Extension::DISABLED;
       initial_enable_incognito =
-          it->second.enable_incognito_on_install;
+          pending_extension_info.enable_incognito_on_install;
     }
-
-    pending_extensions_.erase(it);
   } else {
     // Make sure we don't enable a disabled extension.
     Extension::State existing_state =
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index 1d1406f..c209f59 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -1281,6 +1281,29 @@ TEST_F(ExtensionsServiceTest, UpdatePendingExtension) {
             service_->IsIncognitoEnabled(extension));
 }
 
+// Test updating a pending theme.
+TEST_F(ExtensionsServiceTest, UpdatePendingTheme) {
+  InitializeEmptyExtensionsService();
+  service_->AddPendingExtension(
+      theme_crx, GURL(), true, false, Extension::ENABLED, false);
+  EXPECT_TRUE(ContainsKey(service_->pending_extensions(), theme_crx));
+
+  FilePath extensions_path;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path));
+  extensions_path = extensions_path.AppendASCII("extensions");
+  FilePath path = extensions_path.AppendASCII("theme.crx");
+  UpdateExtension(theme_crx, path, ENABLED);
+
+  EXPECT_FALSE(ContainsKey(service_->pending_extensions(), theme_crx));
+
+  Extension* extension = service_->GetExtensionById(theme_crx, true);
+  ASSERT_TRUE(extension);
+
+  EXPECT_EQ(Extension::ENABLED,
+            service_->extension_prefs()->GetExtensionState(extension->id()));
+  EXPECT_FALSE(service_->IsIncognitoEnabled(extension));
+}
+
 // TODO(akalin): Test updating a pending extension non-silently once
 // we can mock out ExtensionInstallUI and inject our version into
 // UpdateExtension().
@@ -1304,9 +1327,12 @@ TEST_F(ExtensionsServiceTest, UpdatePendingExtensionWrongIsTheme) {
   // TODO(akalin): Figure out how to check that the extensions
   // directory is cleaned up properly in OnExtensionInstalled().
 
-  EXPECT_TRUE(ContainsKey(service_->pending_extensions(), kGoodId));
+  EXPECT_FALSE(ContainsKey(service_->pending_extensions(), kGoodId));
 }
 
+// TODO(akalin): Figure out how to test that installs of pending
+// unsyncable extensions are blocked.
+
 // Test updating a pending extension for one that is not pending.
 TEST_F(ExtensionsServiceTest, UpdatePendingExtensionNotPending) {
   InitializeEmptyExtensionsService();