Use surface copy requests for layout tests.

content/test/layouttest_support.cc

Index: content/test/layouttest_support.cc
diff --git a/content/test/layouttest_support.cc b/content/test/layouttest_support.cc
index 816a48084ecbe9214fcdb4c55d9614fd6863afca..69d058f8c4125de20fec094001e59df316799501 100644
--- a/content/test/layouttest_support.cc
+++ b/content/test/layouttest_support.cc
@@ -12,6 +12,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_util.h"
 #include "build/build_config.h"
+#include "cc/output/copy_output_request.h"
 #include "cc/test/pixel_test_output_surface.h"
 #include "cc/test/test_delegating_output_surface.h"
 #include "components/scheduler/test/renderer_scheduler_test_support.h"
@@ -177,9 +178,46 @@ void SetMockDeviceOrientationData(const WebDeviceOrientationData& data) {
   RendererBlinkPlatformImpl::SetMockDeviceOrientationDataForTesting(data);
 }
 
+namespace {
+
+// Invokes a callback on commit (on the main thread) to obtain the output
+// surface that should be used, then asks that output surface to submit the copy
+// request at SwapBuffers time.
+class CopyRequestSwapPromise : public cc::SwapPromise {
+ public:
+  using OutputSurfaceCallback =

[danakj, 2016/07/22 20:50:26]

FindOutputSurfaceCallback?

[jbroman, 2016/07/25 18:50:55]

Done.

+      base::Callback<base::WeakPtr<cc::TestDelegatingOutputSurface>()>;
+  CopyRequestSwapPromise(std::unique_ptr<cc::CopyOutputRequest> request,
+                         OutputSurfaceCallback output_surface_callback)
+      : copy_request_(std::move(request)),
+        output_surface_callback_(std::move(output_surface_callback)) {}
+
+  // cc::SwapPromise

[danakj, 2016/07/22 20:50:27]

"cc::SwapPromise implementation."

[jbroman, 2016/07/25 18:50:55]

Done.

+  void OnCommit() override { output_surface_ = output_surface_callback_.Run(); }
+  void DidActivate() override {}
+  void DidSwap(cc::CompositorFrameMetadata*) override {
+    if (output_surface_)

[danakj, 2016/07/22 20:50:27]

Is this ever going to be null in a layout test? They don't exactly lose their
context.. well maybe some webgl ones do, but not the compositor context. If we
can dcheck instead that'd be nice. Or make the bound function return a raw
pointer instead of a weakptr.

[jbroman, 2016/07/25 18:50:55]

Fair enough. I don't think it can happen (and if it did, the layout test would
get an empty pixel result which would fail anyhow), so OK. If there were more
general users of WebLayerTreeView::compositeAndReadbackAsync it'd be a bigger
deal to fail gracefully, but there aren't, so I'll simplify as you suggest.

+      output_surface_->RequestCopyOfOutput(std::move(copy_request_));
+    else
+      copy_request_->SendEmptyResult();
+  }
+  void DidNotSwap(DidNotSwapReason r) override {
+    copy_request_->SendEmptyResult();

[danakj, 2016/07/22 20:50:27]

Does this happen outside of test flakiness?

[jbroman, 2016/07/25 18:50:55]

I don't think so, aside from the bug I've already fixed. In a general use it
might be reasonable to send an empty result, but for layout tests it basically
changes where the failure occurs, not whether one does.

+  }
+  int64_t TraceId() const override { return 0; }
+
+ private:
+  std::unique_ptr<cc::CopyOutputRequest> copy_request_;
+  OutputSurfaceCallback output_surface_callback_;
+  base::WeakPtr<cc::TestDelegatingOutputSurface> output_surface_;
+};
+
+}  // namespace
+
 class LayoutTestDependenciesImpl : public LayoutTestDependencies {
  public:
   std::unique_ptr<cc::OutputSurface> CreateOutputSurface(
+      int32_t routing_id,
       scoped_refptr<gpu::GpuChannelHost> gpu_channel,
       scoped_refptr<cc::ContextProvider> compositor_context_provider,
       scoped_refptr<cc::ContextProvider> worker_context_provider,
@@ -219,12 +257,35 @@ class LayoutTestDependenciesImpl : public LayoutTestDependencies {
         RenderWidgetCompositor::GenerateLayerTreeSettings(
             *base::CommandLine::ForCurrentProcess(), deps, 1.f);
 
-    return base::MakeUnique<cc::TestDelegatingOutputSurface>(
+    auto output_surface = base::MakeUnique<cc::TestDelegatingOutputSurface>(
         std::move(compositor_context_provider),
         std::move(worker_context_provider), std::move(display_output_surface),
         deps->GetSharedBitmapManager(), deps->GetGpuMemoryBufferManager(),
         settings.renderer_settings, task_runner, synchronous_composite);
+    output_surfaces_[routing_id] = output_surface->GetWeakPtr();
+    return std::move(output_surface);
   }
+
+  std::unique_ptr<cc::SwapPromise> RequestCopyOfOutput(
+      int32_t routing_id,
+      std::unique_ptr<cc::CopyOutputRequest> request) override {
+    // Note that we can't immediately check output_surfaces_, since it may not
+    // have been created yet. Instead, we wait until OnCommit to check what the
+    // last output surface to be requested was. This takes a raw pointer to

[danakj, 2016/07/22 20:50:26]

"wait until OnCommit to find the currently active OutputSurface for the given
RenderWidget (via the routing_id)." or something?

[jbroman, 2016/07/25 18:50:55]

Done.

+    // LayoutTestDependenciesImpl, so it must not be destroyed before commit.

[danakj, 2016/07/22 20:50:27]

"so the dependencies must not" ("it" is ambiguous)

Maybe rather than saying "must not" you can explain why it will not. Like, the
fact that it's tied to the lifetime of the RenderThreadImpl which is not
destroyed before closing any RenderWidgets' compositors? Or so I believe..

[jbroman, 2016/07/25 18:50:55]

Done.

+    return base::MakeUnique<CopyRequestSwapPromise>(
+        std::move(request),
+        base::Bind(
+            [](const LayoutTestDependenciesImpl* self, int32_t routing_id) {
+              auto it = self->output_surfaces_.find(routing_id);
+              return it == self->output_surfaces_.end() ? nullptr : it->second;
+            },
+            this, routing_id));
+  }
+
+ private:
+  std::unordered_map<int32_t, base::WeakPtr<cc::TestDelegatingOutputSurface>>
+      output_surfaces_;
 };
 
 void EnableRendererLayoutTestMode() {