Fix double install and incorrect uninstall of external extensions.

BUG=chromium-os:7037
TEST=ExtensionsserviceTest.External*,ExtensionManagementTest.ExternalUrlUpdate

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

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

8 files changed, 169 insertions, 115 deletions

diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 33fade0..feb12941 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -185,16 +185,14 @@ class ExtensionsServiceBackend
   // 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<ExtensionsService> frontend,
-                              const std::string& id,
-                              Extension::Location location);
+                              const std::string& id);
 
   // Clear all ExternalExtensionProviders.
   void ClearProvidersForTesting();
 
-  // Sets an ExternalExtensionProvider for the service to use during testing.
-  // |location| specifies what type of provider should be added.
-  void SetProviderForTesting(Extension::Location location,
-                             ExternalExtensionProvider* test_provider);
+  // 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,
@@ -233,16 +231,6 @@ class ExtensionsServiceBackend
   void ReportExtensionLoadError(const FilePath& extension_path,
                                 const std::string& error);
 
-  // Lookup an external extension by |id| by going through all registered
-  // external extension providers until we find a provider that contains an
-  // extension that matches. If |version| is not NULL, the extension version
-  // will be returned (caller is responsible for deleting that pointer).
-  // |location| can also be null, if not needed. Returns true if extension is
-  // found, false otherwise.
-  bool LookupExternalExtension(const std::string& id,
-                               Version** version,
-                               Extension::Location* location);
-
   // This is a naked pointer which is set by each entry point.
   // The entry point is responsible for ensuring lifetime.
   ExtensionsService* frontend_;
@@ -253,12 +241,12 @@ class ExtensionsServiceBackend
   // Whether errors result in noisy alerts.
   bool alert_on_error_;
 
-  // A map from external extension type to the external extension provider
-  // for that type.  Because a single provider may handle more than one
-  // external extension type, more than one key may map to the same object.
-  typedef std::map<Extension::Location,
-                   linked_ptr<ExternalExtensionProvider> > ProviderMap;
-  ProviderMap external_extension_providers_;
+  // 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_;
 
   // Set to true by OnExternalExtensionUpdateUrlFound() when an external
   // extension URL is found.  Used in CheckForExternalUpdates() to see
@@ -281,17 +269,13 @@ ExtensionsServiceBackend::ExtensionsServiceBackend(
   // 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_[Extension::EXTERNAL_PREF] =
+  external_extension_providers_.push_back(
       linked_ptr<ExternalExtensionProvider>(
-          new ExternalPrefExtensionProvider());
-  // EXTERNAL_PREF_DOWNLOAD and EXTERNAL_PREF extensions are handled by the
-  // same object.
-  external_extension_providers_[Extension::EXTERNAL_PREF_DOWNLOAD] =
-      external_extension_providers_[Extension::EXTERNAL_PREF];
+          new ExternalPrefExtensionProvider()));
 #if defined(OS_WIN)
-  external_extension_providers_[Extension::EXTERNAL_REGISTRY] =
+  external_extension_providers_.push_back(
       linked_ptr<ExternalExtensionProvider>(
-          new ExternalRegistryExtensionProvider());
+          new ExternalRegistryExtensionProvider()));
 #endif
 }
 
@@ -338,22 +322,6 @@ void ExtensionsServiceBackend::ReportExtensionLoadError(
           error, NotificationType::EXTENSION_INSTALL_ERROR, alert_on_error_));
 }
 
-bool ExtensionsServiceBackend::LookupExternalExtension(
-    const std::string& id, Version** version, Extension::Location* location) {
-  scoped_ptr<Version> extension_version;
-  for (ProviderMap::const_iterator i = external_extension_providers_.begin();
-       i != external_extension_providers_.end(); ++i) {
-    const ExternalExtensionProvider* provider = i->second.get();
-    extension_version.reset(provider->RegisteredVersion(id, location));
-    if (extension_version.get()) {
-      if (version)
-        *version = extension_version.release();
-      return true;
-    }
-  }
-  return false;
-}
-
 // 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
