Clean up threading model of external extension providers

chrome/browser/extensions/extension_service.cc

-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/extension_service.h"
 
 #include <algorithm>
 
 #include "base/basictypes.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/metrics/histogram.h"
 #include "base/stl_util-inl.h"
 #include "base/string16.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
-#include "base/values_util.h"
 #include "base/version.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/themes/browser_theme_provider.h"
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/debugger/devtools_manager.h"
 #include "chrome/browser/dom_ui/shown_sections_handler.h"
 #include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/extensions/default_apps.h"
 #include "chrome/browser/extensions/extension_accessibility_api.h"
 #include "chrome/browser/extensions/extension_bookmarks_module.h"
 #include "chrome/browser/extensions/extension_browser_event_router.h"
 #include "chrome/browser/extensions/extension_cookies_api.h"
 #include "chrome/browser/extensions/extension_data_deleter.h"
 #include "chrome/browser/extensions/extension_dom_ui.h"
 #include "chrome/browser/extensions/extension_error_reporter.h"
 #include "chrome/browser/extensions/extension_history_api.h"
 #include "chrome/browser/extensions/extension_host.h"
 #include "chrome/browser/extensions/extension_management_api.h"
 #include "chrome/browser/extensions/extension_process_manager.h"
 #include "chrome/browser/extensions/extension_processes_api.h"
 #include "chrome/browser/extensions/extension_updater.h"
 #include "chrome/browser/extensions/extension_webnavigation_api.h"
-#include "chrome/browser/extensions/external_extension_provider.h"
-#include "chrome/browser/extensions/external_policy_extension_provider.h"
-#include "chrome/browser/extensions/external_pref_extension_provider.h"
+#include "chrome/browser/extensions/external_extension_provider_interface.h"
+#include "chrome/browser/extensions/external_extension_provider_impl.h"
 #include "chrome/browser/net/chrome_url_request_context.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search_engines/template_url_model.h"
 #include "chrome/common/child_process_logging.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/extension_error_utils.h"
 #include "chrome/common/extensions/extension_file_util.h"
 #include "chrome/common/extensions/extension_l10n_util.h"
 #include "chrome/common/extensions/extension_resource.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/common/notification_type.h"
 #include "chrome/common/json_value_serializer.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/registry_controlled_domain.h"
 #include "webkit/database/database_tracker.h"
 #include "webkit/database/database_util.h"
 
-#if defined(OS_WIN)
-#include "chrome/browser/extensions/external_registry_extension_provider_win.h"
-#endif
-
 using base::Time;
 
 namespace errors = extension_manifest_errors;
 
 namespace {
 
 #if defined(OS_LINUX)
 static const int kOmniboxIconPaddingLeft = 2;
 static const int kOmniboxIconPaddingRight = 2;
 #elif defined(OS_MACOSX)
@@ skipping 93 matching lines @@
 }
 
 // ExtensionService.
 
 const char* ExtensionService::kInstallDirectoryName = "Extensions";
 const char* ExtensionService::kCurrentVersionFileName = "Current Version";
 
 // Implements IO for the ExtensionService.
 
 class ExtensionServiceBackend
