Simplify and make BlinkTestRunner::CaptureLocalPixelsDump synchronous.

Remove some async callbacks that are not needed in the renderer side
test completion path. This is toward pushing test completion code up
to the browser so we don't depend on keeping a stable renderer main
frame across navigations, which RenderDocument changes.

R=avi@chromium.org

Bug: 866140, 1069111
Change-Id: Ia73ab3ec79ec2a226fc26a92c65962355ff6e1fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2284373Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#785926}

content/shell/renderer/web_test/blink_test_runner.cc

@@ -140,31 +140,12 @@ void BlinkTestRunner::CaptureLocalPixelsDump() {
   // with the current, non-swapped-out RenderView.
   DCHECK(web_view_test_proxy_->GetMainRenderFrame());
 
-  waiting_for_pixels_dump_result_ = true;
-
   TestRunner* test_runner = web_view_test_proxy_->GetTestRunner();
-  test_runner->DumpPixelsAsync(
-      web_view_test_proxy_,
-      base::BindOnce(&BlinkTestRunner::OnPixelsDumpCompleted,
-                     base::Unretained(this)));
-}
-
-void BlinkTestRunner::OnLayoutDumpCompleted(std::string completed_layout_dump) {
-  CHECK(waiting_for_layout_dump_results_);
-  dump_result_->layout.emplace(completed_layout_dump);
-  waiting_for_layout_dump_results_ = false;
-  CaptureDumpComplete();
-}
+  SkBitmap snapshot = test_runner->DumpPixelsInRenderer(web_view_test_proxy_);
 
