Fix flakiness in NewTab() calls of AppApiTest browser_test.

chrome/browser/extensions/app_process_apitest.cc

 // Copyright (c) 2011 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/utf_string_conversions.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_host.h"
 #include "chrome/browser/extensions/extension_process_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
@@ skipping 60 matching lines @@
       L"window.addEventListener('unload', function() {"
       L"    window.domAutomationController.send(true);"
       L"}, false);"
       L"window.location = '" + UTF8ToWide(url.spec()) + L"';",
       &result));
   ASSERT_TRUE(result);
   observer.Wait();
   EXPECT_EQ(url, contents->controller().GetLastCommittedEntry()->url());
 }
 
-#if defined(OS_WIN)
-// AppProcess sometimes hangs on Windows
-// http://crbug.com/88316
-#define MAYBE_AppProcess DISABLED_AppProcess
-#else
-#define MAYBE_AppProcess AppProcess
-#endif
+IN_PROC_BROWSER_TEST_F(AppApiTest, AppProcess) {
+  LOG(INFO) << "Start of test.";
 
-IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_AppProcess) {
   CommandLine::ForCurrentProcess()->AppendSwitch(
       switches::kDisablePopupBlocking);
 
   ExtensionProcessManager* extension_process_manager =
       browser()->profile()->GetExtensionProcessManager();
 
   host_resolver()->AddRule("*", "127.0.0.1");
   ASSERT_TRUE(test_server()->Start());
 
   ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("app_process")));
 
+  LOG(INFO) << "Loaded extension.";
+
   // Open two tabs in the app, one outside it.
   GURL base_url = GetTestBaseURL("app_process");
 
   // Test both opening a URL in a new tab, and opening a tab and then navigating
   // it.  Either way, app tabs should be considered extension processes, but
   // they have no elevated privileges and thus should not have WebUI bindings.
   ui_test_utils::NavigateToURLWithDisposition(
       browser(), base_url.Resolve("path1/empty.html"), NEW_FOREGROUND_TAB,
       ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
   EXPECT_TRUE(extension_process_manager->IsExtensionProcess(
       browser()->GetTabContentsAt(1)->render_view_host()->process()->id()));
   EXPECT_FALSE(browser()->GetTabContentsAt(1)->web_ui());
-  browser()->NewTab();
-  ui_test_utils::NavigateToURL(browser(), base_url.Resolve("path2/empty.html"));
+  LOG(INFO) << "Nav 1.";
+
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), base_url.Resolve("path2/empty.html"), NEW_FOREGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
   EXPECT_TRUE(extension_process_manager->IsExtensionProcess(
       browser()->GetTabContentsAt(2)->render_view_host()->process()->id()));
   EXPECT_FALSE(browser()->GetTabContentsAt(2)->web_ui());
+  LOG(INFO) << "Nav 2.";
+
+  ui_test_utils::WindowedNotificationObserver tab_added_observer(
+      content::NOTIFICATION_TAB_ADDED,
+      content::NotificationService::AllSources());
   browser()->NewTab();
+  tab_added_observer.Wait();
+  LOG(INFO) << "New tab.";
   ui_test_utils::NavigateToURL(browser(), base_url.Resolve("path3/empty.html"));
+  LOG(INFO) << "Nav 3.";
   EXPECT_FALSE(extension_process_manager->IsExtensionProcess(
       browser()->GetTabContentsAt(3)->render_view_host()->process()->id()));
   EXPECT_FALSE(browser()->GetTabContentsAt(3)->web_ui());
 
   // We should have opened 3 new extension tabs. Including the original blank
   // tab, we now have 4 tabs. Because the app_process app has the background
   // permission, all of its instances are in the same process.  Thus two tabs
   // should be part of the extension app and grouped in the same process.
   ASSERT_EQ(4, browser()->tab_count());
   RenderViewHost* host = browser()->GetTabContentsAt(1)->render_view_host();
 
   EXPECT_EQ(host->process(),
             browser()->GetTabContentsAt(2)->render_view_host()->process());
   EXPECT_NE(host->process(),
             browser()->GetTabContentsAt(3)->render_view_host()->process());
 
   // Now let's do the same using window.open. The same should happen.
   ASSERT_EQ(1u, BrowserList::GetBrowserCount(browser()->profile()));
   WindowOpenHelper(browser(), host,
                    base_url.Resolve("path1/empty.html"), true);
+  LOG(INFO) << "WindowOpenHelper 1.";
   WindowOpenHelper(browser(), host,
                    base_url.Resolve("path2/empty.html"), true);
+  LOG(INFO) << "WindowOpenHelper 2.";
   // TODO(creis): This should open in a new process (i.e., false for the last
   // argument), but we temporarily avoid swapping processes away from an app
   // until we're able to support cross-process postMessage calls.
   // See crbug.com/59285.
   WindowOpenHelper(browser(), host,
                    base_url.Resolve("path3/empty.html"), true);
+  LOG(INFO) << "WindowOpenHelper 3.";
 
   // Now let's have these pages navigate, into or out of the extension web
   // extent. They should switch processes.
   const GURL& app_url(base_url.Resolve("path1/empty.html"));
   const GURL& non_app_url(base_url.Resolve("path3/empty.html"));
   NavigateTabHelper(browser()->GetTabContentsAt(2), non_app_url);
+  LOG(INFO) << "NavigateTabHelper 1.";
   NavigateTabHelper(browser()->GetTabContentsAt(3), app_url);
