[run_web_tests] Replace ImageFirst feature with a better solution

Now for tests that don't call any dumpAsXXX methods, they by default
generate pixel results only, and run_web_tests.py just check their
pixel results against the corresponding pixel baselines only.

Provide testRunner.dumpAsLayout() and dumpAsLayoutWithPixelResults()
for tests that do need to dump the layout tree.

We still dump layout tree when running such tests in content_shell
--run-web-tests command line (instead of running from run_web_tests.py
in protocol mode).

Changes to layout tests and their expectations:
- Use testRunner.dumpAsLayoutWithPixelResults() or dumpAsLayout()
  in some tests that still requires the layout dump:
  - editing/ tests need layout dump to show the caret position.
  - two printing/ tests need layout dump to show how the bug is
    fixed.
  In the future, we may just get rid of layout dumps if it's
  feasible.
- Remove testRunner.dumpAsTextWithPixelResults() from some tests
  which used the function just to suppress layout output. They didn't
  actually want text output but we hadn't had a way to dump image only
  until we had the ImageFirst feature.
- Previous platform-specific -expected.txt for tests under previous
  ImageFirst directories are removed and the common baselines are
  created because the baselines are no longer platform-specific without
  the layout dump.

Extra -expected.txt baselines are not deleted in this CL. Will
follow-up in https://chromium-review.googlesource.com/c/chromium/src/+/1286894.

Bug: 703899
Cq-Include-Trybots: luci.chromium.try:linux-blink-gen-property-trees;luci.chromium.try:linux_layout_tests_slimming_paint_v2;luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Ifcf7fea90684e133b4222909ee508469d0c985e0
Reviewed-on: https://chromium-review.googlesource.com/c/1277654
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Robert Ma <robertma@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602161}

content/shell/browser/layout_test/blink_test_controller.cc

@@ -375,7 +375,8 @@ bool BlinkTestController::PrepareForLayoutTest(const TestInfo& test_info) {
       test_url_, &is_devtools_protocol_test);
   did_send_initial_test_configuration_ = false;
 
-  if (test_info.protocol_mode)
+  protocol_mode_ = test_info.protocol_mode;
+  if (protocol_mode_)
     printer_->set_capture_text_only(false);
   printer_->reset();
 
@@ -950,6 +951,7 @@ void BlinkTestController::HandleNewRenderFrameHost(RenderFrameHost* frame) {
             switches::kAllowExternalPages);
     params->expected_pixel_hash = expected_pixel_hash_;
     params->initial_size = initial_size_;