-void BlinkTestRunner::OnPixelsDumpCompleted(const SkBitmap& snapshot) {
-  CHECK(waiting_for_pixels_dump_result_);
-  DCHECK_NE(0, snapshot.info().width());
-  DCHECK_NE(0, snapshot.info().height());
+  DCHECK_GT(snapshot.info().width(), 0);
+  DCHECK_GT(snapshot.info().height(), 0);
 
-  // The snapshot arrives from the GPU process via shared memory. Because MSan
-  // can't track initializedness across processes, we must assure it that the
-  // pixels are in fact initialized.
-  MSAN_UNPOISON(snapshot.getPixels(), snapshot.computeByteSize());
   base::MD5Digest digest;
   base::MD5Sum(snapshot.getPixels(), snapshot.computeByteSize(), &digest);
   std::string actual_pixel_hash = base::MD5DigestToBase16(digest);
@@ -173,13 +154,19 @@ void BlinkTestRunner::OnPixelsDumpCompleted(const SkBitmap& snapshot) {
   if (actual_pixel_hash != test_config_->expected_pixel_hash)
     dump_result_->pixels = snapshot;
 
-  waiting_for_pixels_dump_result_ = false;
+  CaptureDumpComplete();
+}
+
+void BlinkTestRunner::OnLayoutDumpCompleted(std::string completed_layout_dump) {
+  CHECK(waiting_for_layout_dump_results_);
+  dump_result_->layout.emplace(completed_layout_dump);
+  waiting_for_layout_dump_results_ = false;
   CaptureDumpComplete();
 }
 
 void BlinkTestRunner::CaptureDumpComplete() {
   // Abort if we're still waiting for some results.
-  if (waiting_for_layout_dump_results_ || waiting_for_pixels_dump_result_)
+  if (waiting_for_layout_dump_results_)
     return;
 
   // Abort if the browser didn't ask us for the dump yet.

content/shell/renderer/web_test/blink_test_runner.h

@@ -97,7 +97,6 @@ class BlinkTestRunner {
   mojom::WebTestRenderFrame::CaptureDumpCallback dump_callback_;
   mojom::WebTestDumpPtr dump_result_;
   bool waiting_for_layout_dump_results_ = false;
-  bool waiting_for_pixels_dump_result_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(BlinkTestRunner);
 };

content/shell/renderer/web_test/pixel_dump.cc

@@ -40,11 +40,7 @@
 
 namespace content {
 
-namespace {
-
-void CapturePixelsForPrinting(
-    blink::WebLocalFrame* web_frame,
-    base::OnceCallback<void(const SkBitmap&)> callback) {
+SkBitmap PrintFrameToBitmap(blink::WebLocalFrame* web_frame) {
   auto* frame_widget = web_frame->LocalRoot()->FrameWidget();
   frame_widget->UpdateAllLifecyclePhases(blink::DocumentUpdateReason::kTest);
 
@@ -61,8 +57,7 @@ void CapturePixelsForPrinting(
                                 is_opaque)) {
     LOG(ERROR) << "Failed to create bitmap width=" << page_size_in_pixels.width
                << " height=" << spool_size.height;
-    std::move(callback).Run(SkBitmap());
-    return;
+    return SkBitmap();
   }
 
   printing::MetafileSkia metafile(printing::mojom::SkiaDocumentType::kMSKP,
@@ -71,20 +66,7 @@ void CapturePixelsForPrinting(
   canvas.SetPrintingMetafile(&metafile);
   web_frame->PrintPagesForTesting(&canvas, page_size_in_pixels, spool_size);
   web_frame->PrintEnd();
-
-  std::move(callback).Run(bitmap);
-}
-
-}  // namespace
-
-void PrintFrameAsync(blink::WebLocalFrame* web_frame,
-                     base::OnceCallback<void(const SkBitmap&)> callback) {
-  DCHECK(web_frame);
-  DCHECK(callback);
-  web_frame->GetTaskRunner(blink::TaskType::kInternalTest)
-      ->PostTask(FROM_HERE, base::BindOnce(&CapturePixelsForPrinting,
-                                           base::Unretained(web_frame),
-                                           std::move(callback)));
+  return bitmap;
 }
 
 }  // namespace content

content/shell/renderer/web_test/pixel_dump.h

@@ -5,8 +5,6 @@
 #ifndef CONTENT_SHELL_RENDERER_WEB_TEST_PIXEL_DUMP_H_
 #define CONTENT_SHELL_RENDERER_WEB_TEST_PIXEL_DUMP_H_
 
-#include "base/callback_forward.h"
-
 class SkBitmap;
 
 namespace blink {
@@ -15,9 +13,9 @@ class WebLocalFrame;
 
 namespace content {
 
-// Asks |web_frame| to print itself and calls |callback| with the result.
-void PrintFrameAsync(blink::WebLocalFrame* web_frame,
-                     base::OnceCallback<void(const SkBitmap&)> callback);
+// Goes through a test-only path to dump the frame's pixel output as if it was
+// printed.
+SkBitmap PrintFrameToBitmap(blink::WebLocalFrame* web_frame);
 
 }  // namespace content
 

content/shell/renderer/web_test/test_runner.cc

@@ -1871,33 +1871,24 @@ void TestRunnerBindings::GetManifestThen(v8::Local<v8::Function> v8_callback) {
       base::BindOnce(GetManifestReply, WrapV8Callback(std::move(v8_callback))));
 }
 
-static void CapturePrintingPixelsThenReply(
-    base::WeakPtr<TestRunnerBindings> test_runner,
-    blink::WebLocalFrame* frame,
-    BoundV8Callback callback,
-    const SkBitmap& bitmap) {
-  if (!test_runner)  // This guards the validity of the |frame|.
+void TestRunnerBindings::CapturePrintingPixelsThen(
+    v8::Local<v8::Function> v8_callback) {
+  if (invalid_)
     return;
+  SkBitmap bitmap = PrintFrameToBitmap(GetWebFrame());
 
   v8::Isolate* isolate = blink::MainThreadIsolate();
   v8::HandleScope handle_scope(isolate);
 
   // ConvertBitmapToV8() requires a v8::Context.
-  v8::Local<v8::Context> context = frame->MainWorldScriptContext();
+  v8::Local<v8::Context> context = GetWebFrame()->MainWorldScriptContext();
   CHECK(!context.IsEmpty());
   v8::Context::Scope context_scope(context);
 
-  std::move(callback).Run(ConvertBitmapToV8(context_scope, bitmap));
-}
-
-void TestRunnerBindings::CapturePrintingPixelsThen(
-    v8::Local<v8::Function> v8_callback) {
-  if (invalid_)
-    return;
-  PrintFrameAsync(GetWebFrame(),
-                  base::BindOnce(&CapturePrintingPixelsThenReply,
-                                 weak_ptr_factory_.GetWeakPtr(), GetWebFrame(),
-                                 WrapV8Callback(std::move(v8_callback))));
+  WrapV8Callback(std::move(v8_callback))
+      .Run({
+          ConvertBitmapToV8(context_scope, bitmap),
+      });
 }
 
 void TestRunnerBindings::CheckForLeakedWindows() {
@@ -2368,25 +2359,21 @@ bool TestRunner::CanDumpPixelsFromRenderer() const {
          web_test_runtime_flags_.is_printing();
 }
 
-void TestRunner::DumpPixelsAsync(
-    content::RenderView* render_view,
-    base::OnceCallback<void(const SkBitmap&)> callback) {
+SkBitmap TestRunner::DumpPixelsInRenderer(content::RenderView* render_view) {
   auto* view_proxy = static_cast<WebViewTestProxy*>(render_view);
   DCHECK(view_proxy->GetWebView()->MainFrame());
   DCHECK(CanDumpPixelsFromRenderer());
 
   if (web_test_runtime_flags_.dump_drag_image()) {
-    if (!drag_image_.isNull()) {
-      std::move(callback).Run(drag_image_);
-    } else {
-      // This means the test called dumpDragImage but did not initiate a drag.
-      // Return a blank image so that the test fails.
-      SkBitmap bitmap;
-      bitmap.allocN32Pixels(1, 1);
-      bitmap.eraseColor(0);
-      std::move(callback).Run(bitmap);
-    }
-    return;
+    if (!drag_image_.isNull())
+      return drag_image_;
+
+    // This means the test called dumpDragImage but did not initiate a drag.
+    // Return a blank image so that the test fails.
+    SkBitmap bitmap;
+    bitmap.allocN32Pixels(1, 1);
+    bitmap.eraseColor(0);
+    return bitmap;
   }
 
   blink::WebLocalFrame* frame =
@@ -2399,7 +2386,7 @@ void TestRunner::DumpPixelsAsync(
     if (frame_to_print && frame_to_print->IsWebLocalFrame())
       target_frame = frame_to_print->ToWebLocalFrame();
   }
-  PrintFrameAsync(target_frame, std::move(callback));
+  return PrintFrameToBitmap(target_frame);
 }
 
 void TestRunner::ReplicateWebTestRuntimeFlagsChanges(

content/shell/renderer/web_test/test_runner.h

@@ -138,14 +138,13 @@ class TestRunner {
   bool ShouldDumpSelectionRect() const;
 
   // Returns false if the browser should capture the pixel output, true if it
-  // can be done locally in the renderer via DumpPixelsAsync().
+  // can be done locally in the renderer via DumpPixelsInRenderer().
   bool CanDumpPixelsFromRenderer() const;
 
   // Snapshots the content of |render_view| using the mode requested by the
   // current test and calls |callback| with the result.  Caller needs to ensure
   // that |render_view| stays alive until |callback| is called.
-  void DumpPixelsAsync(content::RenderView* render_view,
-                       base::OnceCallback<void(const SkBitmap&)> callback);
+  SkBitmap DumpPixelsInRenderer(content::RenderView* render_view);
 
   // Replicates changes to web test runtime flags (i.e. changes that happened in
   // another renderer). See also BlinkTestRunner::OnWebTestRuntimeFlagsChanged.

third_party/blink/web_tests/printing/resources/webgl-repeated-printing-common.js

@@ -43,7 +43,8 @@ function nextTest() {
   var color = test['clearColor'];
   try {
     draw(color[0], color[1], color[2], color[3]);
-    testRunner.capturePrintingPixelsThen(completionCallback);
+    testRunner.updateAllLifecyclePhasesAndCompositeThen(
+      ()=>testRunner.capturePrintingPixelsThen(completionCallback));
   } catch (e) {
     debug('error in nextTest');
     debug(e);