mac: Flush the autorelease pool after making a browser window in browser tests.

chrome/test/base/in_process_browser_test.cc

 // 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/in_process_browser_test.h"
 
 #include "base/auto_reset.h"
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/lazy_instance.h"
 #include "base/location.h"
 #include "base/path_service.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/test/test_file_util.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/threading/non_thread_safe.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/devtools/devtools_window.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/browser/net/net_error_tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_list_observer.h"
 #include "chrome/browser/ui/browser_navigator.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/host_desktop.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/logging_chrome.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/renderer/chrome_content_renderer_client.h"
 #include "chrome/test/base/chrome_test_suite.h"
 #include "chrome/test/base/test_launcher_utils.h"
 #include "chrome/test/base/test_switches.h"
 #include "chrome/test/base/testing_browser_process.h"
+#include "chrome/test/base/ui_test_utils.h"
 #include "components/google/core/browser/google_util.h"
 #include "components/os_crypt/os_crypt.h"
+#include "content/public/browser/devtools_agent_host.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_launcher.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/test/spawned_test_server/spawned_test_server.h"
 
 #if defined(OS_MACOSX)
 #include "base/mac/scoped_nsautorelease_pool.h"
@@ skipping 344 matching lines @@
 
 void InProcessBrowserTest::CloseAllBrowsers() {
   chrome::CloseAllBrowsers();
 #if defined(OS_MACOSX)
   // BrowserWindowController depends on the auto release pool being recycled
   // in the message loop to delete itself.
   AutoreleasePool()->Recycle();
 #endif
 }
 
+void InProcessBrowserTest::OpenDevToolsWindow(
+    content::WebContents* web_contents) {
+#if defined(OS_MACOSX)
+  // Opening a Devtools Window can cause AppKit to throw objects into the
+  // autorelease pool. Flush the pool when this function returns.
+  base::mac::ScopedNSAutoreleasePool pool;
+#endif  // OS_MACOSX
+  ASSERT_FALSE(content::DevToolsAgentHost::HasFor(web_contents));
+  DevToolsWindow::OpenDevToolsWindow(web_contents);
+  ASSERT_TRUE(content::DevToolsAgentHost::HasFor(web_contents));
+}
+
+Browser* InProcessBrowserTest::OpenURLOffTheRecord(Profile* profile,
+                                                   const GURL& url) {
+#if defined(OS_MACOSX)
+  // Opening an incognito window can cause AppKit to throw objects into the
+  // autorelease pool. Flush the pool when this function returns.
+  base::mac::ScopedNSAutoreleasePool pool;
+#endif
+  chrome::HostDesktopType active_desktop = chrome::GetActiveDesktop();
+  chrome::OpenURLOffTheRecord(profile, url, active_desktop);
+  Browser* browser = chrome::FindTabbedBrowser(
+      profile->GetOffTheRecordProfile(), false, active_desktop);
+  content::TestNavigationObserver observer(
+      browser->tab_strip_model()->GetActiveWebContents());
+  observer.Wait();
+  return browser;
+}
+
 // TODO(alexmos): This function should expose success of the underlying
 // navigation to tests, which should make sure navigations succeed when
 // appropriate. See https://crbug.com/425335
 void InProcessBrowserTest::AddTabAtIndexToBrowser(
     Browser* browser,
     int index,
     const GURL& url,
     ui::PageTransition transition,
     bool check_navigation_success) {
   chrome::NavigateParams params(browser, url, transition);
@@ skipping 14 matching lines @@
   AddTabAtIndexToBrowser(browser(), index, url, transition, true);
 }
 
 bool InProcessBrowserTest::SetUpUserDataDirectory() {
   return true;
 }
 
 // Creates a browser with a single tab (about:blank), waits for the tab to
 // finish loading and shows the browser.
 Browser* InProcessBrowserTest::CreateBrowser(Profile* profile) {
+#if defined(OS_MACOSX)

[James Hawkins, 2015/07/13 18:22:23]

Something doesn't seem right about this change.  I don't think this is the right
'level' to fix this issue.  I'm concerned that we have this same block repeated
in so many places.  I'm also concerned that we are adding so much mac-specific
code here.

[erikchen, 2015/07/13 19:39:21]

I agree that this solution isn't ideal, although I think the alternatives are
not much better. For more details on the problem, see
https://codereview.chromium.org/1229473002/#msg12.

I will propose some alternative solutions. If you prefer one, let me know. If
you don't prefer any, and don't have any alternative proposals, then we should
have some synchronous communication.

1) Move this logic into Browser::Browser.
  Downside: This logic is only necessary for browser tests, and I dislike
