Remove overload for CreateView on WebViewTestProxy.

Move the test logging to the browser side and get rid of the renderer
side override. This change also moves WebTestRuntimeFlags into
common so this class can be used on the browser side so defaults
aren't baked into the code in two places.

BUG=1155202

Change-Id: If53bd8e51737f582efd722559f34b2d80bfe096e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2653768
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#848622}

content/web_test/BUILD.gn

@@ -42,8 +42,12 @@ mojom("web_test_common_mojom") {
 static_library("web_test_common") {
   testonly = true
   sources = [
+    "common/tracked_dictionary.cc",
+    "common/tracked_dictionary.h",
     "common/web_test_constants.cc",
     "common/web_test_constants.h",
+    "common/web_test_runtime_flags.cc",
+    "common/web_test_runtime_flags.h",
     "common/web_test_string_util.cc",
     "common/web_test_string_util.h",
     "common/web_test_switches.cc",
@@ -256,8 +260,6 @@ static_library("web_test_renderer") {
     "renderer/test_websocket_handshake_throttle_provider.h",
     "renderer/text_input_controller.cc",
     "renderer/text_input_controller.h",
-    "renderer/tracked_dictionary.cc",
-    "renderer/tracked_dictionary.h",
     "renderer/web_ax_object_proxy.cc",
     "renderer/web_ax_object_proxy.h",
     "renderer/web_frame_test_proxy.cc",
@@ -270,8 +272,6 @@ static_library("web_test_renderer") {
     "renderer/web_test_grammar_checker.h",
     "renderer/web_test_render_thread_observer.cc",
     "renderer/web_test_render_thread_observer.h",
-    "renderer/web_test_runtime_flags.cc",
-    "renderer/web_test_runtime_flags.h",
     "renderer/web_test_spell_checker.cc",
     "renderer/web_test_spell_checker.h",
     "renderer/web_view_test_proxy.cc",

content/web_test/browser/web_test_content_browser_client.cc

@@ -49,6 +49,7 @@
 #include "content/web_test/browser/web_test_storage_access_manager.h"
 #include "content/web_test/browser/web_test_tts_platform.h"
 #include "content/web_test/common/web_test_bluetooth_fake_adapter_setter.mojom.h"
+#include "content/web_test/common/web_test_string_util.h"
 #include "content/web_test/common/web_test_switches.h"
 #include "device/bluetooth/public/mojom/test/fake_bluetooth.mojom.h"
 #include "device/bluetooth/test/fake_bluetooth.h"
@@ -446,6 +447,19 @@ bool WebTestContentBrowserClient::CanCreateWindow(
     bool opener_suppressed,
     bool* no_javascript_access) {
   *no_javascript_access = false;
+
+  WebTestControlHost* control_host = WebTestControlHost::Get();
+  bool dump_navigation_policy =
+      control_host->web_test_runtime_flags().dump_navigation_policy();
+
+  if (dump_navigation_policy) {
+    static_cast<mojom::WebTestControlHost*>(control_host)
+        ->PrintMessage(
+            "Default policy for createView for '" +
+            web_test_string_util::URLDescription(target_url) + "' is '" +
+            web_test_string_util::WindowOpenDispositionToString(disposition) +
+            "'\n");
+  }
   return !block_popups_ || user_gesture;
 }
 

content/web_test/browser/web_test_control_host.cc

@@ -177,25 +177,23 @@ std::string DumpHistoryForWebContents(WebContents* web_contents) {
 }
 
 std::vector<std::string> DumpTitleWasSet(WebContents* web_contents) {
-  base::Optional<bool> load = WebTestControlHost::Get()
-                                  ->accumulated_web_test_runtime_flags_changes()
-                                  .FindBoolPath("dump_frame_load_callbacks");
+  WebTestControlHost* control_host = WebTestControlHost::Get();
+  bool load =
+      control_host->web_test_runtime_flags().dump_frame_load_callbacks();
 
-  base::Optional<bool> title_changed =
-      WebTestControlHost::Get()
-          ->accumulated_web_test_runtime_flags_changes()
-          .FindBoolPath("dump_title_changes");
+  bool title_changed =
+      control_host->web_test_runtime_flags().dump_title_changes();
 
   std::vector<std::string> logs;
 
-  if (load.has_value() && load.value()) {
+  if (load) {
     // TitleWasSet is only available on top-level frames.
     std::string log = "main frame";
     logs.emplace_back(
         log + " - TitleWasSet: " + base::UTF16ToUTF8(web_contents->GetTitle()));
   }
 
-  if (title_changed.has_value() && title_changed.value()) {
+  if (title_changed) {
     logs.emplace_back("TITLE CHANGED: '" +
                       base::UTF16ToUTF8(web_contents->GetTitle()) + "'");
   }
@@ -204,12 +202,11 @@ std::vector<std::string> DumpTitleWasSet(WebContents* web_contents) {
 
 std::string DumpFailLoad(WebContents* web_contents,
                          RenderFrameHost* render_frame_host) {
-  base::Optional<bool> result =
-      WebTestControlHost::Get()
-          ->accumulated_web_test_runtime_flags_changes()
-          .FindBoolPath("dump_frame_load_callbacks");
+  WebTestControlHost* control_host = WebTestControlHost::Get();
+  bool result =
+      control_host->web_test_runtime_flags().dump_frame_load_callbacks();
 
-  if (!result.has_value())
+  if (!result)
     return std::string();
 
   std::string log = (web_contents->GetMainFrame() == render_frame_host)
@@ -554,6 +551,7 @@ bool WebTestControlHost::PrepareForWebTest(const TestInfo& test_info) {
   printer_->reset();
 
   accumulated_web_test_runtime_flags_changes_.Clear();
+  web_test_runtime_flags_.Reset();
   main_window_render_view_hosts_.clear();
   main_window_render_process_hosts_.clear();
   all_observed_render_process_hosts_.clear();
@@ -1070,10 +1068,7 @@ void WebTestControlHost::WebContentsDestroyed() {
 void WebTestControlHost::DidUpdateFaviconURL(
     RenderFrameHost* render_frame_host,
     const std::vector<blink::mojom::FaviconURLPtr>& candidates) {
-  bool should_dump_icon_changes = false;
-  accumulated_web_test_runtime_flags_changes_.GetBoolean(
-      "dump_icon_changes", &should_dump_icon_changes);
-  if (should_dump_icon_changes) {
+  if (web_test_runtime_flags_.dump_icon_changes()) {
     std::string log = IsMainWindow(web_contents()) ? "main frame " : "frame ";
     printer_->AddMessageRaw(log + "- didChangeIcons\n");
   }
@@ -1249,6 +1244,7 @@ void WebTestControlHost::OnTestFinished() {
   devtools_bindings_.reset();
   devtools_protocol_test_bindings_.reset();
   accumulated_web_test_runtime_flags_changes_.Clear();
+  web_test_runtime_flags_.Reset();
   work_queue_states_.Clear();
 
   ShellBrowserContext* browser_context =
@@ -1675,6 +1671,8 @@ void WebTestControlHost::WebTestRuntimeFlagsChanged(
   // Stash the accumulated changes for future, not-yet-created renderers.
   accumulated_web_test_runtime_flags_changes_.MergeDictionary(
       &changed_web_test_runtime_flags);
+  web_test_runtime_flags_.tracked_dictionary().ApplyUntrackedChanges(
+      accumulated_web_test_runtime_flags_changes_);
 
   // Propagate the changes to all the tracked renderer processes.
   for (RenderProcessHost* process : all_observed_render_process_hosts_) {

content/web_test/browser/web_test_control_host.h

@@ -33,6 +33,7 @@
 #include "content/public/browser/web_contents_observer.h"
 #include "content/web_test/browser/leak_detector.h"
 #include "content/web_test/common/web_test.mojom.h"
+#include "content/web_test/common/web_test_runtime_flags.h"
 #include "mojo/public/cpp/bindings/associated_receiver_set.h"
 #include "mojo/public/cpp/bindings/associated_remote.h"
 #include "third_party/blink/public/common/web_preferences/web_preferences.h"
@@ -173,9 +174,8 @@ class WebTestControlHost : public WebContentsObserver,
   // GpuDataManagerObserver implementation.
   void OnGpuProcessCrashed(base::TerminationStatus exit_code) override;
 
-  const base::DictionaryValue& accumulated_web_test_runtime_flags_changes()
-      const {
-    return accumulated_web_test_runtime_flags_changes_;
+  const WebTestRuntimeFlags& web_test_runtime_flags() const {
+    return web_test_runtime_flags_;
   }
 
  private:
@@ -360,6 +360,9 @@ class WebTestControlHost : public WebContentsObserver,
   // renderer created while test is in progress).
   base::DictionaryValue accumulated_web_test_runtime_flags_changes_;
 
+  // A snasphot of the current runtime flags.
+  WebTestRuntimeFlags web_test_runtime_flags_;
+
   // Work items to be processed in the TestRunner on the renderer process
   // that hosts the main window's main frame.
   base::circular_deque<mojom::WorkItemPtr> work_queue_;

content/web_test/browser/web_test_download_manager_delegate.cc

@@ -53,15 +53,13 @@ void WebTestDownloadManagerDelegate::CheckDownloadAllowed(
     bool content_initiated,
     content::CheckDownloadAllowedCallback check_download_allowed_cb) {
   auto* test_controller = WebTestControlHost::Get();
-  base::Optional<bool> should_wait_until_external_url_load =
-      test_controller->accumulated_web_test_runtime_flags_changes()
-          .FindBoolPath("wait_until_external_url_load");
+  bool should_wait_until_external_url_load =
+      test_controller->web_test_runtime_flags().wait_until_external_url_load();
 
   // The if clause below catches all calls to this method not
   // initiated by content, or even if it does, whose web_test
   // does not call TestRunner::WaitUntilExternalUrlLoad().
-  if (!content_initiated || !should_wait_until_external_url_load.has_value() ||
-      !should_wait_until_external_url_load.value()) {
+  if (!content_initiated || !should_wait_until_external_url_load) {
     ShellDownloadManagerDelegate::CheckDownloadAllowed(
         web_contents_getter, url, request_method, request_initiator,
         from_download_cross_origin_redirect, content_initiated,

content/web_test/browser/web_test_javascript_dialog_manager.cc

@@ -19,23 +19,14 @@ namespace content {
 
 namespace {
 bool DumpJavascriptDialog() {
-  base::Optional<bool> result =
-      WebTestControlHost::Get()
-          ->accumulated_web_test_runtime_flags_changes()
-          .FindBoolPath("dump_javascript_dialogs");
-  if (!result.has_value())
-    return true;
-  return result.value();
+  WebTestControlHost* control_host = WebTestControlHost::Get();
+  return control_host->web_test_runtime_flags().dump_javascript_dialogs();
 }
 
 bool ShouldStayOnPageAfterHandlingBeforeUnload() {
-  base::Optional<bool> result =
-      WebTestControlHost::Get()
-          ->accumulated_web_test_runtime_flags_changes()
-          .FindBoolPath("stay_on_page_after_handling_before_unload");
-  if (!result.has_value())
-    return false;
-  return result.value();
+  WebTestControlHost* control_host = WebTestControlHost::Get();
+  return control_host->web_test_runtime_flags()
+      .stay_on_page_after_handling_before_unload();
 }
 
 }  // namespace

content/web_test/browser/web_test_shell_platform_delegate.cc

@@ -51,9 +51,9 @@ bool WebTestShellPlatformDelegate::HandleRequestToLockMouse(
 
 bool WebTestShellPlatformDelegate::ShouldAllowRunningInsecureContent(
     Shell* shell) {
-  const base::DictionaryValue& flags =
-      WebTestControlHost::Get()->accumulated_web_test_runtime_flags_changes();
-  return flags.FindBoolPath("running_insecure_content_allowed").value_or(false);
+  WebTestControlHost* control_host = WebTestControlHost::Get();
+  return control_host->web_test_runtime_flags()
+      .running_insecure_content_allowed();
 }
 
 }  // namespace content

content/web_test/renderer/tracked_dictionary.cc → content/web_test/common/tracked_dictionary.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "content/web_test/renderer/tracked_dictionary.h"
+#include "content/web_test/common/tracked_dictionary.h"
 
 #include <utility>
 

content/web_test/renderer/tracked_dictionary.h → content/web_test/common/tracked_dictionary.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CONTENT_WEB_TEST_RENDERER_TRACKED_DICTIONARY_H_
-#define CONTENT_WEB_TEST_RENDERER_TRACKED_DICTIONARY_H_
+#ifndef CONTENT_WEB_TEST_COMMON_TRACKED_DICTIONARY_H_
+#define CONTENT_WEB_TEST_COMMON_TRACKED_DICTIONARY_H_
 
 #include <memory>
 #include <string>
@@ -54,4 +54,4 @@ class TrackedDictionary {
 
 }  // namespace content
 
-#endif  // CONTENT_WEB_TEST_RENDERER_TRACKED_DICTIONARY_H_
+#endif  // CONTENT_WEB_TEST_COMMON_TRACKED_DICTIONARY_H_

content/web_test/renderer/web_test_runtime_flags.cc → content/web_test/common/web_test_runtime_flags.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "content/web_test/renderer/web_test_runtime_flags.h"
+#include "content/web_test/common/web_test_runtime_flags.h"
 
 namespace content {
 
@@ -53,7 +53,6 @@ void WebTestRuntimeFlags::Reset() {
 
   set_have_loading_frame(false);
 
-  set_dump_create_view(false);
   set_dump_javascript_dialogs(true);
 
   set_has_custom_text_output(false);

content/web_test/renderer/web_test_runtime_flags.h → content/web_test/common/web_test_runtime_flags.h

@@ -2,15 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CONTENT_WEB_TEST_RENDERER_WEB_TEST_RUNTIME_FLAGS_H_
-#define CONTENT_WEB_TEST_RENDERER_WEB_TEST_RUNTIME_FLAGS_H_
+#ifndef CONTENT_WEB_TEST_COMMON_WEB_TEST_RUNTIME_FLAGS_H_
+#define CONTENT_WEB_TEST_COMMON_WEB_TEST_RUNTIME_FLAGS_H_
 
 #include <string>
 
 #include "base/check.h"
 #include "base/macros.h"
 #include "base/values.h"
-#include "content/web_test/renderer/tracked_dictionary.h"
+#include "content/web_test/common/tracked_dictionary.h"
 
 namespace content {
 
@@ -150,10 +150,6 @@ class WebTestRuntimeFlags {
   // frames. Only one can do it at a time.
   DEFINE_BOOL_WEB_TEST_RUNTIME_FLAG(have_loading_frame)
 
-  // If true, output a descriptive line each time WebViewClient::createView
-  // is invoked.
-  DEFINE_BOOL_WEB_TEST_RUNTIME_FLAG(dump_create_view)
-
   // If true, content_shell will output text for alert(), confirm(), prompt(),
   // etc.
   DEFINE_BOOL_WEB_TEST_RUNTIME_FLAG(dump_javascript_dialogs)
@@ -178,4 +174,4 @@ class WebTestRuntimeFlags {
 
 }  // namespace content
 
-#endif  // CONTENT_WEB_TEST_RENDERER_WEB_TEST_RUNTIME_FLAGS_H_
+#endif  // CONTENT_WEB_TEST_COMMON_WEB_TEST_RUNTIME_FLAGS_H_

content/web_test/common/web_test_string_util.cc

@@ -76,6 +76,25 @@ const char* WebNavigationPolicyToString(
   }
 }
 
+const char* WindowOpenDispositionToString(WindowOpenDisposition disposition) {
+  switch (disposition) {
+    case WindowOpenDisposition::SAVE_TO_DISK:
+      return kPolicyDownload;
+    case WindowOpenDisposition::CURRENT_TAB:
+      return kPolicyCurrentTab;
+    case WindowOpenDisposition::NEW_BACKGROUND_TAB:
+      return kPolicyNewBackgroundTab;
+    case WindowOpenDisposition::NEW_FOREGROUND_TAB:
+      return kPolicyNewForegroundTab;
+    case WindowOpenDisposition::NEW_WINDOW:
+      return kPolicyNewWindow;
+    case WindowOpenDisposition::NEW_POPUP:
+      return kPolicyNewPopup;
+    default:
+      return kIllegalString;
+  }
+}
+
 blink::WebString V8StringToWebString(v8::Isolate* isolate,
                                      v8::Local<v8::String> v8_str) {
   int length = v8_str->Utf8Length(isolate) + 1;

content/web_test/common/web_test_string_util.h

@@ -9,6 +9,7 @@
 
 #include "third_party/blink/public/platform/web_string.h"
 #include "third_party/blink/public/web/web_navigation_policy.h"
+#include "ui/base/window_open_disposition.h"
 #include "v8/include/v8.h"
 
 class GURL;
@@ -22,6 +23,7 @@ std::string NormalizeWebTestURL(const std::string& url);
 std::string URLDescription(const GURL& url);
 const char* WebNavigationPolicyToString(
     const blink::WebNavigationPolicy& policy);
+const char* WindowOpenDispositionToString(WindowOpenDisposition disposition);
 
 blink::WebString V8StringToWebString(v8::Isolate* isolate,
                                      v8::Local<v8::String> v8_str);

content/web_test/renderer/pixel_dump.cc

@@ -17,7 +17,7 @@
 #include "cc/paint/paint_flags.h"
 #include "cc/paint/skia_paint_canvas.h"
 #include "content/public/renderer/render_frame.h"
-#include "content/web_test/renderer/web_test_runtime_flags.h"
+#include "content/web_test/common/web_test_runtime_flags.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "printing/metafile_skia.h"
 #include "printing/mojom/print.mojom.h"