Use a cc::Display for WebView resourceless software draws.

content/renderer/android/synchronous_compositor_output_surface.cc

Index: content/renderer/android/synchronous_compositor_output_surface.cc
diff --git a/content/renderer/android/synchronous_compositor_output_surface.cc b/content/renderer/android/synchronous_compositor_output_surface.cc
index 37d3a23e90fc15f328e5346443836f00898cf0f9..40ee7aa9a36d6f4e6d0ffc41a2ae2b39f6f8cddc 100644
--- a/content/renderer/android/synchronous_compositor_output_surface.cc
+++ b/content/renderer/android/synchronous_compositor_output_surface.cc
@@ -16,7 +16,12 @@
 #include "cc/output/compositor_frame.h"
 #include "cc/output/context_provider.h"
 #include "cc/output/output_surface_client.h"
+#include "cc/output/renderer_settings.h"
 #include "cc/output/software_output_device.h"
+#include "cc/output/texture_mailbox_deleter.h"

[boliu, 2016/07/07 00:37:54]

is this needed?

[danakj, 2016/07/07 00:46:30]

Ya, it's written as nullptr, but it fails to compile otherwise cuz it's making a
unique_ptr<TMD>

+#include "cc/surfaces/surface_factory.h"
+#include "cc/surfaces/surface_id_allocator.h"
+#include "cc/surfaces/surface_manager.h"
 #include "content/common/android/sync_compositor_messages.h"
 #include "content/renderer/android/synchronous_compositor_filter.h"
 #include "content/renderer/android/synchronous_compositor_registry.h"
