Refactor code to make it more testable.
There's an extant problem with external extension providers reporting
that they're ready without actually setting their ready state. This change
allows the implementer of OnExternalProviderReady() to assert that the
caller's ready state is true.
TEST=added
Review URL: http://codereview.chromium.org/8344073
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@106627 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 7c71eb17..e23bba9 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -443,7 +443,7 @@
}
pending_extension_manager()->AddFromExternalUpdateUrl(
id, update_url, location);
- external_extension_url_added_ |= true;
+ external_extension_url_added_ = true;
}
// If a download url matches one of these patterns and has a referrer of the
@@ -2111,17 +2111,21 @@
provider->VisitRegisteredExtension();
}
- // Uninstall of unclaimed extensions will happen after all the providers
- // had reported ready. Every provider calls OnExternalProviderReady()
- // when it finishes, and OnExternalProviderReady() only acts when all
- // providers are ready. In case there are no providers, we call it
- // to trigger removal of extensions that used to have an external source.
- if (external_extension_providers_.empty())
- OnExternalProviderReady();
+ // Do any required work that we would have done after completion of all
+ // providers.
+ if (external_extension_providers_.empty()) {
+ OnAllExternalProvidersReady();
+ }
}
-void ExtensionService::OnExternalProviderReady() {
+void ExtensionService::OnExternalProviderReady(
+ const ExternalExtensionProviderInterface* provider) {
CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+#if 0
+ // TODO(rogerta): Remove guard and this comment when bug described
+ // in http://codereview.chromium.org/8245018/ is fixed.
+ CHECK(provider->IsReady());
+#endif
// An external provider has finished loading. We only take action
// if all of them are finished. So we check them first.
@@ -2133,7 +2137,13 @@
return;
}
- // All the providers are ready. Install any pending extensions.
+ OnAllExternalProvidersReady();
+}
+
+void ExtensionService::OnAllExternalProvidersReady() {
+ CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+ // Install any pending extensions.
if (external_extension_url_added_ && updater()) {
external_extension_url_added_ = false;
updater()->CheckNow();
