Refactor to allow same code to test both sync and async functions.

BUG=none
TEST=none


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

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

chrome/browser/extensions/extension_function.cc

@@ -128,7 +128,7 @@ void ExtensionFunction::HandleBadMessage(base::ProcessHandle process) {
   }
 }
 UIThreadExtensionFunction::UIThreadExtensionFunction()
-    : render_view_host_(NULL), profile_(NULL) {
+    : render_view_host_(NULL), profile_(NULL), delegate_(NULL) {
 }
 
 UIThreadExtensionFunction::~UIThreadExtensionFunction() {
@@ -154,13 +154,17 @@ Browser* UIThreadExtensionFunction::GetCurrentBrowser() {
 }
 
 void UIThreadExtensionFunction::SendResponse(bool success) {
-  if (!render_view_host_ || !dispatcher())
-    return;
+  if (delegate_) {
+    delegate_->OnSendResponse(this, success);
+  } else {
+    if (!render_view_host_ || !dispatcher())
+      return;
 
-  SendResponseImpl(render_view_host_->process()->GetHandle(),
-                   render_view_host_,
-                   render_view_host_->routing_id(),
-                   success);
+    SendResponseImpl(render_view_host_->process()->GetHandle(),
+                     render_view_host_,
+                     render_view_host_->routing_id(),
+                     success);
+  }
 }
 
 IOThreadExtensionFunction::IOThreadExtensionFunction()
@@ -187,14 +191,7 @@ void IOThreadExtensionFunction::SendResponse(bool success) {
                    ipc_sender(), routing_id_, success);
 }
 
-AsyncExtensionFunction::AsyncExtensionFunction() : delegate_(NULL) {
-}
-
-void AsyncExtensionFunction::SendResponse(bool success) {
-  if (delegate_)
-    delegate_->OnSendResponse(this, success);
-  else
-    UIThreadExtensionFunction::SendResponse(success);
+AsyncExtensionFunction::AsyncExtensionFunction() {
 }
 
 AsyncExtensionFunction::~AsyncExtensionFunction() {

chrome/browser/extensions/extension_function.h

@@ -208,10 +208,20 @@ class ExtensionFunction
 // this category.
 class UIThreadExtensionFunction : public ExtensionFunction {
  public:
+  // A delegate for use in testing, to intercept the call to SendResponse.
+  class DelegateForTests {
+   public:
+    virtual void OnSendResponse(UIThreadExtensionFunction* function,
+                                bool success) = 0;
+  };
+
   UIThreadExtensionFunction();
 
   virtual UIThreadExtensionFunction* AsUIThreadExtensionFunction() OVERRIDE;
 
+  void set_test_delegate(DelegateForTests* delegate) {
+    delegate_ = delegate;
+  }
   // Set the profile which contains the extension that has originated this
   // function call.
   void set_profile(Profile* profile) { profile_ = profile; }
@@ -285,6 +295,8 @@ class UIThreadExtensionFunction : public ExtensionFunction {
   virtual void Destruct() const OVERRIDE;
 
   scoped_ptr<RenderViewHostTracker> tracker_;
+
+  DelegateForTests* delegate_;
 };
 
 // Extension functions that run on the IO thread. This type of function avoids
@@ -340,24 +352,10 @@ class IOThreadExtensionFunction : public ExtensionFunction {
 // the browser's UI thread*.
 class AsyncExtensionFunction : public UIThreadExtensionFunction {
  public:
-  // A delegate for use in testing, to intercept the call to SendResponse.
-  class DelegateForTests {
-   public:
-    virtual void OnSendResponse(AsyncExtensionFunction* function,
-                                bool success) = 0;
-  };
-
   AsyncExtensionFunction();
-  virtual void SendResponse(bool success) OVERRIDE;
-
-  void set_test_delegate(DelegateForTests* delegate) {
-    delegate_ = delegate;
-  }
 
  protected:
   virtual ~AsyncExtensionFunction();
-
-  DelegateForTests* delegate_;
 };
 
 // A SyncExtensionFunction is an ExtensionFunction that runs synchronously

chrome/browser/extensions/extension_function_test_utils.cc

@@ -142,28 +142,10 @@ base::Value* RunFunctionAndReturnResult(UIThreadExtensionFunction* function,
   return function->GetResultValue()->DeepCopy();
 }
 
-void RunFunction(UIThreadExtensionFunction* function,
-                 const std::string& args,
-                 Browser* browser,
-                 RunFunctionFlags flags) {
-  scoped_ptr<base::ListValue> parsed_args(ParseList(args));
-  ASSERT_TRUE(parsed_args.get()) <<
-      "Could not parse extension function arguments: " << args;
-  function->SetArgs(parsed_args.get());
-
-  TestFunctionDispatcherDelegate dispatcher_delegate(browser);
-  ExtensionFunctionDispatcher dispatcher(
-      browser->profile(), &dispatcher_delegate);
-  function->set_dispatcher(dispatcher.AsWeakPtr());
-
-  function->set_profile(browser->profile());
-  function->set_include_incognito(flags & INCLUDE_INCOGNITO);
-  function->Run();
-}
-
 // This helps us be able to wait until an AsyncExtensionFunction calls
 // SendResponse.
-class SendResponseDelegate : public AsyncExtensionFunction::DelegateForTests {
+class SendResponseDelegate
+    : public UIThreadExtensionFunction::DelegateForTests {
  public:
   SendResponseDelegate() : should_post_quit_(false) {}
 
@@ -182,7 +164,8 @@ class SendResponseDelegate : public AsyncExtensionFunction::DelegateForTests {
     return *response_.get();
   }
 
-  virtual void OnSendResponse(AsyncExtensionFunction* function, bool success) {
+  virtual void OnSendResponse(UIThreadExtensionFunction* function,
+                              bool success) {
     ASSERT_FALSE(HasResponse());
     response_.reset(new bool);
     *response_ = success;
@@ -196,13 +179,25 @@ class SendResponseDelegate : public AsyncExtensionFunction::DelegateForTests {
   bool should_post_quit_;
 };
 
-bool RunAsyncFunction(AsyncExtensionFunction* function,
-                      const std::string& args,
-                      Browser* browser,
-                      RunFunctionFlags flags) {
+bool RunFunction(UIThreadExtensionFunction* function,
+                 const std::string& args,
+                 Browser* browser,
+                 RunFunctionFlags flags) {
   SendResponseDelegate response_delegate;
   function->set_test_delegate(&response_delegate);
-  RunFunction(function, args, browser, flags);
+  scoped_ptr<base::ListValue> parsed_args(ParseList(args));
+  EXPECT_TRUE(parsed_args.get()) <<
+      "Could not parse extension function arguments: " << args;
+  function->SetArgs(parsed_args.get());
+
+  TestFunctionDispatcherDelegate dispatcher_delegate(browser);
+  ExtensionFunctionDispatcher dispatcher(
+      browser->profile(), &dispatcher_delegate);
+  function->set_dispatcher(dispatcher.AsWeakPtr());
+
+  function->set_profile(browser->profile());
+  function->set_include_incognito(flags & INCLUDE_INCOGNITO);
+  function->Run();
 
   // If the RunImpl of |function| didn't already call SendResponse, run the
   // message loop until they do.

chrome/browser/extensions/extension_function_test_utils.h

@@ -49,8 +49,7 @@ enum RunFunctionFlags {
 };
 
 // Run |function| with |args| and return the resulting error. Adds an error to
-// the current test if |function| returns a result. The caller releases
-// ownership of |function|.
+// the current test if |function| returns a result.
 std::string RunFunctionAndReturnError(UIThreadExtensionFunction* function,
                                       const std::string& args,
                                       Browser* browser,
@@ -60,8 +59,8 @@ std::string RunFunctionAndReturnError(UIThreadExtensionFunction* function,
                                       Browser* browser);
 
 // Run |function| with |args| and return the result. Adds an error to the
-// current test if |function| returns an error. The caller releases ownership of
-// |function|. the caller takes ownership of the result.
+// current test if |function| returns an error. The caller takes ownership of
+// the result.
 base::Value* RunFunctionAndReturnResult(UIThreadExtensionFunction* function,
                                         const std::string& args,
                                         Browser* browser,
@@ -70,28 +69,21 @@ base::Value* RunFunctionAndReturnResult(UIThreadExtensionFunction* function,
                                         const std::string& args,
                                         Browser* browser);
 
-// Create and run |function| with |args|. The caller retains ownership of
-// |function|.
+// Create and run |function| with |args|. Works with both synchronous and async
+// functions.
 //
 // TODO(aa): It would be nice if |args| could be validated against the schema
 // that |function| expects. That way, we know that we are testing something
 // close to what the bindings would actually send.
 //
 // TODO(aa): I'm concerned that this style won't scale to all the bits and bobs
-// we're going to need to frob for all the different extension functions. But we
-// can refactor when we see what is needed.
-void RunFunction(UIThreadExtensionFunction* function,
+// we're going to need to frob for all the different extension functions. But
+// we can refactor when we see what is needed.
+bool RunFunction(UIThreadExtensionFunction* function,
                  const std::string& args,
                  Browser* browser,
                  RunFunctionFlags flags);
 
-// Similar to RunFunction, but doesn't return until |function| calls
-// SendResponse. Returns the value |function| passed to SendResponse.
-bool RunAsyncFunction(AsyncExtensionFunction* function,
-                      const std::string& args,
-                      Browser* browser,
-                      RunFunctionFlags flags);
-
 } // namespace extension_function_test_utils
 
 #endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_FUNCTION_TEST_UTILS_H_

chrome/browser/extensions/extension_management_api_browsertest.cc

@@ -82,7 +82,7 @@ class ExtensionManagementApiEscalationTest : public ExtensionBrowserTest {
     scoped_refptr<SetEnabledFunction> function(new SetEnabledFunction);
     if (user_gesture)
       function->set_user_gesture(true);
-    bool response = util::RunAsyncFunction(
+    bool response = util::RunFunction(
         function.get(),
         base::StringPrintf("[\"%s\", true]", kId),
         browser(),