@@ -38,61 +43,64 @@ namespace content {
 namespace {
 
 const int64_t kFallbackTickTimeoutInMilliseconds = 100;
+const uint32_t kCompositorSurfaceNamespace = 1;
 
 // Do not limit number of resources, so use an unrealistically high value.
 const size_t kNumResourcesLimit = 10 * 1000 * 1000;
 
 }  // namespace
 
-class SynchronousCompositorOutputSurface::SoftwareDevice
-  : public cc::SoftwareOutputDevice {
+class SoftwareDevice : public cc::SoftwareOutputDevice {
  public:
-  SoftwareDevice(SynchronousCompositorOutputSurface* surface)
-    : surface_(surface) {
-  }
+  SoftwareDevice(SkCanvas** canvas) : canvas_(canvas) {}
+
   void Resize(const gfx::Size& pixel_size, float scale_factor) override {
     // Intentional no-op: canvas size is controlled by the embedder.
   }
   SkCanvas* BeginPaint(const gfx::Rect& damage_rect) override {
-    if (!surface_->current_sw_canvas_) {
-      NOTREACHED() << "BeginPaint with no canvas set";
-      return &null_canvas_;
-    }
-    LOG_IF(WARNING, surface_->did_swap_)
-        << "Mutliple calls to BeginPaint per frame";
-    return surface_->current_sw_canvas_;
+    DCHECK(*canvas_) << "BeginPaint with no canvas set";
+    return *canvas_;
   }
   void EndPaint() override {}
 
  private:
-  SynchronousCompositorOutputSurface* surface_;
-  SkCanvas null_canvas_;
+  SkCanvas** canvas_;
 
   DISALLOW_COPY_AND_ASSIGN(SoftwareDevice);
 };
 
+class SoftwareOutputSurface : public cc::OutputSurface {
+ public:
+  SoftwareOutputSurface(std::unique_ptr<SoftwareDevice> software_device)
+      : cc::OutputSurface(nullptr, nullptr, std::move(software_device)) {}
+
+  // cc::OutputSurface implementation.
+  uint32_t GetFramebufferCopyTextureFormat() override { return 0; }
+  void SwapBuffers(cc::CompositorFrame frame) override {}
+};
+
 SynchronousCompositorOutputSurface::SynchronousCompositorOutputSurface(
     scoped_refptr<cc::ContextProvider> context_provider,
     scoped_refptr<cc::ContextProvider> worker_context_provider,
+    cc::SharedBitmapManager* shared_bitmap_manager,
     int routing_id,
     uint32_t output_surface_id,
     SynchronousCompositorRegistry* registry,
     scoped_refptr<FrameSwapMessageQueue> frame_swap_message_queue)
     : cc::OutputSurface(std::move(context_provider),
                         std::move(worker_context_provider),
-                        base::MakeUnique<SoftwareDevice>(this)),
+                        nullptr),
+      shared_bitmap_manager_(shared_bitmap_manager),
       routing_id_(routing_id),
       output_surface_id_(output_surface_id),
       registry_(registry),
       sender_(RenderThreadImpl::current()->sync_compositor_message_filter()),
-      registered_(false),
-      sync_client_(nullptr),
-      current_sw_canvas_(nullptr),
       memory_policy_(0u),
-      did_swap_(false),
       frame_swap_message_queue_(frame_swap_message_queue),
-      fallback_tick_pending_(false),
-      fallback_tick_running_(false) {
+      surface_manager_(new cc::SurfaceManager),
+      surface_id_allocator_(
+          new cc::SurfaceIdAllocator(kCompositorSurfaceNamespace)),
+      surface_factory_(new cc::SurfaceFactory(surface_manager_.get(), this)) {
   DCHECK(registry_);
   DCHECK(sender_);
   thread_checker_.DetachFromThread();
@@ -100,6 +108,7 @@ SynchronousCompositorOutputSurface::SynchronousCompositorOutputSurface(
   capabilities_.delegated_rendering = true;
   memory_policy_.priority_cutoff_when_visible =
       gpu::MemoryAllocation::CUTOFF_ALLOW_NICE_TO_HAVE;
+  surface_id_allocator_->RegisterSurfaceIdNamespace(surface_manager_.get());
 }
 
 SynchronousCompositorOutputSurface::~SynchronousCompositorOutputSurface() {}
@@ -135,6 +144,20 @@ bool SynchronousCompositorOutputSurface::BindToClient(
                  base::Unretained(this)));
   registry_->RegisterOutputSurface(routing_id_, this);
   registered_ = true;
+
+  surface_manager_->RegisterSurfaceFactoryClient(
+      surface_id_allocator_->id_namespace(), this);
+
+  cc::RendererSettings software_renderer_settings;
+
+  display_.reset(new cc::Display(
+      surface_manager_.get(), shared_bitmap_manager_,
+      nullptr /* gpu_memory_buffer_manager */, software_renderer_settings,
+      surface_id_allocator_->id_namespace(), nullptr /* begin_frame_source */,
+      base::MakeUnique<SoftwareOutputSurface>(
+          base::MakeUnique<SoftwareDevice>(&current_sw_canvas_)),

[boliu, 2016/07/07 00:37:54]

just confirming c++11 knowledge :p MakeUnique is doing the perfect forwarding
thingy in c++11?

[danakj, 2016/07/07 00:46:30]

It is a function call, so it returns an rvalue, if that is what you're asking.
(Since the SoftwareOutputSurface constructor is taking a unique_ptr by value,
and it's move-only, it wouldn't compile otherwise :))

+      nullptr /* scheduler */, nullptr /* texture_mailbox_deleter */));
+  display_->Initialize(&display_client_);

[boliu, 2016/07/07 00:37:54]

GLRenderer used to be created on the stack

if this stuff lives for a long time, can the Display hold onto software
resources for longer than before? I assume shared_bitmap_manager_ is the thing
sitting in between compositor and display that maybe can hold onto software
resources?

Also I assume these objects by themsleves don't use that much memory, right?

Basically should always optimize for the case of not having any software draws
of any kind

[danakj, 2016/07/07 00:46:30]

SoftwareRenderer::DrawPictureQuad doesn't store anything onto the
SoftwareRenderer, so I don't think deleting the renderer is going to free
anything.

The SharedBitmapManager would actually only be used if the compositor was
sending quads with bitmap resources in them. It's not.. so I could maybe make it
null, if you prefer. But SoftwareRenderer doesn't generate shared bitmaps, it
consumes them in SoftwareRenderer::DrawTextureQuad and DrawTileQuad.

The one exception to all that is non-root render surfaces, which we don't have
in resourceless mode.

[boliu, 2016/07/07 00:58:01]

Yes please.

[danakj, 2016/07/07 20:34:07]

OK I think that worked. Trying it @ bots now. PTAL

   return true;
 }
 
@@ -144,6 +167,14 @@ void SynchronousCompositorOutputSurface::DetachFromClient() {
     registry_->UnregisterOutputSurface(routing_id_, this);
   }
   client_->SetTreeActivationCallback(base::Closure());
+  if (!delegated_surface_id_.is_null())
+    surface_factory_->Destroy(delegated_surface_id_);
+  surface_manager_->UnregisterSurfaceFactoryClient(
+      surface_id_allocator_->id_namespace());
+  display_ = nullptr;
+  surface_factory_ = nullptr;
+  surface_id_allocator_ = nullptr;
+  surface_manager_ = nullptr;
   cc::OutputSurface::DetachFromClient();
   CancelFallbackTick();
 }
@@ -156,12 +187,41 @@ void SynchronousCompositorOutputSurface::Reshape(
   // Intentional no-op: surface size is controlled by the embedder.
 }
 
