Fix ExtensionApiTest.TabUpdateNoPermissions timeouts

Switch testing of chrome.tabs.update with no tabs permission from an API test
to an InProcessBrowserTest that calls the extension function directly. This
should hopefully be more reliable, especially on overloaded/slow bots.

Test run times (in milliseconds):

                             API Test  C++ Test
      Normal 16-thread z600:    3809       2209
              1 CPU enabled:    4036       2256
1 CPU + lookbusy -c 99 -n 2:    7342       3138
1 CPU + lookbusy -c 99 -n 4:   10705       3481
1 CPU + lookbusy -c 99 -n 8:   17830       6142

Also removes a bunch of StartTestServer() calls from other tabs API tests, since
they're not necessary (and are also causing slower run times).

BUG=98383
R=aa@chromium.org

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

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

4 files changed, 82 insertions, 29 deletions

diff --git a/chrome/browser/extensions/extension_function_test_utils.cc b/chrome/browser/extensions/extension_function_test_utils.cc
index 4f5b8ae..7c0db6d 100644
--- a/chrome/browser/extensions/extension_function_test_utils.cc
+++ b/chrome/browser/extensions/extension_function_test_utils.cc
@@ -6,11 +6,42 @@
 
 #include <string>
 
+#include "base/file_path.h"
 #include "base/json/json_reader.h"
 #include "base/values.h"