+  LOG(INFO) << "NavigateTabHelper 2.";
   // TODO(creis): This should swap out of the app's process (i.e., EXPECT_NE),
   // but we temporarily avoid swapping away from an app in case the window
   // tries to send a postMessage to the app.  See crbug.com/59285.
   EXPECT_EQ(host->process(),
             browser()->GetTabContentsAt(2)->render_view_host()->process());
   EXPECT_EQ(host->process(),
             browser()->GetTabContentsAt(3)->render_view_host()->process());
 
   // If one of the popup tabs navigates back to the app, window.opener should
   // be valid.
   NavigateTabHelper(browser()->GetTabContentsAt(6), app_url);
+  LOG(INFO) << "NavigateTabHelper 3.";
   EXPECT_EQ(host->process(),
             browser()->GetTabContentsAt(6)->render_view_host()->process());
   bool windowOpenerValid = false;
   ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
       browser()->GetTabContentsAt(6)->render_view_host(), L"",
       L"window.domAutomationController.send(window.opener != null)",
       &windowOpenerValid));
   ASSERT_TRUE(windowOpenerValid);
+
+  LOG(INFO) << "End of test.";
 }
 
-
-#if defined(OS_WIN)
-// Seems to timeout sometimes on Windows: http://crbug.com/89766
-#define MAYBE_AppProcessInstances FLAKY_AppProcessInstances
-#else
-#define MAYBE_AppProcessInstances AppProcessInstances
-#endif
-
 // Test that hosted apps without the background permission use a process per app
 // instance model, such that separate instances are in separate processes.
-IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_AppProcessInstances) {
+IN_PROC_BROWSER_TEST_F(AppApiTest, AppProcessInstances) {
+  LOG(INFO) << "Start of test.";
+
   CommandLine::ForCurrentProcess()->AppendSwitch(
       switches::kDisablePopupBlocking);
 
   ExtensionProcessManager* extension_process_manager =
       browser()->profile()->GetExtensionProcessManager();
 
   host_resolver()->AddRule("*", "127.0.0.1");
   ASSERT_TRUE(test_server()->Start());
 
   ASSERT_TRUE(LoadExtension(
       test_data_dir_.AppendASCII("app_process_instances")));
 
   // Open two tabs in the app, one outside it.
   GURL base_url = GetTestBaseURL("app_process_instances");
 
   // Test both opening a URL in a new tab, and opening a tab and then navigating
   // it.  Either way, app tabs should be considered extension processes, but
   // they have no elevated privileges and thus should not have WebUI bindings.
   ui_test_utils::NavigateToURLWithDisposition(
       browser(), base_url.Resolve("path1/empty.html"), NEW_FOREGROUND_TAB,
       ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+  LOG(INFO) << "Nav 1.";
   EXPECT_TRUE(extension_process_manager->IsExtensionProcess(
       browser()->GetTabContentsAt(1)->render_view_host()->process()->id()));
   EXPECT_FALSE(browser()->GetTabContentsAt(1)->web_ui());
+
+  ui_test_utils::WindowedNotificationObserver tab_added_observer(
+      content::NOTIFICATION_TAB_ADDED,
+      content::NotificationService::AllSources());
   browser()->NewTab();
+  tab_added_observer.Wait();
+  LOG(INFO) << "New tab.";
   ui_test_utils::NavigateToURL(browser(), base_url.Resolve("path2/empty.html"));
+  LOG(INFO) << "Nav 2.";
   EXPECT_TRUE(extension_process_manager->IsExtensionProcess(
       browser()->GetTabContentsAt(2)->render_view_host()->process()->id()));
   EXPECT_FALSE(browser()->GetTabContentsAt(2)->web_ui());
 
   // We should have opened 2 new extension tabs. Including the original blank
   // tab, we now have 3 tabs. The two app tabs should not be in the same
   // process, since they do not have the background permission.  (Thus, we want
   // to separate them to improve responsiveness.)
   ASSERT_EQ(3, browser()->tab_count());
   RenderViewHost* host1 = browser()->GetTabContentsAt(1)->render_view_host();
   RenderViewHost* host2 = browser()->GetTabContentsAt(2)->render_view_host();
   EXPECT_NE(host1->process(), host2->process());
 
   // Opening tabs with window.open should keep the page in the opener's process.
   ASSERT_EQ(1u, BrowserList::GetBrowserCount(browser()->profile()));
   WindowOpenHelper(browser(), host1,
                    base_url.Resolve("path1/empty.html"), true);
+  LOG(INFO) << "WindowOpenHelper 1.";
   WindowOpenHelper(browser(), host2,
                    base_url.Resolve("path2/empty.html"), true);
+  LOG(INFO) << "End of test.";
 }
 
 // Tests that app process switching works properly in the following scenario:
 // 1. navigate to a page1 in the app
 // 2. page1 redirects to a page2 outside the app extent (ie, "/server-redirect")
 // 3. page2 redirects back to a page in the app
 // The final navigation should end up in the app process.
 // See http://crbug.com/61757
 IN_PROC_BROWSER_TEST_F(AppApiTest, AppProcessRedirectBack) {
   CommandLine::ForCurrentProcess()->AppendSwitch(
@@ skipping 252 matching lines @@
       content::Source<NavigationController>(
           &browser()->GetSelectedTabContentsWrapper()->controller()));
   browser()->Reload(CURRENT_TAB);
   observer.Wait();
   ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
       contents->render_view_host(), L"",
       L"window.domAutomationController.send(chrome.app.isInstalled)",
       &is_installed));
   ASSERT_TRUE(is_installed);
 }