polluting chrome with test-framework logic.

2) Make a file in_process_browser_test_mac.cc. Write Mac versions of these
functions in that file. These functions in this file only get defined on non-Mac
platforms.
  Downside: More files = more complexity. More duplicated code.

3) Make a new anonymous scoped class in this file, which does nothing on
non-Mac, and is equivalent to ScopedNSAutoreleasePool.
  Downside: This saves some code redundancy, but doesn't address your concern of
Mac logic in this class.

If you insist that the current code is not palatable, then I guess I'm learning
towards (1). Depending on semantics, we /could/ consider this problem to be that
Browser::Browser() has side effects on Mac. (Although if we go down this route,
really most code that interacts with AppKit contains possible side effects, and
we may want to systematically wrap all AppKit interactions in scoped autorelease
pools)

+  // Making a browser window can cause AppKit to throw objects into the
+  // autorelease pool. Flush the pool when this function returns.
+  base::mac::ScopedNSAutoreleasePool pool;
+#endif
   Browser* browser = new Browser(
       Browser::CreateParams(profile, chrome::GetActiveDesktop()));
   AddBlankTabAndShow(browser);
   return browser;
 }
 
 Browser* InProcessBrowserTest::CreateIncognitoBrowser() {
+#if defined(OS_MACOSX)
+  // Making a browser window can cause AppKit to throw objects into the
+  // autorelease pool. Flush the pool when this function returns.
+  base::mac::ScopedNSAutoreleasePool pool;
+#endif
   // Create a new browser with using the incognito profile.
   Browser* incognito = new Browser(
       Browser::CreateParams(browser()->profile()->GetOffTheRecordProfile(),
                             chrome::GetActiveDesktop()));
   AddBlankTabAndShow(incognito);
   return incognito;
 }
 
 Browser* InProcessBrowserTest::CreateBrowserForPopup(Profile* profile) {
+#if defined(OS_MACOSX)
+  // Making a browser window can cause AppKit to throw objects into the
+  // autorelease pool. Flush the pool when this function returns.
+  base::mac::ScopedNSAutoreleasePool pool;
+#endif
   Browser* browser =
       new Browser(Browser::CreateParams(Browser::TYPE_POPUP, profile,
                   chrome::GetActiveDesktop()));
   AddBlankTabAndShow(browser);
   return browser;
 }
 
 Browser* InProcessBrowserTest::CreateBrowserForApp(
     const std::string& app_name,
     Profile* profile) {
+#if defined(OS_MACOSX)
+  // Making a browser window can cause AppKit to throw objects into the
+  // autorelease pool. Flush the pool when this function returns.
+  base::mac::ScopedNSAutoreleasePool pool;
+#endif
   Browser* browser = new Browser(
       Browser::CreateParams::CreateForApp(
           app_name, false /* trusted_source */, gfx::Rect(), profile,
           chrome::GetActiveDesktop()));
   AddBlankTabAndShow(browser);
   return browser;
 }
 
 void InProcessBrowserTest::AddBlankTabAndShow(Browser* browser) {
   content::WindowedNotificationObserver observer(
@@ skipping 141 matching lines @@
   // On the Mac, this eventually reaches
   // -[BrowserWindowController windowWillClose:], which will post a deferred
   // -autorelease on itself to ultimately destroy the Browser object. The line
   // below is necessary to pump these pending messages to ensure all Browsers
   // get deleted.
   content::RunAllPendingInMessageLoop();
   delete autorelease_pool_;
   autorelease_pool_ = NULL;
 #endif
 }