+#include "chrome/browser/extensions/extension_function.h"
+#include "chrome/browser/extensions/extension_function_dispatcher.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/common/extensions/extension.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace {
+
+class TestFunctionDispatcherDelegate
+    : public ExtensionFunctionDispatcher::Delegate {
+ public:
+  explicit TestFunctionDispatcherDelegate(Browser* browser) :
+      browser_(browser) {}
+  virtual ~TestFunctionDispatcherDelegate() {}
+
+ private:
+  virtual Browser* GetBrowser() OVERRIDE {
+    return browser_;
+  }
+
+  virtual gfx::NativeView GetNativeViewOfHost() OVERRIDE {
+    return NULL;
+  }
+
+  virtual TabContents* GetAssociatedTabContents() const OVERRIDE {
+    return NULL;
+  }
+
+  Browser* browser_;
+};
+
+}  // namespace
+
 namespace extension_function_test_utils {
 
 base::Value* ParseJSON(const std::string& data) {
@@ -62,6 +93,22 @@ base::DictionaryValue* ToDictionary(base::Value* val) {
   return static_cast<base::DictionaryValue*>(val);
 }
 
+scoped_refptr<Extension> CreateEmptyExtension() {
+  std::string error;
+  const FilePath test_extension_path;
+  scoped_ptr<base::DictionaryValue> test_extension_value(
+      ParseDictionary("{\"name\": \"Test\", \"version\": \"1.0\"}"));
+  scoped_refptr<Extension> extension(Extension::Create(
+      test_extension_path,
+      Extension::INTERNAL,
+      *test_extension_value.get(),
+      Extension::NO_FLAGS,
+      &error));
+  if (!error.empty())
+    ADD_FAILURE() << "Could not parse test extension " << error;
+  return extension;
+}
+
 std::string RunFunctionAndReturnError(UIThreadExtensionFunction* function,
                                       const std::string& args,
                                       Browser* browser) {
@@ -90,7 +137,9 @@ base::Value* RunFunctionAndReturnResult(UIThreadExtensionFunction* function,
   scoped_refptr<ExtensionFunction> function_owner(function);
   RunFunction(function, args, browser, flags);
   if (!function->GetError().empty())
-    ADD_FAILURE() << function->GetError();
+    ADD_FAILURE() << "Unexpected error: " << function->GetError();
+  if (!function->GetResultValue())
+    ADD_FAILURE() << "No result value found";
   return function->GetResultValue()->DeepCopy();
 }
 
@@ -99,7 +148,16 @@ void RunFunction(UIThreadExtensionFunction* function,
                  Browser* browser,
                  RunFunctionFlags flags) {
   scoped_ptr<base::ListValue> parsed_args(ParseList(args));
+  if (!parsed_args.get()) {
+    ADD_FAILURE() << "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();
diff --git a/chrome/browser/extensions/extension_function_test_utils.h b/chrome/browser/extensions/extension_function_test_utils.h
index acdabda..3b24b5d 100644
--- a/chrome/browser/extensions/extension_function_test_utils.h
+++ b/chrome/browser/extensions/extension_function_test_utils.h
@@ -9,7 +9,10 @@
 #include <string>
 
 #include "base/memory/ref_counted.h"
-#include "chrome/browser/extensions/extension_function.h"
+
+class Browser;
+class Extension;
+class UIThreadExtensionFunction;
 
 namespace base {
 class Value;
@@ -35,6 +38,10 @@ std::string GetString(base::DictionaryValue* val, const std::string& key);
 // If |val| is a dictionary, return it as one, otherwise NULL.
 base::DictionaryValue* ToDictionary(base::Value* val);
 
+// Creates an extension instance that can be attached to an ExtensionFunction
+// before running it.
+scoped_refptr<Extension> CreateEmptyExtension();
+
 enum RunFunctionFlags {
   NONE = 0,
   INCLUDE_INCOGNITO = 1 << 0
diff --git a/chrome/browser/extensions/extension_tabs_apitest.cc b/chrome/browser/extensions/extension_tabs_apitest.cc
index 9c43c99..48cfac5 100644
--- a/chrome/browser/extensions/extension_tabs_apitest.cc
+++ b/chrome/browser/extensions/extension_tabs_apitest.cc
@@ -60,8 +60,6 @@
 #endif
 
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, Tabs) {
-  ASSERT_TRUE(StartTestServer());
-
   // The test creates a tab and checks that the URL of the new tab
   // is that of the new tab page.  Make sure the pref that controls
   // this is set.
@@ -72,60 +70,43 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, Tabs) {
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, Tabs2) {
-  ASSERT_TRUE(StartTestServer());
   ASSERT_TRUE(RunExtensionSubtest("tabs/basics", "crud2.html")) << message_;
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, TabUpdate) {
-  ASSERT_TRUE(StartTestServer());
   ASSERT_TRUE(RunExtensionSubtest("tabs/basics", "update.html")) << message_;
 }
 
-// This times out regularly on all platforms. http://crbug.com/98383
-IN_PROC_BROWSER_TEST_F(ExtensionApiTest, DISABLED_TabUpdateNoPermissions) {
-  ASSERT_TRUE(StartTestServer());
-  ASSERT_TRUE(RunExtensionSubtest(
-      "tabs/no_permissions", "update.html")) << message_;
-}
-
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, TabPinned) {
-  ASSERT_TRUE(StartTestServer());
   ASSERT_TRUE(RunExtensionSubtest("tabs/basics", "pinned.html")) << message_;
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, MAYBE_TabMove) {
-  ASSERT_TRUE(StartTestServer());
   ASSERT_TRUE(RunExtensionSubtest("tabs/basics", "move.html")) << message_;
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, MAYBE_TabEvents) {
-  ASSERT_TRUE(StartTestServer());
   ASSERT_TRUE(RunExtensionSubtest("tabs/basics", "events.html")) << message_;
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, TabRelativeURLs) {
-  ASSERT_TRUE(StartTestServer());
   ASSERT_TRUE(RunExtensionSubtest("tabs/basics", "relative_urls.html"))
       << message_;
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, TabQuery) {
-  ASSERT_TRUE(StartTestServer());
   ASSERT_TRUE(RunExtensionSubtest("tabs/basics", "query.html")) << message_;
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, TabHighlight) {
-  ASSERT_TRUE(StartTestServer());
   ASSERT_TRUE(RunExtensionSubtest("tabs/basics", "highlight.html")) << message_;
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, TabCrashBrowser) {
-  ASSERT_TRUE(StartTestServer());
   ASSERT_TRUE(RunExtensionSubtest("tabs/basics", "crash.html")) << message_;
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, TabGetCurrent) {
-  ASSERT_TRUE(StartTestServer());
   ASSERT_TRUE(RunExtensionTest("tabs/get_current")) << message_;
 }
 
@@ -136,12 +117,10 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, FLAKY_TabConnect) {
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, MAYBE_TabOnRemoved) {
-  ASSERT_TRUE(StartTestServer());
   ASSERT_TRUE(RunExtensionTest("tabs/on_removed")) << message_;
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, TabReload) {
-  ASSERT_TRUE(StartTestServer());
   ASSERT_TRUE(RunExtensionTest("tabs/reload")) << message_;
 }
 
@@ -182,7 +161,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, MAYBE_CaptureVisibleTabPng) {
 #define MAYBE_CaptureVisibleTabRace DISABLED_CaptureVisibleTabRace
 #endif
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, MAYBE_CaptureVisibleTabRace) {
-  ASSERT_TRUE(StartTestServer());
   ASSERT_TRUE(RunExtensionSubtest("tabs/capture_visible_tab",
                                   "test_race.html")) << message_;
 }
@@ -198,7 +176,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, CaptureVisibleNoFile) {
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, TabsOnUpdated) {
-  ASSERT_TRUE(StartTestServer());
   ASSERT_TRUE(RunExtensionTest("tabs/on_updated")) << message_;
 }
 
@@ -227,8 +204,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, FocusWindowDoesNotUnmaximize) {
 #endif  // OS_WIN
 
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, IncognitoDisabledByPref) {
-  ASSERT_TRUE(StartTestServer());
-
   IncognitoModePrefs::SetAvailability(browser()->profile()->GetPrefs(),
                                       IncognitoModePrefs::DISABLED);
 
@@ -238,12 +213,10 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, IncognitoDisabledByPref) {
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, MAYBE_GetViewsOfCreatedPopup) {
-  ASSERT_TRUE(StartTestServer());
   ASSERT_TRUE(RunExtensionSubtest("tabs/basics", "get_views_popup.html"))
       << message_;
 }
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, MAYBE_GetViewsOfCreatedWindow) {
-  ASSERT_TRUE(StartTestServer());
   ASSERT_TRUE(RunExtensionSubtest("tabs/basics", "get_views_window.html"))
       << message_;
 }
