[LayoutTests] Replace multiple dump responses with one callback/struct.

Before this patch:
- We issue a CaptureDump callback from the main frame that has finished
  a test. If a secondary renderer issues a test finished, this routes
  via a browser and goes back to the main frame anyway.
- We issue several IPCs to the browser for each type of dump requested.
- After everything is done, we issue a test done IPC to the browser.

With this patch:
- We capture local dumps (temporarily*)
- We request that a browser initiates a CaptureDump via IPC.
- We get a mojo call to capture the dump with an output struct containing
  all the relevant fields, which the renderer populates from local dumps.
- When it finishes populating everything, it issues a callback with that
  struct.
- The browser then records all of the results from the struct and
  finishes.

The new approach seems to be a bit better in that we can move more logic
to the browser part and make the renderer part more synchronous. For
example, if the browser needs to initiate a layout dump, we can just do
that from the browser instead of requiring that the renderer sends an IPC
to the browser with that request.

Furthermore, because the end goal of this is to have OOPIF pixel dumps,
we need those to be initiated from the browser at some point. This also
aligns with the pattern where the browser is the part that initiates
a capture dump.

* - We would like to capture as little as possible locally and instead
    move things that capture things by asking the browser to the browser
    directly.

R=lukasza@chromium.org, mkwst@chromium.org

Bug: 667551
Change-Id: I07d2b16da687b679f6b3020732706e6bd7a77cd4
Reviewed-on: https://chromium-review.googlesource.com/972578
Commit-Queue: vmpstr <vmpstr@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545880}

content/shell/BUILD.gn

@@ -842,6 +842,7 @@ mojom("mojo_bindings") {
   ]
   public_deps = [
     "//mojo/public/mojom/base",
+    "//skia/public/interfaces",
     "//ui/gfx/geometry/mojo",
     "//url/mojom:url_mojom_gurl",
   ]

content/shell/browser/layout_test/blink_test_controller.cc

