Clean up threading model of external extension providers

chrome/browser/extensions/external_extension_provider_impl.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/stateful_external_extension_provider.h"
+#include "chrome/browser/extensions/external_extension_provider_impl.h"
 
 #include "app/app_paths.h"
 #include "base/file_path.h"
 #include "base/logging.h"
+#include "base/linked_ptr.h"
 #include "base/path_service.h"
 #include "base/values.h"
 #include "base/version.h"
 #include "chrome/browser/browser_thread.h"
+#include "chrome/browser/extensions/external_extension_provider.h"
+#include "chrome/browser/extensions/external_policy_extension_loader.h"
+#include "chrome/browser/extensions/external_pref_extension_loader.h"
+#include "chrome/browser/profiles/profile.h"
 
-namespace {
+#if defined(OS_WIN)
+#include "chrome/browser/extensions/external_registry_extension_loader_win.h"
+#endif
 
-// 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";
+// Constants for keeping track of extension preferences in a dictionary.
+const char ExternalExtensionProviderImpl::kLocation[] = "location";
+const char ExternalExtensionProviderImpl::kState[] = "state";
+const char ExternalExtensionProviderImpl::kExternalCrx[] = "external_crx";
+const char ExternalExtensionProviderImpl::kExternalVersion[] =
+    "external_version";
+const char ExternalExtensionProviderImpl::kExternalUpdateUrl[] =
+    "external_update_url";
 
-}
-
-StatefulExternalExtensionProvider::StatefulExternalExtensionProvider(
+ExternalExtensionProviderImpl::ExternalExtensionProviderImpl(
+    Visitor* service,
+    ExternalExtensionLoader* loader,
     Extension::Location crx_location,
     Extension::Location download_location)
   : crx_location_(crx_location),
-    download_location_(download_location) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    download_location_(download_location),
+    service_(service),
+    prefs_(NULL),
+    ready_(false),
+    loader_(loader) {
+  loader_->Init(this);
 }
 
-StatefulExternalExtensionProvider::~StatefulExternalExtensionProvider() {
+ExternalExtensionProviderImpl::~ExternalExtensionProviderImpl() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  loader_->OwnerShutdown();
 }
 
-void StatefulExternalExtensionProvider::VisitRegisteredExtension(
-    Visitor* visitor) const {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-  DCHECK(prefs_.get());
+void ExternalExtensionProviderImpl::VisitRegisteredExtension() const {
+  loader_->StartLoading();
+}
+
+void ExternalExtensionProviderImpl::SetPrefs(DictionaryValue* prefs) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  prefs_.reset(prefs);
+  if (!service_) return;

[Sam Kerner (Chrome), 2011/01/04 14:06:00]

Can this happen?  I see two ways:  User called ServiceShutdown(), or passed
service=NULL in the constructor.  Both seem like the sort of misuse that should
trigger a CHECK.  Is there another way I missed?

[gfeher, 2011/01/04 23:37:09]

It is possible that ExtensionService calls ServiceShutdown while |loader_| is
working on the FILE thread. The loader will call |SetPrefs| after it has
finished with it's stuff.

+
+  ready_ = true;
   for (DictionaryValue::key_iterator i = prefs_->begin_keys();
        i != prefs_->end_keys(); ++i) {
     const std::string& extension_id = *i;
     DictionaryValue* extension;
     if (!prefs_->GetDictionaryWithoutPathExpansion(extension_id, &extension))
       continue;
 
     FilePath::StringType external_crx;
     std::string external_version;
     std::string external_update_url;
@@ skipping 41 matching lines @@
       }
 
       scoped_ptr<Version> version;
       version.reset(Version::GetVersionFromString(external_version));
       if (!version.get()) {
         LOG(WARNING) << "Malformed extension dictionary for extension: "
                      << extension_id.c_str() << ".  Invalid version string \""
                      << external_version << "\".";
         continue;
       }
-      visitor->OnExternalExtensionFileFound(extension_id, version.get(), path,
+      service_->OnExternalExtensionFileFound(extension_id, version.get(), path,
                                             crx_location_);
     } else { // if (has_external_update_url)
       DCHECK(has_external_update_url);  // Checking of keys above ensures this.
       if (download_location_ == Extension::INVALID) {
         LOG(WARNING) << "This provider does not support installing external "
                      << "extensions from update URLs.";
         continue;
       }
       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.  Saw \"" << external_update_url
                      << "\".";
         continue;
       }
-      visitor->OnExternalExtensionUpdateUrlFound(
+      service_->OnExternalExtensionUpdateUrlFound(
           extension_id, update_url, download_location_);
     }
   }
+  ready_ = true;
+  service_->OnExternalProviderReady();
 }
 
-bool StatefulExternalExtensionProvider::HasExtension(
+void ExternalExtensionProviderImpl::ServiceShutdown() {
+  service_ = NULL;
+}
+
+bool ExternalExtensionProviderImpl::IsReady() {
+  return ready_;
+}
+
+bool ExternalExtensionProviderImpl::HasExtension(
     const std::string& id) const {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(prefs_.get());
+  DCHECK(ready_);
   return prefs_->HasKey(id);
 }
 
-bool StatefulExternalExtensionProvider::GetExtensionDetails(
+bool ExternalExtensionProviderImpl::GetExtensionDetails(
     const std::string& id, Extension::Location* location,
     scoped_ptr<Version>* version) const {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(prefs_.get());
+  DCHECK(ready_);
   DictionaryValue* extension = NULL;
   if (!prefs_->GetDictionary(id, &extension))
     return false;
 
   Extension::Location loc = Extension::INVALID;
   if (extension->HasKey(kExternalUpdateUrl)) {
     loc = download_location_;
 
   } else if (extension->HasKey(kExternalCrx)) {
     loc = crx_location_;
 
     std::string external_version;
     if (!extension->GetString(kExternalVersion, &external_version))
       return false;
 
     if (version)
       version->reset(Version::GetVersionFromString(external_version));
 
   } else {
     NOTREACHED();  // Chrome should not allow prefs to get into this state.
     return false;
   }
 
   if (location)
     *location = loc;
 
   return true;
 }
 
-void StatefulExternalExtensionProvider::set_prefs(DictionaryValue* prefs) {
-  prefs_.reset(prefs);
+// static
+void ExternalExtensionProviderImpl::CreateExternalProviders(
+    Visitor* service,
+    Profile* profile,
+    ProviderCollection* provider_list) {
+  provider_list->push_back(
+      linked_ptr<ExternalExtensionProvider>(
+          new ExternalExtensionProviderImpl(
+              service,
+              new ExternalPrefExtensionLoader,
+              Extension::EXTERNAL_PREF,
+              Extension::EXTERNAL_PREF_DOWNLOAD)));
+#if defined(OS_WIN)
+  provider_list->push_back(
+      linked_ptr<ExternalExtensionProvider>(
+          new ExternalExtensionProviderImpl(
+              service,
+              new ExternalRegistryExtensionLoader,
+              Extension::EXTERNAL_REGISTRY,
+              Extension::INVALID)));
+#endif
+  provider_list->push_back(
+      linked_ptr<ExternalExtensionProvider>(
+          new ExternalExtensionProviderImpl(
+              service,
+              new ExternalPolicyExtensionLoader(profile),
+              Extension::INVALID,
+              Extension::EXTERNAL_POLICY_DOWNLOAD)));
 }