diff --git a/chrome/browser/extensions/extension_tabs_test.cc b/chrome/browser/extensions/extension_tabs_test.cc
index a3f0282..3428abc 100644
--- a/chrome/browser/extensions/extension_tabs_test.cc
+++ b/chrome/browser/extensions/extension_tabs_test.cc
@@ -109,5 +109,20 @@ IN_PROC_BROWSER_TEST_F(ExtensionTabsTest, GetWindow) {
           browser(),
           INCLUDE_INCOGNITO)));
   EXPECT_TRUE(GetBoolean(result.get(), "incognito"));
+}
 
+IN_PROC_BROWSER_TEST_F(ExtensionTabsTest, UpdateNoPermissions) {
+  // The test empty extension has no permissions, therefore it should not get
+  // tab data in the function result.
+  scoped_refptr<UpdateTabFunction> update_tab_function(new UpdateTabFunction());
+  scoped_refptr<Extension> empty_extension(CreateEmptyExtension());
+  update_tab_function->set_extension(empty_extension.get());
+  // Without a callback the function will not generate a result.
+  update_tab_function->set_has_callback(true);
+
+  scoped_ptr<base::Value> result(RunFunctionAndReturnResult(
+          update_tab_function.get(),
+          "[null, {\"url\": \"neutrinos\"}]",
+          browser()));
+  EXPECT_EQ(base::Value::TYPE_NULL, result->GetType());
 }