Clean up threading model of external extension providers

chrome/browser/extensions/extension_service_unittest.cc

 // Copyright (c) 2010 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_unittest.h"
 
 #include <algorithm>
 #include <set>
 #include <vector>
 
@@ skipping 11 matching lines @@
 #include "base/task.h"
 #include "base/utf_string_conversions.h"
 #include "base/version.h"
 #include "chrome/browser/appcache/chrome_appcache_service.h"
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/extensions/extension_creator.h"
 #include "chrome/browser/extensions/extension_error_reporter.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/external_extension_provider.h"
-#include "chrome/browser/extensions/external_pref_extension_provider.h"
+#include "chrome/browser/extensions/external_extension_provider_impl.h"
+#include "chrome/browser/extensions/external_pref_extension_loader.h"
 #include "chrome/browser/extensions/pack_extension_job.cc"
 #include "chrome/browser/file_system/browser_file_system_helper.h"
 #include "chrome/browser/in_process_webkit/dom_storage_context.h"
 #include "chrome/browser/in_process_webkit/webkit_context.h"
 #include "chrome/browser/prefs/browser_prefs.h"
 #include "chrome/browser/prefs/pref_service_mock_builder.h"
 #include "chrome/browser/prefs/scoped_pref_update.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
@@ skipping 77 matching lines @@
   for (size_t i = 0; i < patterns2.size(); ++i)
     strings2.insert(patterns2.at(i).GetAsString());
 
   EXPECT_EQ(strings1, strings2);
 }
 
 }  // namespace
 
 class MockExtensionProvider : public ExternalExtensionProvider {
  public:
-  explicit MockExtensionProvider(Extension::Location location)
-    : location_(location), visit_count_(0) {}
+  explicit MockExtensionProvider(
+      Visitor* visitor,
+      Extension::Location location)
+  : location_(location), visitor_(visitor), visit_count_(0) {
+  }
   virtual ~MockExtensionProvider() {}
 
   void UpdateOrAddExtension(const std::string& id,
                             const std::string& version,
                             const FilePath& path) {
     extension_map_[id] = std::make_pair(version, path);
   }
 
   void RemoveExtension(const std::string& id) {
     extension_map_.erase(id);
   }
 
   // ExternalExtensionProvider implementation:
-  virtual void VisitRegisteredExtension(Visitor* visitor) const {
+  virtual void VisitRegisteredExtension() const {
     visit_count_++;
     for (DataMap::const_iterator i = extension_map_.begin();
          i != extension_map_.end(); ++i) {
       scoped_ptr<Version> version;
       version.reset(Version::GetVersionFromString(i->second.first));
 
-      visitor->OnExternalExtensionFileFound(
+      visitor_->OnExternalExtensionFileFound(
           i->first, version.get(), i->second.second, location_);
     }
+    visitor_->OnExternalProviderReady();
   }
 
   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 false;
 
     if (version)
       version->reset(Version::GetVersionFromString(it->second.first));
 
     if (location)
       *location = location_;
 
     return true;
   }
+
+  virtual bool IsReady() {
+    return true;
+  }
+
+  virtual void ServiceShutdown() {
+  }
+
   int visit_count() const { return visit_count_; }
   void set_visit_count(int visit_count) {
     visit_count_ = visit_count;
   }
 
  private:
   typedef std::map< std::string, std::pair<std::string, FilePath> > DataMap;
   DataMap extension_map_;
   Extension::Location location_;
+  Visitor* visitor_;
 
   // 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);
 };
 
 class MockProviderVisitor : public ExternalExtensionProvider::Visitor {
  public:
   MockProviderVisitor() : ids_found_(0) {
   }
 
   int Visit(const std::string& json_data) {
     // Give the test json file to the provider for parsing.
-    provider_.reset(new ExternalPrefExtensionProvider());
-    provider_->SetPreferencesForTesting(json_data);
+    provider_.reset(new ExternalExtensionProviderImpl(
+        this,
+        new ExternalTestingExtensionLoader(json_data),
+        Extension::EXTERNAL_PREF,
+        Extension::EXTERNAL_PREF_DOWNLOAD));
 
     // We also parse the file into a dictionary to compare what we get back
     // from the provider.
     JSONStringValueSerializer serializer(json_data);
     Value* json_value = serializer.Deserialize(NULL, NULL);
 
     if (!json_value || !json_value->IsType(Value::TYPE_DICTIONARY)) {
       NOTREACHED() << "Unable to deserialize json data";
       return -1;
     } else {
       DictionaryValue* external_extensions =
           static_cast<DictionaryValue*>(json_value);
       prefs_.reset(external_extensions);
     }
 
     // Reset our counter.
     ids_found_ = 0;
     // Ask the provider to look up all extensions and return them.
-    provider_->VisitRegisteredExtension(this);
+    provider_->VisitRegisteredExtension();
 
     return ids_found_;
   }
 
   virtual void OnExternalExtensionFileFound(const std::string& id,
                                             const Version* version,
                                             const FilePath& path,
                                             Extension::Location unused) {
     ++ids_found_;
     DictionaryValue* pref;
@@ skipping 45 matching lines @@
       Extension::Location location1 = Extension::INVALID;
       EXPECT_TRUE(provider_->GetExtensionDetails(id, &location1, &v1));
       EXPECT_FALSE(v1.get());
       EXPECT_EQ(Extension::EXTERNAL_PREF_DOWNLOAD, location1);
 
       // Remove it so we won't count it again.
       prefs_->Remove(id, NULL);
     }
   }
 
+  virtual void OnExternalProviderReady() {
+    EXPECT_TRUE(provider_->IsReady());
+  }
+
  private:
   int ids_found_;
 
-  scoped_ptr<ExternalPrefExtensionProvider> provider_;
+  scoped_ptr<ExternalExtensionProviderImpl> provider_;
   scoped_ptr<DictionaryValue> prefs_;
 
   DISALLOW_COPY_AND_ASSIGN(MockProviderVisitor);
 };
 
 class ExtensionTestingProfile : public TestingProfile {
  public:
   ExtensionTestingProfile() : service_(NULL) {
   }
 
@@ skipping 761 matching lines @@
 // Test the handling of killed extensions.
 TEST_F(ExtensionServiceTest, KilledExtensions) {
   InitializeEmptyExtensionService();
 
   FilePath extensions_path;
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path));
   extensions_path = extensions_path.AppendASCII("extensions");
   FilePath path = extensions_path.AppendASCII("good.crx");
   set_extensions_enabled(true);
 
