Ensure ChromeURLRequestContext finds out first about extension loading.

This prevents races from arising where extension renderers may try to navigate to extension urls before the request context knows how to map the urls to extension resources.

BUG=22668

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

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

5 files changed, 71 insertions, 53 deletions

diff --git a/chrome/browser/extensions/extension_toolstrip_apitest.cc b/chrome/browser/extensions/extension_toolstrip_apitest.cc
index b4d4c4b..5e302db 100644
--- a/chrome/browser/extensions/extension_toolstrip_apitest.cc
+++ b/chrome/browser/extensions/extension_toolstrip_apitest.cc
@@ -4,9 +4,6 @@
 
 #include "chrome/browser/extensions/extension_apitest.h"
 
-// TEMPORARILY ENABLED TO GET DEBUG OUTPUT:
-// TODO(rafaelw,erikkay) disabled due to flakiness
-// BUG=22668 (probably the same bug)
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, Toolstrip) {
   ASSERT_TRUE(RunExtensionTest("toolstrip")) << message_;
 }
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 1258e849..4f61ac4 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -18,6 +18,7 @@
 #include "chrome/browser/extensions/external_extension_provider.h"
 #include "chrome/browser/extensions/external_pref_extension_provider.h"
 #include "chrome/browser/profile.h"
+#include "chrome/browser/net/chrome_url_request_context.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_error_reporter.h"
@@ -242,10 +243,7 @@ void ExtensionsService::EnableExtension(const std::string& extension_id) {
   ExtensionDOMUI::RegisterChromeURLOverrides(profile_,
       extension->GetChromeURLOverrides());
 
-  NotificationService::current()->Notify(
-      NotificationType::EXTENSION_LOADED,
-      Source<ExtensionsService>(this),
-      Details<Extension>(extension));
+  NotifyExtensionLoaded(extension);
 }
 
 void ExtensionsService::DisableExtension(const std::string& extension_id) {
@@ -269,10 +267,7 @@ void ExtensionsService::DisableExtension(const std::string& extension_id) {
   ExtensionDOMUI::UnregisterChromeURLOverrides(profile_,
       extension->GetChromeURLOverrides());
 
-  NotificationService::current()->Notify(
-      NotificationType::EXTENSION_UNLOADED,
-      Source<ExtensionsService>(this),
-      Details<Extension>(extension));
+  NotifyExtensionUnloaded(extension);
 }
 
 void ExtensionsService::LoadExtension(const FilePath& extension_path) {
@@ -324,6 +319,51 @@ void ExtensionsService::LoadInstalledExtension(
   }
 }
 
+void ExtensionsService::NotifyExtensionLoaded(Extension* extension) {
+  LOG(INFO) << "Sending EXTENSION_LOADED";
+
+  // The ChromeURLRequestContext needs to be first to know that the extension
+  // was loaded, otherwise a race can arise where a renderer that is created
+  // for the extension may try to load an extension URL with an extension id
+  // that the request context doesn't yet know about.
+  if (profile_ && !profile_->IsOffTheRecord()) {
+    ChromeURLRequestContext* context = static_cast<ChromeURLRequestContext*>(
+        profile_->GetRequestContext());
+    if (context) {
+      g_browser_process->io_thread()->message_loop()->PostTask(FROM_HERE,
+          NewRunnableMethod(context,
+                            &ChromeURLRequestContext::OnNewExtensions,
+                            extension->id(),
+                            extension->path()));
+    }
+  }
+
+  NotificationService::current()->Notify(
+      NotificationType::EXTENSION_LOADED,
+      Source<ExtensionsService>(this),
+      Details<Extension>(extension));
+}
+
+void ExtensionsService::NotifyExtensionUnloaded(Extension* extension) {
+  LOG(INFO) << "Sending EXTENSION_UNLOADED";
+
+  NotificationService::current()->Notify(
+      NotificationType::EXTENSION_UNLOADED,
+      Source<ExtensionsService>(this),
+      Details<Extension>(extension));
+
+  if (profile_ && !profile_->IsOffTheRecord()) {
+    ChromeURLRequestContext* context = static_cast<ChromeURLRequestContext*>(
+        profile_->GetRequestContext());
+    if (context) {
+      g_browser_process->io_thread()->message_loop()->PostTask(FROM_HERE,
+          NewRunnableMethod(context,
+                            &ChromeURLRequestContext::OnUnloadedExtension,
+                            extension->id()));
+    }
+  }
+}
+
 std::vector<ExtensionAction*> ExtensionsService::GetExtensionActions(
     ExtensionAction::ExtensionActionType action_type) const {
   std::vector<ExtensionAction*> result;
@@ -413,10 +453,7 @@ void ExtensionsService::UnloadExtension(const std::string& extension_id) {
   // Remove the extension from our list.
   extensions_.erase(iter);
 
-  // Tell other services the extension is gone.
-  NotificationService::current()->Notify(NotificationType::EXTENSION_UNLOADED,
-                                         Source<ExtensionsService>(this),
-                                         Details<Extension>(extension.get()));
+  NotifyExtensionUnloaded(extension.get());
 }
 
 void ExtensionsService::UnloadAllExtensions() {
@@ -498,11 +535,7 @@ void ExtensionsService::OnExtensionLoaded(Extension* extension,
         if (extension->location() != Extension::LOAD)
           extension_prefs_->MigrateToPrefs(extension);
 
-        LOG(INFO) << "Sending EXTENSION_LOADED";
-        NotificationService::current()->Notify(
-            NotificationType::EXTENSION_LOADED,
-            Source<ExtensionsService>(this),
-            Details<Extension>(extension));
+        NotifyExtensionLoaded(extension);
 
         if (extension->IsTheme() && extension->location() == Extension::LOAD) {
           NotificationService::current()->Notify(
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index d683585..d4a6805 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -236,6 +236,12 @@ class ExtensionsService
       DictionaryValue* manifest, const std::string& id,
       const FilePath& path, Extension::Location location);
 
+  // Handles sending notification that |extension| was loaded.
+  void NotifyExtensionLoaded(Extension* extension);
+
+  // Handles sending notification that |extension| was unloaded.
+  void NotifyExtensionUnloaded(Extension* extension);
+
   // Retrieves a vector of all page actions or browser actions, irrespective of
   // which extension they belong to.
   std::vector<ExtensionAction*> GetExtensionActions(
diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc
index 5de62c1..2c68419 100644
--- a/chrome/browser/net/chrome_url_request_context.cc
+++ b/chrome/browser/net/chrome_url_request_context.cc
@@ -367,13 +367,6 @@ ChromeURLRequestContext::ChromeURLRequestContext(
   prefs_->AddPrefObserver(prefs::kCookieBehavior, this);
   prefs_->AddPrefObserver(prefs::kDefaultCharset, this);
 
-  if (!is_off_the_record_) {
-    registrar_.Add(this, NotificationType::EXTENSION_LOADED,
-                   NotificationService::AllSources());
-    registrar_.Add(this, NotificationType::EXTENSION_UNLOADED,
-                   NotificationService::AllSources());
-  }
-
   ssl_config_service_ = profile->GetSSLConfigService();
 }
 
@@ -432,20 +425,6 @@ void ChromeURLRequestContext::Observe(NotificationType type,
                             &ChromeURLRequestContext::OnDefaultCharsetChange,
                             default_charset));
     }
-  } else if (NotificationType::EXTENSION_LOADED == type) {
-    ExtensionPaths* new_paths = new ExtensionPaths;
-    Extension* extension = Details<Extension>(details).ptr();
-    DCHECK(extension);
-    new_paths->insert(ExtensionPaths::value_type(extension->id(),
-                                                 extension->path()));
-    g_browser_process->io_thread()->message_loop()->PostTask(FROM_HERE,
-        NewRunnableMethod(this, &ChromeURLRequestContext::OnNewExtensions,
-                          new_paths));
-  } else if (NotificationType::EXTENSION_UNLOADED == type) {
-    Extension* extension = Details<Extension>(details).ptr();
-    g_browser_process->io_thread()->message_loop()->PostTask(FROM_HERE,
-        NewRunnableMethod(this, &ChromeURLRequestContext::OnUnloadedExtension,
-                          extension->id()));
   } else {
     NOTREACHED();
   }
@@ -539,14 +518,17 @@ void ChromeURLRequestContext::OnDefaultCharsetChange(
       net::HttpUtil::GenerateAcceptCharsetHeader(default_charset);
 }
 
-void ChromeURLRequestContext::OnNewExtensions(ExtensionPaths* new_paths) {
-  extension_paths_.insert(new_paths->begin(), new_paths->end());
-  delete new_paths;
+void ChromeURLRequestContext::OnNewExtensions(const std::string& id,
+                                              const FilePath& path) {
+  if (!is_off_the_record_) {
+    extension_paths_[id] = path;
+  }
 }
 
-void ChromeURLRequestContext::OnUnloadedExtension(
-    const std::string& extension_id) {
-  ExtensionPaths::iterator iter = extension_paths_.find(extension_id);
+void ChromeURLRequestContext::OnUnloadedExtension(const std::string& id) {
+  if (is_off_the_record_)
+    return;
+  ExtensionPaths::iterator iter = extension_paths_.find(id);
   DCHECK(iter != extension_paths_.end());
   extension_paths_.erase(iter);
 }
diff --git a/chrome/browser/net/chrome_url_request_context.h b/chrome/browser/net/chrome_url_request_context.h
index 9eb3c90..892593b 100644
--- a/chrome/browser/net/chrome_url_request_context.h
+++ b/chrome/browser/net/chrome_url_request_context.h
@@ -83,6 +83,12 @@ class ChromeURLRequestContext : public URLRequestContext,
   // Gets the Privacy Blacklist, if any for this context.
   const Blacklist* blacklist() const { return blacklist_; }
 
+  // Callback for when new extensions are loaded.
+  void OnNewExtensions(const std::string& id, const FilePath& path);
+
+  // Callback for when an extension is unloaded.
+  void OnUnloadedExtension(const std::string& id);
+
  protected:
   // Private constructors, use the static factory methods instead. This is
   // expected to be called on the UI thread.
@@ -111,12 +117,6 @@ class ChromeURLRequestContext : public URLRequestContext,
   // Callback for when the default charset changes.
   void OnDefaultCharsetChange(const std::string& default_charset);
 
-  // Callback for when new extensions are loaded.
-  void OnNewExtensions(ExtensionPaths* new_paths);
-
-  // Callback for when an extension is unloaded.
-  void OnUnloadedExtension(const std::string& id);
-
   // Destructor.
   virtual ~ChromeURLRequestContext();