Clean up extension loading:

* load extensions from prefs rather than by scanning filesystem
* fix multiple loading bug
* in-place upgrade
* split out Init() behavior into individual pieces that can be called by tests

Also:
* add look up of extension by URL
* rename GetExtensionByID -> GetExtensionById

BUG=12399
BUG=14053
TEST=ExtensionServiceTest.*

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

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

8 files changed, 365 insertions, 303 deletions

diff --git a/chrome/browser/extensions/extension_page_actions_module.cc b/chrome/browser/extensions/extension_page_actions_module.cc
index 5b128ea..ffc5aa2 100644
--- a/chrome/browser/extensions/extension_page_actions_module.cc
+++ b/chrome/browser/extensions/extension_page_actions_module.cc
@@ -50,7 +50,7 @@ bool EnablePageActionFunction::RunImpl() {
   // Find our extension.
   Extension* extension = NULL;
   ExtensionsService* service = profile()->GetExtensionsService();
-  extension = service->GetExtensionByID(extension_id());
+  extension = service->GetExtensionById(extension_id());
   if (!extension) {
     error_ = ExtensionErrorUtils::FormatErrorMessage(keys::kNoExtensionError,
                                                      extension_id());
diff --git a/chrome/browser/extensions/extension_process_manager.cc b/chrome/browser/extensions/extension_process_manager.cc
index dc068e6..a4f2df7 100644
--- a/chrome/browser/extensions/extension_process_manager.cc
+++ b/chrome/browser/extensions/extension_process_manager.cc
@@ -55,6 +55,19 @@ ExtensionHost* ExtensionProcessManager::CreateView(Extension* extension,
   return host;
 }
 
+ExtensionHost* ExtensionProcessManager::CreateView(const GURL& url,
+                                                   Browser* browser) {
+  DCHECK(browser);
+  ExtensionsService* service =
+    browsing_instance_->profile()->GetExtensionsService();
+  if (service) {
+    Extension* extension = service->GetExtensionByURL(url);
+    if (extension)
+      return CreateView(extension, url, browser);
+  }
+  return NULL;
+}
+
 ExtensionHost* ExtensionProcessManager::CreateBackgroundHost(
     Extension* extension, const GURL& url) {
   ExtensionHost* host =
diff --git a/chrome/browser/extensions/extension_process_manager.h b/chrome/browser/extensions/extension_process_manager.h
index f12b49e..9cd861c 100644
--- a/chrome/browser/extensions/extension_process_manager.h
+++ b/chrome/browser/extensions/extension_process_manager.h
@@ -34,6 +34,7 @@ class ExtensionProcessManager : public NotificationObserver {
   ExtensionHost* CreateView(Extension* extension,
                             const GURL& url,
                             Browser* browser);
+  ExtensionHost* CreateView(const GURL& url, Browser* browser);
 
   // Creates a new UI-less extension instance.  Like CreateView, but not
   // displayed anywhere.
diff --git a/chrome/browser/extensions/extension_tabs_module.cc b/chrome/browser/extensions/extension_tabs_module.cc
index 2eb2432..b92677d 100644
--- a/chrome/browser/extensions/extension_tabs_module.cc
+++ b/chrome/browser/extensions/extension_tabs_module.cc
@@ -672,7 +672,7 @@ static Browser* GetBrowserInProfileWithId(Profile* profile,
 static GURL AbsolutePath(Profile* profile, std::string extension_id,
                          std::string relative_url) {
   ExtensionsService* service = profile->GetExtensionsService();
-  Extension* extension = service->GetExtensionByID(extension_id);
+  Extension* extension = service->GetExtensionById(extension_id);
   return Extension::GetResourceURL(extension->url(), relative_url);
 }
 
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index aae153b..5d76b4d 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -60,13 +60,23 @@ const char* ExtensionsServiceBackend::kTempExtensionName = "TEMP_INSTALL";
 
 namespace {
 
-// A preference that keeps track of extension settings. This is a dictionary
+////////////////////
+// Preferences keys
+
+// A preference that keeps track of per-extension settings. This is a dictionary
 // object read from the Preferences file, keyed off of extension id's.
-const wchar_t kExternalExtensionsPref[] = L"extensions.settings";
+const wchar_t kExtensionsPref[] = L"extensions.settings";
+
+// Where an extension was installed from (Extension::Location)
+const wchar_t kPrefLocation[] = L"location";
+
+// Enabled, disabled, killed, etc. (Extension::State)
+const wchar_t kPrefState[] = L"state";
 
-// A preference keeping track of how the extension was installed.
-const wchar_t kLocation[] = L"location";
-const wchar_t kState[] = L"state";
+// The path to the current version's manifest file.
+const wchar_t kPrefPath[] = L"path";
+
+////////////////////
 
 // A temporary subdirectory where we unpack extensions.
 const char* kUnpackExtensionDir = "TEMP_UNPACK";
@@ -93,11 +103,9 @@ class ExtensionsServiceBackend::UnpackerClient
   UnpackerClient(ExtensionsServiceBackend* backend,
                  const FilePath& extension_path,
                  const std::string& public_key,
-                 const std::string& expected_id,
-                 bool from_external)
+                 const std::string& expected_id)
     : backend_(backend), extension_path_(extension_path),
-      public_key_(public_key), expected_id_(expected_id),
-      from_external_(from_external), got_response_(false) {
+      public_key_(public_key), expected_id_(expected_id), got_response_(false) {
   }
 
   // Starts the unpack task.  We call back to the backend when the task is done,
@@ -174,8 +182,7 @@ class ExtensionsServiceBackend::UnpackerClient
     FilePath extension_dir = temp_extension_path_.DirName().AppendASCII(
         ExtensionsServiceBackend::kTempExtensionName);
     backend_->OnExtensionUnpacked(extension_path_, extension_dir,
-                                  expected_id_, from_external_,
-                                  manifest, images);
+                                  expected_id_, manifest, images);
     Cleanup();
   }
 
@@ -216,9 +223,6 @@ class ExtensionsServiceBackend::UnpackerClient
   // The ID we expect this extension to have, if any.
   std::string expected_id_;
 
-  // True if this is being installed from an external source.
-  bool from_external_;
-
   // True if we got a response from the utility process and have cleaned up
   // already.
   bool got_response_;
@@ -234,9 +238,11 @@ ExtensionsService::ExtensionsService(Profile* profile,
           CommandLine::ForCurrentProcess()->
           HasSwitch(switches::kEnableExtensions)),
       show_extensions_prompts_(true) {
+  if (!prefs_->FindPreference(kExtensionsPref))
+    prefs_->RegisterDictionaryPref(kExtensionsPref);
+
   // We pass ownership of this object to the Backend.
   DictionaryValue* external_extensions = new DictionaryValue;
-  prefs_->RegisterDictionaryPref(kExternalExtensionsPref);
   GetExternalExtensions(external_extensions, NULL);
   backend_ = new ExtensionsServiceBackend(
           install_directory_, g_browser_process->resource_dispatcher_host(),
@@ -244,46 +250,90 @@ ExtensionsService::ExtensionsService(Profile* profile,
 }
 
 ExtensionsService::~ExtensionsService() {
-  for (ExtensionList::iterator iter = extensions_.begin();
-       iter != extensions_.end(); ++iter) {
-    delete *iter;
-  }
+  UnloadAllExtensions();
 }
 
-bool ExtensionsService::Init() {
+void ExtensionsService::Init() {
+  DCHECK(extensions_.size() == 0);
+
   // Start up the extension event routers.
   ExtensionBrowserEventRouter::GetInstance()->Init();
 
-  scoped_ptr<DictionaryValue> external_extensions(new DictionaryValue);
-  GetExternalExtensions(external_extensions.get(), NULL);
+  LoadAllExtensions();
 
-  scoped_ptr< std::set<std::string> >
-      killed_extensions(new std::set<std::string>);
-  GetExternalExtensions(NULL, killed_extensions.get());
+  // TODO(erikkay) this should probably be deferred to a future point
+  // rather than running immediately at startup.
+  CheckForUpdates();
 
+  // TODO(erikkay) this should probably be deferred as well.
+  GarbageCollectExtensions();
+}
+
+void ExtensionsService::InstallExtension(const FilePath& extension_path) {
   backend_loop_->PostTask(FROM_HERE, NewRunnableMethod(backend_.get(),
-      &ExtensionsServiceBackend::CheckForExternalUpdates,
-      *killed_extensions.get(),
+      &ExtensionsServiceBackend::InstallExtension,
+      extension_path,
       scoped_refptr<ExtensionsService>(this)));
+}
+
+void ExtensionsService::UninstallExtension(const std::string& extension_id) {
+  Extension* extension = GetExtensionById(extension_id);
 
+  // Callers should not send us nonexistant extensions.
+  CHECK(extension);
+
+  Extension::Location location = extension->location();
+
+  // For external extensions, we save a preference reminding ourself not to try
+  // and install the extension anymore.
+  if (Extension::IsExternalLocation(location)) {
+    UpdateExtensionPref(ASCIIToWide(extension_id), kPrefState,
+                        Value::CreateIntegerValue(Extension::KILLBIT), true);
+  } else {
+    DeleteExtensionPrefs(ASCIIToWide(extension_id));
+  }
+
+  // Tell the backend to start deleting installed extensions on the file thread.
+  if (location == Extension::INTERNAL ||
+      Extension::IsExternalLocation(location)) {
+    backend_loop_->PostTask(FROM_HERE, NewRunnableMethod(backend_.get(),
+        &ExtensionsServiceBackend::UninstallExtension, extension_id));
+  }
+
+  UnloadExtension(extension_id);
+}
+
+void ExtensionsService::LoadExtension(const FilePath& extension_path) {
   backend_loop_->PostTask(FROM_HERE, NewRunnableMethod(backend_.get(),
-      &ExtensionsServiceBackend::LoadExtensionsFromInstallDirectory,
-      scoped_refptr<ExtensionsService>(this),
-      external_extensions.release()));
+      &ExtensionsServiceBackend::LoadSingleExtension,
+      extension_path, scoped_refptr<ExtensionsService>(this)));
+}
 
-  return true;
+void ExtensionsService::LoadAllExtensions() {
+  // Load the extensions we know about from previous runs.
+  const DictionaryValue* extensions = prefs_->GetDictionary(kExtensionsPref);
+  if (extensions) {
+    DictionaryValue* copy =
+        static_cast<DictionaryValue*>(extensions->DeepCopy());
+    backend_loop_->PostTask(FROM_HERE, NewRunnableMethod(backend_.get(),
+        &ExtensionsServiceBackend::LoadExtensionsFromPrefs,
+        scoped_refptr<ExtensionsService>(this),
+        copy));
+  }
 }
 
-void ExtensionsService::InstallExtension(const FilePath& extension_path) {
+void ExtensionsService::CheckForUpdates() {
+  // This installs or updates externally provided extensions.
+  std::set<std::string> killed_extensions;
+  GetExternalExtensions(NULL, &killed_extensions);
   backend_loop_->PostTask(FROM_HERE, NewRunnableMethod(backend_.get(),
-      &ExtensionsServiceBackend::InstallExtension,
-      extension_path,
+      &ExtensionsServiceBackend::CheckForExternalUpdates,
+      killed_extensions,
       scoped_refptr<ExtensionsService>(this)));
 }
 
-void ExtensionsService::UninstallExtension(const std::string& extension_id) {
+void ExtensionsService::UnloadExtension(const std::string& extension_id) {
   Extension* extension = NULL;
-
   ExtensionList::iterator iter;
   for (iter = extensions_.begin(); iter != extensions_.end(); ++iter) {
     if ((*iter)->id() == extension_id) {
@@ -303,33 +353,35 @@ void ExtensionsService::UninstallExtension(const std::string& extension_id) {
                                          NotificationService::AllSources(),
                                          Details<Extension>(extension));
 
-  // For external extensions, we save a preference reminding ourself not to try
-  // and install the extension anymore.
-  if (Extension::IsExternalLocation(extension->location())) {
-    UpdateExtensionPref(ASCIIToWide(extension->id()), kState,
-                        Value::CreateIntegerValue(Extension::KILLBIT), true);
-  } else {
-    UpdateExtensionPref(ASCIIToWide(extension->id()), kState,
-                        Value::CreateIntegerValue(Extension::DISABLED), true);
-  }
+  delete extension;
+}
 
-  // Tell the backend to start deleting installed extensions on the file thread.
-  if (extension->location() == Extension::INTERNAL ||
-      Extension::IsExternalLocation(extension->location())) {
-    backend_loop_->PostTask(FROM_HERE, NewRunnableMethod(backend_.get(),
-        &ExtensionsServiceBackend::UninstallExtension, extension_id));
+void ExtensionsService::UnloadAllExtensions() {
+  ExtensionList::iterator iter;
+  for (iter = extensions_.begin(); iter != extensions_.end(); ++iter) {
+    // Tell other services the extension is gone.
+    NotificationService::current()->Notify(NotificationType::EXTENSION_UNLOADED,
+                                           NotificationService::AllSources(),
+                                           Details<Extension>(*iter));
+    delete *iter;
   }
+  extensions_.clear();
+}
 
-  delete extension;
+void ExtensionsService::ReloadExtensions() {
+  UnloadAllExtensions();
+  LoadAllExtensions();
 }
 
-void ExtensionsService::LoadExtension(const FilePath& extension_path) {
+void ExtensionsService::GarbageCollectExtensions() {
   backend_loop_->PostTask(FROM_HERE, NewRunnableMethod(backend_.get(),
-      &ExtensionsServiceBackend::LoadSingleExtension,
-      extension_path, scoped_refptr<ExtensionsService>(this)));
+      &ExtensionsServiceBackend::GarbageCollectExtensions,
+      scoped_refptr<ExtensionsService>(this)));
 }
 
 void ExtensionsService::OnExtensionsLoaded(ExtensionList* new_extensions) {
+  scoped_ptr<ExtensionList> cleanup(new_extensions);
+
   // Filter out any extensions we don't want to enable. Themes are always
   // enabled, but other extensions are only loaded if --enable-extensions is
   // present.
@@ -337,7 +389,22 @@ void ExtensionsService::OnExtensionsLoaded(ExtensionList* new_extensions) {
   for (ExtensionList::iterator iter = new_extensions->begin();
        iter != new_extensions->end(); ++iter) {
     if (extensions_enabled() || (*iter)->IsTheme()) {
+      Extension* old = GetExtensionById((*iter)->id());
+      if (old) {
+        if ((*iter)->version()->CompareTo(*(old->version())) > 0) {
+          // To upgrade an extension in place, unload the old one and
+          // then load the new one.
+          // TODO(erikkay) issue 12399
+          UnloadExtension(old->id());
+        } else {
+          // We already have the extension of the same or older version.
+          LOG(WARNING) << "Duplicate extension load attempt: " << (*iter)->id();
+          delete *iter;
+          continue;
+        }
+      }
       enabled_extensions.push_back(*iter);
+      extensions_.push_back(*iter);
     } else {
       // Extensions that get enabled get added to extensions_ and deleted later.
       // Anything skipped must be deleted now so we don't leak.
@@ -345,48 +412,28 @@ void ExtensionsService::OnExtensionsLoaded(ExtensionList* new_extensions) {
     }
   }
 
-  for (ExtensionList::const_iterator iter = enabled_extensions.begin();
-       iter != enabled_extensions.end(); ++iter) {
-    std::wstring extension_id = ASCIIToWide((*iter)->id());
-    DictionaryValue* pref = GetOrCreateExtensionPref(extension_id);
-    int location;
-    int state;
-
-    // Ensure all loaded extensions have a preference set. This deals with a
-    // legacy problem where some extensions were installed before we were
-    // storing state in the preferences.
-    // TODO(aa): We should remove this eventually.
-    if (!pref->GetInteger(kLocation, &location) ||
-        !pref->GetInteger(kState, &state)) {
-      UpdateExtensionPref(extension_id,
-          kLocation, Value::CreateIntegerValue(Extension::INTERNAL), false);
-      UpdateExtensionPref(extension_id,
-          kState, Value::CreateIntegerValue(Extension::ENABLED), false);
-    } else {
-      // Sanity check: The kill-bit should only ever be set on external
-      // extensions.
-      DCHECK(state != Extension::KILLBIT ||
-             Extension::IsExternalLocation(
-                static_cast<Extension::Location>(location)));
-    }
-
-    extensions_.push_back(*iter);
-  }
+  // TODO(erikkay) it would be nice if we could just return and send no
+  // notification if the list is empty.  However, UserScriptMaster depends on
+  // getting an initial notification after the first extensions have finished
+  // loading, so even if it's an empty message, we send it anyway.  We should
+  // come up with a way to do a "started" notification that has the semantics
+  // that UserScriptMaster is looking for.
 
   NotificationService::current()->Notify(
       NotificationType::EXTENSIONS_LOADED,
       NotificationService::AllSources(),
       Details<ExtensionList>(&enabled_extensions));
-
-  delete new_extensions;
 }
 
 void ExtensionsService::OnExtensionInstalled(Extension* extension,
     Extension::InstallType install_type) {
-  UpdateExtensionPref(ASCIIToWide(extension->id()), kState,
+  std::wstring id = ASCIIToWide(extension->id());
+  UpdateExtensionPref(id, kPrefState,
                       Value::CreateIntegerValue(Extension::ENABLED), false);
-  UpdateExtensionPref(ASCIIToWide(extension->id()), kLocation,
-                      Value::CreateIntegerValue(Extension::INTERNAL), true);
+  UpdateExtensionPref(id, kPrefLocation,
+                      Value::CreateIntegerValue(extension->location()), false);
+  UpdateExtensionPref(id, kPrefPath,
+      Value::CreateStringValue(extension->path().value()), true);
 
   // If the extension is a theme, tell the profile (and therefore ThemeProvider)
   // to apply it.
@@ -403,17 +450,8 @@ void ExtensionsService::OnExtensionInstalled(Extension* extension,
   }
 }
 
-void ExtensionsService::OnExternalExtensionInstalled(
-    const std::string& id, Extension::Location location) {
-  DCHECK(Extension::IsExternalLocation(location));
-  UpdateExtensionPref(ASCIIToWide(id), kState,
-                      Value::CreateIntegerValue(Extension::ENABLED), false);
-  UpdateExtensionPref(ASCIIToWide(id), kLocation,
-                      Value::CreateIntegerValue(location), true);
-}
-
 void ExtensionsService::OnExtensionOverinstallAttempted(const std::string& id) {
-  Extension* extension = GetExtensionByID(id);
+  Extension* extension = GetExtensionById(id);
   if (extension && extension->IsTheme()) {
     NotificationService::current()->Notify(
         NotificationType::THEME_INSTALLED,
@@ -422,7 +460,7 @@ void ExtensionsService::OnExtensionOverinstallAttempted(const std::string& id) {
   }
 }
 
-Extension* ExtensionsService::GetExtensionByID(std::string id) {
+Extension* ExtensionsService::GetExtensionById(const std::string& id) {
   for (ExtensionList::const_iterator iter = extensions_.begin();
       iter != extensions_.end(); ++iter) {
     if ((*iter)->id() == id)
@@ -431,10 +469,15 @@ Extension* ExtensionsService::GetExtensionByID(std::string id) {
   return NULL;
 }
 
+Extension* ExtensionsService::GetExtensionByURL(const GURL& url) {
+  std::string host = url.host();
+  return GetExtensionById(host);
+}
+
 void ExtensionsService::GetExternalExtensions(
     DictionaryValue* external_extensions,
     std::set<std::string>* killed_extensions) {
-  const DictionaryValue* dict = prefs_->GetDictionary(kExternalExtensionsPref);
+  const DictionaryValue* dict = prefs_->GetDictionary(kExtensionsPref);
   if (!dict || dict->GetSize() == 0)
     return;
 
@@ -455,7 +498,7 @@ void ExtensionsService::GetExternalExtensions(
 
     // Check to see if the extension has been killed.
     int state;
-    if (extension->GetInteger(kState, &state) &&
+    if (extension->GetInteger(kPrefState, &state) &&
         state == static_cast<int>(Extension::KILLBIT)) {
       if (killed_extensions) {
         StringToLowerASCII(&key_name);
@@ -474,7 +517,7 @@ void ExtensionsService::GetExternalExtensions(
 
 DictionaryValue* ExtensionsService::GetOrCreateExtensionPref(
     const std::wstring& extension_id) {
-  DictionaryValue* dict = prefs_->GetMutableDictionary(kExternalExtensionsPref);
+  DictionaryValue* dict = prefs_->GetMutableDictionary(kExtensionsPref);
   DictionaryValue* extension = NULL;
   if (!dict->GetDictionary(extension_id, &extension)) {
     // Extension pref does not exist, create it.
@@ -513,6 +556,16 @@ bool ExtensionsService::UpdateExtensionPref(const std::wstring& extension_id,
   return true;
 }
 
+void ExtensionsService::DeleteExtensionPrefs(
+    const std::wstring& extension_id) {
+  DictionaryValue* dict = prefs_->GetMutableDictionary(kExtensionsPref);
+  if (dict->HasKey(extension_id)) {
+    dict->Remove(extension_id, NULL);
+    prefs_->ScheduleSavePersistentPrefs();
+  }
+}
+
+
 // ExtensionsServicesBackend
 
 ExtensionsServiceBackend::ExtensionsServiceBackend(
@@ -536,37 +589,66 @@ ExtensionsServiceBackend::~ExtensionsServiceBackend() {
                                        external_extension_providers_.end());
 }
 
-void ExtensionsServiceBackend::LoadExtensionsFromInstallDirectory(
+void ExtensionsServiceBackend::LoadExtensionsFromPrefs(
     scoped_refptr<ExtensionsService> frontend,
     DictionaryValue* extension_prefs) {
+  scoped_ptr<DictionaryValue> prefs(extension_prefs);  // for cleanup
   frontend_ = frontend;
   alert_on_error_ = false;
-  scoped_ptr<DictionaryValue> external_extensions(extension_prefs);
-
-#if defined(OS_WIN)
-  // On POSIX, AbsolutePath() calls realpath() which returns NULL if
-  // it does not exist.  Instead we absolute-ify after creation in
-  // case that is needed.
-  // TODO(port): does this really need to happen before
-  // CreateDirectory() on Windows?
-  if (!file_util::AbsolutePath(&install_directory_))
-    NOTREACHED();
-#endif
-
   scoped_ptr<ExtensionList> extensions(new ExtensionList);
+  DictionaryValue::key_iterator extension_id = extension_prefs->begin_keys();
+  for (; extension_id != extension_prefs->end_keys(); ++extension_id) {
+    DictionaryValue* ext;
+    if (!extension_prefs->GetDictionary(*extension_id, &ext)) {
+      NOTREACHED();
+      continue;
+    }
+    FilePath::StringType path;
+    if (ext->GetString(kPrefPath, &path)) {
+      Extension::Location location = Extension::INVALID;
+      int location_value;
+      DictionaryValue* pref = NULL;
+      extension_prefs->GetDictionary(*extension_id, &pref);
+      if (!pref || !pref->GetInteger(kPrefLocation, &location_value)) {
+        // TODO(erikkay) try to recover?
+        continue;
+      } else {
+        location = static_cast<Extension::Location>(location_value);
+      }
+      std::string id = WideToASCII(*extension_id);
+      if (Extension::IsExternalLocation(location) &&
+          CheckExternalUninstall(extension_prefs, FilePath(path), id)) {
+        // TODO(erikkay): Possibly defer this operation to avoid slowing initial
+        // load of extensions.
+        UninstallExtension(id);
+
+        // No error needs to be reported.  The extension effectively doesn't
+        // exist.
+        continue;
+      }
+      Extension* extension =
+          LoadExtension(FilePath(path), location, true);  // require id
+      if (extension)
+        extensions->push_back(extension);
+    } else {
+      // TODO(erikkay) bootstrap?
+    }
+  }
+  LOG(INFO) << "Done.";
+  ReportExtensionsLoaded(extensions.release());
+}
+
+void ExtensionsServiceBackend::GarbageCollectExtensions(
+    scoped_refptr<ExtensionsService> frontend) {
+  frontend_ = frontend;
+  alert_on_error_ = false;
 
-  // Create the <Profile>/Extensions directory if it doesn't exist.
-  if (!file_util::DirectoryExists(install_directory_)) {
-    file_util::CreateDirectory(install_directory_);
-    LOG(INFO) << "Created Extensions directory.  No extensions to install.";
-    ReportExtensionsLoaded(extensions.release());
+  // Nothing to clean up if it doesn't exist.
+  if (!file_util::DirectoryExists(install_directory_))
     return;
-  }
 
-#if !defined(OS_WIN)
   if (!file_util::AbsolutePath(&install_directory_))
     NOTREACHED();
-#endif
 
   LOG(INFO) << "Loading installed extensions...";
 
@@ -586,13 +668,6 @@ void ExtensionsServiceBackend::LoadExtensionsFromInstallDirectory(
     if (extension_id == kUnpackExtensionDir)
       continue;
 
-    // Ignore directories that aren't valid IDs.
-    if (!Extension::IdIsValid(extension_id)) {
-      LOG(WARNING) << "Invalid extension ID encountered in extensions "
-                      "directory: " << extension_id;
-      continue;
-    }
-
     // If there is no Current Version file, just delete the directory and move
     // on. This can legitimately happen when an uninstall does not complete, for
     // example, when a plugin is in use at uninstall time.
@@ -605,45 +680,16 @@ void ExtensionsServiceBackend::LoadExtensionsFromInstallDirectory(
       continue;
     }
 
-    std::string current_version;
-    if (!ReadCurrentVersion(extension_path, &current_version))
-      continue;
-
-    Extension::Location location = Extension::INVALID;
-    int location_value;
-    DictionaryValue* pref = NULL;
-    external_extensions->GetDictionary(ASCIIToWide(extension_id), &pref);
-    if (!pref ||
-        !pref->GetInteger(kLocation, &location_value)) {
-      // Check with the external providers.
-      if (!LookupExternalExtension(extension_id, NULL, &location))
-        location = Extension::INTERNAL;
-    } else {
-      location = static_cast<Extension::Location>(location_value);
-    }
-
-    FilePath version_path = extension_path.AppendASCII(current_version);
-    if (Extension::IsExternalLocation(location) &&
-        CheckExternalUninstall(external_extensions.get(),
-                               version_path, extension_id)) {
-      // TODO(erikkay): Possibly defer this operation to avoid slowing initial
-      // load of extensions.
-      UninstallExtension(extension_id);
-
-      // No error needs to be reported.  The extension effectively doesn't
-      // exist.
+    // Ignore directories that aren't valid IDs.
+    if (!Extension::IdIsValid(extension_id)) {
+      LOG(WARNING) << "Invalid extension ID encountered in extensions "
+                      "directory: " << extension_id;
+      // TODO(erikkay) delete these eventually too...
       continue;
     }
 
-    Extension* extension = LoadExtension(version_path,
-                                         location,
-                                         true);  // require id
-    if (extension)
-      extensions->push_back(extension);
+    // TODO(erikkay) check for extensions that aren't loaded?
   }
-
-  LOG(INFO) << "Done.";
-  ReportExtensionsLoaded(extensions.release());
 }
 
 void ExtensionsServiceBackend::LoadSingleExtension(
@@ -924,20 +970,17 @@ void ExtensionsServiceBackend::InstallExtension(
   frontend_ = frontend;
   alert_on_error_ = true;
 
-  bool from_external = false;
-  InstallOrUpdateExtension(extension_path, std::string(), from_external);
+  InstallOrUpdateExtension(extension_path, std::string());
 }
 
 void ExtensionsServiceBackend::InstallOrUpdateExtension(
-    const FilePath& extension_path,
-    const std::string& expected_id,
-    bool from_external) {
+    const FilePath& extension_path, const std::string& expected_id) {
   std::string actual_public_key;
   if (!ValidateSignature(extension_path, &actual_public_key))
     return;  // Failures reported within ValidateSignature().
 
   UnpackerClient* client = new UnpackerClient(
-      this, extension_path, actual_public_key, expected_id, from_external);
+      this, extension_path, actual_public_key, expected_id);
   client->Start();
 }
 
@@ -1034,7 +1077,6 @@ void ExtensionsServiceBackend::OnExtensionUnpacked(
     const FilePath& extension_path,
     const FilePath& temp_extension_dir,
     const std::string expected_id,
-    bool from_external,
     const DictionaryValue& manifest,
     const std::vector< Tuple2<SkBitmap, FilePath> >& images) {
   Extension extension;
@@ -1062,7 +1104,11 @@ void ExtensionsServiceBackend::OnExtensionUnpacked(
     return;
   }
 
+  Extension::Location location = Extension::INTERNAL;
+  LookupExternalExtension(extension.id(), NULL, &location);
 #if defined(OS_WIN)
+  bool from_external = Extension::IsExternalLocation(location);
+
   if (!extension.IsTheme() && !from_external &&
       frontend_->show_extensions_prompts() &&
       win_util::MessageBox(GetActiveWindow(),
@@ -1188,40 +1234,22 @@ void ExtensionsServiceBackend::OnExtensionUnpacked(
   if (!SetCurrentVersion(dest_dir, version))
     return;
 
-  Extension::Location location = Extension::INVALID;
-  if (from_external)
-    LookupExternalExtension(extension.id(), NULL, &location);
-  else
-    location = Extension::INTERNAL;
-
-  // Load the extension immediately and then report installation success. We
-  // don't load extensions for external installs because external installation
-  // occurs before the normal startup so we just let startup pick them up. We
-  // notify on installation of external extensions because we need to update
-  // the preferences for these extensions to reflect that they've just been
-  // installed.
-  if (!from_external) {
-    Extension* extension = LoadExtension(version_dir,
-                                         location,
-                                         true);  // require id
-    CHECK(extension);
-
-    frontend_loop_->PostTask(FROM_HERE, NewRunnableMethod(
-        frontend_, &ExtensionsService::OnExtensionInstalled, extension,
-        install_type));
-
-    // Only one extension, but ReportExtensionsLoaded can handle multiple,
-    // so we need to construct a list.
-    scoped_ptr<ExtensionList> extensions(new ExtensionList);
-    extensions->push_back(extension);
-    LOG(INFO) << "Done.";
-    // Hand off ownership of the loaded extensions to the frontend.
-    ReportExtensionsLoaded(extensions.release());
-  } else {
-    frontend_loop_->PostTask(FROM_HERE, NewRunnableMethod(
-        frontend_, &ExtensionsService::OnExternalExtensionInstalled,
-        extension.id(), location));
-  }
+  Extension* loaded = LoadExtension(version_dir,
+                                    location,
+                                    true);  // require id
+  CHECK(loaded);
+
+  frontend_loop_->PostTask(FROM_HERE, NewRunnableMethod(
+      frontend_, &ExtensionsService::OnExtensionInstalled, loaded,
+      install_type));
+
+  // Only one extension, but ReportExtensionsLoaded can handle multiple,
+  // so we need to construct a list.
+  scoped_ptr<ExtensionList> extensions(new ExtensionList);
+  extensions->push_back(loaded);
+
+  // Hand off ownership of the loaded extensions to the frontend.
+  ReportExtensionsLoaded(extensions.release());
 
   scoped_version_dir.Take();
 }
@@ -1255,9 +1283,9 @@ bool ExtensionsServiceBackend::ShouldSkipInstallingExtension(
 
 void ExtensionsServiceBackend::CheckVersionAndInstallExtension(
     const std::string& id, const Version* extension_version,
-    const FilePath& extension_path, bool from_external) {
+    const FilePath& extension_path) {
   if (ShouldInstall(id, extension_version))
-    InstallOrUpdateExtension(FilePath(extension_path), id, from_external);
+    InstallOrUpdateExtension(FilePath(extension_path), id);
 }
 
 bool ExtensionsServiceBackend::LookupExternalExtension(
@@ -1303,7 +1331,7 @@ void ExtensionsServiceBackend::CheckForExternalUpdates(
 }
 
 bool ExtensionsServiceBackend::CheckExternalUninstall(
-    DictionaryValue* extension_prefs, const FilePath& version_path,
+    const DictionaryValue* extension_prefs, const FilePath& version_path,
     const std::string& id) {
   // First check the preferences for the kill-bit.
   int location_value = Extension::INVALID;
@@ -1311,9 +1339,9 @@ bool ExtensionsServiceBackend::CheckExternalUninstall(
   if (!extension_prefs->GetDictionary(ASCIIToWide(id), &extension))
     return false;
   int state;
-  if (extension->GetInteger(kLocation, &location_value) &&
+  if (extension->GetInteger(kPrefLocation, &location_value) &&
       location_value == Extension::EXTERNAL_PREF) {
-    return extension->GetInteger(kState, &state) &&
+    return extension->GetInteger(kPrefState, &state) &&
            state == Extension::KILLBIT;
   }
 
@@ -1382,8 +1410,7 @@ void ExtensionsServiceBackend::SetProviderForTesting(
 
 void ExtensionsServiceBackend::OnExternalExtensionFound(
     const std::string& id, const Version* version, const FilePath& path) {
-  bool from_external = true;
-  CheckVersionAndInstallExtension(id, version, path, from_external);
+  CheckVersionAndInstallExtension(id, version, path);
 }
 
 bool ExtensionsServiceBackend::ShouldInstall(const std::string& id,
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index 7def6f1..b9b5e7e 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -82,7 +82,7 @@ class ExtensionsService
   }
 
   // Initialize and start all installed extensions.
-  bool Init();
+  void Init();
 
   // Install the extension file at |extension_path|.  Will install as an
   // update if an older version is already installed.
@@ -97,8 +97,29 @@ class ExtensionsService
   // Load the extension from the directory |extension_path|.
   void LoadExtension(const FilePath& extension_path);
 
+  // Load all known extensions (used by startup and testing code).
+  void LoadAllExtensions();
+
+  // Check for updates (or potentially new extensions from external providers)
+  void CheckForUpdates();
+
+  // Unload the specified extension.
+  void UnloadExtension(const std::string& extension_id);
+
+  // Unload all extensions.
+  void UnloadAllExtensions();
+
+  // Called only by testing.
+  void ReloadExtensions();
+
+  // Scan the extension directory and clean up the cruft.
+  void GarbageCollectExtensions();
+
   // Lookup an extension by |id|.
-  Extension* GetExtensionByID(std::string id);
+  Extension* GetExtensionById(const std::string& id);
+
+  // Lookup an extension by |url|.  This uses the host of the URL as the id.
+  Extension* GetExtensionByURL(const GURL& url);
 
   // Gets a list of external extensions. If |external_extensions| is non-null,
   // a dictionary with all external extensions (including extensions installed
@@ -120,6 +141,9 @@ class ExtensionsService
                            Value* data_value,
                            bool schedule_save);
 
+  // Removes the entire tree of prefs for |extension_id|.
+  void DeleteExtensionPrefs(const std::wstring& extension_id);
+
   // Clear all ExternalExtensionProviders.
   void ClearProvidersForTesting();
 
@@ -153,10 +177,6 @@ class ExtensionsService
   void OnExtensionInstalled(Extension* extension,
       Extension::InstallType install_type);
 
-  // Called by the backend when an external extension has been installed.
-  void OnExternalExtensionInstalled(
-      const std::string& id, Extension::Location location);
-
   // Called by the backend when an attempt was made to reinstall the same
   // version of an existing extension.
   void OnExtensionOverinstallAttempted(const std::string& id);
@@ -205,14 +225,17 @@ class ExtensionsServiceBackend
 
   virtual ~ExtensionsServiceBackend();
 
-  // Loads extensions from the install directory. The extensions are assumed to
-  // be unpacked in directories that are direct children of the specified path.
+  // Loads extensions from the prefs.
   // Errors are reported through ExtensionErrorReporter. On completion,
   // OnExtensionsLoaded() is called with any successfully loaded extensions.
-  void LoadExtensionsFromInstallDirectory(
+  void LoadExtensionsFromPrefs(
       scoped_refptr<ExtensionsService> frontend,
       DictionaryValue* extension_prefs);
 
+  // Scans the extension installation directory to look for partially installed
+  // or extensions to uninstall.
+  void GarbageCollectExtensions(scoped_refptr<ExtensionsService> frontend);
+
   // 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 completion,
@@ -272,14 +295,11 @@ class ExtensionsServiceBackend
   Extension* LoadExtensionCurrentVersion(const FilePath& extension_path);
 
   // Install a crx file at |extension_path|. If |expected_id| is not empty, it's
-  // verified against the extension's manifest before installation. If
-  // |from_external| is true, this extension install is from an external source,
-  // ie the Windows registry, and will be marked as such. If the extension is
-  // already installed, install the new version only if its version number is
-  // greater than the current installed version.
+  // verified against the extension's manifest before installation. If the
+  // extension is already installed, install the new version only if its version
+  // number is greater than the current installed version.
   void InstallOrUpdateExtension(const FilePath& extension_path,
-                                const std::string& expected_id,
-                                bool from_external);
+                                const std::string& expected_id);
 
   // Validates the signature of the extension in |extension_path|. Returns true
   // and the public key (in |key|) if the signature validates, false otherwise.
@@ -294,7 +314,6 @@ class ExtensionsServiceBackend
       const FilePath& extension_path,
       const FilePath& temp_extension_dir,
       const std::string expected_id,
-      bool from_external,
       const DictionaryValue& manifest,
       const std::vector< Tuple2<SkBitmap, FilePath> >& images);
 
@@ -322,8 +341,7 @@ class ExtensionsServiceBackend
   // extension hasn't been installed before.
   void CheckVersionAndInstallExtension(const std::string& id,
                                        const Version* extension_version,
-                                       const FilePath& extension_path,
-                                       bool from_external);
+                                       const FilePath& extension_path);
 
   // Lookup an external extension by |id| by going through all registered
   // external extension providers until we find a provider that contains an
@@ -362,7 +380,7 @@ class ExtensionsServiceBackend
   // For the extension in |version_path| with |id|, check to see if it's an
   // externally managed extension.  If so return true if it should be
   // uninstalled.
-  bool CheckExternalUninstall(DictionaryValue* extension_prefs,
+  bool CheckExternalUninstall(const DictionaryValue* extension_prefs,
                               const FilePath& version_path,
                               const std::string& id);
 
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index cca5299..1150dae 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -168,10 +168,18 @@ class ExtensionsServiceTest
         break;
       }
 
-      case NotificationType::EXTENSION_UNLOADED:
-        unloaded_id_ = Details<Extension>(details).ptr()->id();
+      case NotificationType::EXTENSION_UNLOADED: {
+        Extension* e = Details<Extension>(details).ptr();
+        unloaded_id_ = e->id();
+        ExtensionList::iterator i =
+            std::find(loaded_.begin(), loaded_.end(), e);
+        // TODO(erikkay) fix so this can be an assert.  Right now the tests
+        // are manually calling clear() on loaded_, so this isn't doable.
+        if (i == loaded_.end())
+          return;
+        loaded_.erase(i);
         break;
-
+      }
       case NotificationType::EXTENSION_INSTALLED:
       case NotificationType::THEME_INSTALLED:
         installed_ = Details<Extension>(details).ptr();
@@ -207,7 +215,7 @@ class ExtensionsServiceTest
       EXPECT_EQ(0u, errors.size()) << path.value();
       EXPECT_EQ(total_successes_, service_->extensions()->size()) <<
           path.value();
-      EXPECT_TRUE(service_->GetExtensionByID(loaded_[0]->id())) <<
+      EXPECT_TRUE(service_->GetExtensionById(loaded_[0]->id())) <<
           path.value();
       for (std::vector<std::string>::iterator err = errors.begin();
         err != errors.end(); ++err) {
@@ -274,7 +282,7 @@ class ExtensionsServiceTest
   scoped_refptr<ExtensionsService> service_;
   size_t total_successes_;
   MessageLoop loop_;
-  std::vector<Extension*> loaded_;
+  ExtensionList loaded_;
   std::string unloaded_id_;
   Extension* installed_;
 
@@ -282,8 +290,10 @@ class ExtensionsServiceTest
   NotificationRegistrar registrar_;
 };
 
+// TODO(erikkay) this test and the next need to be replaced with equivalent
+// versions that load from prefs.
 // Test loading good extensions from the profile directory.
-TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectorySuccess) {
+TEST_F(ExtensionsServiceTest, DISABLED_LoadAllExtensionsFromDirectorySuccess) {
   // Copy the test extensions into the test profile.
   FilePath source_path;
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &source_path));
@@ -293,7 +303,7 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectorySuccess) {
   FilePath dest_path = profile_->GetPath().AppendASCII("Extensions");
   file_util::CopyDirectory(source_path, dest_path, true);  // Recursive.
 
-  ASSERT_TRUE(service_->Init());
+  service_->Init();
   loop_.RunAllPending();
 
   std::vector<std::string> errors = GetErrors();
@@ -309,7 +319,7 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectorySuccess) {
   EXPECT_EQ(std::string("The first extension that I made."),
             loaded_[0]->description());
   EXPECT_EQ(Extension::INTERNAL, loaded_[0]->location());
-  EXPECT_TRUE(service_->GetExtensionByID(loaded_[0]->id()));
+  EXPECT_TRUE(service_->GetExtensionById(loaded_[0]->id()));
   EXPECT_EQ(3u, service_->extensions()->size());
 
   ValidatePrefKeyCount(3);
@@ -370,7 +380,7 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectorySuccess) {
 };
 
 // Test loading bad extensions from the profile directory.
-TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectoryFail) {
+TEST_F(ExtensionsServiceTest, DISABLED_LoadAllExtensionsFromDirectoryFail) {
   FilePath source_path;
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &source_path));
   source_path = source_path.AppendASCII("extensions");
@@ -379,7 +389,7 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectoryFail) {
   FilePath dest_path = profile_->GetPath().AppendASCII("Extensions");
   file_util::CopyDirectory(source_path, dest_path, true);  // Recursive.
 
-  ASSERT_TRUE(service_->Init());
+  service_->Init();
   loop_.RunAllPending();
 
   EXPECT_EQ(3u, GetErrors().size());
@@ -414,25 +424,25 @@ TEST_F(ExtensionsServiceTest, CleanupOnStartup) {
 
   // Simulate that one of them got partially deleted by deling the
   // Current Version file.
-  dest_path = dest_path.AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj")
-                       .AppendASCII(ExtensionsService::kCurrentVersionFileName);
-  ASSERT_TRUE(file_util::Delete(dest_path, false));  // not recursive
+  FilePath vers = dest_path.AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj")
+      .AppendASCII(ExtensionsService::kCurrentVersionFileName);
+  ASSERT_TRUE(file_util::Delete(vers, false));  // not recursive
 
   service_->Init();
   loop_.RunAllPending();
 
+  file_util::FileEnumerator dirs(dest_path, false,
+                                 file_util::FileEnumerator::DIRECTORIES);
+  size_t count = 0;
+  while (!dirs.Next().empty())
+    count++;
+
   // We should have only gotten two extensions now.
-  EXPECT_EQ(2u, loaded_.size());
+  EXPECT_EQ(2u, count);
 
   // And extension1 dir should now be toast.
-  dest_path = dest_path.DirName();
-  ASSERT_FALSE(file_util::PathExists(dest_path));
-
-  ValidatePrefKeyCount(2);
-  ValidatePref(good1, L"state", Extension::ENABLED);
-  ValidatePref(good1, L"location", Extension::INTERNAL);
-  ValidatePref(good2, L"state", Extension::ENABLED);
-  ValidatePref(good2, L"location", Extension::INTERNAL);
+  vers = vers.DirName();
+  ASSERT_FALSE(file_util::PathExists(vers));
 }
 
 // Test installing extensions.
@@ -466,7 +476,7 @@ TEST_F(ExtensionsServiceTest, InstallExtension) {
   ValidatePref(page_action, L"state", Extension::ENABLED);
   ValidatePref(page_action, L"location", Extension::INTERNAL);
 
-  // Bad signature. 
+  // Bad signature.
   path = extensions_path.AppendASCII("bad_signature.crx");
   InstallExtension(path, false);
 
@@ -627,6 +637,7 @@ TEST_F(ExtensionsServiceTest, UpgradeSignedGood) {
 
   ASSERT_TRUE(installed_);
   ASSERT_EQ(1u, loaded_.size());
+  ASSERT_EQ("1.0.0.0", loaded_[0]->version()->GetString());
   ASSERT_EQ(0u, GetErrors().size());
 
   // Upgrade to version 2.0
@@ -635,7 +646,8 @@ TEST_F(ExtensionsServiceTest, UpgradeSignedGood) {
   loop_.RunAllPending();
 
   ASSERT_TRUE(installed_);
-  ASSERT_EQ(2u, loaded_.size());
+  ASSERT_EQ(1u, loaded_.size());
+  ASSERT_EQ("1.0.0.1", loaded_[0]->version()->GetString());
   ASSERT_EQ(0u, GetErrors().size());
 }
 
@@ -693,12 +705,10 @@ TEST_F(ExtensionsServiceTest, UninstallExtension) {
   ASSERT_TRUE(unloaded_id_.length());
   EXPECT_EQ(extension_id, unloaded_id_);
 
-  ValidatePrefKeyCount(1);
-  ValidatePref(good_crx, L"state", Extension::DISABLED);
-  ValidatePref(good_crx, L"location", Extension::INTERNAL);
+  ValidatePrefKeyCount(0);
 
   // The extension should not be in the service anymore.
-  ASSERT_FALSE(service_->GetExtensionByID(extension_id));
+  ASSERT_FALSE(service_->GetExtensionById(extension_id));
   loop_.RunAllPending();
 
   // The directory should be gone.
@@ -715,9 +725,7 @@ TEST_F(ExtensionsServiceTest, UninstallExtension) {
   loop_.RunAllPending();
   EXPECT_FALSE(file_util::PathExists(extension_path));
 
-  ValidatePrefKeyCount(1);
-  ValidatePref(good_crx, L"state", Extension::DISABLED);
-  ValidatePref(good_crx, L"location", Extension::INTERNAL);
+  ValidatePrefKeyCount(0);
 }
 
 // Tests loading single extensions (like --load-extension)
@@ -733,11 +741,11 @@ TEST_F(ExtensionsServiceTest, LoadExtension) {
   loop_.RunAllPending();
   EXPECT_EQ(0u, GetErrors().size());
   ASSERT_EQ(1u, loaded_.size());
-  ASSERT_EQ(Extension::LOAD, loaded_[0]->location());
+  EXPECT_EQ(Extension::LOAD, loaded_[0]->location());
+  EXPECT_EQ(1u, service_->extensions()->size());
 
-  ValidatePrefKeyCount(1);
-  ValidatePref(good0, L"state", Extension::ENABLED);
-  ValidatePref(good0, L"location", Extension::INTERNAL);
+  // --load-extension doesn't add entries to prefs
+  ValidatePrefKeyCount(0);
 
   FilePath no_manifest = extensions_path.AppendASCII("bad")
       .AppendASCII("no_manifest").AppendASCII("1");
@@ -745,8 +753,7 @@ TEST_F(ExtensionsServiceTest, LoadExtension) {
   loop_.RunAllPending();
   EXPECT_EQ(1u, GetErrors().size());
   ASSERT_EQ(1u, loaded_.size());
-
-  ValidatePrefKeyCount(1);
+  EXPECT_EQ(1u, service_->extensions()->size());
 
   // Test uninstall.
   std::string id = loaded_[0]->id();
@@ -754,10 +761,8 @@ TEST_F(ExtensionsServiceTest, LoadExtension) {
   service_->UninstallExtension(id);
   loop_.RunAllPending();
   EXPECT_EQ(id, unloaded_id_);
-
-  ValidatePrefKeyCount(1);
-  ValidatePref(good0, L"state", Extension::DISABLED);
-  ValidatePref(good0, L"location", Extension::INTERNAL);
+  ASSERT_EQ(0u, loaded_.size());
+  EXPECT_EQ(0u, service_->extensions()->size());
 }
 
 // Tests that we generate IDs when they are not specified in the manifest for
@@ -775,24 +780,24 @@ TEST_F(ExtensionsServiceTest, GenerateID) {
   EXPECT_EQ(0u, GetErrors().size());
   ASSERT_EQ(1u, loaded_.size());
   std::string id1 = loaded_[0]->id();
-  ASSERT_EQ(all_zero, id1);
-  ASSERT_EQ("chrome-extension://aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/",
+  EXPECT_EQ(all_zero, id1);
+  EXPECT_EQ("chrome-extension://aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/",
             loaded_[0]->url().spec());
+  EXPECT_EQ(loaded_[0]->location(), Extension::LOAD);
 
-  ValidatePrefKeyCount(1);
-  ValidatePref(all_zero, L"state", Extension::ENABLED);
-  ValidatePref(all_zero, L"location", Extension::INTERNAL);
+  // --load-extension doesn't add entries to prefs
+  ValidatePrefKeyCount(0);
 
   service_->LoadExtension(no_id_ext);
   loop_.RunAllPending();
   std::string id2 = loaded_[1]->id();
-  ASSERT_EQ(zero_n_one, id2);
-  ASSERT_EQ("chrome-extension://aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab/",
+  EXPECT_EQ(zero_n_one, id2);
+  EXPECT_EQ("chrome-extension://aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab/",
             loaded_[1]->url().spec());
+  EXPECT_EQ(loaded_[1]->location(), Extension::LOAD);
 
-  ValidatePrefKeyCount(2);
-  ValidatePref(zero_n_one, L"state", Extension::ENABLED);
-  ValidatePref(zero_n_one, L"location", Extension::INTERNAL);
+  // --load-extension doesn't add entries to prefs
+  ValidatePrefKeyCount(0);
 }
 
 // Tests the external installation feature
@@ -817,9 +822,9 @@ TEST_F(ExtensionsServiceTest, ExternalInstallRegistry) {
   // Add the extension.
   reg_provider->UpdateOrAddExtension(good_crx, "1.0.0.0", source_path);
 
-  // Start up the service, it should find our externally registered extension
+  // Reloading extensions should find our externally registered extension
   // and install it.
-  service_->Init();
+  service_->CheckForUpdates();
   loop_.RunAllPending();
 
   ASSERT_EQ(0u, GetErrors().size());
@@ -830,10 +835,10 @@ TEST_F(ExtensionsServiceTest, ExternalInstallRegistry) {
   ValidatePref(good_crx, L"state", Extension::ENABLED);
   ValidatePref(good_crx, L"location", Extension::EXTERNAL_REGISTRY);
 
-  // Reinit the service without changing anything. The extension should be
+  // Reload extensions without changing anything. The extension should be
   // loaded again.
   loaded_.clear();
-  service_->Init();
+  service_->ReloadExtensions();
   loop_.RunAllPending();
   ASSERT_EQ(0u, GetErrors().size());
   ASSERT_EQ(1u, loaded_.size());
@@ -846,7 +851,7 @@ TEST_F(ExtensionsServiceTest, ExternalInstallRegistry) {
   reg_provider->UpdateOrAddExtension(good_crx, "1.0.0.1", source_path);
 
   loaded_.clear();
-  service_->Init();
+  service_->CheckForUpdates();
   loop_.RunAllPending();
   ASSERT_EQ(0u, GetErrors().size());
   ASSERT_EQ(1u, loaded_.size());
@@ -867,7 +872,7 @@ TEST_F(ExtensionsServiceTest, ExternalInstallRegistry) {
   ASSERT_FALSE(file_util::PathExists(install_path));
 
   loaded_.clear();
-  service_->Init();
+  service_->CheckForUpdates();
   loop_.RunAllPending();
   ASSERT_EQ(0u, loaded_.size());
   ValidatePrefKeyCount(1);
@@ -880,7 +885,7 @@ TEST_F(ExtensionsServiceTest, ExternalInstallRegistry) {
   profile_->GetPrefs()->ScheduleSavePersistentPrefs();
 
   loaded_.clear();
-  service_->Init();
+  service_->CheckForUpdates();
   loop_.RunAllPending();
   ASSERT_EQ(1u, loaded_.size());
   ValidatePrefKeyCount(1);
@@ -890,7 +895,7 @@ TEST_F(ExtensionsServiceTest, ExternalInstallRegistry) {
   reg_provider->RemoveExtension(good_crx);
 
   loaded_.clear();
-  service_->Init();
+  service_->CheckForUpdates();
   loop_.RunAllPending();
   ASSERT_EQ(0u, loaded_.size());
 }
@@ -916,9 +921,9 @@ TEST_F(ExtensionsServiceTest, ExternalInstallPref) {
   // Add the extension.
   pref_provider->UpdateOrAddExtension(good_crx, "1.0", source_path);
 
-  // Start up the service, it should find our externally registered extension
+  // Checking for updates should find our externally registered extension
   // and install it.
-  service_->Init();
+  service_->CheckForUpdates();
   loop_.RunAllPending();
 
   ASSERT_EQ(0u, GetErrors().size());
@@ -930,10 +935,10 @@ TEST_F(ExtensionsServiceTest, ExternalInstallPref) {
   ValidatePref(good_crx, L"state", Extension::ENABLED);
   ValidatePref(good_crx, L"location", Extension::EXTERNAL_PREF);
 
-  // Reinit the service without changing anything. The extension should be
+  // Reload extensions without changing anything. The extension should be
   // loaded again.
   loaded_.clear();
-  service_->Init();
+  service_->ReloadExtensions();
   loop_.RunAllPending();
   ASSERT_EQ(0u, GetErrors().size());
   ASSERT_EQ(1u, loaded_.size());
@@ -947,7 +952,7 @@ TEST_F(ExtensionsServiceTest, ExternalInstallPref) {
   pref_provider->UpdateOrAddExtension(good_crx, "1.0.0.1", source_path);
 
   loaded_.clear();
-  service_->Init();
+  service_->CheckForUpdates();
   loop_.RunAllPending();
   ASSERT_EQ(0u, GetErrors().size());
   ASSERT_EQ(1u, loaded_.size());
@@ -957,7 +962,7 @@ TEST_F(ExtensionsServiceTest, ExternalInstallPref) {
   ValidatePref(good_crx, L"state", Extension::ENABLED);
   ValidatePref(good_crx, L"location", Extension::EXTERNAL_PREF);
 
-  // Uninstall the extension and re-init. Nothing should happen because the
+  // Uninstall the extension and reload. Nothing should happen because the
   // preference should prevent us from reinstalling.
   std::string id = loaded_[0]->id();
   service_->UninstallExtension(id);
@@ -969,7 +974,7 @@ TEST_F(ExtensionsServiceTest, ExternalInstallPref) {
   ASSERT_FALSE(file_util::PathExists(install_path));
 
   loaded_.clear();
-  service_->Init();
+  service_->CheckForUpdates();
   loop_.RunAllPending();
   ASSERT_EQ(0u, loaded_.size());
 
@@ -982,7 +987,7 @@ TEST_F(ExtensionsServiceTest, ExternalInstallPref) {
   profile_->GetPrefs()->ScheduleSavePersistentPrefs();
 
   loaded_.clear();
-  service_->Init();
+  service_->CheckForUpdates();
   loop_.RunAllPending();
   ASSERT_EQ(1u, loaded_.size());
 
@@ -995,7 +1000,7 @@ TEST_F(ExtensionsServiceTest, ExternalInstallPref) {
   profile_->GetPrefs()->ScheduleSavePersistentPrefs();
 
   loaded_.clear();
-  service_->Init();
+  service_->ReloadExtensions();
   loop_.RunAllPending();
   ASSERT_EQ(0u, loaded_.size());
 
diff --git a/chrome/browser/extensions/external_pref_extension_provider.cc b/chrome/browser/extensions/external_pref_extension_provider.cc
index 6db3355..ca18753 100644
--- a/chrome/browser/extensions/external_pref_extension_provider.cc
+++ b/chrome/browser/extensions/external_pref_extension_provider.cc
@@ -33,8 +33,6 @@ void ExternalPrefExtensionProvider::VisitRegisteredExtension(
 
     DictionaryValue* extension = NULL;
     if (!prefs_->GetDictionary(extension_id, &extension)) {
-      NOTREACHED() << "Cannot read extension " << extension_id.c_str()
-                   << " from dictionary.";
       continue;
     }