-    : public base::RefCountedThreadSafe<ExtensionServiceBackend>,
-      public ExternalExtensionProvider::Visitor {
+    : public base::RefCountedThreadSafe<ExtensionServiceBackend> {
  public:
   // |install_directory| is a path where to look for extensions to load.
-  ExtensionServiceBackend(PrefService* prefs,
-                           const FilePath& install_directory);
+  explicit ExtensionServiceBackend(const FilePath& install_directory);
 
   // Loads a single extension from |path| where |path| is the top directory of
   // a specific extension where its manifest file lives.
   // Errors are reported through ExtensionErrorReporter. On success,
   // OnExtensionLoaded() is called.
   // TODO(erikkay): It might be useful to be able to load a packed extension
   // (presumably into memory) without installing it.
   void LoadSingleExtension(const FilePath &path,
                            scoped_refptr<ExtensionService> frontend);
 
-  // Check externally updated extensions for updates and install if necessary.
-  // Errors are reported through ExtensionErrorReporter. Succcess is not
-  // reported.
-  void CheckForExternalUpdates(scoped_refptr<ExtensionService> frontend);
-
-  // For the extension in |version_path| with |id|, check to see if it's an
-  // externally managed extension.  If so, tell the frontend to uninstall it.
-  void CheckExternalUninstall(scoped_refptr<ExtensionService> frontend,
-                              const std::string& id);
-
-  // Clear all ExternalExtensionProviders.
-  void ClearProvidersForTesting();
-
-  // Adds an ExternalExtensionProvider for the service to use during testing.
-  // Takes ownership of |test_provider|.
-  void AddProviderForTesting(ExternalExtensionProvider* test_provider);
-
-  // ExternalExtensionProvider::Visitor implementation.
-  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,
-                                                 Extension::Location location);
-
-  virtual void UpdateExternalPolicyExtensionProvider(
-      scoped_refptr<RefCountedList> forcelist);
-
  private:
   friend class base::RefCountedThreadSafe<ExtensionServiceBackend>;
 
   virtual ~ExtensionServiceBackend();
 
   // Finish installing the extension in |crx_path| after it has been unpacked to
   // |unpacked_path|.  If |expected_id| is not empty, it's verified against the
   // extension's manifest before installation. If |silent| is true, there will
   // be no install confirmation dialog. |from_gallery| indicates whether the
   // crx was installed from our gallery, which results in different UI.
@@ skipping 11 matching lines @@
   // This is a naked pointer which is set by each entry point.
   // The entry point is responsible for ensuring lifetime.
   ExtensionService* frontend_;
 
   // The top-level extensions directory being installed to.
   FilePath install_directory_;
 
   // Whether errors result in noisy alerts.
   bool alert_on_error_;
 
-  // A collection of external extension providers.  Each provider reads
-  // a source of external extension information.  Examples include the
-  // windows registry and external_extensions.json.
-  typedef std::vector<linked_ptr<ExternalExtensionProvider> >
-      ProviderCollection;
-  ProviderCollection external_extension_providers_;
-  linked_ptr<ExternalPolicyExtensionProvider>
-      external_policy_extension_provider_;
-
-  // 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(ExtensionServiceBackend);
 };
 
 ExtensionServiceBackend::ExtensionServiceBackend(
-    PrefService* prefs,
     const FilePath& install_directory)
         : frontend_(NULL),
           install_directory_(install_directory),
-          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.
-  external_extension_providers_.push_back(
-      linked_ptr<ExternalExtensionProvider>(
-          new ExternalPrefExtensionProvider()));
-#if defined(OS_WIN)
-  external_extension_providers_.push_back(
-      linked_ptr<ExternalExtensionProvider>(
-          new ExternalRegistryExtensionProvider()));
-#endif
-  // The policy-controlled extension provider is also stored in a member
-  // variable so that UpdateExternalPolicyExtensionProvider can access it and
-  // update its extension list later.
-  external_policy_extension_provider_.reset(
-      new ExternalPolicyExtensionProvider(
-          prefs->GetList(prefs::kExtensionInstallForceList)));
-  external_extension_providers_.push_back(external_policy_extension_provider_);
+          alert_on_error_(false) {
 }
 
 ExtensionServiceBackend::~ExtensionServiceBackend() {
 }
 
 void ExtensionServiceBackend::LoadSingleExtension(
     const FilePath& path_in, scoped_refptr<ExtensionService> frontend) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
   frontend_ = frontend;
@@ skipping 29 matching lines @@
     const FilePath& extension_path, const std::string &error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       NewRunnableMethod(
           frontend_,
           &ExtensionService::ReportExtensionLoadError, extension_path,
           error, NotificationType::EXTENSION_INSTALL_ERROR, alert_on_error_));
 }
 
