diff options
author | aa@chromium.org <aa@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-07-04 16:36:25 +0000 |
---|---|---|
committer | aa@chromium.org <aa@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-07-04 16:36:25 +0000 |
commit | 17c4f3c7c289c840f8b285f7834ab7007f85e35e (patch) | |
tree | e21af5a41aba26623cf56d6fe2412c34a32daafc /chrome/browser | |
parent | cd659ee5e9e02a3fdf261fb9ebdf25801661b5fa (diff) | |
download | chromium_src-17c4f3c7c289c840f8b285f7834ab7007f85e35e.zip chromium_src-17c4f3c7c289c840f8b285f7834ab7007f85e35e.tar.gz chromium_src-17c4f3c7c289c840f8b285f7834ab7007f85e35e.tar.bz2 |
Add an ExtensionBrowserTest base class that allows in-process browser tests of extensions using ExtensionsService directly, rather than TestExtensionLoaded. Use it to re-enable some old browser tests that had been disabled.
Review URL: http://codereview.chromium.org/150213
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@19930 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser')
20 files changed, 328 insertions, 363 deletions
diff --git a/chrome/browser/browser_focus_uitest.cc b/chrome/browser/browser_focus_uitest.cc index 7fcc599..e8269e0 100644 --- a/chrome/browser/browser_focus_uitest.cc +++ b/chrome/browser/browser_focus_uitest.cc @@ -339,7 +339,7 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, FocusTraversal) { // Let's make sure the focus is on the expected element in the page. std::string actual; ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractString( - browser()->GetSelectedTabContents(), + browser()->GetSelectedTabContents()->render_view_host(), L"", L"window.domAutomationController.send(getFocusedElement());", &actual)); @@ -376,7 +376,7 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, FocusTraversal) { // Let's make sure the focus is on the expected element in the page. std::string actual; ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractString( - browser()->GetSelectedTabContents(), + browser()->GetSelectedTabContents()->render_view_host(), L"", L"window.domAutomationController.send(getFocusedElement());", &actual)); diff --git a/chrome/browser/debugger/devtools_sanity_unittest.cc b/chrome/browser/debugger/devtools_sanity_unittest.cc index 0918a0d..df512e3 100644 --- a/chrome/browser/debugger/devtools_sanity_unittest.cc +++ b/chrome/browser/debugger/devtools_sanity_unittest.cc @@ -63,7 +63,7 @@ class DevToolsSanityTest : public InProcessBrowserTest { // files have been loaded) and has runTest method. ASSERT_TRUE( ui_test_utils::ExecuteJavaScriptAndExtractString( - client_contents_, + client_contents_->render_view_host(), L"", L"window.domAutomationController.send(" L"'' + (window.uiTests && (typeof uiTests.runTest)));", @@ -72,7 +72,7 @@ class DevToolsSanityTest : public InProcessBrowserTest { if (result == "function") { ASSERT_TRUE( ui_test_utils::ExecuteJavaScriptAndExtractString( - client_contents_, + client_contents_->render_view_host(), L"", UTF8ToWide(StringPrintf("uiTests.runTest('%s')", test_name.c_str())), &result)); diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc index 74a9fb5..3dcef8b 100644 --- a/chrome/browser/extensions/extension_browsertest.cc +++ b/chrome/browser/extensions/extension_browsertest.cc @@ -1,173 +1,116 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. +#include "chrome/browser/extensions/extension_browsertest.h" -#include "base/ref_counted.h" +#include "base/command_line.h" +#include "base/file_path.h" +#include "base/path_service.h" #include "chrome/browser/browser.h" -#include "chrome/browser/browser_list.h" -#include "chrome/browser/renderer_host/render_view_host.h" #include "chrome/browser/extensions/extension_host.h" -#include "chrome/browser/extensions/extension_process_manager.h" #include "chrome/browser/extensions/extensions_service.h" -#include "chrome/browser/extensions/test_extension_loader.h" #include "chrome/browser/profile.h" -#include "chrome/browser/renderer_host/site_instance.h" -#include "chrome/browser/views/extensions/extension_shelf.h" -#include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_switches.h" -#include "chrome/common/extensions/extension_error_reporter.h" -#include "chrome/common/url_constants.h" -#include "chrome/test/in_process_browser_test.h" +#include "chrome/common/chrome_paths.h" +#include "chrome/common/notification_registrar.h" +#include "chrome/common/notification_service.h" +#include "chrome/common/notification_type.h" #include "chrome/test/ui_test_utils.h" namespace { +// Amount of time to wait to load an extension. This is purposely obscenely +// long because it will only get used in the case of failure and we want to +// minimize false positives. +static const int kTimeoutMs = 60 * 1000; // 1 minute +}; -// How long to wait for the extension to put up a javascript alert before giving -// up. -const int kAlertTimeoutMs = 20000; - -// The extensions we're using as our test case. -const char* kGoodExtension1Id = "behllobkkfkfnphdnhnkndlbkcpglgmj"; -const char* kGoodCrxId = "ldnnhddmnhbkjipkidpdiheffobcpfmf"; - -}; // namespace - -// This class starts up an extension process and waits until it tries to put -// up a javascript alert. -class MockExtensionHost : public ExtensionHost { - public: - MockExtensionHost(Extension* extension, const GURL& url, - SiteInstance* instance) - : ExtensionHost(extension, instance, url), - got_message_(false) { - CreateRenderView(NULL); - MessageLoop::current()->PostDelayedTask(FROM_HERE, - new MessageLoop::QuitTask, kAlertTimeoutMs); - ui_test_utils::RunMessageLoop(); - } +// Base class for extension browser tests. Provides utilities for loading, +// unloading, and installing extensions. +void ExtensionBrowserTest::SetUpCommandLine(CommandLine* command_line) { + // This enables DOM automation for tab contentses. + EnableDOMAutomation(); - virtual ExtensionFunctionDispatcher* CreateExtensionFunctionDispatcher( - RenderViewHost* render_view_host) { - NOTREACHED(); - return NULL; - } + // This enables it for extension hosts. + ExtensionHost::EnableDOMAutomation(); - bool got_message() { return got_message_; } - private: - virtual void RunJavaScriptMessage( - const std::wstring& message, - const std::wstring& default_prompt, - const GURL& frame_url, - const int flags, - IPC::Message* reply_msg, - bool* did_suppress_message) { - got_message_ = true; - MessageLoopForUI::current()->Quit(); - - // Call super, otherwise we'll leak reply_msg. - ExtensionHost::RunJavaScriptMessage( - message, default_prompt, frame_url, flags, - reply_msg, did_suppress_message); - } + command_line->AppendSwitch(switches::kEnableExtensions); - bool got_message_; -}; + PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir_); + test_data_dir_ = test_data_dir_.AppendASCII("extensions"); +} -class ExtensionViewTest : public InProcessBrowserTest { - public: - virtual void SetUp() { - // Initialize the error reporter here, otherwise BrowserMain will create it - // with the wrong MessageLoop. - ExtensionErrorReporter::Init(false); +bool ExtensionBrowserTest::LoadExtension(const FilePath& path) { + ExtensionsService* service = browser()->profile()->GetExtensionsService(); + size_t num_before = service->extensions()->size(); + registrar_.Add(this, NotificationType::EXTENSIONS_LOADED, + NotificationService::AllSources()); + service->LoadExtension(path); + MessageLoop::current()->PostDelayedTask(FROM_HERE, new MessageLoop::QuitTask, + kTimeoutMs); + ui_test_utils::RunMessageLoop(); + registrar_.Remove(this, NotificationType::EXTENSIONS_LOADED, + NotificationService::AllSources()); + size_t num_after = service->extensions()->size(); + if (num_after != (num_before + 1)) + return false; + + return WaitForExtensionHostsToLoad(); +} - InProcessBrowserTest::SetUp(); - } - virtual void SetUpCommandLine(CommandLine* command_line) { - command_line->AppendSwitch(switches::kEnableExtensions); - } -}; +bool ExtensionBrowserTest::InstallExtension(const FilePath& path) { + ExtensionsService* service = browser()->profile()->GetExtensionsService(); + service->set_show_extensions_prompts(false); + size_t num_before = service->extensions()->size(); + + registrar_.Add(this, NotificationType::EXTENSION_INSTALLED, + NotificationService::AllSources()); + service->InstallExtension(path); + MessageLoop::current()->PostDelayedTask(FROM_HERE, new MessageLoop::QuitTask, + kTimeoutMs); + ui_test_utils::RunMessageLoop(); + registrar_.Remove(this, NotificationType::EXTENSION_INSTALLED, + NotificationService::AllSources()); + size_t num_after = service->extensions()->size(); + if (num_after != (num_before + 1)) + return false; + + return WaitForExtensionHostsToLoad(); +} -// Tests that ExtensionView starts an extension process and runs the script -// contained in the extension's toolstrip. -// TODO(mpcomplete): http://crbug.com/15081 Disabled because it fails. -IN_PROC_BROWSER_TEST_F(ExtensionViewTest, DISABLED_Toolstrip) { - // Get the path to our extension. - FilePath path; - ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &path)); - path = path.AppendASCII("extensions") - .AppendASCII("good") - .AppendASCII("Extensions") - .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj") - .AppendASCII("1.0.0.0"); - ASSERT_TRUE(file_util::DirectoryExists(path)); // sanity check - - // Wait for the extension to load and grab a pointer to it. - TestExtensionLoader loader(browser()->profile()); - Extension* extension = loader.Load(kGoodExtension1Id, path); - ASSERT_TRUE(extension); - GURL url = Extension::GetResourceURL(extension->url(), "toolstrip1.html"); - - // Start the extension process and wait for it to show a javascript alert. - MockExtensionHost host(extension, url, - browser()->profile()->GetExtensionProcessManager()-> - GetSiteInstanceForURL(url)); - EXPECT_TRUE(host.got_message()); +void ExtensionBrowserTest::UninstallExtension(const std::string& extension_id) { + ExtensionsService* service = browser()->profile()->GetExtensionsService(); + service->UninstallExtension(extension_id, false); } -// Tests that the ExtensionShelf initializes properly, notices that -// an extension loaded and has a view available, and then sets that up -// properly. -// TODO(mpcomplete): http://crbug.com/15081 Disabled because it fails. -IN_PROC_BROWSER_TEST_F(ExtensionViewTest, DISABLED_Shelf) { - // When initialized, there are no extension views and the preferred height - // should be zero. - scoped_ptr<ExtensionShelf> shelf(new ExtensionShelf(browser())); - EXPECT_EQ(shelf->GetChildViewCount(), 0); - EXPECT_EQ(shelf->GetPreferredSize().height(), 0); - - // Get the path to our extension. - FilePath path; - ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &path)); - path = path.AppendASCII("extensions") - .AppendASCII("good") - .AppendASCII("Extensions") - .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj") - .AppendASCII("1.0.0.0"); - ASSERT_TRUE(file_util::DirectoryExists(path)); // sanity check - - // Wait for the extension to load and grab a pointer to it. - TestExtensionLoader loader(browser()->profile()); - Extension* extension = loader.Load(kGoodExtension1Id, path); - ASSERT_TRUE(extension); - - // There should now be two extension views and preferred height of the view - // should be non-zero. - EXPECT_EQ(shelf->GetChildViewCount(), 2); - EXPECT_NE(shelf->GetPreferredSize().height(), 0); +bool ExtensionBrowserTest::WaitForExtensionHostsToLoad() { + // Wait for all the extension hosts that exist to finish loading. + // NOTE: This assumes that the extension host list is not changing while + // this method is running. + ExtensionProcessManager* manager = + browser()->profile()->GetExtensionProcessManager(); + base::Time start_time = base::Time::Now(); + for (ExtensionProcessManager::const_iterator iter = manager->begin(); + iter != manager->end(); ++iter) { + while (!(*iter)->did_stop_loading()) { + if ((base::Time::Now() - start_time).InMilliseconds() > kTimeoutMs) + return false; + + MessageLoop::current()->PostDelayedTask(FROM_HERE, + new MessageLoop::QuitTask, 200); + ui_test_utils::RunMessageLoop(); + } + } + + return true; } -// Tests that installing and uninstalling extensions don't crash with an -// incognito window open. -// TODO(mpcomplete): http://crbug.com/14947 disabled because it crashes. -IN_PROC_BROWSER_TEST_F(ExtensionViewTest, DISABLED_Incognito) { - // Open an incognito window to the extensions management page. We just - // want to make sure that we don't crash while playing with extensions when - // this guy is around. - Browser::OpenURLOffTheRecord(browser()->profile(), - GURL(chrome::kChromeUIExtensionsURL)); - - // Get the path to our extension. - FilePath path; - ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &path)); - path = path.AppendASCII("extensions").AppendASCII("good.crx"); - ASSERT_TRUE(file_util::PathExists(path)); // sanity check - - // Wait for the extension to load and grab a pointer to it. - TestExtensionLoader loader(browser()->profile()); - Extension* extension = loader.Install(kGoodCrxId, path); - ASSERT_TRUE(extension); - - // TODO(mpcomplete): wait for uninstall to complete? - browser()->profile()->GetExtensionsService()-> - UninstallExtension(kGoodCrxId, false); +void ExtensionBrowserTest::Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + switch (type.value) { + case NotificationType::EXTENSION_INSTALLED: + case NotificationType::EXTENSIONS_LOADED: + MessageLoopForUI::current()->Quit(); + break; + default: + NOTREACHED(); + break; + } } diff --git a/chrome/browser/extensions/extension_browsertest.h b/chrome/browser/extensions/extension_browsertest.h new file mode 100644 index 0000000..385fd9e --- /dev/null +++ b/chrome/browser/extensions/extension_browsertest.h @@ -0,0 +1,41 @@ +// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_BROWSER_TEST_H_ +#define CHROME_BROWSER_EXTENSIONS_EXTENSION_BROWSER_TEST_H_ + +#include <string> + +#include "base/command_line.h" +#include "base/file_path.h" +#include "chrome/common/notification_details.h" +#include "chrome/common/notification_observer.h" +#include "chrome/common/notification_registrar.h" +#include "chrome/common/notification_type.h" +#include "chrome/test/in_process_browser_test.h" + +// Base class for extension browser tests. Provides utilities for loading, +// unloading, and installing extensions. +class ExtensionBrowserTest + : public InProcessBrowserTest, public NotificationObserver { + protected: + virtual void SetUpCommandLine(CommandLine* command_line); + bool LoadExtension(const FilePath& path); + bool InstallExtension(const FilePath& path); + void UninstallExtension(const std::string& extension_id); + + bool loaded_; + bool installed_; + FilePath test_data_dir_; + + private: + virtual void Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details); + bool WaitForExtensionHostsToLoad(); + + NotificationRegistrar registrar_; +}; + +#endif // CHROME_BROWSER_EXTENSIONS_EXTENSION_BROWSER_TEST_H_ diff --git a/chrome/browser/extensions/extension_browsertests_misc.cc b/chrome/browser/extensions/extension_browsertests_misc.cc new file mode 100644 index 0000000..1f149d4 --- /dev/null +++ b/chrome/browser/extensions/extension_browsertests_misc.cc @@ -0,0 +1,106 @@ +// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "base/ref_counted.h" +#include "chrome/browser/browser.h" +#include "chrome/browser/browser_list.h" +#include "chrome/browser/renderer_host/render_view_host.h" +#include "chrome/browser/extensions/extension_browsertest.h" +#include "chrome/browser/extensions/extension_host.h" +#include "chrome/browser/extensions/extension_process_manager.h" +#include "chrome/browser/extensions/extensions_service.h" +#include "chrome/browser/profile.h" +#include "chrome/browser/renderer_host/site_instance.h" +#include "chrome/browser/tab_contents/tab_contents.h" +#include "chrome/browser/views/extensions/extension_shelf.h" +#include "chrome/browser/views/frame/browser_view.h" +#include "chrome/common/chrome_paths.h" +#include "chrome/common/extensions/extension_error_reporter.h" +#include "chrome/common/url_constants.h" +#include "chrome/test/ui_test_utils.h" + +// Tests that toolstrips initializes properly and can run basic extension js. +IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, Toolstrip) { + ASSERT_TRUE(LoadExtension( + test_data_dir_.AppendASCII("good").AppendASCII("Extensions") + .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj") + .AppendASCII("1.0.0.0"))); + + // At this point, there should be two ExtensionHosts loaded because this + // extension has two toolstrips. Find the one that is hosting toolstrip1.html. + ExtensionProcessManager* manager = + browser()->profile()->GetExtensionProcessManager(); + ExtensionHost* host = NULL; + int num_hosts = 0; + for (ExtensionProcessManager::const_iterator iter = manager->begin(); + iter != manager->end(); ++iter) { + if ((*iter)->GetURL().path() == "/toolstrip1.html") { + ASSERT_FALSE(host); + host = *iter; + } + num_hosts++; + } + EXPECT_EQ(2, num_hosts); + + // Tell it to run some JavaScript that tests that basic extension code works. + bool result = false; + ui_test_utils::ExecuteJavaScriptAndExtractBool( + host->render_view_host(), L"", L"testTabsAPI()", &result); + EXPECT_TRUE(result); +} + +// Tests that the ExtensionShelf initializes properly, notices that +// an extension loaded and has a view available, and then sets that up +// properly. +IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, Shelf) { + // When initialized, there are no extension views and the preferred height + // should be zero. + BrowserView* browser_view = static_cast<BrowserView*>(browser()->window()); + ExtensionShelf* shelf = browser_view->extension_shelf(); + ASSERT_TRUE(shelf); + EXPECT_EQ(shelf->GetChildViewCount(), 0); + EXPECT_EQ(shelf->GetPreferredSize().height(), 0); + + ASSERT_TRUE(LoadExtension( + test_data_dir_.AppendASCII("good").AppendASCII("Extensions") + .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj") + .AppendASCII("1.0.0.0"))); + + // There should now be two extension views and preferred height of the view + // should be non-zero. + EXPECT_EQ(shelf->GetChildViewCount(), 2); + EXPECT_NE(shelf->GetPreferredSize().height(), 0); +} + +// Tests that installing and uninstalling extensions don't crash with an +// incognito window open. +IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, Incognito) { + // Open an incognito window to the extensions management page. We just + // want to make sure that we don't crash while playing with extensions when + // this guy is around. + Browser::OpenURLOffTheRecord(browser()->profile(), + GURL(chrome::kChromeUIExtensionsURL)); + + ASSERT_TRUE(InstallExtension(test_data_dir_.AppendASCII("good.crx"))); + UninstallExtension("ldnnhddmnhbkjipkidpdiheffobcpfmf"); +} + +// Tests that we can load extension pages into the tab area and they can call +// extension APIs. +IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, TabContents) { + ASSERT_TRUE(LoadExtension( + test_data_dir_.AppendASCII("good").AppendASCII("Extensions") + .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj") + .AppendASCII("1.0.0.0"))); + + ui_test_utils::NavigateToURL( + browser(), + GURL("chrome-extension://behllobkkfkfnphdnhnkndlbkcpglgmj/page.html")); + + bool result = false; + ui_test_utils::ExecuteJavaScriptAndExtractBool( + browser()->GetSelectedTabContents()->render_view_host(), L"", + L"testTabsAPI()", &result); + EXPECT_TRUE(result); +} diff --git a/chrome/browser/extensions/extension_host.cc b/chrome/browser/extensions/extension_host.cc index 48853d8..15319b3 100644 --- a/chrome/browser/extensions/extension_host.cc +++ b/chrome/browser/extensions/extension_host.cc @@ -77,6 +77,10 @@ class CrashedExtensionInfobarDelegate : public ConfirmInfoBarDelegate { } // namespace + +// static +bool ExtensionHost::enable_dom_automation_ = false; + ExtensionHost::ExtensionHost(Extension* extension, SiteInstance* site_instance, const GURL& url) : extension_(extension), @@ -86,6 +90,8 @@ ExtensionHost::ExtensionHost(Extension* extension, SiteInstance* site_instance, render_view_host_ = new RenderViewHost( site_instance, this, MSG_ROUTING_NONE, NULL); render_view_host_->AllowBindings(BindingsPolicy::EXTENSION); + if (enable_dom_automation_) + render_view_host_->AllowBindings(BindingsPolicy::DOM_AUTOMATION); } ExtensionHost::~ExtensionHost() { diff --git a/chrome/browser/extensions/extension_host.h b/chrome/browser/extensions/extension_host.h index e54fc39..a3b04f2 100644 --- a/chrome/browser/extensions/extension_host.h +++ b/chrome/browser/extensions/extension_host.h @@ -32,6 +32,9 @@ class ExtensionHost : public RenderViewHostDelegate, public RenderViewHostDelegate::View, public ExtensionFunctionDispatcher::Delegate { public: + // Enable DOM automation in created render view hosts. + static void EnableDOMAutomation() { enable_dom_automation_ = true; } + ExtensionHost(Extension* extension, SiteInstance* site_instance, const GURL& url); ~ExtensionHost(); @@ -104,6 +107,10 @@ class ExtensionHost : public RenderViewHostDelegate, virtual void UpdatePreferredWidth(int pref_width); private: + // Whether to allow DOM automation for created RenderViewHosts. This is used + // for testing. + static bool enable_dom_automation_; + // ExtensionFunctionDispatcher::Delegate // If this ExtensionHost has a view, this returns the Browser that view is a // part of. If this is a global background page, we use the active Browser diff --git a/chrome/browser/extensions/extension_shelf_model_unittest.cc b/chrome/browser/extensions/extension_shelf_model_unittest.cc index b41754c..d1e2217 100644 --- a/chrome/browser/extensions/extension_shelf_model_unittest.cc +++ b/chrome/browser/extensions/extension_shelf_model_unittest.cc @@ -3,11 +3,13 @@ // found in the LICENSE file. #include "chrome/browser/browser.h" +#include "chrome/browser/extensions/extension_browsertest.h" #include "chrome/browser/extensions/extension_host.h" #include "chrome/browser/extensions/extension_shelf_model.h" #include "chrome/browser/extensions/extensions_service.h" -#include "chrome/browser/extensions/test_extension_loader.h" #include "chrome/browser/profile.h" +#include "chrome/browser/views/extensions/extension_shelf.h" +#include "chrome/browser/views/frame/browser_view.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/extensions/extension_error_reporter.h" @@ -26,31 +28,28 @@ const char* kExtensionId = "behllobkkfkfnphdnhnkndlbkcpglgmj"; // It would be nice to refactor things so that ExtensionShelfModel, // ExtensionHost and ExtensionsService could run without so much of the browser // in place. -class ExtensionShelfModelTest : public InProcessBrowserTest, +class ExtensionShelfModelTest : public ExtensionBrowserTest, public ExtensionShelfModelObserver { public: virtual void SetUp() { - // Initialize the error reporter here, or BrowserMain will create it with - // the wrong MessageLoop. - ExtensionErrorReporter::Init(false); inserted_count_ = 0; removed_count_ = 0; moved_count_ = 0; - InProcessBrowserTest::SetUp(); } - virtual void SetUpCommandLine(CommandLine* command_line) { - command_line->AppendSwitch(switches::kEnableExtensions); - } - virtual Browser* CreateBrowser(Profile* profile) { Browser* b = InProcessBrowserTest::CreateBrowser(profile); - model_ = new ExtensionShelfModel(b); + BrowserView* browser_view = static_cast<BrowserView*>(b->window()); + model_ = browser_view->extension_shelf()->model(); model_->AddObserver(this); return b; } + virtual void CleanUpOnMainThread() { + model_->RemoveObserver(this); + } + virtual void ToolstripInsertedAt(ExtensionHost* toolstrip, int index) { inserted_count_++; } @@ -73,21 +72,11 @@ class ExtensionShelfModelTest : public InProcessBrowserTest, int moved_count_; }; -// TODO(erikkay): http://crbug.com/15291 disabled because fails on build-bot. -IN_PROC_BROWSER_TEST_F(ExtensionShelfModelTest, DISABLED_Basic) { - // Get the path to our extension. - FilePath path; - ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &path)); - path = path.AppendASCII("extensions") - .AppendASCII("good") - .AppendASCII("Extensions") - .AppendASCII(kExtensionId).AppendASCII("1.0.0.0"); - ASSERT_TRUE(file_util::DirectoryExists(path)); // sanity check - - // Wait for the extension to load and grab a pointer to it. - TestExtensionLoader loader(browser()->profile()); - Extension* extension = loader.Load(kExtensionId, path); - ASSERT_TRUE(extension); +IN_PROC_BROWSER_TEST_F(ExtensionShelfModelTest, Basic) { + ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("good") + .AppendASCII("Extensions") + .AppendASCII(kExtensionId) + .AppendASCII("1.0.0.0"))); // extension1 has two toolstrips EXPECT_EQ(inserted_count_, 2); @@ -105,12 +94,4 @@ IN_PROC_BROWSER_TEST_F(ExtensionShelfModelTest, DISABLED_Basic) { EXPECT_EQ(one, model_->ToolstripAt(0)); EXPECT_EQ(1, model_->count()); EXPECT_EQ(removed_count_, 1); - - // Tear down |model_| manually here rather than in the destructor or with - // a scoped_ptr. InProcessBrowserTest doesn't give us a chance to clean - // up before the browser and all of its services have been shut down, - // and |model_| depends on these existing. - model_->RemoveObserver(this); - delete model_; - model_ = NULL; } diff --git a/chrome/browser/extensions/extension_startup_unittest.cc b/chrome/browser/extensions/extension_startup_unittest.cc index a9c494a..540671e 100644 --- a/chrome/browser/extensions/extension_startup_unittest.cc +++ b/chrome/browser/extensions/extension_startup_unittest.cc @@ -7,6 +7,7 @@ #include "chrome/browser/browser.h" #include "chrome/browser/extensions/extensions_service.h" #include "chrome/browser/profile.h" +#include "chrome/browser/tab_contents/tab_contents.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/notification_details.h" @@ -18,22 +19,22 @@ #include "chrome/test/ui_test_utils.h" #include "net/base/net_util.h" -// This file contains high-level regression tests for the extensions system. The -// goal here is not to test everything in depth, but to run the system as close -// as possible end-to-end to find any gaps in test coverage in the lower-level -// unit tests. +// This file contains high-level startup tests for the extensions system. We've +// had many silly bugs where command line flags did not get propagated correctly +// into the services, so we didn't start correctly. class ExtensionStartupTestBase : public InProcessBrowserTest, public NotificationObserver { public: ExtensionStartupTestBase() : enable_extensions_(false), enable_user_scripts_(false) { - EnableDOMAutomation(); } protected: // InProcessBrowserTest virtual void SetUpCommandLine(CommandLine* command_line) { + EnableDOMAutomation(); + FilePath profile_dir; PathService::Get(chrome::DIR_USER_DATA, &profile_dir); profile_dir = profile_dir.AppendASCII("Default"); @@ -145,7 +146,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionsStartupTest, Test) { // Test that the content script ran. bool result = false; ui_test_utils::ExecuteJavaScriptAndExtractBool( - browser()->GetSelectedTabContents(), L"", + browser()->GetSelectedTabContents()->render_view_host(), L"", L"window.domAutomationController.send(" L"document.defaultView.getComputedStyle(document.body, null)." L"getPropertyValue('background-color') == 'rgb(245, 245, 220)')", @@ -154,21 +155,10 @@ IN_PROC_BROWSER_TEST_F(ExtensionsStartupTest, Test) { result = false; ui_test_utils::ExecuteJavaScriptAndExtractBool( - browser()->GetSelectedTabContents(), L"", + browser()->GetSelectedTabContents()->render_view_host(), L"", L"window.domAutomationController.send(document.title == 'Modified')", &result); EXPECT_TRUE(result); - - // Load an extension page into the tab area to make sure it works. - result = false; - ui_test_utils::NavigateToURL( - browser(), - GURL("chrome-extension://behllobkkfkfnphdnhnkndlbkcpglgmj/page.html")); - ui_test_utils::ExecuteJavaScriptAndExtractBool( - browser()->GetSelectedTabContents(), L"", L"testTabsAPI()", &result); - EXPECT_TRUE(result); - - // TODO(aa): Move the stuff in ExtensionBrowserTest here? } @@ -207,7 +197,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionsStartupUserScriptTest, Test) { // Test that the user script ran. bool result = false; ui_test_utils::ExecuteJavaScriptAndExtractBool( - browser()->GetSelectedTabContents(), L"", + browser()->GetSelectedTabContents()->render_view_host(), L"", L"window.domAutomationController.send(document.title == 'Modified')", &result); EXPECT_TRUE(result); diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h index 4b28169..5afa547 100644 --- a/chrome/browser/extensions/extensions_service.h +++ b/chrome/browser/extensions/extensions_service.h @@ -129,7 +129,11 @@ class ExtensionsService ExtensionInstallCallback* callback); // Uninstalls the specified extension. Callers should only call this method - // with extensions that exist. + // with extensions that exist. |external_uninstall| is a magical parameter + // that is only used to send information to ExtensionPrefs, which external + // callers should never set to true. + // TODO(aa): Remove |external_uninstall| -- this information should be passed + // to ExtensionPrefs some other way. void UninstallExtension(const std::string& extension_id, bool external_uninstall); diff --git a/chrome/browser/extensions/test_extension_loader.cc b/chrome/browser/extensions/test_extension_loader.cc deleted file mode 100644 index d17f557..0000000 --- a/chrome/browser/extensions/test_extension_loader.cc +++ /dev/null @@ -1,82 +0,0 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "chrome/browser/extensions/test_extension_loader.h" - -#include "base/file_path.h" -#include "base/message_loop.h" -#include "chrome/browser/profile.h" -#include "chrome/browser/extensions/extensions_service.h" -#include "chrome/common/notification_service.h" -#include "chrome/test/ui_test_utils.h" - -namespace { - -// How long to wait for the extension to load before giving up. -const int kLoadTimeoutMs = 5000; -const int kInstallTimeoutMs = 10000; - -} // namespace - -TestExtensionLoader::TestExtensionLoader(Profile* profile) - : profile_(profile), - extension_(NULL) { - registrar_.Add(this, NotificationType::EXTENSIONS_LOADED, - NotificationService::AllSources()); - - profile_->GetExtensionsService()->Init(); - profile_->GetExtensionsService()->set_show_extensions_prompts(false); - DCHECK(profile_->GetExtensionsService()->extensions()->empty()); -} - -Extension* TestExtensionLoader::Load(const char* extension_id, - const FilePath& path) { - loading_extension_id_ = extension_id; - - // Load the extension. - profile_->GetExtensionsService()->LoadExtension(path); - - // Wait for the load to complete. Stick a QuitTask into the message loop - // with the timeout so it will exit if the extension never loads. - extension_ = NULL; - MessageLoop::current()->PostDelayedTask(FROM_HERE, - new MessageLoop::QuitTask, kLoadTimeoutMs); - ui_test_utils::RunMessageLoop(); - - return extension_; -} - -Extension* TestExtensionLoader::Install(const char* extension_id, - const FilePath& path) { - loading_extension_id_ = extension_id; - - // Install the extension. When installed, the extension will automatically - // be loaded. - profile_->GetExtensionsService()->InstallExtension(path); - - // Wait for the load to complete. - extension_ = NULL; - MessageLoop::current()->PostDelayedTask(FROM_HERE, - new MessageLoop::QuitTask, kInstallTimeoutMs); - ui_test_utils::RunMessageLoop(); - - return extension_; -} - -void TestExtensionLoader::Observe(NotificationType type, - const NotificationSource& source, - const NotificationDetails& details) { - if (type == NotificationType::EXTENSIONS_LOADED) { - ExtensionList* extensions = Details<ExtensionList>(details).ptr(); - for (size_t i = 0; i < (*extensions).size(); ++i) { - if ((*extensions)[i]->id() == loading_extension_id_) { - extension_ = (*extensions)[i]; - MessageLoopForUI::current()->Quit(); - break; - } - } - } else { - NOTREACHED(); - } -} diff --git a/chrome/browser/extensions/test_extension_loader.h b/chrome/browser/extensions/test_extension_loader.h deleted file mode 100644 index ddf5585..0000000 --- a/chrome/browser/extensions/test_extension_loader.h +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef CHROME_BROWSER_EXTENSIONS_TEST_EXTENSION_LOADER_H_ -#define CHROME_BROWSER_EXTENSIONS_TEST_EXTENSION_LOADER_H_ - -#include "chrome/common/notification_observer.h" -#include "chrome/common/notification_registrar.h" - -class Extension; -class FilePath; -class Profile; - -class TestExtensionLoader : public NotificationObserver { - public: - explicit TestExtensionLoader(Profile* profile); - - // Tells the extension service to load the extension at the given path. It - // waits for the extension with the expected ID to load, then returns a - // handle to it. - Extension* Load(const char* extension_id, const FilePath& path); - - // Same as above, but installs from a CRX first. - Extension* Install(const char* extension_id, const FilePath& path); - - virtual void Observe(NotificationType type, - const NotificationSource& source, - const NotificationDetails& details); - - private: - Profile* profile_; - Extension* extension_; - NotificationRegistrar registrar_; - std::string loading_extension_id_; - - DISALLOW_COPY_AND_ASSIGN(TestExtensionLoader); -}; - -#endif // CHROME_BROWSER_EXTENSIONS_TEST_EXTENSION_LOADER_H_ diff --git a/chrome/browser/renderer_host/render_view_host.cc b/chrome/browser/renderer_host/render_view_host.cc index 1a0574d..82182db 100644 --- a/chrome/browser/renderer_host/render_view_host.cc +++ b/chrome/browser/renderer_host/render_view_host.cc @@ -15,6 +15,7 @@ #include "chrome/browser/child_process_security_policy.h" #include "chrome/browser/cross_site_request_manager.h" #include "chrome/browser/debugger/devtools_manager.h" +#include "chrome/browser/dom_operation_notification_details.h" #include "chrome/browser/extensions/extension_message_service.h" #include "chrome/browser/metrics/user_metrics.h" #include "chrome/browser/profile.h" @@ -24,6 +25,7 @@ #include "chrome/browser/renderer_host/render_widget_host_view.h" #include "chrome/browser/renderer_host/site_instance.h" #include "chrome/common/bindings_policy.h" +#include "chrome/common/notification_details.h" #include "chrome/common/notification_service.h" #include "chrome/common/notification_type.h" #include "chrome/common/render_messages.h" @@ -1043,6 +1045,12 @@ void RenderViewHost::OnMsgDidContentsPreferredWidthChange(int pref_width) { void RenderViewHost::OnMsgDomOperationResponse( const std::string& json_string, int automation_id) { delegate_->DomOperationResponse(json_string, automation_id); + + // We also fire a notification for more loosely-coupled use cases. + DomOperationNotificationDetails details(json_string, automation_id); + NotificationService::current()->Notify( + NotificationType::DOM_OPERATION_RESPONSE, Source<RenderViewHost>(this), + Details<DomOperationNotificationDetails>(&details)); } void RenderViewHost::OnMsgDOMUISend( diff --git a/chrome/browser/renderer_host/render_view_host_manager_browsertest.cc b/chrome/browser/renderer_host/render_view_host_manager_browsertest.cc index 86bf069..7d2ff82 100755 --- a/chrome/browser/renderer_host/render_view_host_manager_browsertest.cc +++ b/chrome/browser/renderer_host/render_view_host_manager_browsertest.cc @@ -49,7 +49,7 @@ IN_PROC_BROWSER_TEST_F(RenderViewHostManagerTest, ASSERT_TRUE(contents); bool domui_responded = false; EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool( - contents, + contents->render_view_host(), L"", L"window.domAutomationController.send(window.domui_responded_);", &domui_responded)); @@ -96,7 +96,7 @@ IN_PROC_BROWSER_TEST_F(RenderViewHostManagerTest, ASSERT_TRUE(contents); bool result = false; EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool( - contents, + contents->render_view_host(), L"", L"window.onunload = function() { var do_nothing = 0; }; " L"window.domAutomationController.send(true);", diff --git a/chrome/browser/ssl/ssl_browser_tests.cc b/chrome/browser/ssl/ssl_browser_tests.cc index 304b16a..78047d2 100644 --- a/chrome/browser/ssl/ssl_browser_tests.cc +++ b/chrome/browser/ssl/ssl_browser_tests.cc @@ -232,8 +232,8 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestUnsafeContents) { int img_width; EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractInt( - tab, L"", L"window.domAutomationController.send(ImageWidth());", - &img_width)); + tab->render_view_host(), L"", + L"window.domAutomationController.send(ImageWidth());", &img_width)); // In order to check that the image was not loaded, we check its width. // The actual image (Google logo) is 114 pixels wide, we assume the broken // image is less than 100. @@ -241,8 +241,8 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestUnsafeContents) { bool js_result = false; EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool( - tab, L"", L"window.domAutomationController.send(IsFooSet());", - &js_result)); + tab->render_view_host(), L"", + L"window.domAutomationController.send(IsFooSet());", &js_result)); EXPECT_FALSE(js_result); } @@ -260,7 +260,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestMixedContentsLoadedFromJS) { // Load the insecure image. bool js_result = false; EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool( - tab, L"", L"loadBadImage();", &js_result)); + tab->render_view_host(), L"", L"loadBadImage();", &js_result)); EXPECT_TRUE(js_result); // We should now have mixed-contents. @@ -578,8 +578,8 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestGoodFrameNavigation) { bool success = false; // Now navigate inside the frame. - EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(tab, - L"", + EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool( + tab->render_view_host(), L"", L"window.domAutomationController.send(clickLink('goodHTTPSLink'));", &success)); EXPECT_TRUE(success); @@ -589,8 +589,8 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestGoodFrameNavigation) { CheckAuthenticatedState(tab, false, false); // Now let's hit a bad page. - EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(tab, - L"", + EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool( + tab->render_view_host(), L"", L"window.domAutomationController.send(clickLink('badHTTPSLink'));", &success)); EXPECT_TRUE(success); @@ -605,7 +605,8 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestGoodFrameNavigation) { std::wstring is_frame_evil_js( L"window.domAutomationController" L".send(document.getElementById('evilDiv') != null);"); - EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(tab, + EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool( + tab->render_view_host(), content_frame_xpath, is_frame_evil_js, &is_content_evil)); @@ -617,7 +618,8 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestGoodFrameNavigation) { CheckAuthenticatedState(tab, false, false); // Navigate to a page served over HTTP. - EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(tab, + EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool( + tab->render_view_host(), L"", L"window.domAutomationController.send(clickLink('HTTPLink'));", &success)); @@ -654,7 +656,8 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestBadFrameNavigation) { // Navigate to a good frame. bool success = false; - EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(tab, + EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool( + tab->render_view_host(), L"", L"window.domAutomationController.send(clickLink('goodHTTPSLink'));", &success)); @@ -680,8 +683,8 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestUnauthenticatedFrameNavigation) { // Now navigate inside the frame to a secure HTTPS frame. bool success = false; - EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(tab, - L"", + EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool( + tab->render_view_host(), L"", L"window.domAutomationController.send(clickLink('goodHTTPSLink'));", &success)); EXPECT_TRUE(success); @@ -691,7 +694,8 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestUnauthenticatedFrameNavigation) { CheckUnauthenticatedState(tab); // Now navigate to a bad HTTPS frame. - EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(tab, + EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool( + tab->render_view_host(), L"", L"window.domAutomationController.send(clickLink('badHTTPSLink'));", &success)); @@ -707,8 +711,9 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestUnauthenticatedFrameNavigation) { std::wstring is_frame_evil_js( L"window.domAutomationController" L".send(document.getElementById('evilDiv') != null);"); - EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(tab, - content_frame_xpath, is_frame_evil_js, &is_content_evil)); + EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool( + tab->render_view_host(), content_frame_xpath, is_frame_evil_js, + &is_content_evil)); EXPECT_FALSE(is_content_evil); } diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc index d303c6a..3b46d99 100644 --- a/chrome/browser/tab_contents/tab_contents.cc +++ b/chrome/browser/tab_contents/tab_contents.cc @@ -1862,14 +1862,6 @@ void TabContents::RequestOpenURL(const GURL& url, const GURL& referrer, } } -void TabContents::DomOperationResponse(const std::string& json_string, - int automation_id) { - DomOperationNotificationDetails details(json_string, automation_id); - NotificationService::current()->Notify( - NotificationType::DOM_OPERATION_RESPONSE, Source<TabContents>(this), - Details<DomOperationNotificationDetails>(&details)); -} - void TabContents::ProcessDOMUIMessage(const std::string& message, const std::string& content, int request_id, diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h index 56939d8..f7d8f69 100644 --- a/chrome/browser/tab_contents/tab_contents.h +++ b/chrome/browser/tab_contents/tab_contents.h @@ -794,8 +794,6 @@ class TabContents : public PageNavigator, const SkBitmap& image); virtual void RequestOpenURL(const GURL& url, const GURL& referrer, WindowOpenDisposition disposition); - virtual void DomOperationResponse(const std::string& json_string, - int automation_id); virtual void ProcessDOMUIMessage(const std::string& message, const std::string& content, int request_id, diff --git a/chrome/browser/views/extensions/extension_shelf.h b/chrome/browser/views/extensions/extension_shelf.h index a4baaec2..789d758 100644 --- a/chrome/browser/views/extensions/extension_shelf.h +++ b/chrome/browser/views/extensions/extension_shelf.h @@ -28,6 +28,9 @@ class ExtensionShelf : public views::View, explicit ExtensionShelf(Browser* browser); virtual ~ExtensionShelf(); + // Get the current model. + ExtensionShelfModel* model() { return model_.get(); } + // Return the current active ExtensionShelfHandle (if any). BrowserBubble* GetHandle(); diff --git a/chrome/browser/views/find_bar_win_browsertest.cc b/chrome/browser/views/find_bar_win_browsertest.cc index de66f7a..47d3c2e 100644 --- a/chrome/browser/views/find_bar_win_browsertest.cc +++ b/chrome/browser/views/find_bar_win_browsertest.cc @@ -173,7 +173,7 @@ IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, FindInPageFrames) { std::string FocusedOnPage(TabContents* tab_contents) { std::string result; ui_test_utils::ExecuteJavaScriptAndExtractString( - tab_contents, + tab_contents->render_view_host(), L"", L"window.domAutomationController.send(getFocusedElement());", &result); @@ -214,7 +214,7 @@ IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, FindInPageEndState) { // Move the selection to link 1, after searching. std::string result; ui_test_utils::ExecuteJavaScriptAndExtractString( - tab_contents, + tab_contents->render_view_host(), L"", L"window.domAutomationController.send(selectLink1());", &result); diff --git a/chrome/browser/views/frame/browser_view.h b/chrome/browser/views/frame/browser_view.h index fd321ce..3af778f 100644 --- a/chrome/browser/views/frame/browser_view.h +++ b/chrome/browser/views/frame/browser_view.h @@ -122,6 +122,9 @@ class BrowserView : public BrowserWindow, // Accessor for the TabStrip. TabStrip* tabstrip() const { return tabstrip_; } + // Accessor for the ExtensionShelf. + ExtensionShelf* extension_shelf() const { return extension_shelf_; } + // Returns true if various window components are visible. bool IsToolbarVisible() const; bool IsTabStripVisible() const; |