Allow update URLs in external_extensions.json.

Doc updates will follow in another CL.

BUG=48117
TEST=ExtensionsServiceTest.(UpdatePendingExternalCrx,UpdatePendingCrxThemeMismatch,ExternalPrefProvider), ExtensionManagmentTest.ExternalUrlUpdate, manual testing.

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

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

13 files changed, 428 insertions, 114 deletions

diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
index b24a02d..b8adc1f 100644
--- a/chrome/browser/extensions/crx_installer.cc
+++ b/chrome/browser/extensions/crx_installer.cc
@@ -144,7 +144,7 @@ void CrxInstaller::OnUnpackSuccess(const FilePath& temp_dir,
     return;
   }
 
-  // The unpack dir we don't have to delete explicity since it is a child of
+  // We don't have to delete the unpack dir explicity since it is a child of
   // the temp dir.
   unpacked_extension_root_ = extension_dir;
 
diff --git a/chrome/browser/extensions/extension_management_browsertest.cc b/chrome/browser/extensions/extension_management_browsertest.cc
index ff94bfa..f91bd07 100644
--- a/chrome/browser/extensions/extension_management_browsertest.cc
+++ b/chrome/browser/extensions/extension_management_browsertest.cc
@@ -253,3 +253,65 @@ IN_PROC_BROWSER_TEST_F(ExtensionManagementTest, AutoUpdate) {
             extensions->at(size_before)->id());
   ASSERT_EQ("2.0", extensions->at(size_before)->VersionString());
 }
+
+IN_PROC_BROWSER_TEST_F(ExtensionManagementTest, ExternalUrlUpdate) {
+  ExtensionsService* service = browser()->profile()->GetExtensionsService();
+  const char* kExtensionId = "ogjcoiohnmldgjemafoockdghcjciccf";
+  // We don't want autoupdate blacklist checks.
+  service->updater()->set_blacklist_checks_enabled(false);
+
+  FilePath basedir = test_data_dir_.AppendASCII("autoupdate");
+
+  // Note: This interceptor gets requests on the IO thread.
+  scoped_refptr<AutoUpdateInterceptor> interceptor(new AutoUpdateInterceptor());
+  URLFetcher::enable_interception_for_tests(true);
+
+  interceptor->SetResponseOnIOThread("http://localhost/autoupdate/manifest",
+                                     basedir.AppendASCII("manifest_v2.xml"));
+  interceptor->SetResponseOnIOThread("http://localhost/autoupdate/v2.crx",
+                                     basedir.AppendASCII("v2.crx"));
+
+  const size_t size_before = service->extensions()->size();
+  ASSERT_TRUE(service->disabled_extensions()->empty());
+
+  // The code that reads external_extensions.json uses this method to inform
+  // the ExtensionsService of an extension to download.  Using the real code
+  // is race-prone, because instantating the ExtensionService starts a read
+  // of external_extensions.json before this test function starts.
+  service->AddPendingExtensionFromExternalUpdateUrl(
+      kExtensionId, GURL("http://localhost/autoupdate/manifest"));
+
+  // Run autoupdate and make sure version 2 of the extension was installed.
+  service->updater()->CheckNow();
+  ASSERT_TRUE(WaitForExtensionInstall());
+  const ExtensionList* extensions = service->extensions();
+  ASSERT_EQ(size_before + 1, extensions->size());
+  ASSERT_EQ(kExtensionId, extensions->at(size_before)->id());
+  ASSERT_EQ("2.0", extensions->at(size_before)->VersionString());
+
+  // Uninstalling the extension should set a pref that keeps the extension from
+  // being installed again the next time external_extensions.json is read.
+
+  UninstallExtension(kExtensionId);
+
+  std::set<std::string> killed_ids;
+  service->extension_prefs()->GetKilledExtensionIds(&killed_ids);
+  EXPECT_TRUE(killed_ids.end() != killed_ids.find(kExtensionId))
+      << "Uninstalling should set kill bit on externaly installed extension.";
+
+  // Installing from non-external source.
+  ASSERT_TRUE(InstallExtension(basedir.AppendASCII("v2.crx"), 1));
+
+  killed_ids.clear();
+  service->extension_prefs()->GetKilledExtensionIds(&killed_ids);
+  EXPECT_TRUE(killed_ids.end() == killed_ids.find(kExtensionId))
+      << "Reinstalling should clear the kill bit.";
+
+  // Uninstalling from a non-external source should not set the kill bit.
+  UninstallExtension(kExtensionId);
+
+  killed_ids.clear();
+  service->extension_prefs()->GetKilledExtensionIds(&killed_ids);
+  EXPECT_TRUE(killed_ids.end() == killed_ids.find(kExtensionId))
+      << "Uninstalling non-external extension should not set kill bit.";
+}
diff --git a/chrome/browser/extensions/extension_updater.cc b/chrome/browser/extensions/extension_updater.cc
index 8201edd..88696f0 100644
--- a/chrome/browser/extensions/extension_updater.cc
+++ b/chrome/browser/extensions/extension_updater.cc
@@ -161,7 +161,8 @@ void ManifestFetchesBuilder::AddExtension(const Extension& extension) {
   AddExtensionData(extension.location(),
                    extension.id(),
                    *extension.version(),
-                   extension.is_theme(),
+                   (extension.is_theme() ? PendingExtensionInfo::THEME
+                                         : PendingExtensionInfo::EXTENSION),
                    extension.update_url());
 }
 
