From 304a317201cf92f40c6ba7c9e969939c69205b2e Mon Sep 17 00:00:00 2001 From: "viettrungluu@chromium.org" Date: Tue, 4 May 2010 05:38:44 +0000 Subject: Mac: Don't run nested loop for browser_tests (matching Windows and Linux). See sky's commit at r44412. Also fix loading of MainMenu.nib when running browser_tests. (These two fixes are mutually dependent.) BUG=43148 TEST=browser_tests still pass (or pass even more) Review URL: http://codereview.chromium.org/1689001 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@46331 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/browser_main.cc | 7 ---- chrome/browser/browser_main_mac.mm | 11 ++++- chrome/test/in_process_browser_test.cc | 77 +++++++--------------------------- chrome/test/in_process_browser_test.h | 6 --- 4 files changed, 24 insertions(+), 77 deletions(-) diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc index 179201d..b2b4489 100644 --- a/chrome/browser/browser_main.cc +++ b/chrome/browser/browser_main.cc @@ -1176,15 +1176,8 @@ int BrowserMain(const MainFunctionParams& parameters) { // We are in test mode. Run one task and enter the main message loop. if (pool) pool->Recycle(); - // TODO(sky): revisit once Trung fixs mac exiting, mac should be just like - // windows/linux. -#if defined(OS_MACOSX) - MessageLoopForUI::current()->PostTask(FROM_HERE, parameters.ui_task); - RunUIMessageLoop(browser_process.get()); -#else parameters.ui_task->Run(); delete parameters.ui_task; -#endif } else { // We are in regular browser boot sequence. Open initial stabs and enter // the main message loop. diff --git a/chrome/browser/browser_main_mac.mm b/chrome/browser/browser_main_mac.mm index c1e0aca..ad158dd 100644 --- a/chrome/browser/browser_main_mac.mm +++ b/chrome/browser/browser_main_mac.mm @@ -11,6 +11,8 @@ #include "app/resource_bundle.h" #include "base/command_line.h" #include "base/debug_util.h" +#include "base/mac_util.h" +#include "base/scoped_nsobject.h" #include "chrome/app/breakpad_mac.h" #import "chrome/browser/app_controller_mac.h" #include "chrome/browser/browser_main_win.h" @@ -44,8 +46,13 @@ void WillInitializeMainMessageLoop(const MainFunctionParams& parameters) { ResourceBundle::InitSharedInstance(std::wstring()); } - // Now load the nib. - [NSBundle loadNibNamed:@"MainMenu" owner:NSApp]; + // Now load the nib (from the right bundle). + scoped_nsobject + nib([[NSNib alloc] initWithNibNamed:@"MainMenu" + bundle:mac_util::MainAppBundle()]); + [nib instantiateNibWithOwner:NSApp topLevelObjects:nil]; + // Make sure the app controller has been created. + DCHECK([NSApp delegate]); // This is a no-op if the KeystoneRegistration framework is not present. // The framework is only distributed with branded Google Chrome builds. diff --git a/chrome/test/in_process_browser_test.cc b/chrome/test/in_process_browser_test.cc index 941150e..0f377d3 100644 --- a/chrome/test/in_process_browser_test.cc +++ b/chrome/test/in_process_browser_test.cc @@ -8,6 +8,7 @@ #include "base/file_path.h" #include "base/file_util.h" #include "base/path_service.h" +#include "base/scoped_nsautorelease_pool.h" #include "base/test/test_file_util.h" #include "chrome/browser/browser.h" #include "chrome/browser/browser_list.h" @@ -164,14 +165,8 @@ void InProcessBrowserTest::SetUp() { SandboxInitWrapper sandbox_wrapper; MainFunctionParams params(*command_line, sandbox_wrapper, NULL); -#if defined(OS_MACOSX) - params.ui_task = - NewRunnableMethod(this, - &InProcessBrowserTest::RunTestOnMainThreadLoopDeprecated); -#else params.ui_task = NewRunnableMethod(this, &InProcessBrowserTest::RunTestOnMainThreadLoop); -#endif host_resolver_ = new net::RuleBasedHostResolverProc( new IntranetRedirectHostResolverProc(NULL)); @@ -256,63 +251,16 @@ Browser* InProcessBrowserTest::CreateBrowser(Profile* profile) { return browser; } -#if defined(OS_MACOSX) -void InProcessBrowserTest::RunTestOnMainThreadLoopDeprecated() { - // In the long term it would be great if we could use a TestingProfile - // here and only enable services you want tested, but that requires all - // consumers of Profile to handle NULL services. - Profile* profile = ProfileManager::GetDefaultProfile(); - if (!profile) { - // We should only be able to get here if the profile already exists and - // has been created. - NOTREACHED(); - MessageLoopForUI::current()->Quit(); - return; - } - - ChromeThread::PostTask( - ChromeThread::IO, FROM_HERE, - NewRunnableFunction(chrome_browser_net::SetUrlRequestMocksEnabled, true)); - - browser_ = CreateBrowser(profile); - - // Start the timeout timer to prevent hangs. - MessageLoopForUI::current()->PostDelayedTask(FROM_HERE, - NewRunnableMethod(this, &InProcessBrowserTest::TimedOut), - initial_timeout_); - - // If an ASSERT_ failed during SetUp, skip the InProcessBrowserTest test body. - if (!HasFatalFailure()) - RunTestOnMainThread(); - CleanUpOnMainThread(); - - // Close all browser windows. This might not happen immediately, since some - // may need to wait for beforeunload and unload handlers to fire in a tab. - // When all windows are closed, the last window will call Quit(). Call - // Quit() explicitly if no windows are open. -#if defined(OS_MACOSX) - // When the browser window closes, Cocoa will generate an inner-loop that - // processes the RenderProcessHost delete task, so allow task nesting. - bool old_state = MessageLoopForUI::current()->NestableTasksAllowed(); - MessageLoopForUI::current()->SetNestableTasksAllowed(true); -#endif - BrowserList::const_iterator browser = BrowserList::begin(); - if (browser == BrowserList::end()) { - MessageLoopForUI::current()->Quit(); - } else { - for (; browser != BrowserList::end(); ++browser) - (*browser)->CloseWindow(); - } -#if defined(OS_MACOSX) - MessageLoopForUI::current()->SetNestableTasksAllowed(old_state); -#endif - - // Stop the HTTP server. - http_server_ = NULL; -} -#endif // defined(OS_MACOSX) - void InProcessBrowserTest::RunTestOnMainThreadLoop() { + // On Mac, without the following autorelease pool, code which is directly + // executed (as opposed to executed inside a message loop) would autorelease + // objects into a higher-level pool. This pool is not recycled in-sync with + // the message loops' pools and causes problems with code relying on + // deallocation via an autorelease pool (such as browser window closure and + // browser shutdown). To avoid this, the following pool is recycled after each + // time code is directly executed. + base::ScopedNSAutoreleasePool pool; + // Pump startup related events. MessageLoopForUI::current()->RunAllPending(); @@ -326,12 +274,14 @@ void InProcessBrowserTest::RunTestOnMainThreadLoop() { NOTREACHED(); return; } + pool.Recycle(); ChromeThread::PostTask( ChromeThread::IO, FROM_HERE, NewRunnableFunction(chrome_browser_net::SetUrlRequestMocksEnabled, true)); browser_ = CreateBrowser(profile); + pool.Recycle(); // Start the timeout timer to prevent hangs. MessageLoopForUI::current()->PostDelayedTask(FROM_HERE, @@ -343,10 +293,13 @@ void InProcessBrowserTest::RunTestOnMainThreadLoop() { MessageLoopForUI::current()->RunAllPending(); RunTestOnMainThread(); + pool.Recycle(); CleanUpOnMainThread(); + pool.Recycle(); QuitBrowsers(); + pool.Recycle(); // Stop the HTTP server. http_server_ = NULL; diff --git a/chrome/test/in_process_browser_test.h b/chrome/test/in_process_browser_test.h index 63cad3b..8b69f6a 100644 --- a/chrome/test/in_process_browser_test.h +++ b/chrome/test/in_process_browser_test.h @@ -117,12 +117,6 @@ class InProcessBrowserTest : public testing::Test { void EnableSingleProcess() { single_process_ = true; } private: -#if defined(OS_MACOSX) - // Old variant of RunTestOnMainThreadLoop that assumes a nested message loop. - // TODO(sky): nuke this once we straighten out properly exiting on the mac. - void RunTestOnMainThreadLoopDeprecated(); -#endif - // This is invoked from main after browser_init/browser_main have completed. // This prepares for the test by creating a new browser, runs the test // (RunTestOnMainThread), quits the browsers and returns. -- cgit v1.1