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 f7dbe2b31c3237117be28a0e2f4da2f9201aa01f..50d592a52a7e1390c5f7e20b1a946f1af39f84f8 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"
@@ -179,9 +180,47 @@ 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 FindOutputSurfaceCallback =
+      base::Callback<cc::TestDelegatingOutputSurface*()>;
+  CopyRequestSwapPromise(std::unique_ptr<cc::CopyOutputRequest> request,
+                         FindOutputSurfaceCallback output_surface_callback)
+      : copy_request_(std::move(request)),
+        output_surface_callback_(std::move(output_surface_callback)) {}
+
+  // cc::SwapPromise implementation.
+  void OnCommit() override {
+    output_surface_from_commit_ = output_surface_callback_.Run();
+    DCHECK(output_surface_from_commit_);
+  }
+  void DidActivate() override {}
+  void DidSwap(cc::CompositorFrameMetadata*) override {
+    output_surface_from_commit_->RequestCopyOfOutput(std::move(copy_request_));
+  }
+  void DidNotSwap(DidNotSwapReason r) override {
+    // The compositor should always swap in layout test mode.
+    NOTREACHED() << "did not swap for reason " << r;
+  }
+  int64_t TraceId() const override { return 0; }
+
+ private:
+  std::unique_ptr<cc::CopyOutputRequest> copy_request_;
+  FindOutputSurfaceCallback output_surface_callback_;
+  cc::TestDelegatingOutputSurface* output_surface_from_commit_ = nullptr;
+};
+
+}  // 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,
@@ -221,12 +260,38 @@ 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.get();

[piman, 2016/07/26 06:56:32]

Where is the entry removed? Also, what guarantees the entry outlives the output
surface (otherwise we have a stale pointer)?

[jbroman, 2016/07/26 13:28:42]

It isn't, though this object is only used in layout tests, so while it can grow
unbounded over a single content_shell runner's lifetime, it cannot occur in
production.
The entry has the lifetime of the LayoutTestDependenciesImpl, which in turn has
the lifetime of the RenderThreadImpl (which should outlive all renderer output
surfaces, IIUC).

I'd originally used WeakPtr here and intended to have the surfaces remove
themselves from the map on destruction, but danakj suggested that was overkill
for layout test support code. I'm happy either way.

[piman, 2016/07/26 20:34:36]

Ok, can you leave a comment though? I wouldn't want something to start using an
arbitrary entry, and then cause UAF and other hard to track issues.

+    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 find the
+    // currently active OutputSurface for the given RenderWidget routing_id.
+    return base::MakeUnique<CopyRequestSwapPromise>(
+        std::move(request),
+        base::Bind(
+            &LayoutTestDependenciesImpl::FindOutputSurface,
+            // |this| will still be valid, because its lifetime is tied to
+            // RenderThreadImpl, which outlives layout test execution.
+            base::Unretained(this), routing_id));
+  }
+
+ private:
+  cc::TestDelegatingOutputSurface* FindOutputSurface(int32_t routing_id) {
+    auto it = output_surfaces_.find(routing_id);
+    return it == output_surfaces_.end() ? nullptr : it->second;
   }
+
+  std::unordered_map<int32_t, cc::TestDelegatingOutputSurface*>
+      output_surfaces_;
 };
 
 void EnableRendererLayoutTestMode() {