@@ -374,10 +342,10 @@ void ExtensionsServiceBackend::CheckForExternalUpdates(
 
   // Ask each external extension provider to give us a call back for each
   // extension they know about. See OnExternalExtension(File|UpdateUrl)Found.
-
-  for (ProviderMap::const_iterator i = external_extension_providers_.begin();
+  ProviderCollection::const_iterator i;
+  for (i = external_extension_providers_.begin();
        i != external_extension_providers_.end(); ++i) {
-    ExternalExtensionProvider* provider = i->second.get();
+    ExternalExtensionProvider* provider = i->get();
     provider->VisitRegisteredExtension(this, ids_to_ignore);
   }
 
@@ -390,21 +358,15 @@ void ExtensionsServiceBackend::CheckForExternalUpdates(
 }
 
 void ExtensionsServiceBackend::CheckExternalUninstall(
-    scoped_refptr<ExtensionsService> frontend, const std::string& id,
-    Extension::Location location) {
+    scoped_refptr<ExtensionsService> frontend, const std::string& id) {
   // Check if the providers know about this extension.
-  ProviderMap::const_iterator i = external_extension_providers_.find(location);
-  if (i == external_extension_providers_.end()) {
-    NOTREACHED() << "CheckExternalUninstall called for non-external extension "
-                 << location;
-    return;
+  ProviderCollection::const_iterator i;
+  for (i = external_extension_providers_.begin();
+       i != external_extension_providers_.end(); ++i) {
+    if (i->get()->HasExtension(id))
+      return;  // Yup, known extension, don't uninstall.
   }
 
-  scoped_ptr<Version> version;
-  version.reset(i->second->RegisteredVersion(id, NULL));
-  if (version.get())
-    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,
@@ -416,12 +378,11 @@ void ExtensionsServiceBackend::ClearProvidersForTesting() {
   external_extension_providers_.clear();
 }
 
-void ExtensionsServiceBackend::SetProviderForTesting(
-    Extension::Location location,
+void ExtensionsServiceBackend::AddProviderForTesting(
     ExternalExtensionProvider* test_provider) {
   DCHECK(test_provider);
-  external_extension_providers_[location] =
-      linked_ptr<ExternalExtensionProvider>(test_provider);
+  external_extension_providers_.push_back(
+      linked_ptr<ExternalExtensionProvider>(test_provider));
 }
 
 void ExtensionsServiceBackend::OnExternalExtensionFileFound(
@@ -1108,8 +1069,7 @@ void ExtensionsService::LoadInstalledExtension(const ExtensionInfo& info,
             backend_.get(),
             &ExtensionsServiceBackend::CheckExternalUninstall,
             scoped_refptr<ExtensionsService>(this),
-            info.extension_id,
-            info.extension_location));
+            info.extension_id));
   }
 }
 
@@ -1749,13 +1709,13 @@ void ExtensionsService::ClearProvidersForTesting() {
           backend_.get(), &ExtensionsServiceBackend::ClearProvidersForTesting));
 }
 