-// Some extensions will autoupdate themselves externally from Chrome.  These
-// are typically part of some larger client application package.  To support
-// these, the extension will register its location in the the preferences file
-// (and also, on Windows, in the registry) and this code will periodically
-// check that location for a .crx file, which it will then install locally if
-// a new version is available.
-void ExtensionServiceBackend::CheckForExternalUpdates(
-    scoped_refptr<ExtensionService> frontend) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-
-  // Note that this installation is intentionally silent (since it didn't
-  // go through the front-end).  Extensions that are registered in this
-  // way are effectively considered 'pre-bundled', and so implicitly
-  // trusted.  In general, if something has HKLM or filesystem access,
-  // 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 OnExternalExtension(File|UpdateUrl)Found.
-  ProviderCollection::const_iterator i;
-  for (i = external_extension_providers_.begin();
-       i != external_extension_providers_.end(); ++i) {
-    ExternalExtensionProvider* provider = i->get();
-    provider->VisitRegisteredExtension(this);
-  }
-
-  if (external_extension_added_ && frontend->updater()) {
-    BrowserThread::PostTask(
-        BrowserThread::UI, FROM_HERE,
-            NewRunnableMethod(
-                frontend->updater(), &ExtensionUpdater::CheckNow));
-  }
-}
-
-void ExtensionServiceBackend::CheckExternalUninstall(
-    scoped_refptr<ExtensionService> frontend, const std::string& id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+void ExtensionService::CheckExternalUninstall(const std::string& id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // Check if the providers know about this extension.
   ProviderCollection::const_iterator i;
   for (i = external_extension_providers_.begin();
        i != external_extension_providers_.end(); ++i) {
+    DCHECK(i->get()->IsReady());
     if (i->get()->HasExtension(id))
       return;  // Yup, known extension, don't uninstall.
   }
 
   // This is an external extension that we don't have registered.  Uninstall.
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      NewRunnableMethod(
-          frontend.get(), &ExtensionService::UninstallExtension, id, true));
+  UninstallExtension(id, true);
 }
 
