Revert "blink_web_tests: Ensure content_shell window is created and...

Revert "blink_web_tests: Ensure content_shell window is created and initialized before logging #READY"

This reverts commit 9e8ae362.

Reason for revert: Shutdown crash when using --run-web-tests, breaks ClusterFuzz fuzzing bots.

Original change's description:
> blink_web_tests: Ensure content_shell window is created and initialized before logging #READY
>
> Sometimes, especially on Windows, initial creation of the content_shell window
> takes multiple seconds to complete. Tests then result in flaky timeouts. This change
> ensures the window is initialized before logging #READY. The run_web_tests harness has
> code to not count the time between launching content_shell and logging #READY.
>
> Bug: 953991
> Change-Id: I50c2449e204cdb2d66996bcb4c553a2f083604f7
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1584476
> Commit-Queue: Austin Eng <enga@chromium.org>
> Reviewed-by: Peter Beverloo <peter@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#658594}

TBR=peter@chromium.org,dpranke@chromium.org,pdr@chromium.org,robertma@chromium.org,enga@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 953991, 964522
Change-Id: Ief50f7a4da3e459056c31f5d12a526987591309b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1618542Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Abhishek Arya <inferno@chromium.org>
Commit-Queue: Abhishek Arya <inferno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#661124}

content/shell/browser/web_test/blink_test_controller.cc

@@ -368,37 +368,6 @@ BlinkTestController::~BlinkTestController() {
   instance_ = nullptr;
 }
 
-void BlinkTestController::EnsureMainWindow() {
-  if (main_window_)
-    return;
-
-  ShellBrowserContext* browser_context =
-      ShellContentBrowserClient::Get()->browser_context();
-
-  initial_size_ = Shell::GetShellDefaultSize();
-  main_window_ = content::Shell::CreateNewWindow(
-      browser_context, GURL(url::kAboutBlankURL), nullptr, initial_size_);
-
-  WebContents* web_contents = main_window_->web_contents();
-  WebContentsObserver::Observe(web_contents);
-
-  RenderViewHost* render_view_host = web_contents->GetRenderViewHost();
-  RenderWidgetHost* render_view_host_widget = render_view_host->GetWidget();
-
-  current_pid_ = base::kNullProcessId;
-  default_prefs_ = render_view_host->GetWebkitPreferences();
-
-  // Focus the RenderWidgetHost. This will send an IPC message to the
-  // renderer to propagate the state change.
-  render_view_host_widget->Focus();
-
-  // Flush various interfaces to ensure a test run begins from a known
-  // state. This will block until page navigation to about:blank successfully
-  // completes.
-  render_view_host_widget->FlushForTesting();
-  GetWebTestControlPtr(render_view_host->GetMainFrame()).FlushForTesting();
-}
-
 bool BlinkTestController::PrepareForWebTest(const TestInfo& test_info) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   test_phase_ = DURING_TEST;
