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

chrome/test/base/in_process_browser_test.cc

Index: chrome/test/base/in_process_browser_test.cc
diff --git a/chrome/test/base/in_process_browser_test.cc b/chrome/test/base/in_process_browser_test.cc
index 96f521b12bfb8a73f2b18e500d480eca98ad2eb3..83f3a2181c8fbd27ec84135968f3e3fbbda5a3a8 100644
--- a/chrome/test/base/in_process_browser_test.cc
+++ b/chrome/test/base/in_process_browser_test.cc
@@ -20,11 +20,13 @@
 #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"
@@ -43,8 +45,10 @@
 #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"
@@ -409,6 +413,35 @@ void InProcessBrowserTest::CloseAllBrowsers() {
 #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
@@ -443,6 +476,11 @@ bool InProcessBrowserTest::SetUpUserDataDirectory() {
 // 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);
@@ -450,6 +488,11 @@ Browser* InProcessBrowserTest::CreateBrowser(Profile* profile) {
 }
 
 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(),
@@ -459,6 +502,11 @@ Browser* InProcessBrowserTest::CreateIncognitoBrowser() {
 }
 
 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()));
@@ -469,6 +517,11 @@ Browser* InProcessBrowserTest::CreateBrowserForPopup(Profile* profile) {
 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,