+    params->protocol_mode = protocol_mode_;
 
     if (did_send_initial_test_configuration_) {
       GetLayoutTestControlPtr(frame)->ReplicateTestConfiguration(

content/shell/browser/layout_test/blink_test_controller.h

@@ -296,6 +296,7 @@ class BlinkTestController : public WebContentsObserver,
   std::string expected_pixel_hash_;
   gfx::Size initial_size_;
   GURL test_url_;
+  bool protocol_mode_;
 
   // Stores the default test-adapted WebPreferences which is then used to fully
   // reset the main window's preferences if and when it is reused.

content/shell/common/layout_test.mojom

@@ -27,6 +27,10 @@ struct ShellTestConfiguration {
 
   // The initial size of the test window.
   gfx.mojom.Size initial_size;
+
+  // Whether the test is running in protocol mode.
+  // See TestInfo::protocol_mode in browser/layout_test/test_info_extractor.h.
+  bool protocol_mode;
 };
 
 // Results of a CaptureDump call.

content/shell/renderer/layout_test/blink_test_runner.cc

@@ -809,7 +809,7 @@ void BlinkTestRunner::ApplyTestConfiguration(
   interfaces->SetMainView(render_view()->GetWebView());
 
   interfaces->SetTestIsRunning(true);
-  interfaces->ConfigureForTestWithURL(params->test_url);
+  interfaces->ConfigureForTestWithURL(params->test_url, params->protocol_mode);
 }
 
 void BlinkTestRunner::OnReplicateTestConfiguration(

content/shell/test_runner/layout_dump.cc

@@ -61,22 +61,16 @@ std::string DumpLayout(WebLocalFrame* frame,
 
   if (flags.dump_as_text()) {
     result = DumpFrameHeaderIfNeeded(frame);
-    if (flags.is_printing() && frame->GetDocument().IsHTMLDocument()) {
-      result += WebFrameContentDumper::DumpLayoutTreeAsText(
-                    frame, WebFrameContentDumper::kLayoutAsTextPrinting)
-                    .Utf8();
-    } else {
-      result += frame->GetDocument()
-                    .ContentAsTextForTesting(flags.should_use_inner_text_dump())
-                    .Utf8();
-    }
+    result += frame->GetDocument()
+                  .ContentAsTextForTesting(flags.should_use_inner_text_dump())
+                  .Utf8();
     result += "\n";
   } else if (flags.dump_as_markup()) {
     DCHECK(!flags.is_printing());
     result = DumpFrameHeaderIfNeeded(frame);
     result += WebFrameContentDumper::DumpAsMarkup(frame).Utf8();
     result += "\n";
-  } else {
+  } else if (flags.dump_as_layout()) {
     if (frame->Parent() == nullptr) {
       WebFrameContentDumper::LayoutAsTextControls layout_text_behavior =
           WebFrameContentDumper::kLayoutAsTextNormal;

content/shell/test_runner/layout_test_runtime_flags.cc

@@ -15,6 +15,7 @@ void LayoutTestRuntimeFlags::Reset() {
 
   set_dump_as_text(false);
   set_dump_as_markup(false);
+  set_dump_as_layout(false);
   set_dump_child_frames(false);
 
   set_is_printing(false);

content/shell/test_runner/layout_test_runtime_flags.h

@@ -48,19 +48,23 @@ class TEST_RUNNER_EXPORT LayoutTestRuntimeFlags {
     dict_.SetString(#name, new_value);                             \
   }
 
-  // If true, the test_shell will generate pixel results.
+  // If true, the test runner will generate pixel results.
   DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(generate_pixel_results)
 
-  // If true, the test_shell will produce a plain text dump rather than a
-  // text representation of the renderer.
+  // If true, the test runner will produce a plain text dump.
   DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(dump_as_text)
 
-  // If true, the test_shell will produce a dump of the DOM rather than a text
-  // representation of the renderer.
+  // If true and dump_as_text is false, the test runner will produce a dump of
+  // the DOM.
   DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(dump_as_markup)
 
-  // If true, the test_shell will recursively dump all frames as text, markup
-  // or layout if dump_as_text, dump_as_markup or none of them is effective.
+  // If true and both dump_as_text and dump_as_markup are false, the test runner
+  // will dump a text representation of the layout.
+  DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(dump_as_layout)
+
+  // If true, the test runner will recursively dump all frames as text, markup
+  // or layout depending on which of dump_as_text, dump_as_markup and
+  // dump_as_layout is effective.
   DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(dump_child_frames)
 
   // If true, layout is to target printed pages.
@@ -89,11 +93,11 @@ class TEST_RUNNER_EXPORT LayoutTestRuntimeFlags {
   // If true, the policy delegate will signal layout test completion.
   DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(policy_delegate_should_notify_done)
 
-  // If true, the test_shell will draw the bounds of the current selection rect
+  // If true, the test runner will draw the bounds of the current selection rect
   // taking possible transforms of the selection rect into account.
   DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(dump_selection_rect)
 
-  // If true, the test_shell will dump the drag image as pixel results.
+  // If true, the test runner will dump the drag image as pixel results.
   DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(dump_drag_image)
 
   // Contents of Accept-Language HTTP header requested by the test.
@@ -109,28 +113,28 @@ class TEST_RUNNER_EXPORT LayoutTestRuntimeFlags {
       dump_web_content_settings_client_callbacks)
   DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(autoplay_allowed)
 
-  // If true, the test_shell will write a descriptive line for each editing
+  // If true, the test runner will write a descriptive line for each editing
   // command.
   DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(dump_editting_callbacks)
 
-  // If true, the test_shell will output a descriptive line for each frame
+  // If true, the test runner will output a descriptive line for each frame
   // load callback.
   DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(dump_frame_load_callbacks)
 
-  // If true, the test_shell will output a descriptive line for each
+  // If true, the test runner will output a descriptive line for each
   // PingLoader dispatched.
   DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(dump_ping_loader_callbacks)
 
-  // If true, the test_shell will output a line of the user gesture status
+  // If true, the test runner will output a line of the user gesture status
   // text for some frame load callbacks.
   DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(
       dump_user_gesture_in_frame_load_callbacks)
 
-  // If true, the test_shell will output a descriptive line for each resource
+  // If true, the test runner will output a descriptive line for each resource
   // load callback.
   DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(dump_resource_load_callbacks)
 
-  // If true, the test_shell will output the MIME type for each resource that
+  // If true, the test runner will output the MIME type for each resource that
   // was loaded.
   DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(dump_resource_response_mime_types)
 
@@ -141,7 +145,7 @@ class TEST_RUNNER_EXPORT LayoutTestRuntimeFlags {
   // If true, output a message when the page title is changed.
   DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(dump_title_changes)
 
-  // If true, the test_shell will print out the icon change notifications.
+  // If true, the test runner will print out the icon change notifications.
   DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(dump_icon_changes)
 
   // If true, the console messages produced by the page will
@@ -167,7 +171,7 @@ class TEST_RUNNER_EXPORT LayoutTestRuntimeFlags {
   // is invoked.
   DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(dump_create_view)
 
-  // If true, the test_shell will output descriptive test for spellcheck
+  // If true, the test runner will output descriptive test for spellcheck
   // execution.
   DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(dump_spell_check_callbacks)
 

content/shell/test_runner/test_interfaces.cc

@@ -93,7 +93,8 @@ void TestInterfaces::SetTestIsRunning(bool running) {
   test_runner_->SetTestIsRunning(running);
 }
 
-void TestInterfaces::ConfigureForTestWithURL(const blink::WebURL& test_url) {
+void TestInterfaces::ConfigureForTestWithURL(const blink::WebURL& test_url,
+                                             bool protocol_mode) {
   std::string spec = GURL(test_url).spec();
   size_t path_start = spec.rfind("LayoutTests/");
   if (path_start != std::string::npos) {
@@ -108,6 +109,17 @@ void TestInterfaces::ConfigureForTestWithURL(const blink::WebURL& test_url) {
   if (is_devtools_test) {
     test_runner_->SetDumpConsoleMessages(false);
   }
+
+  // In protocol mode (see TestInfo::protocol_mode), we dump layout only when
+  // requested by the test. In non-protocol mode, we dump layout by default
+  // because the layout may be the only interesting thing to the user while
+  // we don't dump non-human-readable binary data. In non-protocol mode, we
+  // still generate pixel results (though don't dump them) to let the renderer
+  // execute the same code regardless of the protocol mode, e.g. for ease of
+  // debugging a layout test issue.
+  if (!protocol_mode)
+    test_runner_->setShouldDumpAsLayout(true);
+
   // For http/tests/loading/, which is served via httpd and becomes /loading/.
   if (spec.find("/loading/") != std::string::npos)
     test_runner_->setShouldDumpFrameLoadCallbacks(true);

content/shell/test_runner/test_interfaces.h

@@ -38,7 +38,8 @@ class TestInterfaces {
   void ResetAll();
   bool TestIsRunning();
   void SetTestIsRunning(bool running);
-  void ConfigureForTestWithURL(const blink::WebURL& test_url);
+  void ConfigureForTestWithURL(const blink::WebURL& test_url,
+                               bool protocol_mode);
 
   void WindowOpened(WebViewTestProxyBase* proxy);
   void WindowClosed(WebViewTestProxyBase* proxy);

content/shell/test_runner/test_runner.cc

@@ -174,6 +174,8 @@ class TestRunnerBindings : public gin::Wrappable<TestRunnerBindings> {
   void DumpAsMarkup();
   void DumpAsText();
   void DumpAsTextWithPixelResults();
+  void DumpAsLayout();
+  void DumpAsLayoutWithPixelResults();
   void DumpChildFrames();
   void DumpBackForwardList();
   void DumpCreateView();
@@ -431,6 +433,9 @@ gin::ObjectTemplateBuilder TestRunnerBindings::GetObjectTemplateBuilder(
       .SetMethod("dumpAsText", &TestRunnerBindings::DumpAsText)
       .SetMethod("dumpAsTextWithPixelResults",
                  &TestRunnerBindings::DumpAsTextWithPixelResults)
+      .SetMethod("dumpAsLayout", &TestRunnerBindings::DumpAsLayout)
+      .SetMethod("dumpAsLayoutWithPixelResults",
+                 &TestRunnerBindings::DumpAsLayoutWithPixelResults)
       .SetMethod("dumpBackForwardList",
                  &TestRunnerBindings::DumpBackForwardList)
       .SetMethod("dumpChildFrames", &TestRunnerBindings::DumpChildFrames)
@@ -1036,6 +1041,16 @@ void TestRunnerBindings::DumpAsTextWithPixelResults() {
     runner_->DumpAsTextWithPixelResults();
 }
 
+void TestRunnerBindings::DumpAsLayout() {
+  if (runner_)
+    runner_->DumpAsLayout();
+}
+
+void TestRunnerBindings::DumpAsLayoutWithPixelResults() {
+  if (runner_)
+    runner_->DumpAsLayoutWithPixelResults();
+}
+
 void TestRunnerBindings::DumpChildFrames() {
   if (runner_)
     runner_->DumpChildFrames();
@@ -1634,6 +1649,11 @@ void TestRunner::setShouldDumpAsMarkup(bool value) {
   OnLayoutTestRuntimeFlagsChanged();
 }
 
+void TestRunner::setShouldDumpAsLayout(bool value) {
+  layout_test_runtime_flags_.set_dump_as_layout(value);
+  OnLayoutTestRuntimeFlagsChanged();
+}
+
 bool TestRunner::shouldDumpAsCustomText() const {
   return layout_test_runtime_flags_.has_custom_text_output();
 }
@@ -2295,6 +2315,18 @@ void TestRunner::DumpAsTextWithPixelResults() {
   OnLayoutTestRuntimeFlagsChanged();
 }
 
+void TestRunner::DumpAsLayout() {
+  layout_test_runtime_flags_.set_dump_as_layout(true);
+  layout_test_runtime_flags_.set_generate_pixel_results(false);
+  OnLayoutTestRuntimeFlagsChanged();
+}
+
+void TestRunner::DumpAsLayoutWithPixelResults() {
+  layout_test_runtime_flags_.set_dump_as_layout(true);
+  layout_test_runtime_flags_.set_generate_pixel_results(true);
+  OnLayoutTestRuntimeFlagsChanged();
+}
+
 void TestRunner::DumpChildFrames() {
   layout_test_runtime_flags_.set_dump_child_frames(true);
   OnLayoutTestRuntimeFlagsChanged();

content/shell/test_runner/test_runner.h

@@ -112,6 +112,7 @@ class TestRunner : public WebTestRunner {
   void SetV8CacheDisabled(bool);
   void setShouldDumpAsText(bool);
   void setShouldDumpAsMarkup(bool);
+  void setShouldDumpAsLayout(bool);
   void setCustomTextOutput(const std::string& text);
   void setShouldGeneratePixelResults(bool);
   void setShouldDumpFrameLoadCallbacks(bool);
@@ -306,24 +307,34 @@ class TestRunner : public WebTestRunner {
   // ignores any that may be present.
   void DumpEditingCallbacks();
 
-  // This function sets a flag that tells the test_shell to dump pages as
-  // plain text, rather than as a text representation of the renderer's state.
-  // The pixel results will not be generated for this test.
+  // This function sets a flag that tells the test runner to dump pages as
+  // plain text. The pixel results will not be generated for this test.
+  // It has higher priority than DumpAsMarkup() and DumpAsLayout().
   void DumpAsText();
 
   // This function sets a flag that tells the test runner to dump pages as
   // the DOM contents, rather than as a text representation of the renderer's
-  // state. The pixel results will not be generated for this test.
+  // state. The pixel results will not be generated for this test. It has
+  // higher priority than DumpAsLayout(), but lower than DumpAsText().
   void DumpAsMarkup();
 
-  // This function sets a flag that tells the test_shell to dump pages as
-  // plain text, rather than as a text representation of the renderer's state.
-  // It will also generate a pixel dump for the test.
+  // This function sets a flag that tells the test runner to dump pages as
+  // plain text. It will also generate a pixel dump for the test.
   void DumpAsTextWithPixelResults();
 
+  // This function sets a flag that tells the test runner to dump pages as
+  // text representation of the layout. The pixel results will not be generated
+  // for this test. It has lower priority than DumpAsText() and DumpAsMarkup().
+  void DumpAsLayout();
+
+  // This function sets a flag that tells the test runner to dump pages as
+  // text representation of the layout. It will also generate a pixel dump for
+  // the test.
+  void DumpAsLayoutWithPixelResults();
+
   // This function sets a flag that tells the test runner to recursively dump
-  // all frames as text, markup or layout if DumpAsText, DumpAsMarkup or none
-  // of them is effective.
+  // all frames as text, markup or layout depending on which of DumpAsText,
+  // DumpAsMarkup and DumpAsLayout is effective.
   void DumpChildFrames();
 
   // This function sets a flag that tells the test runner to print out the

content/shell/test_runner/web_test_interfaces.cc

@@ -42,8 +42,9 @@ void WebTestInterfaces::SetTestIsRunning(bool running) {
   interfaces_->SetTestIsRunning(running);
 }
 
-void WebTestInterfaces::ConfigureForTestWithURL(const blink::WebURL& test_url) {
-  interfaces_->ConfigureForTestWithURL(test_url);
+void WebTestInterfaces::ConfigureForTestWithURL(const blink::WebURL& test_url,
+                                                bool protocol_mode) {
+  interfaces_->ConfigureForTestWithURL(test_url, protocol_mode);
 }
 
 WebTestRunner* WebTestInterfaces::TestRunner() {

content/shell/test_runner/web_test_interfaces.h

@@ -45,8 +45,10 @@ class TEST_RUNNER_EXPORT WebTestInterfaces {
   bool TestIsRunning();
   void SetTestIsRunning(bool running);
 
-  // Configures the renderer for the test, based on |test_url|.
-  void ConfigureForTestWithURL(const blink::WebURL& test_url);
+  // Configures the renderer for the test, based on |test_url| and
+  // |procotol_mode|.
+  void ConfigureForTestWithURL(const blink::WebURL& test_url,
+                               bool protocol_mode);
 
   WebTestRunner* TestRunner();
   blink::WebThemeEngine* ThemeEngine();