+  scoped_ptr<Version> version;
+  version.reset(Version::GetVersionFromString("1.0.0.0"));
   // Install an external extension.
-  service_->OnExternalExtensionFileFound(good_crx, "1.0.0.0",
+  service_->OnExternalExtensionFileFound(good_crx, version.get(),
                                          path, Extension::EXTERNAL_PREF);
   loop_.RunAllPending();
   ASSERT_TRUE(NULL != service_->GetExtensionById(good_crx, false));
 
   // Uninstall it and check that its killbit gets set.
   service_->UninstallExtension(good_crx, false);
   loop_.RunAllPending();
   ValidateIntegerPref(good_crx, "location", Extension::KILLBIT);
 
   // Try to re-install it externally. This should fail because of the killbit.
-  service_->OnExternalExtensionFileFound(good_crx, "1.0.0.0",
+  service_->OnExternalExtensionFileFound(good_crx, version.get(),
                                          path, Extension::EXTERNAL_PREF);
   loop_.RunAllPending();
   ASSERT_TRUE(NULL == service_->GetExtensionById(good_crx, false));
   ValidateIntegerPref(good_crx, "location", Extension::KILLBIT);
 
+  version.reset(Version::GetVersionFromString("1.0.0.1"));
   // Repeat the same thing with a newer version of the extension.
   path = extensions_path.AppendASCII("good2.crx");
-  service_->OnExternalExtensionFileFound(good_crx, "1.0.0.1",
+  service_->OnExternalExtensionFileFound(good_crx, version.get(),
                                          path, Extension::EXTERNAL_PREF);
   loop_.RunAllPending();
   ASSERT_TRUE(NULL == service_->GetExtensionById(good_crx, false));
   ValidateIntegerPref(good_crx, "location", Extension::KILLBIT);
 
   // Try adding the same extension from an external update URL.
   service_->AddPendingExtensionFromExternalUpdateUrl(
       good_crx,
       GURL("http:://fake.update/url"),
       Extension::EXTERNAL_PREF_DOWNLOAD);
@@ skipping 1750 matching lines @@
   ASSERT_EQ(1u, loaded_.size());
   ValidatePrefKeyCount(1);
   ValidateIntegerPref(good_crx, "state", Extension::ENABLED);
   ValidateIntegerPref(good_crx, "location", location);
 
   // Now test an externally triggered uninstall (deleting the registry key or
   // the pref entry).
   provider->RemoveExtension(good_crx);
 
   loaded_.clear();
-  service_->UnloadAllExtensions();
-  service_->LoadAllExtensions();
+  service_->OnExternalProviderReady();
   loop_.RunAllPending();
   ASSERT_EQ(0u, loaded_.size());
   ValidatePrefKeyCount(0);
 
   // The extension should also be gone from the install directory.
   ASSERT_FALSE(file_util::PathExists(install_path));
 
   // Now test the case where user uninstalls and then the extension is removed
   // from the external provider.
 
@@ skipping 25 matching lines @@
 
 // Tests the external installation feature
 #if defined(OS_WIN)
 TEST_F(ExtensionServiceTest, ExternalInstallRegistry) {
   // This should all work, even when normal extension installation is disabled.
   InitializeEmptyExtensionService();
   set_extensions_enabled(false);
 
   // Now add providers. Extension system takes ownership of the objects.
   MockExtensionProvider* reg_provider =
-      new MockExtensionProvider(Extension::EXTERNAL_REGISTRY);
+      new MockExtensionProvider(service_.get(), Extension::EXTERNAL_REGISTRY);
   AddMockExternalProvider(reg_provider);
   TestExternalProvider(reg_provider, Extension::EXTERNAL_REGISTRY);
 }
 #endif
 
 TEST_F(ExtensionServiceTest, ExternalInstallPref) {
   InitializeEmptyExtensionService();
 
   // Now add providers. Extension system takes ownership of the objects.
   MockExtensionProvider* pref_provider =
-      new MockExtensionProvider(Extension::EXTERNAL_PREF);
+      new MockExtensionProvider(service_.get(), Extension::EXTERNAL_PREF);
 
   AddMockExternalProvider(pref_provider);
   TestExternalProvider(pref_provider, Extension::EXTERNAL_PREF);
 }
 
 TEST_F(ExtensionServiceTest, ExternalInstallPrefUpdateUrl) {
   // This should all work, even when normal extension installation is disabled.
   InitializeEmptyExtensionService();
   set_extensions_enabled(false);
 
   // 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);
+      new MockExtensionProvider(service_.get(),
+                                Extension::EXTERNAL_PREF_DOWNLOAD);
   AddMockExternalProvider(pref_provider);
   TestExternalProvider(pref_provider, Extension::EXTERNAL_PREF_DOWNLOAD);
 }
 
 // Tests that external extensions get uninstalled when the external extension
 // providers can't account for them.
 TEST_F(ExtensionServiceTest, ExternalUninstall) {
   // Start the extensions service with one external extension already installed.
   FilePath source_install_dir;
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &source_install_dir));
@@ skipping 159 matching lines @@
   BrowserThread ui_thread(BrowserThread::UI, &loop);
   BrowserThread file_thread(BrowserThread::FILE, &loop);
   scoped_ptr<CommandLine> command_line;
   scoped_refptr<ExtensionService> service;
   FilePath install_dir = profile->GetPath()
       .AppendASCII(ExtensionService::kInstallDirectoryName);
 
   // By default, we are enabled.
   command_line.reset(new CommandLine(CommandLine::NO_PROGRAM));
   service = profile->CreateExtensionService(command_line.get(),
-                                             install_dir);
+                                            install_dir);
   EXPECT_TRUE(service->extensions_enabled());
   service->Init();
   loop.RunAllPending();
   EXPECT_TRUE(recorder.ready());
 
   // If either the command line or pref is set, we are disabled.
   recorder.set_ready(false);
   profile.reset(new TestingProfile());
   command_line->AppendSwitch(switches::kDisableExtensions);
   service = profile->CreateExtensionService(command_line.get(),
                                              install_dir);
   EXPECT_FALSE(service->extensions_enabled());
   service->Init();
   loop.RunAllPending();
   EXPECT_TRUE(recorder.ready());
 
   recorder.set_ready(false);
   profile.reset(new TestingProfile());
   profile->GetPrefs()->SetBoolean(prefs::kDisableExtensions, true);
   service = profile->CreateExtensionService(command_line.get(),
-                                             install_dir);
+                                            install_dir);
   EXPECT_FALSE(service->extensions_enabled());
   service->Init();
   loop.RunAllPending();
   EXPECT_TRUE(recorder.ready());
 
   recorder.set_ready(false);
   profile.reset(new TestingProfile());
   profile->GetPrefs()->SetBoolean(prefs::kDisableExtensions, true);
   command_line.reset(new CommandLine(CommandLine::NO_PROGRAM));
   service = profile->CreateExtensionService(command_line.get(),
@@ skipping 102 matching lines @@
   // Component extensions shouldn't get recourded in the prefs.
   ValidatePrefKeyCount(0);
 
   // Reload all extensions, and make sure it comes back.
   std::string extension_id = service_->extensions()->at(0)->id();
   loaded_.clear();
   service_->ReloadExtensions();
   ASSERT_EQ(1u, service_->extensions()->size());
   EXPECT_EQ(extension_id, service_->extensions()->at(0)->id());
 }