Switching to CompositorFrameSinkSupport in android_webview::SurfacesInstance

android_webview/browser/surfaces_instance.cc

Index: android_webview/browser/surfaces_instance.cc
diff --git a/android_webview/browser/surfaces_instance.cc b/android_webview/browser/surfaces_instance.cc
index 7e3324f38de47148a2dd75bf8b96e9b6ef353f27..41366ed369fcab946f27d0a7169351b3b3ed97c8 100644
--- a/android_webview/browser/surfaces_instance.cc
+++ b/android_webview/browser/surfaces_instance.cc
@@ -16,6 +16,7 @@
 #include "cc/output/texture_mailbox_deleter.h"
 #include "cc/quads/surface_draw_quad.h"
 #include "cc/scheduler/begin_frame_source.h"
+#include "cc/surfaces/compositor_frame_sink_support.h"
 #include "cc/surfaces/display.h"
 #include "cc/surfaces/display_scheduler.h"
 #include "cc/surfaces/surface_factory.h"
@@ -52,10 +53,8 @@ SurfacesInstance::SurfacesInstance()
   surface_manager_.reset(new cc::SurfaceManager);
   surface_id_allocator_.reset(new cc::SurfaceIdAllocator());
   surface_manager_->RegisterFrameSinkId(frame_sink_id_);

[Fady Samuel, 2017/01/18 23:14:40]

I don't think this is necessary? CompositorFrameSinkSupport should take care of
it.

[Saman Sami, 2017/01/18 23:22:09]

Forgot to remove it.

-  surface_factory_.reset(
-      new cc::SurfaceFactory(frame_sink_id_, surface_manager_.get(), this));
 
-  begin_frame_source_.reset(new cc::StubBeginFrameSource);
+  auto begin_frame_source = base::MakeUnique<cc::StubBeginFrameSource>();
   std::unique_ptr<cc::TextureMailboxDeleter> texture_mailbox_deleter(
       new cc::TextureMailboxDeleter(nullptr));
   std::unique_ptr<ParentOutputSurface> output_surface_holder(
@@ -65,13 +64,16 @@ SurfacesInstance::SurfacesInstance()
   output_surface_ = output_surface_holder.get();
   std::unique_ptr<cc::DisplayScheduler> scheduler(new cc::DisplayScheduler(
       nullptr, output_surface_holder->capabilities().max_frames_pending));
-  display_.reset(new cc::Display(
+
+  auto display = base::MakeUnique<cc::Display>(
       nullptr /* shared_bitmap_manager */,
       nullptr /* gpu_memory_buffer_manager */, settings, frame_sink_id_,
-      begin_frame_source_.get(), std::move(output_surface_holder),
-      std::move(scheduler), std::move(texture_mailbox_deleter)));
-  display_->Initialize(this, surface_manager_.get());
-  display_->SetVisible(true);
+      begin_frame_source.get(), std::move(output_surface_holder),
+      std::move(scheduler), std::move(texture_mailbox_deleter));
+
+  support_.reset(new cc::CompositorFrameSinkSupport(
+      nullptr, surface_manager_.get(), frame_sink_id_, std::move(display),
+      std::move(begin_frame_source)));
 
   DCHECK(!g_surfaces_instance);
   g_surfaces_instance = this;
@@ -83,7 +85,7 @@ SurfacesInstance::~SurfacesInstance() {
   g_surfaces_instance = nullptr;
 
   DCHECK(child_ids_.empty());
-  surface_factory_->EvictSurface();
+  support_->EvictFrame();
 
   surface_manager_->InvalidateFrameSinkId(frame_sink_id_);

[Fady Samuel, 2017/01/18 23:14:40]

I don't think this is necessary, CompositorFrameSinkSupport destruction should
take care of it.

[Saman Sami, 2017/01/18 23:22:09]

Forgot to remove it.

 }
@@ -133,15 +135,11 @@ void SurfacesInstance::DrawAndSwap(const gfx::Size& viewport,
   frame.render_pass_list.push_back(std::move(render_pass));
   frame.metadata.referenced_surfaces = child_ids_;
 
-  if (!root_id_.is_valid()) {
+  if (!root_id_.is_valid())
     root_id_ = surface_id_allocator_->GenerateId();
-    display_->SetLocalFrameId(root_id_, 1.f);
-  }
-  surface_factory_->SubmitCompositorFrame(root_id_, std::move(frame),
-                                          cc::SurfaceFactory::DrawCallback());
 
-  display_->Resize(viewport);
-  display_->DrawAndSwap();

[Fady Samuel, 2017/01/18 23:14:40]

I guess this is more a question to boliu@. Why is this here? Shouldn't display
scheduler take care of this?

[boliu, 2017/01/19 00:24:13]

Basically android does the scheduling part for us, ie when it wants to draw, it
calls DrawGL, which calls this. This means everything, from receiving and
submitting a frame, all the way to command buffer swap frame needs to happen
within that call.

If you really want to, can probably massage DisplayScheduler and
BeginFrameSource to fit into this, but needs to be super careful. Eg I see it
takes a TaskRunner, which doesn't exist on this thread.

+  support_->SubmitCompositorFrame(root_id_, std::move(frame));
+  support_->display()->DrawAndSwap();
 }
 
 void SurfacesInstance::AddChildId(const cc::SurfaceId& child_id) {
@@ -163,8 +161,7 @@ void SurfacesInstance::RemoveChildId(const cc::SurfaceId& child_id) {
 void SurfacesInstance::SetEmptyRootFrame() {
   cc::CompositorFrame empty_frame;
   empty_frame.metadata.referenced_surfaces = child_ids_;
-  surface_factory_->SubmitCompositorFrame(root_id_, std::move(empty_frame),
-                                          cc::SurfaceFactory::DrawCallback());
+  support_->SubmitCompositorFrame(root_id_, std::move(empty_frame));
 }
 
 void SurfacesInstance::ReturnResources(