Move GetExtension() from ExtensionFunctionDispatcher to ExtensionFunction.

EFD's can expire during the processing of AsyncExtensionFunctions, so this change enables them to retain access to their extension to complete their work.

This fixes a bug in executeScript where when a file source was used, but the call was made from a popup which which had closed, by the time the file source was loaded, the EFD had been destroyed. Go boom.

BUG=32431
TEST=All tests should pass

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

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

9 files changed, 43 insertions, 40 deletions

diff --git a/chrome/browser/extensions/execute_script_apitest.cc b/chrome/browser/extensions/execute_script_apitest.cc
index 4ef3d81..3b07add 100644
--- a/chrome/browser/extensions/execute_script_apitest.cc
+++ b/chrome/browser/extensions/execute_script_apitest.cc
@@ -18,3 +18,12 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, DISABLED_ExecuteScript) {
   ASSERT_TRUE(RunExtensionTest("executescript/in_frame")) << message_;
   ASSERT_TRUE(RunExtensionTest("executescript/permissions")) << message_;
 }
+
+// TODO(rafaelw) - This case is split out per Pawel's request. When the above
+// (ExecuteScript) tests are de-flakified, reunite this case with it's brethern.
+IN_PROC_BROWSER_TEST_F(ExtensionApiTest, ExecuteScriptFileAfterClose) {
+  host_resolver()->AddRule("b.com", "127.0.0.1");
+  StartHTTPServer();
+
+  ASSERT_TRUE(RunExtensionTest("executescript/file_after_close")) << message_;
+}
diff --git a/chrome/browser/extensions/extension_browser_actions_api.cc b/chrome/browser/extensions/extension_browser_actions_api.cc
index c415d02..c1ffe8e 100644
--- a/chrome/browser/extensions/extension_browser_actions_api.cc
+++ b/chrome/browser/extensions/extension_browser_actions_api.cc
@@ -25,7 +25,7 @@ bool BrowserActionFunction::RunImpl() {
   if (details_->HasKey(L"tabId"))
     EXTENSION_FUNCTION_VALIDATE(details_->GetInteger(L"tabId", &tab_id_));
 
-  Extension* extension = dispatcher()->GetExtension();
+  Extension* extension = GetExtension();
   browser_action_ = extension->browser_action();
   if (!browser_action_) {
     error_ = kNoBrowserActionError;
@@ -67,7 +67,7 @@ bool BrowserActionSetPopupFunction::RunBrowserAction() {
 
   GURL popup_url;
   if (!popup_string.empty())
-    popup_url = dispatcher()->GetExtension()->GetResourceURL(popup_string);
+    popup_url = GetExtension()->GetResourceURL(popup_string);
 
   browser_action_->SetPopupUrl(tab_id_, popup_url);
   return true;
diff --git a/chrome/browser/extensions/extension_function.cc b/chrome/browser/extensions/extension_function.cc
index f319315..f3c3532 100644
--- a/chrome/browser/extensions/extension_function.cc
+++ b/chrome/browser/extensions/extension_function.cc
@@ -37,13 +37,3 @@ bool AsyncExtensionFunction::HasOptionalArgument(size_t index) {
   Value* value;
   return args_list->Get(index, &value) && !value->IsType(Value::TYPE_NULL);
 }
-
-std::string AsyncExtensionFunction::extension_id() {
-  DCHECK(dispatcher());
-  return dispatcher()->extension_id();
-}
-
-Profile* AsyncExtensionFunction::profile() const {
-  DCHECK(dispatcher());
-  return dispatcher()->profile();
-}
diff --git a/chrome/browser/extensions/extension_function.h b/chrome/browser/extensions/extension_function.h
index 1727ae0..0e51321 100644
--- a/chrome/browser/extensions/extension_function.h
+++ b/chrome/browser/extensions/extension_function.h
@@ -12,6 +12,8 @@
 #include "base/scoped_ptr.h"
 #include "base/ref_counted.h"
 #include "chrome/browser/extensions/extension_function_dispatcher.h"
+#include "chrome/browser/extensions/extensions_service.h"
+#include "chrome/browser/profile.h"
 
 class ExtensionFunctionDispatcher;
 class Profile;
@@ -46,6 +48,17 @@ class ExtensionFunction : public base::RefCounted<ExtensionFunction> {
   void set_name(const std::string& name) { name_ = name; }
   const std::string name() const { return name_; }
 
+  // Set the profile which contains the extension that has originated this
+  // function call.
+  void set_profile(Profile* profile) { profile_ = profile; }
+  Profile* profile() const { return profile_; }
+
+  // Set the id of this function call's extension.
+  void set_extension_id(std::string extension_id) {
+    extension_id_ = extension_id;
+  }
+  std::string extension_id() const { return extension_id_; }
+
   // Specifies the raw arguments to the function, as a JSON value.
   virtual void SetArgs(const Value* args) = 0;
 
@@ -92,12 +105,12 @@ class ExtensionFunction : public base::RefCounted<ExtensionFunction> {
   virtual ~ExtensionFunction() {}
 
   // Gets the extension that called this function. This can return NULL for
-  // async functions.
+  // async functions, for example if the extension is unloaded while the
+  // function is running.
   Extension* GetExtension() {
-    if (dispatcher())
-      return dispatcher()->GetExtension();
-    else
-      return NULL;
+    ExtensionsService* service = profile_->GetExtensionsService();
+    DCHECK(service);
+    return service->GetExtensionById(extension_id_, false);
   }
 
   // Gets the "current" browser, if any.
@@ -126,6 +139,12 @@ class ExtensionFunction : public base::RefCounted<ExtensionFunction> {
   // Id of this request, used to map the response back to the caller.
   int request_id_;
 
+  // The Profile of this function's extension.
+  Profile* profile_;
+
+  // The id of this function's extension.
+  std::string extension_id_;
+
   // The name of this function.
   std::string name_;
 
@@ -176,13 +195,10 @@ class AsyncExtensionFunction : public ExtensionFunction {
     return static_cast<DictionaryValue*>(args_.get());
   }
 
+  // Return true if the argument to this function at |index| was provided and
+  // is non-null.
   bool HasOptionalArgument(size_t index);
 
-  // Note: After Run() returns, dispatcher() can be NULL.  Since these getters
-  // rely on dispatcher(), make sure it is valid before using them.
-  std::string extension_id();
-  Profile* profile() const;
-
   // The arguments to the API. Only non-null if argument were specified.
   scoped_ptr<Value> args_;
 
diff --git a/chrome/browser/extensions/extension_function_dispatcher.cc b/chrome/browser/extensions/extension_function_dispatcher.cc
index 8abc84c..7476cb0 100644
--- a/chrome/browser/extensions/extension_function_dispatcher.cc
+++ b/chrome/browser/extensions/extension_function_dispatcher.cc
@@ -360,16 +360,6 @@ Browser* ExtensionFunctionDispatcher::GetCurrentBrowser(
   return browser;
 }
 
-Extension* ExtensionFunctionDispatcher::GetExtension() {
-  ExtensionsService* service = profile()->GetExtensionsService();
-  DCHECK(service);
-
-  Extension* extension = service->GetExtensionById(extension_id(), false);
-  DCHECK(extension);
-
-  return extension;
-}
-
 void ExtensionFunctionDispatcher::HandleRequest(const std::string& name,
                                                 const Value* args,
                                                 const GURL& source_url,
@@ -378,6 +368,8 @@ void ExtensionFunctionDispatcher::HandleRequest(const std::string& name,
   scoped_refptr<ExtensionFunction> function(
       FactoryRegistry::instance()->NewFunction(name));
   function->set_dispatcher_peer(peer_);
+  function->set_profile(profile_);
+  function->set_extension_id(extension_id());
   function->SetArgs(args);
   function->set_source_url(source_url);
   function->set_request_id(request_id);
diff --git a/chrome/browser/extensions/extension_function_dispatcher.h b/chrome/browser/extensions/extension_function_dispatcher.h
index 08e74bc1..fc50c17 100644
--- a/chrome/browser/extensions/extension_function_dispatcher.h
+++ b/chrome/browser/extensions/extension_function_dispatcher.h
@@ -105,10 +105,6 @@ class ExtensionFunctionDispatcher {
   // details.
   Browser* GetCurrentBrowser(bool include_incognito);
 
-  // Gets the extension the function is being invoked by. This should not ever
-  // return NULL.
-  Extension* GetExtension();
-
   // Handle a malformed message.  Possibly the result of an attack, so kill
   // the renderer.
   void HandleBadMessage(ExtensionFunction* api);
diff --git a/chrome/browser/extensions/extension_infobar_module.cc b/chrome/browser/extensions/extension_infobar_module.cc
index 73768fb..d8cea95 100644
--- a/chrome/browser/extensions/extension_infobar_module.cc
+++ b/chrome/browser/extensions/extension_infobar_module.cc
@@ -26,7 +26,7 @@ bool ShowInfoBarFunction::RunImpl() {
   std::string html_path;
   EXTENSION_FUNCTION_VALIDATE(args->GetString(keys::kHtmlPath, &html_path));
 
-  Extension* extension = dispatcher()->GetExtension();
+  Extension* extension = GetExtension();
   GURL url = extension->GetResourceURL(extension->url(), html_path);
 
   Browser* browser = NULL;
diff --git a/chrome/browser/extensions/extension_page_actions_module.cc b/chrome/browser/extensions/extension_page_actions_module.cc
index 5fcbf45..01ca992 100644
--- a/chrome/browser/extensions/extension_page_actions_module.cc
+++ b/chrome/browser/extensions/extension_page_actions_module.cc
@@ -56,7 +56,7 @@ bool PageActionFunction::SetPageActionEnabled(bool enable) {
     }
   }
 
-  ExtensionAction* page_action = dispatcher()->GetExtension()->page_action();
+  ExtensionAction* page_action = GetExtension()->page_action();
   if (!page_action) {
     error_ = kNoPageActionError;
     return false;
@@ -95,7 +95,7 @@ bool PageActionFunction::SetPageActionEnabled(bool enable) {
 }
 
 bool PageActionFunction::InitCommon(int tab_id) {
-  page_action_ = dispatcher()->GetExtension()->page_action();
+  page_action_ = GetExtension()->page_action();
   if (!page_action_) {
     error_ = kNoPageActionError;
     return false;
diff --git a/chrome/browser/extensions/extension_popup_api.cc b/chrome/browser/extensions/extension_popup_api.cc
index c7b28ed..f73dc7f 100644
--- a/chrome/browser/extensions/extension_popup_api.cc
+++ b/chrome/browser/extensions/extension_popup_api.cc
@@ -266,7 +266,7 @@ bool PopupShowFunction::RunImpl() {
   // Disallow non-extension requests, or requests outside of the requesting
   // extension view's extension.
   const std::string& extension_id = url.host();
-  if (extension_id != dispatcher()->GetExtension()->id() ||
+  if (extension_id != GetExtension()->id() ||
       !url.SchemeIs(chrome::kExtensionScheme)) {
     error_ = kInvalidURLError;
     return false;