-void ExtensionsService::SetProviderForTesting(
-    Extension::Location location, ExternalExtensionProvider* test_provider) {
+void ExtensionsService::AddProviderForTesting(
+    ExternalExtensionProvider* test_provider) {
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       NewRunnableMethod(
-          backend_.get(), &ExtensionsServiceBackend::SetProviderForTesting,
-          location, test_provider));
+          backend_.get(), &ExtensionsServiceBackend::AddProviderForTesting,
+          test_provider));
 }
 
 void ExtensionsService::OnExternalExtensionFileFound(
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index bf398de..c00c264 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -314,9 +314,8 @@ class ExtensionsService
   void ClearProvidersForTesting();
 
   // Sets an ExternalExtensionProvider for the service to use during testing.
-  // |location| specifies what type of provider should be added.
-  void SetProviderForTesting(Extension::Location location,
-                             ExternalExtensionProvider* test_provider);
+  // Takes ownership of |test_provider|.
+  void AddProviderForTesting(ExternalExtensionProvider* test_provider);
 
   // Called when the initial extensions load has completed.
   virtual void OnLoadedInstalledExtensions();
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index df29995..57d2cff 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -103,7 +103,7 @@ static std::vector<std::string> GetErrors() {
 class MockExtensionProvider : public ExternalExtensionProvider {
  public:
   explicit MockExtensionProvider(Extension::Location location)
-    : location_(location) {}
+    : location_(location), visit_count_(0) {}
   virtual ~MockExtensionProvider() {}
 
   void UpdateOrAddExtension(const std::string& id,
@@ -119,6 +119,7 @@ class MockExtensionProvider : public ExternalExtensionProvider {
   // ExternalExtensionProvider implementation:
   virtual void VisitRegisteredExtension(
       Visitor* visitor, const std::set<std::string>& ids_to_ignore) const {
+    visit_count_++;
     for (DataMap::const_iterator i = extension_map_.begin();
          i != extension_map_.end(); ++i) {
       if (ids_to_ignore.find(i->first) != ids_to_ignore.end())
@@ -131,15 +132,28 @@ class MockExtensionProvider : public ExternalExtensionProvider {
     }
   }
 
-  virtual Version* RegisteredVersion(const std::string& id,
-                                     Extension::Location* location) const {
+  virtual bool HasExtension(const std::string& id) const {
+    return extension_map_.find(id) != extension_map_.end();
+  }
+
+  virtual bool GetExtensionDetails(const std::string& id,
+                                   Extension::Location* location,
+                                   scoped_ptr<Version>* version) const {
     DataMap::const_iterator it = extension_map_.find(id);
     if (it == extension_map_.end())
-      return NULL;
+      return false;
+
+    if (version)
+      version->reset(Version::GetVersionFromString(it->second.first));
 
     if (location)
       *location = location_;
-    return Version::GetVersionFromString(it->second.first);
+
+    return true;
+  }
+  int visit_count() const { return visit_count_; }
+  void set_visit_count(int visit_count) {
+    visit_count_ = visit_count;
   }
 
  private:
@@ -147,6 +161,12 @@ class MockExtensionProvider : public ExternalExtensionProvider {
   DataMap extension_map_;
   Extension::Location location_;
 
+  // visit_count_ tracks the number of calls to VisitRegisteredExtension().
+  // Mutable because it must be incremented on each call to
+  // VisitRegisteredExtension(), which must be a const method to inherit
+  // from the class being mocked.
+  mutable int visit_count_;
+
   DISALLOW_COPY_AND_ASSIGN(MockExtensionProvider);
 };
 
@@ -197,10 +217,18 @@ class MockProviderVisitor : public ExternalExtensionProvider::Visitor {
        << "Got back ID (" << id.c_str() << ") we weren't expecting";
 
     if (pref) {
+      EXPECT_TRUE(provider_->HasExtension(id));
+
       // Ask provider if the extension we got back is registered.
       Extension::Location location = Extension::INVALID;
-      scoped_ptr<Version> v1(provider_->RegisteredVersion(id, NULL));
-      scoped_ptr<Version> v2(provider_->RegisteredVersion(id, &location));
+      scoped_ptr<Version> v1;
+      FilePath crx_path;
+
+      EXPECT_TRUE(provider_->GetExtensionDetails(id, NULL, &v1));
+      EXPECT_STREQ(version->GetString().c_str(), v1->GetString().c_str());
+
+      scoped_ptr<Version> v2;
+      EXPECT_TRUE(provider_->GetExtensionDetails(id, &location, &v2));
       EXPECT_STREQ(version->GetString().c_str(), v1->GetString().c_str());
       EXPECT_STREQ(version->GetString().c_str(), v2->GetString().c_str());
       EXPECT_EQ(Extension::EXTERNAL_PREF, location);
@@ -221,6 +249,13 @@ class MockProviderVisitor : public ExternalExtensionProvider::Visitor {
        << L"Got back ID (" << id.c_str() << ") we weren't expecting";
 
     if (pref) {
+      EXPECT_TRUE(provider_->HasExtension(id));
+
+      // External extensions with update URLs do not have versions.
+      scoped_ptr<Version> v1;
+      EXPECT_TRUE(provider_->GetExtensionDetails(id, NULL, &v1));
+      EXPECT_FALSE(v1.get());
+
       // Remove it so we won't count it again.
       prefs_->Remove(id, NULL);
     }
@@ -416,9 +451,8 @@ class ExtensionsServiceTest
     }
   }
 
-  void SetMockExternalProvider(Extension::Location location,
-                               ExternalExtensionProvider* provider) {
-    service_->SetProviderForTesting(location, provider);
+  void AddMockExternalProvider(ExternalExtensionProvider* provider) {
+    service_->AddProviderForTesting(provider);
   }
 
  protected:
@@ -2227,6 +2261,8 @@ void ExtensionsServiceTest::TestExternalProvider(
   loop_.RunAllPending();
   ASSERT_EQ(0u, loaded_.size());
 
+  provider->set_visit_count(0);
+
   // Register a test extension externally using the mock registry provider.
   FilePath source_path;
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &source_path));
@@ -2342,6 +2378,8 @@ void ExtensionsServiceTest::TestExternalProvider(
   loop_.RunAllPending();
   ASSERT_EQ(0u, loaded_.size());
   ValidatePrefKeyCount(1);
+
+  EXPECT_EQ(5, provider->visit_count());
 }
 
 // Tests the external installation feature
@@ -2354,7 +2392,7 @@ TEST_F(ExtensionsServiceTest, ExternalInstallRegistry) {
   // Now add providers. Extension system takes ownership of the objects.
   MockExtensionProvider* reg_provider =
       new MockExtensionProvider(Extension::EXTERNAL_REGISTRY);
-  SetMockExternalProvider(Extension::EXTERNAL_REGISTRY, reg_provider);
+  AddMockExternalProvider(reg_provider);
   TestExternalProvider(reg_provider, Extension::EXTERNAL_REGISTRY);
 }
 #endif
@@ -2367,7 +2405,8 @@ TEST_F(ExtensionsServiceTest, ExternalInstallPref) {
   // Now add providers. Extension system takes ownership of the objects.
   MockExtensionProvider* pref_provider =
       new MockExtensionProvider(Extension::EXTERNAL_PREF);
-  SetMockExternalProvider(Extension::EXTERNAL_PREF, pref_provider);
+
+  AddMockExternalProvider(pref_provider);
   TestExternalProvider(pref_provider, Extension::EXTERNAL_PREF);
 }
 
@@ -2376,10 +2415,16 @@ TEST_F(ExtensionsServiceTest, ExternalInstallPrefUpdateUrl) {
   InitializeEmptyExtensionsService();
   set_extensions_enabled(false);
 
-  // Now add providers. Extension system takes ownership of the objects.
+  // TODO(skerner): The mock provider is not a good model of a provider
+  // that works with update URLs, because it adds file and version info.
+  // Extend the mock to work with update URLs.  This test checks the
+  // behavior that is common to all external extension visitors.  The
+  // browser test ExtensionManagementTest.ExternalUrlUpdate tests that
+  // what the visitor does results in an extension being downloaded and
+  // installed.
   MockExtensionProvider* pref_provider =
       new MockExtensionProvider(Extension::EXTERNAL_PREF_DOWNLOAD);
-  SetMockExternalProvider(Extension::EXTERNAL_PREF_DOWNLOAD, pref_provider);
+  AddMockExternalProvider(pref_provider);
   TestExternalProvider(pref_provider, Extension::EXTERNAL_PREF_DOWNLOAD);
 }
 
diff --git a/chrome/browser/extensions/external_extension_provider.h b/chrome/browser/extensions/external_extension_provider.h
index eddea62..2a0b967 100644
--- a/chrome/browser/extensions/external_extension_provider.h
+++ b/chrome/browser/extensions/external_extension_provider.h
@@ -46,11 +46,16 @@ class ExternalExtensionProvider {
   virtual void VisitRegisteredExtension(
       Visitor* visitor, const std::set<std::string>& ids_to_ignore) const = 0;
 
-  // Gets the version of extension with |id| and its |location|. |location| can
-  // be NULL. The caller is responsible for cleaning up the Version object
-  // returned. This function returns NULL if the extension is not found.
-  virtual Version* RegisteredVersion(const std::string& id,
-                                     Extension::Location* location) const = 0;
+  // Test if this provider has an extension with id |id| registered.
+  virtual bool HasExtension(const std::string& id) const = 0;
+
+  // Gets details of an extension by its id.  Output params will be set only
+  // if they are not NULL.  If an output parameter is not specified by the
+  // provider type, it will not be changed.
+  // This function is no longer used outside unit tests.
+  virtual bool GetExtensionDetails(const std::string& id,
+                                   Extension::Location* location,
+                                   scoped_ptr<Version>* version) const = 0;
 };
 
 #endif  // CHROME_BROWSER_EXTENSIONS_EXTERNAL_EXTENSION_PROVIDER_H_
diff --git a/chrome/browser/extensions/external_pref_extension_provider.cc b/chrome/browser/extensions/external_pref_extension_provider.cc
index 3eaafc9..4f4db41 100644
--- a/chrome/browser/extensions/external_pref_extension_provider.cc
+++ b/chrome/browser/extensions/external_pref_extension_provider.cc
@@ -121,19 +121,39 @@ void ExternalPrefExtensionProvider::VisitRegisteredExtension(
   }
 }
 
-Version* ExternalPrefExtensionProvider::RegisteredVersion(
-    const std::string& id, Extension::Location* location) const {
+bool ExternalPrefExtensionProvider::HasExtension(const std::string& id) const {
+  return prefs_->HasKey(id);
+}
+
+bool ExternalPrefExtensionProvider::GetExtensionDetails(
+    const std::string& id, Extension::Location* location,
+    scoped_ptr<Version>* version) const {
   DictionaryValue* extension = NULL;
   if (!prefs_->GetDictionary(id, &extension))
-    return NULL;
+    return false;
+
+  Extension::Location loc;
+  if (extension->HasKey(kExternalUpdateUrl)) {
+    loc = Extension::EXTERNAL_PREF_DOWNLOAD;
+
+  } else if (extension->HasKey(kExternalCrx)) {
+    loc = Extension::EXTERNAL_PREF;
+
+    std::string external_version;
+    if (!extension->GetString(kExternalVersion, &external_version))
+      return false;
 
-  std::string external_version;
-  if (!extension->GetString(kExternalVersion, &external_version))
-    return NULL;
+    if (version)
+      version->reset(Version::GetVersionFromString(external_version));
+
+  } else {
+    NOTREACHED();  // Chrome should not allow prefs to get into this state.
+  }
 
   if (location)
-    *location = Extension::EXTERNAL_PREF;
-  return Version::GetVersionFromString(external_version);
+    *location = loc;
+
+  return true;
 }
 
 void ExternalPrefExtensionProvider::SetPreferences(
diff --git a/chrome/browser/extensions/external_pref_extension_provider.h b/chrome/browser/extensions/external_pref_extension_provider.h
index aef6b17..9c54348 100644
--- a/chrome/browser/extensions/external_pref_extension_provider.h
+++ b/chrome/browser/extensions/external_pref_extension_provider.h
@@ -30,8 +30,11 @@ class ExternalPrefExtensionProvider : public ExternalExtensionProvider {
   virtual void VisitRegisteredExtension(
       Visitor* visitor, const std::set<std::string>& ids_to_ignore) const;
 
-  virtual Version* RegisteredVersion(const std::string& id,
-                                     Extension::Location* location) const;
+  virtual bool HasExtension(const std::string& id) const;
+
+  virtual bool GetExtensionDetails(const std::string& id,
+                                   Extension::Location* location,
+                                   scoped_ptr<Version>* version) const;
  protected:
   scoped_ptr<DictionaryValue> prefs_;
 
diff --git a/chrome/browser/extensions/external_registry_extension_provider_win.cc b/chrome/browser/extensions/external_registry_extension_provider_win.cc
index 02ae850..0a66ac0 100644
--- a/chrome/browser/extensions/external_registry_extension_provider_win.cc
+++ b/chrome/browser/extensions/external_registry_extension_provider_win.cc
@@ -10,6 +10,8 @@
 #include "base/version.h"
 #include "base/win/registry.h"
 
+namespace {
+
 // The Registry hive where to look for external extensions.
 const HKEY kRegRoot = HKEY_LOCAL_MACHINE;
 
@@ -22,6 +24,16 @@ const wchar_t kRegistryExtensionPath[] = L"path";
 // Registry value of that key that defines the current version of the .crx file.
 const wchar_t kRegistryExtensionVersion[] = L"version";
 
+bool OpenKeyById(const std::string& id, base::win::RegKey *key) {
+  std::wstring key_path = ASCIIToWide(kRegistryExtensions);
+  key_path.append(L"\\");
+  key_path.append(ASCIIToWide(id));
+
+  return key->Open(kRegRoot, key_path.c_str(), KEY_READ);
+}
+
+}  // namespace
+
 ExternalRegistryExtensionProvider::ExternalRegistryExtensionProvider() {
 }
 
@@ -75,22 +87,29 @@ void ExternalRegistryExtensionProvider::VisitRegisteredExtension(
   }
 }
 
-Version* ExternalRegistryExtensionProvider::RegisteredVersion(
-    const std::string& id,
-    Extension::Location* location) const  {
+
+bool ExternalRegistryExtensionProvider::HasExtension(
+    const std::string& id) const {
   base::win::RegKey key;
-  std::wstring key_path = ASCIIToWide(kRegistryExtensions);
-  key_path.append(L"\\");
-  key_path.append(ASCIIToWide(id));
+  return OpenKeyById(id, &key);
+}
 
-  if (!key.Open(kRegRoot, key_path.c_str(), KEY_READ))
-    return NULL;
+bool ExternalRegistryExtensionProvider::GetExtensionDetails(
+    const std::string& id,
+    Extension::Location* location,
+    scoped_ptr<Version>* version) const  {
+  base::win::RegKey key;
+  if (!OpenKeyById(id, &key))
+    return false;
 
   std::wstring extension_version;
   if (!key.ReadValue(kRegistryExtensionVersion, &extension_version))
-    return NULL;
+    return false;
+
+  if (version)
+    version->reset(Version::GetVersionFromString(extension_version));
 
   if (location)
     *location = Extension::EXTERNAL_REGISTRY;
-  return Version::GetVersionFromString(extension_version);
+  return true;
 }
diff --git a/chrome/browser/extensions/external_registry_extension_provider_win.h b/chrome/browser/extensions/external_registry_extension_provider_win.h
index b5b44bb..bb60106 100644
--- a/chrome/browser/extensions/external_registry_extension_provider_win.h
+++ b/chrome/browser/extensions/external_registry_extension_provider_win.h
@@ -24,8 +24,11 @@ class ExternalRegistryExtensionProvider : public ExternalExtensionProvider {
   virtual void VisitRegisteredExtension(
       Visitor* visitor, const std::set<std::string>& ids_to_ignore) const;
 
-  virtual Version* RegisteredVersion(const std::string& id,
-                                     Extension::Location* location) const;
+  virtual bool HasExtension(const std::string& id) const;
+
+  virtual bool GetExtensionDetails(const std::string& id,
+                                   Extension::Location* location,
+                                   scoped_ptr<Version>* version) const;
 };
 
 #endif  // CHROME_BROWSER_EXTENSIONS_EXTERNAL_REGISTRY_EXTENSION_PROVIDER_WIN_H_