-void ExtensionServiceBackend::UpdateExternalPolicyExtensionProvider(
-    scoped_refptr<RefCountedList> forcelist) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-  external_policy_extension_provider_->SetPreferences(forcelist->Get());
-}
-
-void ExtensionServiceBackend::ClearProvidersForTesting() {
+void ExtensionService::ClearProvidersForTesting() {
   external_extension_providers_.clear();
 }
 
-void ExtensionServiceBackend::AddProviderForTesting(
-    ExternalExtensionProvider* test_provider) {
+void ExtensionService::AddProviderForTesting(
+    ExternalExtensionProviderInterface* test_provider) {
   DCHECK(test_provider);
   external_extension_providers_.push_back(
-      linked_ptr<ExternalExtensionProvider>(test_provider));
+      linked_ptr<ExternalExtensionProviderInterface>(test_provider));
 }
 
-void ExtensionServiceBackend::OnExternalExtensionFileFound(
-    const std::string& id, const Version* version, const FilePath& path,
-    Extension::Location location) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-
-  DCHECK(version);
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      NewRunnableMethod(
-          frontend_, &ExtensionService::OnExternalExtensionFileFound, id,
-          version->GetString(), path, location));
-}
-
-void ExtensionServiceBackend::OnExternalExtensionUpdateUrlFound(
+void ExtensionService::OnExternalExtensionUpdateUrlFound(
     const std::string& id,
     const GURL& update_url,
     Extension::Location location) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  if (frontend_->GetExtensionById(id, true)) {
+  if (GetExtensionById(id, true)) {
     // Already installed.  Do not change the update URL that the extension set.
     return;
   }
-
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      NewRunnableMethod(
-          frontend_,
-          &ExtensionService::AddPendingExtensionFromExternalUpdateUrl,
-          id, update_url, location));
+  AddPendingExtensionFromExternalUpdateUrl(id, update_url, location);
   external_extension_added_ |= true;
 }
 
 bool ExtensionService::IsDownloadFromGallery(const GURL& download_url,
                                               const GURL& referrer_url) {
   // Special-case the themes mini-gallery.
   // TODO(erikkay) When that gallery goes away, remove this code.
   if (IsDownloadFromMiniGallery(download_url) &&
       StartsWithASCII(referrer_url.spec(),
                       extension_urls::kMiniGalleryBrowsePrefix, false)) {
@@ skipping 96 matching lines @@
     extensions_enabled_ = false;
   } else if (profile->GetPrefs()->GetBoolean(prefs::kDisableExtensions)) {
     extensions_enabled_ = false;
   }
 
   registrar_.Add(this, NotificationType::EXTENSION_PROCESS_TERMINATED,
                  NotificationService::AllSources());
   pref_change_registrar_.Init(profile->GetPrefs());
   pref_change_registrar_.Add(prefs::kExtensionInstallAllowList, this);
   pref_change_registrar_.Add(prefs::kExtensionInstallDenyList, this);
-  pref_change_registrar_.Add(prefs::kExtensionInstallForceList, this);
 
   // Set up the ExtensionUpdater
   if (autoupdate_enabled) {
     int update_frequency = kDefaultUpdateFrequencySeconds;
     if (command_line->HasSwitch(switches::kExtensionsUpdateFrequency)) {
       base::StringToInt(command_line->GetSwitchValueASCII(
           switches::kExtensionsUpdateFrequency),
           &update_frequency);
     }
     updater_ = new ExtensionUpdater(this,
                                     profile->GetPrefs(),
                                     update_frequency);
   }
 
-  backend_ = new ExtensionServiceBackend(profile->GetPrefs(),
-                                          install_directory_);
+  backend_ = new ExtensionServiceBackend(install_directory_);
+
+  ExternalExtensionProviderImpl::CreateExternalProviders(
+      this, profile_, &external_extension_providers_);
 
   // Use monochrome icons for Omnibox icons.
   omnibox_popup_icon_manager_.set_monochrome(true);
   omnibox_icon_manager_.set_monochrome(true);
   omnibox_icon_manager_.set_padding(gfx::Insets(0, kOmniboxIconPaddingLeft,
                                                 0, kOmniboxIconPaddingRight));
 }
 
 const ExtensionList* ExtensionService::extensions() const {
   return &extensions_;
@@ skipping 14 matching lines @@
 bool ExtensionService::HasInstalledExtensions() {
   return !(extensions_.empty() && disabled_extensions_.empty());
 }
 
 ExtensionService::~ExtensionService() {
   DCHECK(!profile_);  // Profile should have told us it's going away.
   UnloadAllExtensions();
   if (updater_.get()) {
     updater_->Stop();
   }
+  ProviderCollection::const_iterator i;
+  for (i = external_extension_providers_.begin();
+       i != external_extension_providers_.end(); ++i) {
+    ExternalExtensionProviderInterface* provider = i->get();
+    provider->ServiceShutdown();
+  }
 }
 
 void ExtensionService::InitEventRouters() {
   if (event_routers_initialized_)
     return;
 
   ExtensionHistoryEventRouter::GetInstance()->ObserveProfile(profile_);
   ExtensionAccessibilityEventRouter::GetInstance()->ObserveProfile(profile_);
   ExtensionBrowserEventRouter::GetInstance()->Init(profile_);
   ExtensionBookmarkEventRouter::GetInstance()->Observe(
@@ skipping 564 matching lines @@
                              error,
                              NotificationType::EXTENSION_INSTALL_ERROR,
                              false);
     return;
   }
 
   if (write_to_prefs)
     extension_prefs_->UpdateManifest(extension);
 
   OnExtensionLoaded(extension);
-
-  if (Extension::IsExternalLocation(info.extension_location)) {
-    BrowserThread::PostTask(
-        BrowserThread::FILE, FROM_HERE,
-        NewRunnableMethod(
-            backend_.get(),
-            &ExtensionServiceBackend::CheckExternalUninstall,
-            scoped_refptr<ExtensionService>(this),
-            info.extension_id));
-  }
 }
 
 void ExtensionService::NotifyExtensionLoaded(const Extension* extension) {
   // The ChromeURLRequestContexts need to be first to know that the extension
   // was loaded, otherwise a race can arise where a renderer that is created
   // for the extension may try to load an extension URL with an extension id
   // that the request context doesn't yet know about. The profile is responsible
   // for ensuring its URLRequestContexts appropriately discover the loaded
   // extension.
   if (profile_) {
@@ skipping 225 matching lines @@
       NotificationType::EXTENSION_USER_SCRIPTS_UPDATED,
       Source<Profile>(profile_),
       Details<const Extension>(extension));
 }
 
 bool ExtensionService::GetBrowserActionVisibility(const Extension* extension) {
   return extension_prefs_->GetBrowserActionVisibility(extension);
 }
 
 void ExtensionService::SetBrowserActionVisibility(const Extension* extension,
-                                                   bool visible) {
+                                                  bool visible) {
   extension_prefs_->SetBrowserActionVisibility(extension, visible);
 }
 