@@ -424,60 +393,113 @@ bool BlinkTestController::PrepareForWebTest(const TestInfo& test_info) {
   accumulated_web_test_runtime_flags_changes_.Clear();
   web_test_control_map_.clear();
 
+  ShellBrowserContext* browser_context =
+      ShellContentBrowserClient::Get()->browser_context();
   is_compositing_test_ =
       test_url_.spec().find("compositing/") != std::string::npos;
   initial_size_ = Shell::GetShellDefaultSize();
-
-  EnsureMainWindow();
-
+  if (!main_window_) {
+    main_window_ = content::Shell::CreateNewWindow(
+        browser_context, GURL(url::kAboutBlankURL), nullptr, initial_size_);
+    WebContentsObserver::Observe(main_window_->web_contents());
+
+    // The render frame host is constructed before the call to
+    // WebContentsObserver::Observe, so we need to manually handle the creation
+    // of the new render frame host.
+    HandleNewRenderFrameHost(main_window_->web_contents()->GetMainFrame());
+
+    if (is_devtools_protocol_test) {
+      devtools_protocol_test_bindings_.reset(
+          new DevToolsProtocolTestBindings(main_window_->web_contents()));
+    }
+    current_pid_ = base::kNullProcessId;
+    default_prefs_ = main_window_->web_contents()
+                         ->GetRenderViewHost()
+                         ->GetWebkitPreferences();
+    if (is_devtools_js_test) {
+      LoadDevToolsJSTest();
+    } else {
+      // Focus the RenderWidgetHost. This will send an IPC message to the
+      // renderer to propagate the state change.
+      main_window_->web_contents()->GetRenderViewHost()->GetWidget()->Focus();
+
+      // Flush various interfaces to ensure a test run begins from a known
+      // state.
+      main_window_->web_contents()
+          ->GetRenderViewHost()
+          ->GetWidget()
+          ->FlushForTesting();
+      GetWebTestControlPtr(
+          main_window_->web_contents()->GetRenderViewHost()->GetMainFrame())
+          .FlushForTesting();
+
+      // Loading the URL will immediately start the web test. Manually call
+      // LoadURLWithParams on the WebContents to avoid extraneous calls from
+      // content::Shell such as SetFocus(), which could race with the web
+      // test.
+      NavigationController::LoadURLParams params(test_url_);
+
+      // Using PAGE_TRANSITION_TYPED replicates an omnibox navigation.
+      params.transition_type =
+          ui::PageTransitionFromInt(ui::PAGE_TRANSITION_TYPED);
+
+      // Clear history to purge the prior navigation to about:blank.
+      params.should_clear_history_list = true;
+      main_window_->web_contents()->GetController().LoadURLWithParams(params);
+    }
+  } else {
 #if defined(OS_MACOSX)
-  // Shell::SizeTo is not implemented on all platforms.
-  main_window_->SizeTo(initial_size_);
+    // Shell::SizeTo is not implemented on all platforms.
+    main_window_->SizeTo(initial_size_);
 #endif
-  WebContents* web_contents = main_window_->web_contents();
-  RenderViewHost* render_view_host = web_contents->GetRenderViewHost();
-  RenderWidgetHost* render_view_host_widget = render_view_host->GetWidget();
-
-  render_view_host_widget->GetView()->SetSize(initial_size_);
-  // Try to reset the window size. This can fail, see crbug.com/772811
-  render_view_host_widget->SynchronizeVisualProperties();
-
-  if (is_devtools_protocol_test) {
-    devtools_protocol_test_bindings_.reset(
-        new DevToolsProtocolTestBindings(web_contents));
-  }
-
-  // Compositing tests override the default preferences (see
-  // BlinkTestController::OverrideWebkitPrefs) so we force them to be
-  // calculated again to ensure is_compositing_test_ changes are picked up.
-  OverrideWebkitPrefs(&default_prefs_);
-
-  render_view_host->UpdateWebkitPreferences(default_prefs_);
+    main_window_->web_contents()
+        ->GetRenderViewHost()
+        ->GetWidget()
+        ->GetView()
+        ->SetSize(initial_size_);
+    // Try to reset the window size. This can fail, see crbug.com/772811
+    main_window_->web_contents()
+        ->GetRenderViewHost()
+        ->GetWidget()
+        ->SynchronizeVisualProperties();
+    RenderViewHost* render_view_host =
+        main_window_->web_contents()->GetRenderViewHost();
+
+    if (is_devtools_protocol_test) {
+      devtools_protocol_test_bindings_.reset(
+          new DevToolsProtocolTestBindings(main_window_->web_contents()));
+    }
 
-  RenderFrameHost* main_frame = render_view_host->GetMainFrame();
+    // Compositing tests override the default preferences (see
+    // BlinkTestController::OverrideWebkitPrefs) so we force them to be
+    // calculated again to ensure is_compositing_test_ changes are picked up.
+    OverrideWebkitPrefs(&default_prefs_);
 
-  bool new_frame = !base::ContainsKey(main_window_render_process_hosts_,
-                                      main_frame->GetProcess());
-  HandleNewRenderFrameHost(main_frame);
+    render_view_host->UpdateWebkitPreferences(default_prefs_);
+    HandleNewRenderFrameHost(render_view_host->GetMainFrame());
 
-  // Focus the RenderWidgetHost. This will send an IPC message to the
-  // renderer to propagate the state change.
-  render_view_host_widget->Focus();
+    // Focus the RenderWidgetHost. This will send an IPC message to the
+    // renderer to propagate the state change.
+    main_window_->web_contents()->GetRenderViewHost()->GetWidget()->Focus();
 
-  // Flush various interfaces to ensure a test run begins from a known state.
-  render_view_host_widget->FlushForTesting();
-  GetWebTestControlPtr(main_frame).FlushForTesting();
+    // Flush various interfaces to ensure a test run begins from a known state.
+    main_window_->web_contents()
+        ->GetRenderViewHost()
+        ->GetWidget()
+        ->FlushForTesting();
+    GetWebTestControlPtr(render_view_host->GetMainFrame()).FlushForTesting();
 
-  if (is_devtools_js_test) {
-    LoadDevToolsJSTest();
-  } else {
-    NavigationController::LoadURLParams params(test_url_);
-    // Using PAGE_TRANSITION_LINK avoids a BrowsingInstance/process swap
-    // between web tests.
-    params.transition_type = ui::PageTransitionFromInt(
-        new_frame ? ui::PAGE_TRANSITION_TYPED : ui::PAGE_TRANSITION_LINK);
-    params.should_clear_history_list = true;
-    web_contents->GetController().LoadURLWithParams(params);
+    if (is_devtools_js_test) {
+      LoadDevToolsJSTest();
+    } else {
+      NavigationController::LoadURLParams params(test_url_);
+      // Using PAGE_TRANSITION_LINK avoids a BrowsingInstance/process swap
+      // between web tests.
+      params.transition_type =
+          ui::PageTransitionFromInt(ui::PAGE_TRANSITION_LINK);
+      params.should_clear_history_list = true;
+      main_window_->web_contents()->GetController().LoadURLWithParams(params);
+    }
   }
   return true;
 }

content/shell/browser/web_test/blink_test_controller.h

@@ -114,9 +114,6 @@ class BlinkTestController : public WebContentsObserver,
   BlinkTestController();
   ~BlinkTestController() override;
 
-  // Make sure the browser window has been created and ready to load a test.
-  void EnsureMainWindow();
-
   // True if the controller is ready for testing.
   bool PrepareForWebTest(const TestInfo& test_info);
   // True if the controller was reset successfully.

content/shell/browser/web_test/web_test_browser_main.cc

@@ -80,8 +80,6 @@ void RunTests(content::BrowserMainRunner* main_runner) {
     test_controller.SetTempPath(temp_path);
   }
 
-  test_controller.EnsureMainWindow();
-
   std::cout << "#READY\n";
   std::cout.flush();