@@ -173,8 +174,13 @@ void ManifestFetchesBuilder::AddPendingExtension(
   // non-zero versions).
   scoped_ptr<Version> version(
       Version::GetVersionFromString("0.0.0.0"));
-  AddExtensionData(Extension::INTERNAL, id, *version,
-                   info.is_theme, info.update_url);
+
+  Extension::Location location =
+      (info.is_from_sync ? Extension::INTERNAL
+                         : Extension::EXTERNAL_PREF_DOWNLOAD);
+
+  AddExtensionData(location, id, *version,
+                   info.expected_crx_type, info.update_url);
 }
 
 void ManifestFetchesBuilder::ReportStats() const {
@@ -208,8 +214,9 @@ void ManifestFetchesBuilder::AddExtensionData(
     Extension::Location location,
     const std::string& id,
     const Version& version,
-    bool is_theme,
+    PendingExtensionInfo::ExpectedCrxType crx_type,
     GURL update_url) {
+
   // Only internal and external extensions can be autoupdated.
   if (location != Extension::INTERNAL &&
       !Extension::IsExternalLocation(location)) {
@@ -241,7 +248,7 @@ void ManifestFetchesBuilder::AddExtensionData(
     url_stats_.other_url_count++;
   }
 
-  if (is_theme) {
+  if (crx_type == PendingExtensionInfo::THEME) {
     url_stats_.theme_count++;
   }
 
diff --git a/chrome/browser/extensions/extension_updater.h b/chrome/browser/extensions/extension_updater.h
index 9b650cd..ad8d243 100644
--- a/chrome/browser/extensions/extension_updater.h
+++ b/chrome/browser/extensions/extension_updater.h
@@ -111,7 +111,7 @@ class ManifestFetchesBuilder {
   void AddExtensionData(Extension::Location location,
                         const std::string& id,
                         const Version& version,
-                        bool is_theme,
+                        PendingExtensionInfo::ExpectedCrxType crx_type,
                         GURL update_url);
 
   ExtensionUpdateService* service_;
diff --git a/chrome/browser/extensions/extension_updater_unittest.cc b/chrome/browser/extensions/extension_updater_unittest.cc
index c9d472e7..2d2c6d03 100644
--- a/chrome/browser/extensions/extension_updater_unittest.cc
+++ b/chrome/browser/extensions/extension_updater_unittest.cc
@@ -112,15 +112,19 @@ std::string GenerateId(std::string input) {
 // name and version are all based on their index.
 void CreateTestPendingExtensions(int count, const GURL& update_url,
                                  PendingExtensionMap* pending_extensions) {
+  PendingExtensionInfo::ExpectedCrxType crx_type;
   for (int i = 1; i <= count; i++) {
-    bool is_theme = (i % 2) == 0;
+    crx_type = ((i % 2) ? PendingExtensionInfo::EXTENSION
+                        : PendingExtensionInfo::THEME);
+    const bool kIsFromSync = true;
     const bool kInstallSilently = true;
     const Extension::State kInitialState = Extension::ENABLED;
     const bool kInitialIncognitoEnabled = false;
     std::string id = GenerateId(StringPrintf("extension%i", i));
     (*pending_extensions)[id] =
-        PendingExtensionInfo(update_url, is_theme, kInstallSilently,
-                             kInitialState, kInitialIncognitoEnabled);
+        PendingExtensionInfo(update_url, crx_type, kIsFromSync,
+                             kInstallSilently, kInitialState,
+                             kInitialIncognitoEnabled);
   }
 }
 
@@ -548,14 +552,17 @@ class ExtensionUpdaterTest : public testing::Test {
     updater->FetchUpdatedExtension(id, test_url, hash, version->GetString());
 
     if (pending) {
-      const bool kIsTheme = false;
+      const PendingExtensionInfo::ExpectedCrxType kExpectedCrxType =
+          PendingExtensionInfo::EXTENSION;
+      const bool kIsFromSync = true;
       const bool kInstallSilently = true;
       const Extension::State kInitialState = Extension::ENABLED;
       const bool kInitialIncognitoEnabled = false;
       PendingExtensionMap pending_extensions;
       pending_extensions[id] =
-          PendingExtensionInfo(test_url, kIsTheme, kInstallSilently,
-                               kInitialState, kInitialIncognitoEnabled);
+          PendingExtensionInfo(test_url, kExpectedCrxType, kIsFromSync,
+                               kInstallSilently, kInitialState,
+                               kInitialIncognitoEnabled);
       service.set_pending_extensions(pending_extensions);
     }
 
@@ -883,18 +890,21 @@ TEST(ExtensionUpdaterTest, TestManifestFetchesBuilderAddExtension) {
   // Extensions with invalid update URLs should be rejected.
   builder.AddPendingExtension(
       GenerateId("foo"), PendingExtensionInfo(GURL("http:google.com:foo"),
+                                              PendingExtensionInfo::EXTENSION,
                                               false, false, true, false));
   EXPECT_TRUE(builder.GetFetches().empty());
 
   // Extensions with empty IDs should be rejected.
   builder.AddPendingExtension(
-      "", PendingExtensionInfo(GURL(), false, false, true, false));
+      "", PendingExtensionInfo(GURL(), PendingExtensionInfo::EXTENSION,
+                               false, false, true, false));
   EXPECT_TRUE(builder.GetFetches().empty());
 
   // Extensions with empty update URLs should have a default one
   // filled in.
   builder.AddPendingExtension(
       GenerateId("foo"), PendingExtensionInfo(GURL(),
+                                              PendingExtensionInfo::EXTENSION,
                                               false, false, true, false));
   std::vector<ManifestFetchData*> fetches = builder.GetFetches();
   ASSERT_EQ(1u, fetches.size());
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 61de28b..e9d635c 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -97,20 +97,24 @@ void GetExplicitOriginsInExtent(Extension* extension,
 
 }  // namespace
 
-PendingExtensionInfo::PendingExtensionInfo(const GURL& update_url,
-                                           bool is_theme,
-                                           bool install_silently,
-                                           bool enable_on_install,
-                                           bool enable_incognito_on_install)
+PendingExtensionInfo::PendingExtensionInfo(
+    const GURL& update_url,
+    PendingExtensionInfo::ExpectedCrxType expected_crx_type,
+    bool is_from_sync,
+    bool install_silently,
+    bool enable_on_install,
+    bool enable_incognito_on_install)
     : update_url(update_url),
-      is_theme(is_theme),
+      expected_crx_type(expected_crx_type),
+      is_from_sync(is_from_sync),
       install_silently(install_silently),
       enable_on_install(enable_on_install),
       enable_incognito_on_install(enable_incognito_on_install) {}
 
 PendingExtensionInfo::PendingExtensionInfo()
     : update_url(),
-      is_theme(false),
+      expected_crx_type(PendingExtensionInfo::UNKNOWN),
+      is_from_sync(true),
       install_silently(false),
       enable_on_install(false),
       enable_incognito_on_install(false) {}
@@ -274,7 +278,9 @@ void ExtensionsService::UpdateExtension(const std::string& id,
                                         const FilePath& extension_path,
                                         const GURL& download_url) {
   PendingExtensionMap::const_iterator it = pending_extensions_.find(id);
-  if ((it == pending_extensions_.end()) &&
+  bool is_pending_extension = (it != pending_extensions_.end());
+
+  if (!is_pending_extension &&
       !GetExtensionByIdInternal(id, true, true)) {
     LOG(WARNING) << "Will not update extension " << id
                  << " because it is not installed or pending";
@@ -289,7 +295,7 @@ void ExtensionsService::UpdateExtension(const std::string& id,
   // We want a silent install only for non-pending extensions and
   // pending extensions that have install_silently set.
   ExtensionInstallUI* client =
-      ((it == pending_extensions_.end()) || it->second.install_silently) ?
+      (!is_pending_extension || it->second.install_silently) ?
       NULL : new ExtensionInstallUI(profile_);
 
   scoped_refptr<CrxInstaller> installer(
@@ -297,32 +303,59 @@ void ExtensionsService::UpdateExtension(const std::string& id,
                        this,  // frontend
                        client));
   installer->set_expected_id(id);
+  if (is_pending_extension && !it->second.is_from_sync)
+    installer->set_install_source(Extension::EXTERNAL_PREF_DOWNLOAD);
   installer->set_delete_source(true);
   installer->set_original_url(download_url);
   installer->InstallCrx(extension_path);
 }
 
-void ExtensionsService::AddPendingExtension(
+void ExtensionsService::AddPendingExtensionFromSync(
     const std::string& id, const GURL& update_url,
-    bool is_theme, bool install_silently,
-    bool enable_on_install, bool enable_incognito_on_install) {
+    PendingExtensionInfo::ExpectedCrxType expected_crx_type,
+    bool install_silently, bool enable_on_install,
+    bool enable_incognito_on_install) {
   if (GetExtensionByIdInternal(id, true, true)) {
     LOG(DFATAL) << "Trying to add pending extension " << id
                 << " which already exists";
     return;
   }
   AddPendingExtensionInternal(
-      id, update_url, is_theme, install_silently,
+      id, update_url, expected_crx_type, true, install_silently,
       enable_on_install, enable_incognito_on_install);
 }
 
+void ExtensionsService::AddPendingExtensionFromExternalUpdateUrl(
+    const std::string& id, const GURL& update_url) {
+  // Add the extension to this list of extensions to update.
+  // We do not know if the id refers to a theme, so make is_theme unknown.
+  const PendingExtensionInfo::ExpectedCrxType kExpectedCrxType =
+      PendingExtensionInfo::UNKNOWN;
+  const bool kIsFromSync = false;
+  const bool kInstallSilently = true;
+  const bool kEnableOnInstall = true;
+  const bool kEnableIncognitoOnInstall = false;
+
+  if (GetExtensionByIdInternal(id, true, true)) {
+    LOG(DFATAL) << "Trying to add extension " << id
+                << " by external update, but it is already installed.";
+    return;
+  }
+
+  AddPendingExtensionInternal(id, update_url, kExpectedCrxType, kIsFromSync,
+                              kInstallSilently, kEnableOnInstall,
+                              kEnableIncognitoOnInstall);
+}
+
 void ExtensionsService::AddPendingExtensionInternal(
     const std::string& id, const GURL& update_url,
-    bool is_theme, bool install_silently,
+    PendingExtensionInfo::ExpectedCrxType expected_crx_type,
+    bool is_from_sync, bool install_silently,
     bool enable_on_install, bool enable_incognito_on_install) {
   pending_extensions_[id] =
-      PendingExtensionInfo(update_url, is_theme, install_silently,
-                           enable_on_install, enable_incognito_on_install);
+      PendingExtensionInfo(update_url, expected_crx_type, is_from_sync,
+                           install_silently, enable_on_install,
+                           enable_incognito_on_install);
 }
 
 void ExtensionsService::ReloadExtension(const std::string& extension_id) {
@@ -1024,14 +1057,22 @@ void ExtensionsService::OnExtensionInstalled(Extension* extension,
       pending_extensions_.find(extension->id());
   if (it != pending_extensions_.end()) {
     PendingExtensionInfo pending_extension_info = it->second;
+    PendingExtensionInfo::ExpectedCrxType expected_crx_type =
+        pending_extension_info.expected_crx_type;
+    bool is_from_sync = pending_extension_info.is_from_sync;
     pending_extensions_.erase(it);
     it = pending_extensions_.end();
+
     // Set initial state from pending extension data.
-    if (pending_extension_info.is_theme != extension->is_theme()) {
+    PendingExtensionInfo::ExpectedCrxType actual_crx_type =
+        (extension->is_theme() ? PendingExtensionInfo::THEME
+                               : PendingExtensionInfo::EXTENSION);
+
+    if (expected_crx_type != PendingExtensionInfo::UNKNOWN &&
+        expected_crx_type != actual_crx_type) {
       LOG(WARNING)
           << "Not installing pending extension " << extension->id()
-          << " with is_theme = " << extension->is_theme()
-          << "; expected is_theme = " << pending_extension_info.is_theme;
+          << " with is_theme = " << extension->is_theme();
       // Delete the extension directory since we're not going to
       // load it.
       ChromeThread::PostTask(
@@ -1039,13 +1080,29 @@ void ExtensionsService::OnExtensionInstalled(Extension* extension,
           NewRunnableFunction(&DeleteFileHelper, extension->path(), true));
       return;
     }
+
+    // If |extension| is not syncable, and was installed via sync, disallow
+    // the instanation.
+    //
+    // Themes are always allowed.  Because they contain no active code, they
+    // are less of a risk than extensions.
+    //
+    // If |is_from_sync| is false, then the install was not initiated by sync,
+    // and this check should pass.  Extensions that were installed from an
+    // update URL in external_extensions.json are an example.  They are not
+    // syncable, because the user did not make an explicit choice to install
+    // them.  However, they were installed through the update mechanism, so
+    // control must pass into this function.
+    //
+    // TODO(akalin): When we do apps sync, we have to work with its
+    // traits, too.
     const browser_sync::ExtensionSyncTraits extension_sync_traits =
         browser_sync::GetExtensionSyncTraits();
     const browser_sync::ExtensionSyncTraits app_sync_traits =
         browser_sync::GetAppSyncTraits();
     // If an extension is a theme, we bypass the valid/syncable check
     // as themes are harmless.
-    if (!extension->is_theme() &&
+    if (!extension->is_theme() && is_from_sync &&
         !browser_sync::IsExtensionValidAndSyncable(
             *extension, extension_sync_traits.allowed_extension_types) &&
         !browser_sync::IsExtensionValidAndSyncable(
@@ -1073,7 +1130,7 @@ void ExtensionsService::OnExtensionInstalled(Extension* extension,
           NewRunnableFunction(&DeleteFileHelper, extension->path(), true));
       return;
     }
-    if (pending_extension_info.is_theme) {
+    if (extension->is_theme()) {
       DCHECK(pending_extension_info.enable_on_install);
       initial_state = Extension::ENABLED;
       DCHECK(!pending_extension_info.enable_incognito_on_install);
@@ -1187,10 +1244,11 @@ void ExtensionsService::SetProviderForTesting(
           location, test_provider));
 }
 
-void ExtensionsService::OnExternalExtensionFound(const std::string& id,
-                                                 const std::string& version,
-                                                 const FilePath& path,
-                                                 Extension::Location location) {
+void ExtensionsService::OnExternalExtensionFileFound(
+         const std::string& id,
+         const std::string& version,
+         const FilePath& path,
+         Extension::Location location) {
   // Before even bothering to unpack, check and see if we already have this
   // version. This is important because these extensions are going to get
   // installed on every startup.
@@ -1303,7 +1361,8 @@ ExtensionsServiceBackend::ExtensionsServiceBackend(
     const FilePath& install_directory)
         : frontend_(NULL),
           install_directory_(install_directory),
-          alert_on_error_(false) {
+          alert_on_error_(false),
+          external_extension_added_(false) {
   // TODO(aa): This ends up doing blocking IO on the UI thread because it reads
   // pref data in the ctor and that is called on the UI thread. Would be better
   // to re-read data each time we list external extensions, anyway.
@@ -1396,14 +1455,23 @@ void ExtensionsServiceBackend::CheckForExternalUpdates(
   // they could install an extension manually themselves anyway.
   alert_on_error_ = false;
   frontend_ = frontend;
+  external_extension_added_ = false;
 
   // Ask each external extension provider to give us a call back for each
-  // extension they know about. See OnExternalExtensionFound.
+  // extension they know about. See OnExternalExtension(File|UpdateUrl)Found.
+
   for (ProviderMap::const_iterator i = external_extension_providers_.begin();
        i != external_extension_providers_.end(); ++i) {
     ExternalExtensionProvider* provider = i->second.get();
     provider->VisitRegisteredExtension(this, ids_to_ignore);
   }
+
+  if (external_extension_added_ && frontend->updater()) {
+    ChromeThread::PostTask(
+        ChromeThread::UI, FROM_HERE,
+            NewRunnableMethod(
+                frontend->updater(), &ExtensionUpdater::CheckNow));
+  }
 }
 
 void ExtensionsServiceBackend::CheckExternalUninstall(
@@ -1441,16 +1509,27 @@ void ExtensionsServiceBackend::SetProviderForTesting(
       linked_ptr<ExternalExtensionProvider>(test_provider);
 }
 
-void ExtensionsServiceBackend::OnExternalExtensionFound(
+void ExtensionsServiceBackend::OnExternalExtensionFileFound(
     const std::string& id, const Version* version, const FilePath& path,
     Extension::Location location) {
   ChromeThread::PostTask(
       ChromeThread::UI, FROM_HERE,
       NewRunnableMethod(
-          frontend_, &ExtensionsService::OnExternalExtensionFound, id,
+          frontend_, &ExtensionsService::OnExternalExtensionFileFound, id,
           version->GetString(), path, location));
 }
 
+void ExtensionsServiceBackend::OnExternalExtensionUpdateUrlFound(
+         const std::string& id, const GURL& update_url) {
+  if (frontend_->GetExtensionById(id, true)) {
+    // Already installed.  Do not change the update URL that the extension set.
+    return;
+  }
+
+  frontend_->AddPendingExtensionFromExternalUpdateUrl(id, update_url);
+  external_extension_added_ |= true;
+}
+
 void ExtensionsServiceBackend::ReloadExtensionManifests(
     ExtensionPrefs::ExtensionsInfo* extensions_to_reload,
     base::TimeTicks start_time,
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index 25182ee..3191a06 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -45,8 +45,18 @@ class Version;
 // update URL of a pending extension may be blank, in which case a
 // default one is assumed.
 struct PendingExtensionInfo {
+  // TODO(skerner): Consider merging ExpectedCrxType with
+  // browser_sync::ExtensionType.
+  enum ExpectedCrxType {
+    UNKNOWN,  // Sometimes we don't know the type of a pending item.  An
+              // update URL from external_extensions.json is one such case.
+    THEME,
+    EXTENSION
+  };
+
   PendingExtensionInfo(const GURL& update_url,
-                       bool is_theme,
+                       ExpectedCrxType expected_crx_type,
+                       bool is_from_sync,
                        bool install_silently,
                        bool enable_on_install,
                        bool enable_incognito_on_install);
@@ -54,7 +64,8 @@ struct PendingExtensionInfo {
   PendingExtensionInfo();
 
   GURL update_url;
-  bool is_theme;
+  ExpectedCrxType expected_crx_type;
+  bool is_from_sync;  // This update check was initiated from sync.
   bool install_silently;
   bool enable_on_install;
   bool enable_incognito_on_install;
@@ -205,10 +216,16 @@ class ExtensionsService
   //
   // TODO(akalin): Replace |install_silently| with a list of
   // pre-enabled permissions.
-  void AddPendingExtension(
+  void AddPendingExtensionFromSync(
       const std::string& id, const GURL& update_url,
-      bool is_theme, bool install_silently,
-      bool enable_on_install, bool enable_incognito_on_install);
+      const PendingExtensionInfo::ExpectedCrxType expected_crx_type,
+      bool install_silently, bool enable_on_install,
+      bool enable_incognito_on_install);
+
+  // Given an extension id and an update URL, schedule the extension
+  // to be fetched, installed, and activated.
+  void AddPendingExtensionFromExternalUpdateUrl(const std::string& id,
+                                                const GURL& update_url);
 
   // Reloads the specified extension.
   void ReloadExtension(const std::string& extension_id);
@@ -293,10 +310,10 @@ class ExtensionsService
                             bool allow_privilege_increase);
 
   // Called by the backend when an external extension is found.
-  void OnExternalExtensionFound(const std::string& id,
-                                const std::string& version,
-                                const FilePath& path,
-                                Extension::Location location);
+  void OnExternalExtensionFileFound(const std::string& id,
+                                    const std::string& version,
+                                    const FilePath& path,
+                                    Extension::Location location);
 
   // Go through each extensions in pref, unload blacklisted extensions
   // and update the blacklist state in pref.
@@ -366,7 +383,8 @@ class ExtensionsService
   // id is not already installed.
   void AddPendingExtensionInternal(
       const std::string& id, const GURL& update_url,
-      bool is_theme, bool install_silently,
+      PendingExtensionInfo::ExpectedCrxType crx_type,
+      bool is_from_sync, bool install_silently,
       bool enable_on_install, bool enable_incognito_on_install);
 
   // Handles sending notification that |extension| was loaded.
@@ -507,10 +525,13 @@ class ExtensionsServiceBackend
                              ExternalExtensionProvider* test_provider);
 
   // ExternalExtensionProvider::Visitor implementation.
-  virtual void OnExternalExtensionFound(const std::string& id,
-                                        const Version* version,
-                                        const FilePath& path,
-                                        Extension::Location location);
+  virtual void OnExternalExtensionFileFound(const std::string& id,
+                                            const Version* version,
+                                            const FilePath& path,
+                                            Extension::Location location);
+
+  virtual void OnExternalExtensionUpdateUrlFound(const std::string& id,
+                                                 const GURL& update_url);
 
   // Reloads the given extensions from their manifests on disk (instead of what
   // we have cached in the prefs).
@@ -566,6 +587,11 @@ class ExtensionsServiceBackend
                    linked_ptr<ExternalExtensionProvider> > ProviderMap;
   ProviderMap external_extension_providers_;
 
+  // Set to true by OnExternalExtensionUpdateUrlFound() when an external
+  // extension URL is found.  Used in CheckForExternalUpdates() to see
+  // if an update check is needed to install pending extensions.
+  bool external_extension_added_;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionsServiceBackend);
 };
 
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index b8c176b..e1b77c4 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -125,7 +125,7 @@ class MockExtensionProvider : public ExternalExtensionProvider {
       scoped_ptr<Version> version;
       version.reset(Version::GetVersionFromString(i->second.first));
 
-      visitor->OnExternalExtensionFound(
+      visitor->OnExternalExtensionFileFound(
           i->first, version.get(), i->second.second, location_);
     }
   }
@@ -183,10 +183,10 @@ class MockProviderVisitor : public ExternalExtensionProvider::Visitor {
     return ids_found_;
   }
 
-  virtual void OnExternalExtensionFound(const std::string& id,
-                                        const Version* version,
-                                        const FilePath& path,
-                                        Extension::Location unused) {
+  virtual void OnExternalExtensionFileFound(const std::string& id,
+                                            const Version* version,
+                                            const FilePath& path,
+                                            Extension::Location unused) {
     ++ids_found_;
     DictionaryValue* pref;
     // This tests is to make sure that the provider only notifies us of the
@@ -209,6 +209,22 @@ class MockProviderVisitor : public ExternalExtensionProvider::Visitor {
     }
   }
 
+  virtual void OnExternalExtensionUpdateUrlFound(const std::string& id,
+                                                 const GURL& update_url) {
+    ++ids_found_;
+    DictionaryValue* pref;
+    // This tests is to make sure that the provider only notifies us of the
+    // values we gave it. So if the id we doesn't exist in our internal
+    // dictionary then something is wrong.
+    EXPECT_TRUE(prefs_->GetDictionary(id, &pref))
+       << L"Got back ID (" << id.c_str() << ") we weren't expecting";
+
+    if (pref) {
+      // Remove it so we won't count it again.
+      prefs_->Remove(id, NULL);
+    }
+  }
+
  private:
   int ids_found_;
 
@@ -895,7 +911,7 @@ TEST_F(ExtensionsServiceTest, InstallExtension) {
   InstallExtension(path, false);
   ValidatePrefKeyCount(pref_count);
 
-  // Extensions cannot have folders or files that have underscores except ofr in
+  // Extensions cannot have folders or files that have underscores except in
   // certain whitelisted cases (eg _locales). This is an example of a broader
   // class of validation that we do to the directory structure of the extension.
   // We did not used to handle this correctly for installation.
@@ -1443,26 +1459,31 @@ TEST_F(ExtensionsServiceTest, AddPendingExtension) {
 
   const std::string kFakeId("fake-id");
   const GURL kFakeUpdateURL("http:://fake.update/url");
-  const bool kFakeIsTheme(false);
+  const PendingExtensionInfo::ExpectedCrxType kFakeExpectedCrxType =
+      PendingExtensionInfo::EXTENSION;
   const bool kFakeInstallSilently(true);
   const Extension::State kFakeInitialState(Extension::ENABLED);
   const bool kFakeInitialIncognitoEnabled(false);
 
-  service_->AddPendingExtension(
-      kFakeId, kFakeUpdateURL, kFakeIsTheme, kFakeInstallSilently,
+  service_->AddPendingExtensionFromSync(
+      kFakeId, kFakeUpdateURL, kFakeExpectedCrxType, kFakeInstallSilently,
       kFakeInitialState, kFakeInitialIncognitoEnabled);
   PendingExtensionMap::const_iterator it =
       service_->pending_extensions().find(kFakeId);
   ASSERT_TRUE(it != service_->pending_extensions().end());
   EXPECT_EQ(kFakeUpdateURL, it->second.update_url);
-  EXPECT_EQ(kFakeIsTheme, it->second.is_theme);
+  EXPECT_EQ(kFakeExpectedCrxType, it->second.expected_crx_type);
   EXPECT_EQ(kFakeInstallSilently, it->second.install_silently);
 }
 
 namespace {
 const char kGoodId[] = "ldnnhddmnhbkjipkidpdiheffobcpfmf";
 const char kGoodUpdateURL[] = "http://good.update/url";
-const bool kGoodIsTheme = false;
+const PendingExtensionInfo::ExpectedCrxType kCrxTypeTheme =
+    PendingExtensionInfo::THEME;
+const PendingExtensionInfo::ExpectedCrxType kCrxTypeExtension =
+    PendingExtensionInfo::EXTENSION;
+const bool kGoodIsFromSync = true;
 const bool kGoodInstallSilently = true;
 const Extension::State kGoodInitialState = Extension::DISABLED;
 const bool kGoodInitialIncognitoEnabled = true;
@@ -1471,8 +1492,8 @@ const bool kGoodInitialIncognitoEnabled = true;
 // Test updating a pending extension.
 TEST_F(ExtensionsServiceTest, UpdatePendingExtension) {
   InitializeEmptyExtensionsService();
-  service_->AddPendingExtension(
-      kGoodId, GURL(kGoodUpdateURL), kGoodIsTheme,
+  service_->AddPendingExtensionFromSync(
+      kGoodId, GURL(kGoodUpdateURL), kCrxTypeExtension,
       kGoodInstallSilently, kGoodInitialState,
       kGoodInitialIncognitoEnabled);
   EXPECT_TRUE(ContainsKey(service_->pending_extensions(), kGoodId));
@@ -1499,8 +1520,9 @@ TEST_F(ExtensionsServiceTest, UpdatePendingExtension) {
 // Test updating a pending theme.
 TEST_F(ExtensionsServiceTest, UpdatePendingTheme) {
   InitializeEmptyExtensionsService();
-  service_->AddPendingExtension(
-      theme_crx, GURL(), true, false, Extension::ENABLED, false);
+  service_->AddPendingExtensionFromSync(
+      theme_crx, GURL(), PendingExtensionInfo::THEME,
+      false, Extension::ENABLED, false);
   EXPECT_TRUE(ContainsKey(service_->pending_extensions(), theme_crx));
 
   FilePath extensions_path;
@@ -1519,6 +1541,54 @@ TEST_F(ExtensionsServiceTest, UpdatePendingTheme) {
   EXPECT_FALSE(service_->IsIncognitoEnabled(extension));
 }
 
+// Test updating a pending CRX as if the source is an external extension
+// with an update URL.  In this case we don't know if the CRX is a theme
+// or not.
+TEST_F(ExtensionsServiceTest, UpdatePendingExternalCrx) {
+  InitializeEmptyExtensionsService();
+  service_->AddPendingExtensionFromExternalUpdateUrl(theme_crx, GURL());
+
+  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));
+}
+
+// Updating a theme should fail if the updater is explicitly told that
+// the CRX is not a theme.
+TEST_F(ExtensionsServiceTest, UpdatePendingCrxThemeMismatch) {
+  InitializeEmptyExtensionsService();
+  service_->AddPendingExtensionFromSync(
+      theme_crx, GURL(),
+      PendingExtensionInfo::EXTENSION,
+      true, 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, FAILED_SILENTLY);
+
+  EXPECT_FALSE(ContainsKey(service_->pending_extensions(), theme_crx));
+
+  Extension* extension = service_->GetExtensionById(theme_crx, true);
+  ASSERT_FALSE(extension);
+}
+
 // TODO(akalin): Test updating a pending extension non-silently once
 // we can mock out ExtensionInstallUI and inject our version into
 // UpdateExtension().
@@ -1527,9 +1597,9 @@ TEST_F(ExtensionsServiceTest, UpdatePendingTheme) {
 TEST_F(ExtensionsServiceTest, UpdatePendingExtensionWrongIsTheme) {
   InitializeEmptyExtensionsService();
   // Add pending extension with a flipped is_theme.
-  service_->AddPendingExtension(
-      kGoodId, GURL(kGoodUpdateURL), !kGoodIsTheme,
-      kGoodInstallSilently, kGoodInitialState,
+  service_->AddPendingExtensionFromSync(
+      kGoodId, GURL(kGoodUpdateURL),
+      kCrxTypeTheme, kGoodInstallSilently, kGoodInitialState,
       kGoodInitialIncognitoEnabled);
   EXPECT_TRUE(ContainsKey(service_->pending_extensions(), kGoodId));
 
@@ -1574,13 +1644,15 @@ TEST_F(ExtensionsServiceTest, UpdatePendingExtensionAlreadyInstalled) {
   ASSERT_EQ(1u, service_->extensions()->size());
   Extension* good = service_->extensions()->at(0);
 
+  EXPECT_FALSE(good->is_theme());
+
   // Use AddPendingExtensionInternal() as AddPendingExtension() would
   // balk.
   service_->AddPendingExtensionInternal(
-      good->id(), good->update_url(), good->is_theme(),
-      kGoodInstallSilently, kGoodInitialState,
+      good->id(), good->update_url(),
+      PendingExtensionInfo::EXTENSION,
+      kGoodIsFromSync, kGoodInstallSilently, kGoodInitialState,
       kGoodInitialIncognitoEnabled);
-
   UpdateExtension(good->id(), path, INSTALLED);
 
   EXPECT_FALSE(ContainsKey(service_->pending_extensions(), kGoodId));
@@ -2172,25 +2244,33 @@ TEST_F(ExtensionsServiceTest, ExternalPrefProvider) {
         "\"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\": {"
           "\"external_crx\": \"RandomExtension2.crx\","
           "\"external_version\": \"2.0\""
+        "},"
+        "\"cccccccccccccccccccccccccccccccc\": {"
+          "\"external_update_url\": \"http:\\\\foo.com/update\""
         "}"
       "}";
 
   MockProviderVisitor visitor;
   std::set<std::string> ignore_list;
-  EXPECT_EQ(2, visitor.Visit(json_data, ignore_list));
+  EXPECT_EQ(3, visitor.Visit(json_data, ignore_list));
   ignore_list.insert("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
-  EXPECT_EQ(1, visitor.Visit(json_data, ignore_list));
+  EXPECT_EQ(2, visitor.Visit(json_data, ignore_list));
   ignore_list.insert("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");
+  EXPECT_EQ(1, visitor.Visit(json_data, ignore_list));
+  ignore_list.insert("cccccccccccccccccccccccccccccccc");
   EXPECT_EQ(0, visitor.Visit(json_data, ignore_list));
 
-  // Use a json that contains three invalid extensions:
+  // Use a json that contains six invalid extensions:
   // - One that is missing the 'external_crx' key.
   // - One that is missing the 'external_version' key.
   // - One that is specifying .. in the path.
+  // - One that specifies both a file and update URL.
+  // - One that specifies no file or update URL.
+  // - One that has an update URL that is not well formed.
   // - Plus one valid extension to make sure the json file is parsed properly.
   json_data =
       "{"
-        "\"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\": {"
+        "\"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\": {"
           "\"external_version\": \"1.0\""
         "},"
         "\"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\": {"
@@ -2200,7 +2280,17 @@ TEST_F(ExtensionsServiceTest, ExternalPrefProvider) {
           "\"external_crx\": \"..\\\\foo\\\\RandomExtension2.crx\","
           "\"external_version\": \"2.0\""
         "},"
-        "\"dddddddddddddddddddddddddddddddddd\": {"
+        "\"dddddddddddddddddddddddddddddddd\": {"
+          "\"external_crx\": \"..\\\\foo\\\\RandomExtension2.crx\","
+          "\"external_version\": \"2.0\","
+          "\"external_update_url\": \"http:\\\\foo.com/update\""
+        "},"
+        "\"eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\": {"
+        "},"
+        "\"ffffffffffffffffffffffffffffffff\": {"
+          "\"external_update_url\": \"This string is not avalid URL\""
+        "},"
+        "\"gggggggggggggggggggggggggggggggg\": {"
           "\"external_crx\": \"RandomValidExtension.crx\","
           "\"external_version\": \"1.0\""
         "}"
diff --git a/chrome/browser/extensions/external_extension_provider.h b/chrome/browser/extensions/external_extension_provider.h
index 7291f5b..eddea62 100644
--- a/chrome/browser/extensions/external_extension_provider.h
+++ b/chrome/browser/extensions/external_extension_provider.h
@@ -24,10 +24,15 @@ class ExternalExtensionProvider {
   // is not transferred to the visitor.
   class Visitor {
    public:
-     virtual void OnExternalExtensionFound(const std::string& id,
-                                           const Version* version,
-                                           const FilePath& path,
-                                           Extension::Location location) = 0;
+     virtual void OnExternalExtensionFileFound(
+         const std::string& id,
+         const Version* version,
+         const FilePath& path,
+         Extension::Location location) = 0;
+
+     virtual void OnExternalExtensionUpdateUrlFound(
+         const std::string& id,
+         const GURL& update_url) = 0;
 
    protected:
      virtual ~Visitor() {}
diff --git a/chrome/browser/extensions/external_pref_extension_provider.cc b/chrome/browser/extensions/external_pref_extension_provider.cc
index d9b2748..9020151 100644
--- a/chrome/browser/extensions/external_pref_extension_provider.cc
+++ b/chrome/browser/extensions/external_pref_extension_provider.cc
@@ -14,8 +14,11 @@
 #include "chrome/common/json_value_serializer.h"
 
 // Constants for keeping track of extension preferences.
+const char kLocation[] = "location";
+const char kState[] = "state";
 const char kExternalCrx[] = "external_crx";
 const char kExternalVersion[] = "external_version";
+const char kExternalUpdateUrl[] = "external_update_url";
 
 ExternalPrefExtensionProvider::ExternalPrefExtensionProvider() {
   FilePath json_file;
@@ -53,33 +56,61 @@ void ExternalPrefExtensionProvider::VisitRegisteredExtension(
 
     FilePath::StringType external_crx;
     std::string external_version;
-    if (!extension->GetString(kExternalCrx, &external_crx) ||
-        !extension->GetString(kExternalVersion, &external_version)) {
+    std::string external_update_url;
+
+    bool has_external_crx = extension->GetString(kExternalCrx, &external_crx);
+    bool has_external_version = extension->GetString(kExternalVersion,
+                                                     &external_version);
+    bool has_external_update_url = extension->GetString(kExternalUpdateUrl,
+                                                        &external_update_url);
+    if (has_external_crx != has_external_version) {
       LOG(WARNING) << "Malformed extension dictionary for extension: "
-                   << extension_id.c_str();
+                   << extension_id.c_str() << ".  " << kExternalCrx
+                   << " and " << kExternalVersion << " must be used together.";
       continue;
     }
 
-    if (external_crx.find(FilePath::kParentDirectory) !=
-        base::StringPiece::npos) {
-      LOG(WARNING) << "Path traversal not allowed in path: "
-                   << external_crx.c_str();
+    if (has_external_crx == has_external_update_url) {
+      LOG(WARNING) << "Malformed extension dictionary for extension: "
+                   << extension_id.c_str() << ".  Exactly one of the "
+                   << "followng keys should be used: " << kExternalCrx
+                   << ", " << kExternalUpdateUrl << ".";
       continue;
     }
 
-    // See if it's an absolute path...
-    FilePath path(external_crx);
-    if (!path.IsAbsolute()) {
-      // Try path as relative path from external extension dir.
-      FilePath base_path;
-      PathService::Get(app::DIR_EXTERNAL_EXTENSIONS, &base_path);
-      path = base_path.Append(external_crx);
+    if (has_external_crx) {
+      if (external_crx.find(FilePath::kParentDirectory) !=
+          base::StringPiece::npos) {
+        LOG(WARNING) << "Path traversal not allowed in path: "
+                     << external_crx.c_str();
+        continue;
+      }
+
+      // See if it's an absolute path...
+      FilePath path(external_crx);
+      if (!path.IsAbsolute()) {
+        // Try path as relative path from external extension dir.
+        FilePath base_path;
+        PathService::Get(app::DIR_EXTERNAL_EXTENSIONS, &base_path);
+        path = base_path.Append(external_crx);
+      }
+
+      scoped_ptr<Version> version;
+      version.reset(Version::GetVersionFromString(external_version));
+      visitor->OnExternalExtensionFileFound(extension_id, version.get(), path,
+                                            Extension::EXTERNAL_PREF);
+      continue;
     }
 
-    scoped_ptr<Version> version;
-    version.reset(Version::GetVersionFromString(external_version));
-    visitor->OnExternalExtensionFound(extension_id, version.get(), path,
-                                      Extension::EXTERNAL_PREF);
+    DCHECK(has_external_update_url);  // Checking of keys above ensures this.
+    GURL update_url(external_update_url);
+    if (!update_url.is_valid()) {
+      LOG(WARNING) << "Malformed extension dictionary for extension: "
+                   << extension_id.c_str() << ".  " << kExternalUpdateUrl
+                   << " must be a valid URL: " << external_update_url;
+      continue;
+    }
+    visitor->OnExternalExtensionUpdateUrlFound(extension_id, update_url);
   }
 }
 
diff --git a/chrome/browser/extensions/external_registry_extension_provider_win.cc b/chrome/browser/extensions/external_registry_extension_provider_win.cc
index 9189c3a..d34735f 100644
--- a/chrome/browser/extensions/external_registry_extension_provider_win.cc
+++ b/chrome/browser/extensions/external_registry_extension_provider_win.cc
@@ -52,8 +52,8 @@ void ExternalRegistryExtensionProvider::VisitRegisteredExtension(
           scoped_ptr<Version> version;
           version.reset(Version::GetVersionFromString(extension_version));
           FilePath path = FilePath::FromWStringHack(extension_path);
-          visitor->OnExternalExtensionFound(id, version.get(), path,
-                                            Extension::EXTERNAL_REGISTRY);
+          visitor->OnExternalExtensionFileFound(id, version.get(), path,
+                                                Extension::EXTERNAL_REGISTRY);
         } else {
           // TODO(erikkay): find a way to get this into about:extensions
           LOG(WARNING) << "Missing value " << kRegistryExtensionVersion <<
diff --git a/chrome/browser/sync/glue/extension_sync.cc b/chrome/browser/sync/glue/extension_sync.cc
index 3b31ab9..b85f677 100644
--- a/chrome/browser/sync/glue/extension_sync.cc
+++ b/chrome/browser/sync/glue/extension_sync.cc
@@ -292,9 +292,12 @@ void TryUpdateClient(
     GURL update_url(specifics.update_url());
     // TODO(akalin): Replace silent update with a list of enabled
     // permissions.
-    extensions_service->AddPendingExtension(
-        id, update_url, false, true,
-        specifics.enabled(), specifics.incognito_enabled());
+    extensions_service->AddPendingExtensionFromSync(
+        id, update_url,
+        PendingExtensionInfo::EXTENSION,
+        true,  // install_silently
+        specifics.enabled(),
+        specifics.incognito_enabled());
   }
   DCHECK(!extension_data->NeedsUpdate(ExtensionData::SERVER));
 }
diff --git a/chrome/browser/sync/glue/theme_util.cc b/chrome/browser/sync/glue/theme_util.cc
index a384ef4..47a8604 100644
--- a/chrome/browser/sync/glue/theme_util.cc
+++ b/chrome/browser/sync/glue/theme_util.cc
@@ -126,16 +126,17 @@ void SetCurrentThemeFromThemeSpecifics(
       // No extension with this id exists -- we must install it; we do
       // so by adding it as a pending extension and then triggering an
       // auto-update cycle.
-      const bool kIsTheme = true;
+      const PendingExtensionInfo::ExpectedCrxType kExpectedCrxType =
+          PendingExtensionInfo::THEME;
       // Themes don't need to install silently as they just pop up an
       // informational dialog after installation instead of a
       // confirmation dialog.
       const bool kInstallSilently = false;
       const bool kEnableOnInstall = true;
       const bool kEnableIncognitoOnInstall = false;
-      extensions_service->AddPendingExtension(
-          id, update_url, kIsTheme, kInstallSilently,
-          kEnableOnInstall, kEnableIncognitoOnInstall);
+      extensions_service->AddPendingExtensionFromSync(
+          id, update_url, kExpectedCrxType,
+          kInstallSilently, kEnableOnInstall, kEnableIncognitoOnInstall);
       ExtensionUpdater* extension_updater = extensions_service->updater();
       // Auto-updates should now be on always (see the construction of
       // the ExtensionsService in ProfileImpl::InitExtensions()).