+// Some extensions will autoupdate themselves externally from Chrome.  These
+// are typically part of some larger client application package.  To support
+// these, the extension will register its location in the the preferences file
+// (and also, on Windows, in the registry) and this code will periodically
+// check that location for a .crx file, which it will then install locally if
+// a new version is available.
+// Errors are reported through ExtensionErrorReporter. Succcess is not
+// reported.
 void ExtensionService::CheckForExternalUpdates() {
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      NewRunnableMethod(
-          backend_.get(), &ExtensionServiceBackend::CheckForExternalUpdates,
-          scoped_refptr<ExtensionService>(this)));
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  // Note that this installation is intentionally silent (since it didn't
+  // go through the front-end).  Extensions that are registered in this
+  // way are effectively considered 'pre-bundled', and so implicitly
+  // trusted.  In general, if something has HKLM or filesystem access,
+  // they could install an extension manually themselves anyway.
+  external_extension_added_ = false;
+
+  // Ask each external extension provider to give us a call back for each
+  // extension they know about. See OnExternalExtension(File|UpdateUrl)Found.
+  ProviderCollection::const_iterator i;
+  for (i = external_extension_providers_.begin();
+       i != external_extension_providers_.end(); ++i) {
+    ExternalExtensionProviderInterface* provider = i->get();
+    provider->VisitRegisteredExtension();
+  }
+
+  // Uninstall of unclaimed extensions will happen after all the providers
+  // had reported ready. Trigger uninstall even if there are no providers
+  // installed:
+  OnExternalProviderReady();
 }
 
