diff options
Diffstat (limited to 'chrome')
21 files changed, 239 insertions, 205 deletions
diff --git a/chrome/browser/autocomplete/search_provider_unittest.cc b/chrome/browser/autocomplete/search_provider_unittest.cc index abe66f2..06e1fde 100644 --- a/chrome/browser/autocomplete/search_provider_unittest.cc +++ b/chrome/browser/autocomplete/search_provider_unittest.cc @@ -4,6 +4,7 @@ #include "chrome/browser/autocomplete/search_provider.h" +#include "base/run_loop.h" #include "base/string_util.h" #include "base/time.h" #include "base/utf_string_conversions.h" @@ -178,13 +179,12 @@ void SearchProviderTest::RunTillProviderDone() { return; quit_when_done_ = true; -#if defined(OS_MACOSX) - message_loop_.Run(); -#elif defined(OS_ANDROID) +#if defined(OS_ANDROID) // Android doesn't have Run(), only Start(). message_loop_.Start(); #else - message_loop_.RunWithDispatcher(NULL); + base::RunLoop run_loop; + run_loop.Run(); #endif } diff --git a/chrome/browser/automation/automation_provider_win.cc b/chrome/browser/automation/automation_provider_win.cc index 13885c6..d156b5e 100644 --- a/chrome/browser/automation/automation_provider_win.cc +++ b/chrome/browser/automation/automation_provider_win.cc @@ -8,6 +8,7 @@ #include "base/callback.h" #include "base/debug/trace_event.h" #include "base/json/json_reader.h" +#include "base/run_loop.h" #include "base/utf_string_conversions.h" #include "chrome/browser/automation/automation_browser_tracker.h" #include "chrome/browser/automation/automation_tab_tracker.h" @@ -36,6 +37,17 @@ using content::WebContents; namespace { +// Allow some pending tasks up to |num_deferrals| generations to complete. +void DeferredQuitRunLoop(const base::Closure& quit_task, + int num_quit_deferrals) { + if (num_quit_deferrals <= 0) { + quit_task.Run(); + } else { + MessageLoop::current()->PostTask(FROM_HERE, + base::Bind(&DeferredQuitRunLoop, quit_task, num_quit_deferrals - 1)); + } +} + // This callback just adds another callback to the event queue. This is useful // if you want to ensure that any callbacks added to the event queue after this // one have already been processed by the time |callback| is run. @@ -139,6 +151,11 @@ void AutomationProvider::WindowSimulateDrag( if (press_escape_en_route) { // Press Escape, making sure we wait until chrome processes the escape. // TODO(phajdan.jr): make this use ui_test_utils::SendKeyPressSync. + views::AcceleratorHandler handler; + base::RunLoop run_loop(&handler); + // Number of times to repost Quit task to allow pending tasks to complete. + // See kNumQuitDeferrals in ui_test_utils.cc for explanation. + int num_quit_deferrals = 10; ui_controls::SendKeyPressNotifyWhenDone( window, ui::VKEY_ESCAPE, ((flags & ui::EF_CONTROL_DOWN) == @@ -147,11 +164,11 @@ void AutomationProvider::WindowSimulateDrag( ui::EF_SHIFT_DOWN), ((flags & ui::EF_ALT_DOWN) == ui::EF_ALT_DOWN), false, - MessageLoop::QuitClosure()); + base::Bind(&DeferredQuitRunLoop, run_loop.QuitClosure(), + num_quit_deferrals)); MessageLoopForUI* loop = MessageLoopForUI::current(); - views::AcceleratorHandler handler; MessageLoop::ScopedNestableTaskAllower allow(loop); - loop->RunWithDispatcher(&handler); + run_loop.Run(); } SendMessage(top_level_hwnd, up_message, wparam_flags, MAKELPARAM(end.x, end.y)); diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc index bf106a4..54f308b 100644 --- a/chrome/browser/chrome_browser_main.cc +++ b/chrome/browser/chrome_browser_main.cc @@ -19,6 +19,7 @@ #include "base/path_service.h" #include "base/process_info.h" #include "base/process_util.h" +#include "base/run_loop.h" #include "base/string_number_conversions.h" #include "base/string_piece.h" #include "base/string_split.h" @@ -1900,16 +1901,14 @@ bool ChromeBrowserMainParts::MainMessageLoopRun(int* result_code) { // UI thread message loop as possible to get a stable measurement // across versions. RecordBrowserStartupTime(); -#if defined(USE_AURA) - MessageLoopForUI::current()->Run(); -#elif defined(TOOLKIT_VIEWS) + DCHECK_EQ(MessageLoop::TYPE_UI, MessageLoop::current()->type()); +#if !defined(USE_AURA) && defined(TOOLKIT_VIEWS) views::AcceleratorHandler accelerator_handler; - MessageLoopForUI::current()->RunWithDispatcher(&accelerator_handler); -#elif defined(USE_X11) - MessageLoopForUI::current()->RunWithDispatcher(NULL); -#elif defined(OS_POSIX) - MessageLoopForUI::current()->Run(); + base::RunLoop run_loop(&accelerator_handler); +#else + base::RunLoop run_loop; #endif + run_loop.Run(); return true; } diff --git a/chrome/browser/referrer_policy_browsertest.cc b/chrome/browser/referrer_policy_browsertest.cc index 3e6fc20..7d59dbd 100644 --- a/chrome/browser/referrer_policy_browsertest.cc +++ b/chrome/browser/referrer_policy_browsertest.cc @@ -255,8 +255,7 @@ IN_PROC_BROWSER_TEST_F(ReferrerPolicyTest, HttpsMiddleClickTargetBlankOrigin) { // Context menu, from HTTP to HTTP. IN_PROC_BROWSER_TEST_F(ReferrerPolicyTest, MAYBE_ContextMenuOrigin) { ContextMenuNotificationObserver context_menu_observer( - IDC_CONTENT_CONTEXT_OPENLINKNEWTAB, - chrome::NOTIFICATION_TAB_ADDED); + IDC_CONTENT_CONTEXT_OPENLINKNEWTAB); RunReferrerTest("origin", false, false, false, true, WebKit::WebMouseEvent::ButtonRight, EXPECT_ORIGIN_AS_REFERRER); @@ -265,8 +264,7 @@ IN_PROC_BROWSER_TEST_F(ReferrerPolicyTest, MAYBE_ContextMenuOrigin) { // Context menu, from HTTPS to HTTP. IN_PROC_BROWSER_TEST_F(ReferrerPolicyTest, MAYBE_HttpsContextMenuOrigin) { ContextMenuNotificationObserver context_menu_observer( - IDC_CONTENT_CONTEXT_OPENLINKNEWTAB, - chrome::NOTIFICATION_TAB_ADDED); + IDC_CONTENT_CONTEXT_OPENLINKNEWTAB); RunReferrerTest("origin", true, false, false, true, WebKit::WebMouseEvent::ButtonRight, EXPECT_ORIGIN_AS_REFERRER); @@ -352,8 +350,7 @@ IN_PROC_BROWSER_TEST_F(ReferrerPolicyTest, // Context menu, from HTTP to HTTP via server redirect. IN_PROC_BROWSER_TEST_F(ReferrerPolicyTest, MAYBE_ContextMenuRedirect) { ContextMenuNotificationObserver context_menu_observer( - IDC_CONTENT_CONTEXT_OPENLINKNEWTAB, - chrome::NOTIFICATION_TAB_ADDED); + IDC_CONTENT_CONTEXT_OPENLINKNEWTAB); RunReferrerTest("origin", false, false, true, true, WebKit::WebMouseEvent::ButtonRight, EXPECT_ORIGIN_AS_REFERRER); @@ -362,8 +359,7 @@ IN_PROC_BROWSER_TEST_F(ReferrerPolicyTest, MAYBE_ContextMenuRedirect) { // Context menu, from HTTPS to HTTP via server redirect. IN_PROC_BROWSER_TEST_F(ReferrerPolicyTest, MAYBE_HttpsContextMenuRedirect) { ContextMenuNotificationObserver context_menu_observer( - IDC_CONTENT_CONTEXT_OPENLINKNEWTAB, - chrome::NOTIFICATION_TAB_ADDED); + IDC_CONTENT_CONTEXT_OPENLINKNEWTAB); RunReferrerTest("origin", true, false, true, true, WebKit::WebMouseEvent::ButtonRight, EXPECT_ORIGIN_AS_REFERRER); diff --git a/chrome/browser/tab_contents/render_view_context_menu_browsertest.cc b/chrome/browser/tab_contents/render_view_context_menu_browsertest.cc index e95b26c..362d57f 100644 --- a/chrome/browser/tab_contents/render_view_context_menu_browsertest.cc +++ b/chrome/browser/tab_contents/render_view_context_menu_browsertest.cc @@ -101,8 +101,7 @@ IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest, IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest, MAYBE_RealMenu) { ContextMenuNotificationObserver menu_observer( - IDC_CONTENT_CONTEXT_OPENLINKNEWTAB, - chrome::NOTIFICATION_TAB_ADDED); + IDC_CONTENT_CONTEXT_OPENLINKNEWTAB); ui_test_utils::WindowedTabAddedNotificationObserver tab_observer( content::NotificationService::AllSources()); diff --git a/chrome/browser/tab_contents/render_view_context_menu_browsertest_util.cc b/chrome/browser/tab_contents/render_view_context_menu_browsertest_util.cc index 900581b..b86fbe4 100644 --- a/chrome/browser/tab_contents/render_view_context_menu_browsertest_util.cc +++ b/chrome/browser/tab_contents/render_view_context_menu_browsertest_util.cc @@ -11,20 +11,11 @@ #include "content/public/browser/notification_service.h" ContextMenuNotificationObserver::ContextMenuNotificationObserver( - int command_to_execute, - int expected_notification) - : command_to_execute_(command_to_execute), - expected_notification_(expected_notification), - seen_expected_notification_(false) { + int command_to_execute) + : command_to_execute_(command_to_execute) { registrar_.Add(this, chrome::NOTIFICATION_RENDER_VIEW_CONTEXT_MENU_SHOWN, content::NotificationService::AllSources()); - registrar_.Add(this, - chrome::NOTIFICATION_RENDER_VIEW_CONTEXT_MENU_CLOSED, - content::NotificationService::AllSources()); - registrar_.Add(this, - expected_notification_, - content::NotificationService::AllSources()); } ContextMenuNotificationObserver::~ContextMenuNotificationObserver() { @@ -34,11 +25,6 @@ void ContextMenuNotificationObserver::Observe( int type, const content::NotificationSource& source, const content::NotificationDetails& details) { - if (type == expected_notification_) { - seen_expected_notification_ = true; - return; - } - switch (type) { case chrome::NOTIFICATION_RENDER_VIEW_CONTEXT_MENU_SHOWN: { RenderViewContextMenu* context_menu = @@ -49,17 +35,6 @@ void ContextMenuNotificationObserver::Observe( base::Unretained(this), context_menu)); break; } - case chrome::NOTIFICATION_RENDER_VIEW_CONTEXT_MENU_CLOSED: { - // Aura is running a nested message loop for menus. That means that - // whatever the test running us is waiting for (e.g. a new tab to be - // added) happened while the menu message loop was running, so the - // message loop run by this test never quits. - if (seen_expected_notification_ && - MessageLoop::current()->is_running()) { - MessageLoop::current()->QuitNow(); - } - break; - } default: NOTREACHED(); diff --git a/chrome/browser/tab_contents/render_view_context_menu_browsertest_util.h b/chrome/browser/tab_contents/render_view_context_menu_browsertest_util.h index 815454f..dac0c03 100644 --- a/chrome/browser/tab_contents/render_view_context_menu_browsertest_util.h +++ b/chrome/browser/tab_contents/render_view_context_menu_browsertest_util.h @@ -15,14 +15,7 @@ class RenderViewContextMenu; class ContextMenuNotificationObserver : public content::NotificationObserver { public: // Wait for a context menu to be shown, and then execute |command_to_execute|. - // As the context menu might spin a nested message loop, the usual way to wait - // for the result of the |command_to_execute|, i.e. running - // ui_test_utils::WindowedNotificationObserver::Wait, won't work. The - // ContextMenuNotificationObserver can work around this problem. In order to - // do so, you need to also specify what notification the - // WindowedNotificationObserver is waiting for. - ContextMenuNotificationObserver(int command_to_execute, - int expected_notification); + explicit ContextMenuNotificationObserver(int command_to_execute); virtual ~ContextMenuNotificationObserver(); private: @@ -34,8 +27,6 @@ class ContextMenuNotificationObserver : public content::NotificationObserver { content::NotificationRegistrar registrar_; int command_to_execute_; - int expected_notification_; - bool seen_expected_notification_; DISALLOW_COPY_AND_ASSIGN(ContextMenuNotificationObserver); }; diff --git a/chrome/browser/ui/views/simple_message_box_views.cc b/chrome/browser/ui/views/simple_message_box_views.cc index 805c6b5..a399885 100644 --- a/chrome/browser/ui/views/simple_message_box_views.cc +++ b/chrome/browser/ui/views/simple_message_box_views.cc @@ -8,6 +8,7 @@ #include "base/compiler_specific.h" #include "base/memory/ref_counted.h" #include "base/message_loop.h" +#include "base/run_loop.h" #include "chrome/browser/browser_process.h" #include "grit/generated_resources.h" #include "ui/base/l10n/l10n_util.h" @@ -179,7 +180,8 @@ MessageBoxResult ShowMessageBox(gfx::NativeWindow parent, #else { MessageLoop::ScopedNestableTaskAllower allow(MessageLoopForUI::current()); - MessageLoopForUI::current()->RunWithDispatcher(dialog); + base::RunLoop run_loop(dialog); + run_loop.Run(); } #endif return dialog->result(); diff --git a/chrome/browser/ui/views/uninstall_view.cc b/chrome/browser/ui/views/uninstall_view.cc index 808d245..41fee09 100644 --- a/chrome/browser/ui/views/uninstall_view.cc +++ b/chrome/browser/ui/views/uninstall_view.cc @@ -6,6 +6,7 @@ #include "base/message_loop.h" #include "base/process_util.h" +#include "base/run_loop.h" #include "base/string16.h" #include "base/utf_string_conversions.h" #include "chrome/browser/shell_integration.h" @@ -23,19 +24,21 @@ #include "ui/views/layout/layout_constants.h" #include "ui/views/widget/widget.h" -UninstallView::UninstallView(int* user_selection) +UninstallView::UninstallView(int* user_selection, + const base::Closure& quit_closure) : confirm_label_(NULL), delete_profile_(NULL), change_default_browser_(NULL), browsers_combo_(NULL), browsers_(NULL), - user_selection_(*user_selection) { + user_selection_(*user_selection), + quit_closure_(quit_closure) { SetupControls(); } UninstallView::~UninstallView() { // Exit the message loop we were started with so that uninstall can continue. - MessageLoop::current()->Quit(); + quit_closure_.Run(); } void UninstallView::SetupControls() { @@ -164,10 +167,13 @@ string16 UninstallView::GetItemAt(int index) { namespace browser { int ShowUninstallBrowserPrompt() { + DCHECK_EQ(MessageLoop::TYPE_UI, MessageLoop::current()->type()); int result = content::RESULT_CODE_NORMAL_EXIT; - views::Widget::CreateWindow(new UninstallView(&result))->Show(); views::AcceleratorHandler accelerator_handler; - MessageLoopForUI::current()->RunWithDispatcher(&accelerator_handler); + base::RunLoop run_loop(&accelerator_handler); + UninstallView* view = new UninstallView(&result, run_loop.QuitClosure()); + views::Widget::CreateWindow(view)->Show(); + run_loop.Run(); return result; } diff --git a/chrome/browser/ui/views/uninstall_view.h b/chrome/browser/ui/views/uninstall_view.h index 181c07a..47c02f2 100644 --- a/chrome/browser/ui/views/uninstall_view.h +++ b/chrome/browser/ui/views/uninstall_view.h @@ -8,6 +8,7 @@ #include <map> +#include "base/callback.h" #include "base/string16.h" #include "ui/base/models/combobox_model.h" #include "ui/views/window/dialog_delegate.h" @@ -25,7 +26,8 @@ class UninstallView : public views::ButtonListener, public views::DialogDelegateView, public ui::ComboboxModel { public: - explicit UninstallView(int* user_selection); + explicit UninstallView(int* user_selection, + const base::Closure& quit_closure); virtual ~UninstallView(); // Overridden form views::ButtonListener. @@ -56,6 +58,7 @@ class UninstallView : public views::ButtonListener, typedef std::map<std::wstring, std::wstring> BrowsersMap; scoped_ptr<BrowsersMap> browsers_; int& user_selection_; + base::Closure quit_closure_; DISALLOW_COPY_AND_ASSIGN(UninstallView); }; diff --git a/chrome/browser/ui/views/user_data_dir_dialog_view.cc b/chrome/browser/ui/views/user_data_dir_dialog_view.cc index a0e7128..2a1f29c 100644 --- a/chrome/browser/ui/views/user_data_dir_dialog_view.cc +++ b/chrome/browser/ui/views/user_data_dir_dialog_view.cc @@ -5,6 +5,7 @@ #include "chrome/browser/ui/views/user_data_dir_dialog_view.h" #include "base/logging.h" +#include "base/run_loop.h" #include "base/utf_string_conversions.h" #include "chrome/browser/ui/user_data_dir_dialog.h" #include "grit/chromium_strings.h" @@ -100,10 +101,12 @@ void UserDataDirDialogView::FileSelectionCanceled(void* params) { namespace browser { FilePath ShowUserDataDirDialog(const FilePath& user_data_dir) { + DCHECK_EQ(MessageLoop::TYPE_UI, MessageLoop::current()->type()); // When the window closes, it will delete itself. UserDataDirDialogView* dialog = new UserDataDirDialogView(user_data_dir); views::Widget::CreateWindow(dialog)->Show(); - MessageLoopForUI::current()->RunWithDispatcher(dialog); + base::RunLoop run_loop(dialog); + run_loop.Run(); return dialog->user_data_dir(); } diff --git a/chrome/test/base/bookmark_load_observer.cc b/chrome/test/base/bookmark_load_observer.cc index 5b80070..3bc1b7f 100644 --- a/chrome/test/base/bookmark_load_observer.cc +++ b/chrome/test/base/bookmark_load_observer.cc @@ -1,15 +1,16 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 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/test/base/bookmark_load_observer.h" -#include "base/message_loop.h" +#include "chrome/test/base/ui_test_utils.h" -BookmarkLoadObserver::BookmarkLoadObserver() {} +BookmarkLoadObserver::BookmarkLoadObserver(const base::Closure& quit_task) + : quit_task_(quit_task) {} BookmarkLoadObserver::~BookmarkLoadObserver() {} void BookmarkLoadObserver::Loaded(BookmarkModel* model, bool ids_reassigned) { - MessageLoop::current()->Quit(); + quit_task_.Run(); } diff --git a/chrome/test/base/bookmark_load_observer.h b/chrome/test/base/bookmark_load_observer.h index 0619838e..bb21615 100644 --- a/chrome/test/base/bookmark_load_observer.h +++ b/chrome/test/base/bookmark_load_observer.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 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. @@ -7,7 +7,9 @@ #pragma once #include "base/basictypes.h" +#include "base/callback.h" #include "base/compiler_specific.h" +#include "base/memory/weak_ptr.h" #include "chrome/browser/bookmarks/bookmark_model_observer.h" // BookmarkLoadObserver is used when blocking until the BookmarkModel @@ -15,7 +17,7 @@ // loop is quit. class BookmarkLoadObserver : public BookmarkModelObserver { public: - BookmarkLoadObserver(); + explicit BookmarkLoadObserver(const base::Closure& quit_task); virtual ~BookmarkLoadObserver(); virtual void Loaded(BookmarkModel* model, bool ids_reassigned) OVERRIDE; @@ -40,6 +42,7 @@ class BookmarkLoadObserver : public BookmarkModelObserver { const BookmarkNode* node) OVERRIDE {} private: + base::Closure quit_task_; DISALLOW_COPY_AND_ASSIGN(BookmarkLoadObserver); }; diff --git a/chrome/test/base/in_process_browser_test.cc b/chrome/test/base/in_process_browser_test.cc index 34a9c70..30a2eca 100644 --- a/chrome/test/base/in_process_browser_test.cc +++ b/chrome/test/base/in_process_browser_test.cc @@ -374,6 +374,12 @@ void InProcessBrowserTest::RunTestOnMainThreadLoop() { autorelease_pool_->Recycle(); #endif + // Sometimes tests leave Quit tasks in the MessageLoop (for shame), so let's + // run all pending messages here to avoid preempting the QuitBrowsers tasks. + // TODO(jbates) Once crbug.com/134753 is fixed, this can be removed because it + // will not be possible to post Quit tasks. + ui_test_utils::RunAllPendingInMessageLoop(); + QuitBrowsers(); CHECK(BrowserList::empty()); } diff --git a/chrome/test/base/test_web_dialog_observer.cc b/chrome/test/base/test_web_dialog_observer.cc index 2056592..75b7a34 100644 --- a/chrome/test/base/test_web_dialog_observer.cc +++ b/chrome/test/base/test_web_dialog_observer.cc @@ -42,7 +42,7 @@ void TestWebDialogObserver::Observe( // If the message loop is running stop it. if (running_) { running_ = false; - MessageLoopForUI::current()->Quit(); + message_loop_runner_->Quit(); } break; default: @@ -73,7 +73,8 @@ content::WebUI* TestWebDialogObserver::GetWebUI() { if (!done_) { EXPECT_FALSE(running_); running_ = true; - ui_test_utils::RunMessageLoop(); + message_loop_runner_ = new ui_test_utils::MessageLoopRunner; + message_loop_runner_->Run(); } return web_ui_; } diff --git a/chrome/test/base/test_web_dialog_observer.h b/chrome/test/base/test_web_dialog_observer.h index b77d667..67f1f74 100644 --- a/chrome/test/base/test_web_dialog_observer.h +++ b/chrome/test/base/test_web_dialog_observer.h @@ -8,6 +8,7 @@ #include "base/basictypes.h" #include "base/compiler_specific.h" +#include "chrome/test/base/ui_test_utils.h" #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" #include "ui/web_dialogs/web_dialog_observer.h" @@ -55,6 +56,7 @@ class TestWebDialogObserver : public content::NotificationObserver, content::WebUI* web_ui_; bool done_; bool running_; + scoped_refptr<ui_test_utils::MessageLoopRunner> message_loop_runner_; DISALLOW_COPY_AND_ASSIGN(TestWebDialogObserver); }; diff --git a/chrome/test/base/testing_profile.cc b/chrome/test/base/testing_profile.cc index a652086..c69447f 100644 --- a/chrome/test/base/testing_profile.cc +++ b/chrome/test/base/testing_profile.cc @@ -11,6 +11,7 @@ #include "base/file_util.h" #include "base/message_loop_proxy.h" #include "base/path_service.h" +#include "base/run_loop.h" #include "base/string_number_conversions.h" #include "chrome/browser/autocomplete/autocomplete_classifier.h" #include "chrome/browser/bookmarks/bookmark_model.h" @@ -378,9 +379,11 @@ void TestingProfile::BlockUntilBookmarkModelLoaded() { DCHECK(GetBookmarkModel()); if (GetBookmarkModel()->IsLoaded()) return; - BookmarkLoadObserver observer; + base::RunLoop run_loop; + BookmarkLoadObserver observer( + ui_test_utils::GetQuitTaskForRunLoop(&run_loop)); GetBookmarkModel()->AddObserver(&observer); - MessageLoop::current()->Run(); + run_loop.Run(); GetBookmarkModel()->RemoveObserver(&observer); DCHECK(GetBookmarkModel()->IsLoaded()); } diff --git a/chrome/test/base/tracing.cc b/chrome/test/base/tracing.cc index 790ab3c..3c29bba 100644 --- a/chrome/test/base/tracing.cc +++ b/chrome/test/base/tracing.cc @@ -38,7 +38,8 @@ class InProcessTraceController : public content::TraceSubscriber { return false; // Wait for OnEndTracingComplete() to quit the message loop. // OnTraceDataCollected may be called multiple times while blocking here. - ui_test_utils::RunMessageLoop(); + message_loop_runner_ = new ui_test_utils::MessageLoopRunner; + message_loop_runner_->Run(); trace_buffer_.Finish(); trace_buffer_.SetOutputCallback(TraceResultBuffer::OutputCallback()); @@ -51,7 +52,7 @@ class InProcessTraceController : public content::TraceSubscriber { // TraceSubscriber virtual void OnEndTracingComplete() OVERRIDE { - MessageLoopForUI::current()->Quit(); + message_loop_runner_->Quit(); } // TraceSubscriber @@ -63,6 +64,8 @@ class InProcessTraceController : public content::TraceSubscriber { // For collecting trace data asynchronously. base::debug::TraceResultBuffer trace_buffer_; + scoped_refptr<ui_test_utils::MessageLoopRunner> message_loop_runner_; + DISALLOW_COPY_AND_ASSIGN(InProcessTraceController); }; diff --git a/chrome/test/base/ui_test_utils.cc b/chrome/test/base/ui_test_utils.cc index f3f51b7..7b4af42 100644 --- a/chrome/test/base/ui_test_utils.cc +++ b/chrome/test/base/ui_test_utils.cc @@ -93,6 +93,17 @@ using content::WebContents; static const int kDefaultWsPort = 8880; +// Number of times to repost a Quit task so that the MessageLoop finishes up +// pending tasks and tasks posted by those pending tasks without risking the +// potential hang behavior of MessageLoop::QuitWhenIdle. +// The criteria for choosing this number: it should be high enough to make the +// quit act like QuitWhenIdle, while taking into account that any page which is +// animating may be rendering another frame for each quit deferral. For an +// animating page, the potential delay to quitting the RunLoop would be +// kNumQuitDeferrals * frame_render_time. Some perf tests run slow, such as +// 200ms/frame. +static const int kNumQuitDeferrals = 10; + namespace ui_test_utils { namespace { @@ -106,7 +117,8 @@ class DOMOperationObserver : public content::NotificationObserver, did_respond_(false) { registrar_.Add(this, content::NOTIFICATION_DOM_OPERATION_RESPONSE, content::Source<RenderViewHost>(render_view_host)); - ui_test_utils::RunMessageLoop(); + message_loop_runner_ = new MessageLoopRunner; + message_loop_runner_->Run(); } virtual void Observe(int type, @@ -116,12 +128,12 @@ class DOMOperationObserver : public content::NotificationObserver, content::Details<DomOperationNotificationDetails> dom_op_details(details); response_ = dom_op_details->json; did_respond_ = true; - MessageLoopForUI::current()->Quit(); + message_loop_runner_->Quit(); } // Overridden from content::WebContentsObserver: virtual void RenderViewGone(base::TerminationStatus status) OVERRIDE { - MessageLoopForUI::current()->Quit(); + message_loop_runner_->Quit(); } bool GetResponse(std::string* response) WARN_UNUSED_RESULT { @@ -133,6 +145,7 @@ class DOMOperationObserver : public content::NotificationObserver, content::NotificationRegistrar registrar_; std::string response_; bool did_respond_; + scoped_refptr<MessageLoopRunner> message_loop_runner_; DISALLOW_COPY_AND_ASSIGN(DOMOperationObserver); }; @@ -147,7 +160,8 @@ class FindInPageNotificationObserver : public content::NotificationObserver { parent_tab->find_tab_helper()->current_find_request_id(); registrar_.Add(this, chrome::NOTIFICATION_FIND_RESULT_AVAILABLE, content::Source<WebContents>(parent_tab_->web_contents())); - ui_test_utils::RunMessageLoop(); + message_loop_runner_ = new MessageLoopRunner; + message_loop_runner_->Run(); } int active_match_ordinal() const { return active_match_ordinal_; } @@ -165,7 +179,7 @@ class FindInPageNotificationObserver : public content::NotificationObserver { active_match_ordinal_ = find_details->active_match_ordinal(); if (find_details->final_update()) { number_of_matches_ = find_details->number_of_matches(); - MessageLoopForUI::current()->Quit(); + message_loop_runner_->Quit(); } else { DVLOG(1) << "Ignoring, since we only care about the final message"; } @@ -185,6 +199,7 @@ class FindInPageNotificationObserver : public content::NotificationObserver { // The id of the current find request, obtained from WebContents. Allows us // to monitor when the search completes. int current_find_request_id_; + scoped_refptr<MessageLoopRunner> message_loop_runner_; DISALLOW_COPY_AND_ASSIGN(FindInPageNotificationObserver); }; @@ -263,39 +278,52 @@ bool ExecuteJavaScriptHelper(RenderViewHost* render_view_host, return true; } -void RunAllPendingMessageAndSendQuit(content::BrowserThread::ID thread_id) { - MessageLoop::current()->PostTask(FROM_HERE, MessageLoop::QuitClosure()); +void RunAllPendingMessageAndSendQuit(content::BrowserThread::ID thread_id, + const base::Closure& quit_task) { + MessageLoop::current()->PostTask(FROM_HERE, + MessageLoop::QuitWhenIdleClosure()); RunMessageLoop(); - content::BrowserThread::PostTask(thread_id, FROM_HERE, - MessageLoop::QuitClosure()); + content::BrowserThread::PostTask(thread_id, FROM_HERE, quit_task); } } // namespace void RunMessageLoop() { - MessageLoop* loop = MessageLoop::current(); - MessageLoopForUI* ui_loop = - content::BrowserThread::CurrentlyOn(content::BrowserThread::UI) ? - MessageLoopForUI::current() : NULL; - MessageLoop::ScopedNestableTaskAllower allow(loop); - if (ui_loop) { -#if defined(USE_AURA) - ui_loop->Run(); -#elif defined(TOOLKIT_VIEWS) - views::AcceleratorHandler handler; - ui_loop->RunWithDispatcher(&handler); -#elif defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID) - ui_loop->RunWithDispatcher(NULL); -#else - ui_loop->Run(); + base::RunLoop run_loop; + RunThisRunLoop(&run_loop); +} + +void RunThisRunLoop(base::RunLoop* run_loop) { + MessageLoop::ScopedNestableTaskAllower allow(MessageLoop::current()); +#if !defined(USE_AURA) && defined(TOOLKIT_VIEWS) + scoped_ptr<views::AcceleratorHandler> handler; + if (content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)) { + handler.reset(new views::AcceleratorHandler); + run_loop->set_dispatcher(handler.get()); + } #endif + run_loop->Run(); +} + +// TODO(jbates) move this to a new test_utils.cc in content/test/ +static void DeferredQuitRunLoop(const base::Closure& quit_task, + int num_quit_deferrals) { + if (num_quit_deferrals <= 0) { + quit_task.Run(); } else { - loop->Run(); + MessageLoop::current()->PostTask(FROM_HERE, + base::Bind(&DeferredQuitRunLoop, quit_task, num_quit_deferrals - 1)); } } +base::Closure GetQuitTaskForRunLoop(base::RunLoop* run_loop) { + return base::Bind(&DeferredQuitRunLoop, run_loop->QuitClosure(), + kNumQuitDeferrals); +} + void RunAllPendingInMessageLoop() { - MessageLoop::current()->PostTask(FROM_HERE, MessageLoop::QuitClosure()); + MessageLoop::current()->PostTask(FROM_HERE, + MessageLoop::QuitWhenIdleClosure()); ui_test_utils::RunMessageLoop(); } @@ -309,10 +337,12 @@ void RunAllPendingInMessageLoop(content::BrowserThread::ID thread_id) { NOTREACHED(); return; } - content::BrowserThread::PostTask(thread_id, FROM_HERE, - base::Bind(&RunAllPendingMessageAndSendQuit, current_thread_id)); - ui_test_utils::RunMessageLoop(); + base::RunLoop run_loop; + content::BrowserThread::PostTask(thread_id, FROM_HERE, + base::Bind(&RunAllPendingMessageAndSendQuit, current_thread_id, + run_loop.QuitClosure())); + ui_test_utils::RunThisRunLoop(&run_loop); } bool GetCurrentTabTitle(const Browser* browser, string16* title) { @@ -331,10 +361,10 @@ void WaitForNavigations(NavigationController* controller, content::TestNavigationObserver observer( content::Source<NavigationController>(controller), NULL, number_of_navigations); + base::RunLoop run_loop; observer.WaitForObservation( - base::Bind(&ui_test_utils::RunMessageLoop), - base::Bind(&MessageLoop::Quit, - base::Unretained(MessageLoopForUI::current()))); + base::Bind(&ui_test_utils::RunThisRunLoop, base::Unretained(&run_loop)), + ui_test_utils::GetQuitTaskForRunLoop(&run_loop)); } void WaitForNewTab(Browser* browser) { @@ -377,11 +407,10 @@ void NavigateToURL(browser::NavigateParams* params) { content::TestNavigationObserver observer( content::NotificationService::AllSources(), NULL, 1); browser::Navigate(params); + base::RunLoop run_loop; observer.WaitForObservation( - base::Bind(&ui_test_utils::RunMessageLoop), - base::Bind(&MessageLoop::Quit, - base::Unretained(MessageLoopForUI::current()))); - + base::Bind(&ui_test_utils::RunThisRunLoop, base::Unretained(&run_loop)), + ui_test_utils::GetQuitTaskForRunLoop(&run_loop)); } void NavigateToURL(Browser* browser, const GURL& url) { @@ -451,10 +480,10 @@ static void NavigateToURLWithDispositionBlockUntilNavigationsComplete( web_contents = browser->GetActiveWebContents(); } if (disposition == CURRENT_TAB) { + base::RunLoop run_loop; same_tab_observer.WaitForObservation( - base::Bind(&ui_test_utils::RunMessageLoop), - base::Bind(&MessageLoop::Quit, - base::Unretained(MessageLoopForUI::current()))); + base::Bind(&ui_test_utils::RunThisRunLoop, base::Unretained(&run_loop)), + ui_test_utils::GetQuitTaskForRunLoop(&run_loop)); return; } else if (web_contents) { NavigationController* controller = &web_contents->GetController(); @@ -704,9 +733,10 @@ void RegisterAndWait(content::NotificationObserver* observer, void WaitForBookmarkModelToLoad(BookmarkModel* model) { if (model->IsLoaded()) return; - BookmarkLoadObserver observer; + base::RunLoop run_loop; + BookmarkLoadObserver observer(GetQuitTaskForRunLoop(&run_loop)); model->AddObserver(&observer); - RunMessageLoop(); + RunThisRunLoop(&run_loop); model->RemoveObserver(&observer); ASSERT_TRUE(model->IsLoaded()); } @@ -764,14 +794,14 @@ bool SendKeyPressSync(const Browser* browser, gfx::NativeWindow window = NULL; if (!GetNativeWindow(browser, &window)) return false; - + scoped_refptr<MessageLoopRunner> runner = new MessageLoopRunner; bool result; result = ui_controls::SendKeyPressNotifyWhenDone( - window, key, control, shift, alt, command, MessageLoop::QuitClosure()); + window, key, control, shift, alt, command, runner->QuitClosure()); #if defined(OS_WIN) if (!result && BringBrowserWindowToFront(browser)) { result = ui_controls::SendKeyPressNotifyWhenDone( - window, key, control, shift, alt, command, MessageLoop::QuitClosure()); + window, key, control, shift, alt, command, runner->QuitClosure()); } #endif if (!result) { @@ -782,7 +812,7 @@ bool SendKeyPressSync(const Browser* browser, // Run the message loop. It'll stop running when either the key was received // or the test timed out (in which case testing::Test::HasFatalFailure should // be set). - RunMessageLoop(); + runner->Run(); return !testing::Test::HasFatalFailure(); } @@ -804,51 +834,41 @@ bool SendKeyPressAndWait(const Browser* browser, } bool SendMouseMoveSync(const gfx::Point& location) { - if (!ui_controls::SendMouseMoveNotifyWhenDone(location.x(), location.y(), - MessageLoop::QuitClosure())) { + scoped_refptr<MessageLoopRunner> runner = new MessageLoopRunner; + if (!ui_controls::SendMouseMoveNotifyWhenDone( + location.x(), location.y(), runner->QuitClosure())) { return false; } - RunMessageLoop(); + runner->Run(); return !testing::Test::HasFatalFailure(); } bool SendMouseEventsSync(ui_controls::MouseButton type, int state) { + scoped_refptr<MessageLoopRunner> runner = new MessageLoopRunner; if (!ui_controls::SendMouseEventsNotifyWhenDone( - type, state, MessageLoop::QuitClosure())) { + type, state, runner->QuitClosure())) { return false; } - RunMessageLoop(); + runner->Run(); return !testing::Test::HasFatalFailure(); } -TimedMessageLoopRunner::TimedMessageLoopRunner() - : loop_(new MessageLoopForUI()), - owned_(true), - quit_loop_invoked_(false) { +MessageLoopRunner::MessageLoopRunner() { } -TimedMessageLoopRunner::~TimedMessageLoopRunner() { - if (owned_) - delete loop_; +MessageLoopRunner::~MessageLoopRunner() { } -void TimedMessageLoopRunner::RunFor(int ms) { - QuitAfter(ms); - quit_loop_invoked_ = false; - loop_->Run(); +void MessageLoopRunner::Run() { + ui_test_utils::RunThisRunLoop(&run_loop_); } -void TimedMessageLoopRunner::Quit() { - quit_loop_invoked_ = true; - loop_->PostTask(FROM_HERE, MessageLoop::QuitClosure()); +base::Closure MessageLoopRunner::QuitClosure() { + return base::Bind(&MessageLoopRunner::Quit, this); } -void TimedMessageLoopRunner::QuitAfter(int ms) { - quit_loop_invoked_ = true; - loop_->PostDelayedTask( - FROM_HERE, - MessageLoop::QuitClosure(), - base::TimeDelta::FromMilliseconds(ms)); +void MessageLoopRunner::Quit() { + ui_test_utils::GetQuitTaskForRunLoop(&run_loop_).Run(); } TestWebSocketServer::TestWebSocketServer() @@ -1013,7 +1033,8 @@ void WindowedNotificationObserver::Wait() { return; running_ = true; - ui_test_utils::RunMessageLoop(); + message_loop_runner_ = new MessageLoopRunner; + message_loop_runner_->Run(); EXPECT_TRUE(seen_); } @@ -1027,7 +1048,7 @@ void WindowedNotificationObserver::Observe( if (!running_) return; - MessageLoopForUI::current()->Quit(); + message_loop_runner_->Quit(); running_ = false; } @@ -1078,7 +1099,8 @@ const string16& TitleWatcher::WaitAndGetTitle() { if (expected_title_observed_) return observed_title_; quit_loop_on_observation_ = true; - ui_test_utils::RunMessageLoop(); + message_loop_runner_ = new MessageLoopRunner; + message_loop_runner_->Run(); return observed_title_; } @@ -1104,8 +1126,11 @@ void TitleWatcher::Observe(int type, return; observed_title_ = *it; expected_title_observed_ = true; - if (quit_loop_on_observation_) - MessageLoopForUI::current()->Quit(); + if (quit_loop_on_observation_) { + // Only call Quit once, on first Observe: + quit_loop_on_observation_ = false; + message_loop_runner_->Quit(); + } } BrowserAddedObserver::BrowserAddedObserver() @@ -1140,7 +1165,7 @@ void DOMMessageQueue::Observe(int type, message_queue_.push(dom_op_details->json); if (waiting_for_message_) { waiting_for_message_ = false; - MessageLoopForUI::current()->Quit(); + message_loop_runner_->Quit(); } } @@ -1152,7 +1177,8 @@ bool DOMMessageQueue::WaitForMessage(std::string* message) { if (message_queue_.empty()) { waiting_for_message_ = true; // This will be quit when a new message comes in. - RunMessageLoop(); + message_loop_runner_ = new MessageLoopRunner; + message_loop_runner_->Run(); } // The queue should not be empty, unless we were quit because of a timeout. if (message_queue_.empty()) @@ -1180,7 +1206,8 @@ class SnapshotTaker { base::Bind(&SnapshotTaker::OnSnapshotTaken, base::Unretained(this)), page_size, desired_size); - ui_test_utils::RunMessageLoop(); + message_loop_runner_ = new MessageLoopRunner; + message_loop_runner_->Run(); return snapshot_taken_; } @@ -1215,13 +1242,14 @@ class SnapshotTaker { void OnSnapshotTaken(const SkBitmap& bitmap) { *bitmap_ = bitmap; snapshot_taken_ = true; - MessageLoop::current()->Quit(); + message_loop_runner_->Quit(); } SkBitmap* bitmap_; // Whether the snapshot was actually taken and received by this SnapshotTaker. // This will be false if the test times out. bool snapshot_taken_; + scoped_refptr<MessageLoopRunner> message_loop_runner_; DISALLOW_COPY_AND_ASSIGN(SnapshotTaker); }; @@ -1247,9 +1275,9 @@ void OverrideGeolocation(double latitude, double longitude) { position.altitude = 0.; position.accuracy = 0.; position.timestamp = base::Time::Now(); - content::OverrideLocationForTesting(position, - base::Bind(MessageLoop::QuitClosure())); - RunMessageLoop(); + scoped_refptr<MessageLoopRunner> runner = new MessageLoopRunner; + content::OverrideLocationForTesting(position, runner->QuitClosure()); + runner->Run(); } namespace internal { diff --git a/chrome/test/base/ui_test_utils.h b/chrome/test/base/ui_test_utils.h index 9af9372..a2ec811 100644 --- a/chrome/test/base/ui_test_utils.h +++ b/chrome/test/base/ui_test_utils.h @@ -13,8 +13,8 @@ #include <vector> #include "base/basictypes.h" -#include "base/message_loop.h" #include "base/process.h" +#include "base/run_loop.h" #include "base/scoped_temp_dir.h" #include "base/string16.h" #include "chrome/browser/ui/view_ids.h" @@ -96,6 +96,13 @@ enum BrowserTestWaitFlags { // process browser tests that need to block until a condition is met. void RunMessageLoop(); +// Variant of RunMessageLoop that takes RunLoop. +void RunThisRunLoop(base::RunLoop* run_loop); + +// Get task to quit the given RunLoop. It allows a few generations of pending +// tasks to run as opposed to run_loop->QuitClosure(). +base::Closure GetQuitTaskForRunLoop(base::RunLoop* run_loop); + // Turns on nestable tasks, runs all pending tasks in the message loop, // then resets nestable tasks to what they were originally. Prefer this // over MessageLoop::RunAllPending for in process browser tests to run @@ -300,37 +307,34 @@ bool SendMouseMoveSync(const gfx::Point& location) WARN_UNUSED_RESULT; bool SendMouseEventsSync(ui_controls::MouseButton type, int state) WARN_UNUSED_RESULT; -// Run a message loop only for the specified amount of time. -class TimedMessageLoopRunner { +// Helper class to Run and Quit the message loop. Run and Quit can only happen +// once per instance. Make a new instance for each use. Calling Quit after Run +// has returned is safe and has no effect. +class MessageLoopRunner + : public base::RefCounted<MessageLoopRunner> { public: - // Create new MessageLoopForUI and attach to it. - TimedMessageLoopRunner(); - - // Attach to an existing message loop. - explicit TimedMessageLoopRunner(MessageLoop* loop) - : loop_(loop), owned_(false), quit_loop_invoked_(false) {} - - ~TimedMessageLoopRunner(); + MessageLoopRunner(); - // Run the message loop for ms milliseconds. - void RunFor(int ms); + // Run the current MessageLoop. + void Run(); - // Post Quit task to the message loop. + // Quit the matching call to Run (nested MessageLoops are unaffected). void Quit(); - // Post delayed Quit task to the message loop. - void QuitAfter(int ms); - - bool WasTimedOut() const { - return !quit_loop_invoked_; - } + // Hand this closure off to code that uses callbacks to notify completion. + // Example: + // scoped_refptr<MessageLoopRunner> runner = new MessageLoopRunner; + // kick_off_some_api(runner.QuitNowClosure()); + // runner.Run(); + base::Closure QuitClosure(); private: - MessageLoop* loop_; - bool owned_; - bool quit_loop_invoked_; + friend class base::RefCounted<MessageLoopRunner>; + ~MessageLoopRunner(); + + base::RunLoop run_loop_; - DISALLOW_COPY_AND_ASSIGN(TimedMessageLoopRunner); + DISALLOW_COPY_AND_ASSIGN(MessageLoopRunner); }; // This is a utility class for running a python websocket server @@ -445,6 +449,7 @@ class WindowedNotificationObserver : public content::NotificationObserver { content::NotificationSource source_; content::NotificationDetails details_; + scoped_refptr<MessageLoopRunner> message_loop_runner_; DISALLOW_COPY_AND_ASSIGN(WindowedNotificationObserver); }; @@ -538,6 +543,7 @@ class TitleWatcher : public content::NotificationObserver { content::WebContents* web_contents_; std::vector<string16> expected_titles_; content::NotificationRegistrar notification_registrar_; + scoped_refptr<MessageLoopRunner> message_loop_runner_; // The most recently observed expected title, if any. string16 observed_title_; @@ -637,6 +643,7 @@ class DOMMessageQueue : public content::NotificationObserver { content::NotificationRegistrar registrar_; std::queue<std::string> message_queue_; bool waiting_for_message_; + scoped_refptr<MessageLoopRunner> message_loop_runner_; DISALLOW_COPY_AND_ASSIGN(DOMMessageQueue); }; diff --git a/chrome/test/perf/rendering/throughput_tests.cc b/chrome/test/perf/rendering/throughput_tests.cc index 606fb94..18965ca 100644 --- a/chrome/test/perf/rendering/throughput_tests.cc +++ b/chrome/test/perf/rendering/throughput_tests.cc @@ -8,6 +8,7 @@ #include "base/json/json_reader.h" #include "base/memory/scoped_ptr.h" #include "base/path_service.h" +#include "base/run_loop.h" #include "base/string_number_conversions.h" #include "base/stringprintf.h" #include "base/test/trace_event_analyzer.h" @@ -195,11 +196,11 @@ class ThroughputTest : public BrowserPerfTest { } void Wait(int ms) { + base::RunLoop run_loop; MessageLoop::current()->PostDelayedTask( - FROM_HERE, - MessageLoop::QuitClosure(), + FROM_HERE, run_loop.QuitClosure(), base::TimeDelta::FromMilliseconds(ms)); - ui_test_utils::RunMessageLoop(); + ui_test_utils::RunThisRunLoop(&run_loop); } // Take snapshot of the current tab, encode it as PNG, and save to a SkBitmap. @@ -468,13 +469,7 @@ IN_PROC_BROWSER_TEST_F(ThroughputTestGPU, DISABLED_TestURL) { RunTestWithURL(kAllowExternalDNS); } -// crbug.com/124049 -#if defined(OS_MACOSX) -#define MAYBE_Particles DISABLED_Particles -#else -#define MAYBE_Particles Particles -#endif -IN_PROC_BROWSER_TEST_F(ThroughputTestGPU, MAYBE_Particles) { +IN_PROC_BROWSER_TEST_F(ThroughputTestGPU, Particles) { RunTest("particles", kInternal); } @@ -527,13 +522,7 @@ IN_PROC_BROWSER_TEST_F(ThroughputTestSW, CanvasTextSW) { RunTest("canvas2d_balls_text", kNone); } -// crbug.com/124049 -#if defined(OS_MACOSX) -#define MAYBE_CanvasTextGPU DISABLED_CanvasTextGPU -#else -#define MAYBE_CanvasTextGPU CanvasTextGPU -#endif -IN_PROC_BROWSER_TEST_F(ThroughputTestGPU, MAYBE_CanvasTextGPU) { +IN_PROC_BROWSER_TEST_F(ThroughputTestGPU, CanvasTextGPU) { RunTest("canvas2d_balls_text", kNone | kIsGpuCanvasTest); } |