+static void NoOpDrawCallback(cc::SurfaceDrawStatus s) {}
+
 void SynchronousCompositorOutputSurface::SwapBuffers(
     cc::CompositorFrame frame) {
   DCHECK(CalledOnValidThread());
   DCHECK(sync_client_);
+
+  cc::CompositorFrame swap_frame;
+
+  if (in_software_draw_) {

[boliu, 2016/07/07 00:37:54]

should be ok just skip this and return the resources if it's fallback tick? no
worse than before though I guess

[danakj, 2016/07/07 00:46:30]

Don't produce a frame in fallback tick? So before we would have drawn
regardless, right? I think this is the same behaviour, or can you point out why
not?

[boliu, 2016/07/07 00:58:01]

This is the same behavior. I'm just pointing out a potential optimization this
CL enables. Can totally do it in a follow up.

[danakj, 2016/07/07 20:34:07]

OK ya let me send you a patch and we can see if I understand.

+    // The frame we send to the client is actually just the metadata. Preserve
+    // the |frame| for the software path below.
+    swap_frame.metadata = frame.metadata.Clone();
+
+    if (delegated_surface_id_.is_null()) {
+      delegated_surface_id_ = surface_id_allocator_->GenerateId();
+      surface_factory_->Create(delegated_surface_id_);
+    }
+
+    display_->SetSurfaceId(delegated_surface_id_,
+                           frame.metadata.device_scale_factor);
+
+    gfx::Size frame_size =
+        frame.delegated_frame_data->render_pass_list.back()->output_rect.size();
+    display_->Resize(frame_size);
+
+    surface_factory_->SubmitCompositorFrame(
+        delegated_surface_id_, std::move(frame), base::Bind(&NoOpDrawCallback));
+    display_->DrawAndSwap();
+  } else {
+    swap_frame = std::move(frame);
+  }
+
   if (!fallback_tick_running_) {
-    sync_client_->SwapBuffers(output_surface_id_, std::move(frame));
+    sync_client_->SwapBuffers(output_surface_id_, std::move(swap_frame));
     DeliverMessages();
   }
   client_->DidSwapBuffers();
@@ -228,8 +288,7 @@ void SynchronousCompositorOutputSurface::DemandDrawHw(
   surface_size_ = surface_size;
   client_->SetExternalTilePriorityConstraints(viewport_rect_for_tile_priority,
                                               transform_for_tile_priority);
-  const bool software_draw = false;
-  InvokeComposite(transform, viewport, clip, software_draw);
+  InvokeComposite(transform, viewport, clip);
 }
 
 void SynchronousCompositorOutputSurface::DemandDrawSw(SkCanvas* canvas) {
@@ -249,19 +308,18 @@ void SynchronousCompositorOutputSurface::DemandDrawSw(SkCanvas* canvas) {
 
   surface_size_ = gfx::Size(canvas->getBaseLayerSize().width(),
                             canvas->getBaseLayerSize().height());
-  const bool software_draw = true;
-  InvokeComposite(transform, clip, clip, software_draw);
+  base::AutoReset<bool> set_in_software_draw(&in_software_draw_, true);
+  InvokeComposite(transform, clip, clip);
 }
 
 void SynchronousCompositorOutputSurface::InvokeComposite(
     const gfx::Transform& transform,
     const gfx::Rect& viewport,
-    const gfx::Rect& clip,
-    bool software_draw) {
+    const gfx::Rect& clip) {
   gfx::Transform adjusted_transform = transform;
   adjusted_transform.matrix().postTranslate(-viewport.x(), -viewport.y(), 0);
   did_swap_ = false;
-  client_->OnDraw(adjusted_transform, viewport, clip, software_draw);
+  client_->OnDraw(adjusted_transform, viewport, clip, in_software_draw_);
 
   if (did_swap_)
     client_->DidSwapBuffersComplete();
@@ -325,4 +383,17 @@ bool SynchronousCompositorOutputSurface::CalledOnValidThread() const {
   return thread_checker_.CalledOnValidThread();
 }
 
+void SynchronousCompositorOutputSurface::ReturnResources(
+    const cc::ReturnedResourceArray& resources) {
+  cc::CompositorFrameAck ack;
+  ack.resources = resources;
+  client_->ReclaimResources(&ack);
+}
+
+void SynchronousCompositorOutputSurface::SetBeginFrameSource(
+    cc::BeginFrameSource* begin_frame_source) {
+  // Software output is synchronous and doesn't use a BeginFrameSource.
+  NOTREACHED();
+}
+
 }  // namespace content