-void ExtensionService::UpdateExternalPolicyExtensionProvider() {
-  const ListValue* list_pref =
-      profile_->GetPrefs()->GetList(prefs::kExtensionInstallForceList);
-  ListValue* list_copy = NULL;
-  if (list_pref)
-    list_copy = static_cast<ListValue*>(list_pref->DeepCopy());
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      NewRunnableMethod(
-          backend_.get(),
-          &ExtensionServiceBackend::UpdateExternalPolicyExtensionProvider,
-          scoped_refptr<RefCountedList>(
-              new RefCountedList(list_copy))));
+void ExtensionService::OnExternalProviderReady() {
+  // An external provider has finished loading. We only take action
+  // if all of them are finished. So we check them first.
+  ProviderCollection::const_iterator i;
+  for (i = external_extension_providers_.begin();
+       i != external_extension_providers_.end(); ++i) {
+    ExternalExtensionProviderInterface* provider = i->get();
+    if (!provider->IsReady()) {
+      return;
+    }
+  }
+
+  // All the providers are ready. Install any pending extensions.
+  if (external_extension_added_ && updater()) {
+    external_extension_added_ = false;
+    updater()->CheckNow();
+  }
+
+  // Uninstall all the unclaimed extensions.
+  scoped_ptr<ExtensionPrefs::ExtensionsInfo> extensions_info(
+      extension_prefs_->GetInstalledExtensionsInfo());
+  for (size_t i = 0; i < extensions_info->size(); ++i) {
+    ExtensionInfo* info = extensions_info->at(i).get();
+    if (Extension::IsExternalLocation(info->extension_location))
+      CheckExternalUninstall(info->extension_id);
+  }
 }
 
 void ExtensionService::UnloadExtension(
     const std::string& extension_id,
     UnloadedExtensionInfo::Reason reason) {
   // Make sure the extension gets deleted after we return from this function.
   scoped_refptr<const Extension> extension(
       GetExtensionByIdInternal(extension_id, true, true));
 
   // This method can be called via PostTask, so the extension may have been
@@ skipping 414 matching lines @@
 const SkBitmap& ExtensionService::GetOmniboxIcon(
     const std::string& extension_id) {
   return omnibox_icon_manager_.GetIcon(extension_id);
 }
 
 const SkBitmap& ExtensionService::GetOmniboxPopupIcon(
     const std::string& extension_id) {
   return omnibox_popup_icon_manager_.GetIcon(extension_id);
 }
 
-void ExtensionService::ClearProvidersForTesting() {
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      NewRunnableMethod(
-          backend_.get(), &ExtensionServiceBackend::ClearProvidersForTesting));
-}
-
-void ExtensionService::AddProviderForTesting(
-    ExternalExtensionProvider* test_provider) {
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      NewRunnableMethod(
-          backend_.get(), &ExtensionServiceBackend::AddProviderForTesting,
-          test_provider));
-}
-
 void ExtensionService::OnExternalExtensionFileFound(
          const std::string& id,
-         const std::string& version,
+         const Version* version,
          const FilePath& path,
          Extension::Location location) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (extension_prefs_->IsExtensionKilled(id))
     return;
 
+  DCHECK(version);
+
   // 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.
   const Extension* existing = GetExtensionById(id, true);
-  scoped_ptr<Version> other(Version::GetVersionFromString(version));
   if (existing) {
-    switch (existing->version()->CompareTo(*other)) {
+    switch (existing->version()->CompareTo(*version)) {
       case -1:  // existing version is older, we should upgrade
         break;
       case 0:  // existing version is same, do nothing
         return;
       case 1:  // existing version is newer, uh-oh
         LOG(WARNING) << "Found external version of extension " << id
                      << "that is older than current version. Current version "
                      << "is: " << existing->VersionString() << ". New version "
                      << "is: " << version << ". Keeping current version.";
         return;
@@ skipping 73 matching lines @@
                             host->extension()->id(),
                             UnloadedExtensionInfo::DISABLE));
       break;
     }
 
     case NotificationType::PREF_CHANGED: {
       std::string* pref_name = Details<std::string>(details).ptr();
       if (*pref_name == prefs::kExtensionInstallAllowList ||
           *pref_name == prefs::kExtensionInstallDenyList) {
         CheckAdminBlacklist();
-      } else if (*pref_name == prefs::kExtensionInstallForceList) {
-        UpdateExternalPolicyExtensionProvider();
-        CheckForExternalUpdates();
-        // TODO(gfeher): Also check for external extensions that can be
-        // uninstalled because they were removed from the pref.
-        // (crbug.com/63667)
       } else {
         NOTREACHED() << "Unexpected preference name.";
       }
       break;
     }
 
     default:
       NOTREACHED() << "Unexpected notification type.";
   }
 }
@@ skipping 32 matching lines @@
 }
 
 void ExtensionService::SetBeingUpgraded(const Extension* extension,
                                          bool value) {
   extension_runtime_data_[extension->id()].being_upgraded = value;
 }
 
 PropertyBag* ExtensionService::GetPropertyBag(const Extension* extension) {
   return &extension_runtime_data_[extension->id()].property_bag;
 }