@@ -177,8 +177,12 @@ BlinkTestResultPrinter::BlinkTestResultPrinter(std::ostream* output,
 BlinkTestResultPrinter::~BlinkTestResultPrinter() {
 }
 
+void BlinkTestResultPrinter::StartStateDump() {
+  state_ = DURING_STATE_DUMP;
+}
+
 void BlinkTestResultPrinter::PrintTextHeader() {
-  if (state_ != DURING_TEST)
+  if (state_ != DURING_STATE_DUMP)
     return;
   if (!capture_text_only_)
     *output_ << "Content-Type: text/plain\n";
@@ -237,7 +241,7 @@ void BlinkTestResultPrinter::PrintImageFooter() {
 }
 
 void BlinkTestResultPrinter::PrintAudioHeader() {
-  DCHECK_EQ(state_, DURING_TEST);
+  DCHECK_EQ(state_, DURING_STATE_DUMP);
   if (!capture_text_only_)
     *output_ << "Content-Type: audio/wav\n";
   state_ = IN_AUDIO_BLOCK;
@@ -284,7 +288,7 @@ void BlinkTestResultPrinter::AddMessageRaw(const std::string& message) {
 void BlinkTestResultPrinter::AddErrorMessage(const std::string& message) {
   if (!capture_text_only_)
     *error_ << message << "\n";
-  if (state_ != DURING_TEST)
+  if (state_ != DURING_TEST && state_ != DURING_STATE_DUMP)
     return;
   PrintTextHeader();
   *output_ << message << "\n";
@@ -498,6 +502,7 @@ bool BlinkTestController::ResetAfterLayoutTest() {
   prefs_ = WebPreferences();
   should_override_prefs_ = false;
   LayoutTestContentBrowserClient::Get()->SetPopupBlockingEnabled(false);
+  navigation_history_dump_ = "";
 
 #if defined(OS_ANDROID)
   // Re-using the shell's main window on Android causes issues with networking
@@ -547,6 +552,36 @@ void BlinkTestController::OnTestFinishedInSecondaryRenderer() {
       main_render_view_host->GetRoutingID()));
 }
 
+void BlinkTestController::OnInitiateCaptureDump(
+    bool capture_navigation_history) {
+  if (capture_navigation_history) {
+    RenderFrameHost* main_rfh = main_window_->web_contents()->GetMainFrame();
+    for (auto* window : Shell::windows()) {
+      WebContents* web_contents = window->web_contents();
+      // Only capture the history from windows in the same process as the main
+      // window. During layout tests, we only use two processes when a devtools
+      // window is open.
+      // TODO(https://crbug.com/771003): Dump history for all WebContentses, not
+      // just ones that happen to be in the same process as the main test
+      // window's main frame.
+      if (main_rfh->GetProcess() != web_contents->GetMainFrame()->GetProcess())
+        continue;
+
+      navigation_history_dump_ +=
+          "\n============== Back Forward List ==============\n";
+      navigation_history_dump_ += DumpHistoryForWebContents(web_contents);
+      navigation_history_dump_ +=
+          "===============================================\n";
+    }
+  }
+
+  RenderFrameHost* rfh = main_window_->web_contents()->GetMainFrame();
+  printer_->StartStateDump();
+  GetLayoutTestControlPtr(rfh)->CaptureDump(
+      base::BindOnce(&BlinkTestController::OnCaptureDumpCompleted,
+                     weak_factory_.GetWeakPtr()));
+}
+
 bool BlinkTestController::IsMainWindow(WebContents* web_contents) const {
   return main_window_ && web_contents == main_window_->web_contents();
 }
@@ -576,16 +611,12 @@ bool BlinkTestController::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_HANDLER(ShellViewHostMsg_PrintMessage, OnPrintMessage)
     IPC_MESSAGE_HANDLER(ShellViewHostMsg_PrintMessageToStderr,
                         OnPrintMessageToStderr)
-    IPC_MESSAGE_HANDLER(ShellViewHostMsg_TextDump, OnTextDump)
     IPC_MESSAGE_HANDLER(ShellViewHostMsg_InitiateLayoutDump,
                         OnInitiateLayoutDump)
-    IPC_MESSAGE_HANDLER(ShellViewHostMsg_ImageDump, OnImageDump)
-    IPC_MESSAGE_HANDLER(ShellViewHostMsg_AudioDump, OnAudioDump)
     IPC_MESSAGE_HANDLER(ShellViewHostMsg_OverridePreferences,
                         OnOverridePreferences)
     IPC_MESSAGE_HANDLER(ShellViewHostMsg_SetPopupBlockingEnabled,
                         OnSetPopupBlockingEnabled)
-    IPC_MESSAGE_HANDLER(ShellViewHostMsg_TestFinished, OnTestFinished)
     IPC_MESSAGE_HANDLER(ShellViewHostMsg_NavigateSecondaryWindow,
                         OnNavigateSecondaryWindow)
     IPC_MESSAGE_HANDLER(ShellViewHostMsg_GoToOffset, OnGoToOffset)
@@ -823,6 +854,17 @@ void BlinkTestController::OnAllSharedWorkersDestroyed() {
   }
 }
 
+void BlinkTestController::OnCaptureDumpCompleted(
+    mojom::LayoutTestDumpPtr dump) {
+  if (dump->audio)
+    OnAudioDump(*dump->audio);
+  if (dump->layout)
+    OnTextDump(*dump->layout);
+  if (!dump->actual_pixel_hash.empty())
+    OnImageDump(dump->actual_pixel_hash, dump->pixels);
+  OnTestFinished();
+}
+
 void BlinkTestController::OnImageDump(const std::string& actual_pixel_hash,
                                       const SkBitmap& image) {
   printer_->PrintImageHeader(actual_pixel_hash, expected_pixel_hash_);
@@ -870,30 +912,11 @@ void BlinkTestController::OnAudioDump(const std::vector<unsigned char>& dump) {
   printer_->PrintAudioFooter();
 }
 
-void BlinkTestController::OnTextDump(const std::string& dump,
-                                     bool should_dump_history) {
+void BlinkTestController::OnTextDump(const std::string& dump) {
   printer_->PrintTextHeader();
   printer_->PrintTextBlock(dump);
-  if (should_dump_history) {
-    RenderFrameHost* main_rfh = main_window_->web_contents()->GetMainFrame();
-    for (auto* window : Shell::windows()) {
-      WebContents* web_contents = window->web_contents();
-      // Only capture the history from windows in the same process as the main
-      // window. During layout tests, we only use two processes when a devtools
-      // window is open.
-      // TODO(https://crbug.com/771003): Dump history for all WebContentses, not
-      // just ones that happen to be in the same process as the main test
-      // window's main frame.
-      if (main_rfh->GetProcess() != web_contents->GetMainFrame()->GetProcess())
-        continue;
-
-      printer_->PrintTextBlock(
-          "\n============== Back Forward List ==============\n");
-      printer_->PrintTextBlock(DumpHistoryForWebContents(web_contents));
-      printer_->PrintTextBlock(
-          "===============================================\n");
-    }
-  }
+  if (!navigation_history_dump_.empty())
+    printer_->PrintTextBlock(navigation_history_dump_);
   printer_->PrintTextFooter();
 }
 

content/shell/browser/layout_test/blink_test_controller.h

@@ -93,12 +93,14 @@ class BlinkTestResultPrinter {
   void AddErrorMessage(const std::string& message);
 
   void CloseStderr();
+  void StartStateDump();
 
  private:
   void PrintEncodedBinaryData(const std::vector<unsigned char>& data);
 
   enum State {
     DURING_TEST,
+    DURING_STATE_DUMP,
     IN_TEXT_BLOCK,
     IN_AUDIO_BLOCK,
     IN_IMAGE_BLOCK,
@@ -138,6 +140,7 @@ class BlinkTestController : public WebContentsObserver,
       int sender_process_host_id,
       const base::DictionaryValue& changed_layout_test_runtime_flags);
   void OnTestFinishedInSecondaryRenderer();
+  void OnInitiateCaptureDump(bool capture_navigation_history);
   void OnInspectSecondaryWindow();
 
   // Makes sure that the potentially new renderer associated with |frame| is 1)
@@ -202,7 +205,7 @@ class BlinkTestController : public WebContentsObserver,
   // Message handlers.
   void OnAudioDump(const std::vector<unsigned char>& audio_dump);
   void OnImageDump(const std::string& actual_pixel_hash, const SkBitmap& image);
-  void OnTextDump(const std::string& dump, bool should_dump_history);
+  void OnTextDump(const std::string& dump);
   void OnInitiateLayoutDump();
   void OnDumpFrameLayoutResponse(int frame_tree_node_id,
                                  const std::string& dump);
@@ -228,6 +231,7 @@ class BlinkTestController : public WebContentsObserver,
 
   void OnAllServiceWorkersCleared();
   void OnAllSharedWorkersDestroyed();
+  void OnCaptureDumpCompleted(mojom::LayoutTestDumpPtr dump);
 
   std::unique_ptr<BlinkTestResultPrinter> printer_;
 
@@ -295,6 +299,8 @@ class BlinkTestController : public WebContentsObserver,
   // renderer created while test is in progress).
   base::DictionaryValue accumulated_layout_test_runtime_flags_changes_;
 
+  std::string navigation_history_dump_;
+
   // Map from one frame to one mojo pipe.
   std::map<RenderFrameHost*, mojom::LayoutTestControlAssociatedPtr>
       layout_test_control_map_;

content/shell/browser/layout_test/layout_test_message_filter.cc

@@ -65,6 +65,7 @@ base::TaskRunner* LayoutTestMessageFilter::OverrideTaskRunnerForMessage(
     case LayoutTestHostMsg_ResetPermissions::ID:
     case LayoutTestHostMsg_LayoutTestRuntimeFlagsChanged::ID:
     case LayoutTestHostMsg_TestFinishedInSecondaryRenderer::ID:
+    case LayoutTestHostMsg_InitiateCaptureDump::ID:
     case LayoutTestHostMsg_InspectSecondaryWindow::ID:
     case LayoutTestHostMsg_DeleteAllCookiesForNetworkService::ID:
       return BrowserThread::GetTaskRunnerForThread(BrowserThread::UI).get();
@@ -96,6 +97,8 @@ bool LayoutTestMessageFilter::OnMessageReceived(const IPC::Message& message) {
                         OnLayoutTestRuntimeFlagsChanged)
     IPC_MESSAGE_HANDLER(LayoutTestHostMsg_TestFinishedInSecondaryRenderer,
                         OnTestFinishedInSecondaryRenderer)
+    IPC_MESSAGE_HANDLER(LayoutTestHostMsg_InitiateCaptureDump,
+                        OnInitiateCaptureDump);
     IPC_MESSAGE_HANDLER(LayoutTestHostMsg_InspectSecondaryWindow,
                         OnInspectSecondaryWindow)
     IPC_MESSAGE_UNHANDLED(handled = false)
@@ -248,6 +251,15 @@ void LayoutTestMessageFilter::OnTestFinishedInSecondaryRenderer() {
     BlinkTestController::Get()->OnTestFinishedInSecondaryRenderer();
 }
 
+void LayoutTestMessageFilter::OnInitiateCaptureDump(
+    bool capture_navigation_history) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (BlinkTestController::Get()) {
+    BlinkTestController::Get()->OnInitiateCaptureDump(
+        capture_navigation_history);
+  }
+}
+
 void LayoutTestMessageFilter::OnInspectSecondaryWindow() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   if (BlinkTestController::Get())

content/shell/browser/layout_test/layout_test_message_filter.h

@@ -92,6 +92,7 @@ class LayoutTestMessageFilter : public BrowserMessageFilter {
   void OnLayoutTestRuntimeFlagsChanged(
       const base::DictionaryValue& changed_layout_test_runtime_flags);
   void OnTestFinishedInSecondaryRenderer();
+  void OnInitiateCaptureDump(bool capture_navigation_history);
   void OnInspectSecondaryWindow();
 
   int render_process_id_;

content/shell/common/layout_test.mojom

@@ -5,6 +5,7 @@
 module content.mojom;
 
 import "mojo/public/mojom/base/file_path.mojom";
+import "skia/public/interfaces/bitmap.mojom";
 import "ui/gfx/geometry/mojo/geometry.mojom";
 import "url/mojom/url.mojom";
 
@@ -34,7 +35,22 @@ struct ShellTestConfiguration {
   gfx.mojom.Size initial_size;
 };
 
+// Results of a CaptureDump call.
+struct LayoutTestDump {
+  // Audio dump.
+  array<uint8>? audio;
+
+  // Layout dump.
+  string? layout;
+
+  // Image dump.
+  skia.mojom.Bitmap? pixels;
+  string actual_pixel_hash;
+};
+
 interface LayoutTestControl {
+  CaptureDump() => (LayoutTestDump result);
+
   // Dumps the frame's contents into a string.
   DumpFrameLayout() => (string frame_layout_dump);
 

content/shell/common/layout_test/layout_test_messages.h

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Multiply-included file, no traditional include guard.
+// no-include-guard-because-multiply-included
 #include <string>
 #include <vector>
 
@@ -44,6 +44,8 @@ IPC_MESSAGE_ROUTED4(LayoutTestHostMsg_SetPermission,
                     GURL /* embedding_origin */)
 IPC_MESSAGE_ROUTED0(LayoutTestHostMsg_ResetPermissions)
 IPC_MESSAGE_ROUTED0(LayoutTestHostMsg_InspectSecondaryWindow)
+IPC_MESSAGE_ROUTED1(LayoutTestHostMsg_InitiateCaptureDump,
+                    bool /* should dump navigation history */)
 
 // Notifies the browser that one of renderers has changed layout test runtime
 // flags (i.e. has set dump_as_text).

content/shell/common/shell_messages.h

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Multiply-included file, no traditional include guard.
+// no-include-guard-because-multiply-included
 #include <string>
 #include <vector>
 
@@ -36,28 +36,12 @@ IPC_MESSAGE_ROUTED0(ShellViewMsg_TryLeakDetection)
 IPC_MESSAGE_ROUTED1(ShellViewMsg_LayoutDumpCompleted,
                     std::string /* completed/stitched layout dump */)
 
-// Send a text dump of the WebContents to the render host.
-IPC_MESSAGE_ROUTED2(ShellViewHostMsg_TextDump,
-                    std::string /* dump */,
-                    bool /* should_dump_history */)
-
 // Asks the browser process to perform a layout dump spanning all the
 // (potentially cross-process) frames.  This goes through multiple
 // LayoutTestControl.DumpFrameLayout calls and ends with sending of
 // ShellViewMsg_LayoutDumpCompleted.
 IPC_MESSAGE_ROUTED0(ShellViewHostMsg_InitiateLayoutDump)
 
-// Send an image dump of the WebContents to the render host.
-IPC_MESSAGE_ROUTED2(ShellViewHostMsg_ImageDump,
-                    std::string /* actual pixel hash */,
-                    SkBitmap /* image */)
-
-// Send an audio dump to the render host.
-IPC_MESSAGE_ROUTED1(ShellViewHostMsg_AudioDump,
-                    std::vector<unsigned char> /* audio data */)
-
-IPC_MESSAGE_ROUTED0(ShellViewHostMsg_TestFinished)
-
 IPC_MESSAGE_ROUTED0(ShellViewHostMsg_ResetDone)
 
 // WebTestDelegate related.

content/shell/renderer/layout_test/blink_test_runner.h

@@ -172,6 +172,7 @@ class BlinkTestRunner : public RenderViewObserver,
   void OnSetTestConfiguration(mojom::ShellTestConfigurationPtr params);
   void OnReplicateTestConfiguration(mojom::ShellTestConfigurationPtr params);
   void OnSetupSecondaryRenderer();
+  void CaptureDump(mojom::LayoutTestControl::CaptureDumpCallback callback);
 
  private:
   // Message handlers.
@@ -195,11 +196,12 @@ class BlinkTestRunner : public RenderViewObserver,
 
   // After finishing the test, retrieves the audio, text, and pixel dumps from
   // the TestRunner library and sends them to the browser process.
-  void CaptureDump();
   void OnLayoutDumpCompleted(std::string completed_layout_dump);
-  void CaptureDumpContinued();
   void OnPixelsDumpCompleted(const SkBitmap& snapshot);
   void CaptureDumpComplete();
+  void CaptureLocalAudioDump();
+  void CaptureLocalLayoutDump();
+  void CaptureLocalPixelsDump();
 
   mojom::LayoutTestBluetoothFakeAdapterSetter&
   GetBluetoothFakeAdapterSetter();
@@ -220,6 +222,11 @@ class BlinkTestRunner : public RenderViewObserver,
 
   std::unique_ptr<LeakDetector> leak_detector_;
 
+  mojom::LayoutTestControl::CaptureDumpCallback dump_callback_;
+  mojom::LayoutTestDumpPtr dump_result_;
+  bool waiting_for_layout_dump_results_ = false;
+  bool waiting_for_pixels_dump_result_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(BlinkTestRunner);
 };
 

content/shell/renderer/layout_test/layout_test_render_frame_observer.cc

@@ -42,6 +42,11 @@ void LayoutTestRenderFrameObserver::OnDestruct() {
   delete this;
 }
 
+void LayoutTestRenderFrameObserver::CaptureDump(CaptureDumpCallback callback) {
+  BlinkTestRunner::Get(render_frame()->GetRenderView())
+      ->CaptureDump(std::move(callback));
+}
+
 void LayoutTestRenderFrameObserver::DumpFrameLayout(
     DumpFrameLayoutCallback callback) {
   std::string dump =

content/shell/renderer/layout_test/layout_test_render_frame_observer.h

@@ -22,6 +22,7 @@ class LayoutTestRenderFrameObserver : public RenderFrameObserver,
   // RenderFrameObserver implementation.
   void OnDestruct() override;
 
+  void CaptureDump(CaptureDumpCallback callback) override;
   void DumpFrameLayout(DumpFrameLayoutCallback callback) override;
   void SetTestConfiguration(mojom::ShellTestConfigurationPtr config) override;
   void ReplicateTestConfiguration(