cc: Make separate interface for BeginFrame ipc from OutputSurface

content/browser/android/in_process/synchronous_compositor_impl.cc

Index: content/browser/android/in_process/synchronous_compositor_impl.cc
diff --git a/content/browser/android/in_process/synchronous_compositor_impl.cc b/content/browser/android/in_process/synchronous_compositor_impl.cc
index 702164b5a8fcefc139ed4ea4b00d57e9dbec8071..420d3f8722decf02923e482cfdab45c1c8763dfb 100644
--- a/content/browser/android/in_process/synchronous_compositor_impl.cc
+++ b/content/browser/android/in_process/synchronous_compositor_impl.cc
@@ -4,9 +4,11 @@
 
 #include "content/browser/android/in_process/synchronous_compositor_impl.h"
 
+#include "base/auto_reset.h"
 #include "base/lazy_instance.h"
 #include "base/message_loop/message_loop.h"
 #include "cc/input/input_handler.h"
+#include "content/browser/android/in_process/synchronous_compositor_external_begin_frame_source.h"
 #include "content/browser/android/in_process/synchronous_compositor_factory_impl.h"
 #include "content/browser/android/in_process/synchronous_input_event_filter.h"
 #include "content/browser/renderer_host/render_widget_host_view_android.h"
@@ -65,8 +67,10 @@ SynchronousCompositorImpl* SynchronousCompositorImpl::FromRoutingID(
 SynchronousCompositorImpl::SynchronousCompositorImpl(WebContents* contents)
     : compositor_client_(NULL),
       output_surface_(NULL),
+      begin_frame_source_(NULL),
       contents_(contents),
       input_handler_(NULL),
+      invoking_composite_(false),
       weak_ptr_factory_(this) {
   DCHECK(contents);
 }
@@ -94,6 +98,11 @@ void SynchronousCompositor::SetRecordFullDocument(bool record_full_document) {
   g_factory.Get().SetRecordFullDocument(record_full_document);
 }
 
+void SynchronousCompositorImpl::SetExternalBeginFrameSource(
+    SynchronousCompositorExternalBeginFrameSource* begin_frame_source) {
+  begin_frame_source_ = begin_frame_source;

[boliu, 2014/10/24 17:42:40]

Need keep BeginFrameSource and compositor_client_ state in sync, ie call
compositor_client_->SetContinuousInvalidate(NeedsBeginFrames()); 

[simonhong, 2014/10/29 14:47:13]

DCHECK(compositor_client_) is added.
And, begin_frame_source_ is set to null when
DidDestroySynchronousOutputSurface() is called.

+}
+
 bool SynchronousCompositorImpl::InitializeHwDraw() {
   DCHECK(CalledOnValidThread());
   DCHECK(output_surface_);
@@ -124,6 +133,9 @@ scoped_ptr<cc::CompositorFrame> SynchronousCompositorImpl::DemandDrawHw(
     const gfx::Transform& transform_for_tile_priority) {
   DCHECK(CalledOnValidThread());
   DCHECK(output_surface_);
+  DCHECK(!invoking_composite_);
+  DCHECK(begin_frame_source_);

[boliu, 2014/10/24 17:42:40]

This DCHECK is not guaranteed.

Note we only call DidInitializeCompositor/DidDestroyCompositor while compositor
has an output surface.

Now this condition needs to be having an output surface *and* a begin frame
source.

[simonhong, 2014/10/29 14:47:13]

Removed.

+  base::AutoReset<bool> invoking_composite_resetter(&invoking_composite_, true);
 
   scoped_ptr<cc::CompositorFrame> frame =
       output_surface_->DemandDrawHw(surface_size,
@@ -138,6 +150,11 @@ scoped_ptr<cc::CompositorFrame> SynchronousCompositorImpl::DemandDrawHw(
   return frame.Pass();
 }
 
+void SynchronousCompositorImpl::RequestBeginFrame() {

[boliu, 2014/10/24 17:42:40]

Super confusing name, should be just BeginFrame. But this shouldn't exist at
all.

[simonhong, 2014/10/29 14:47:13]

This function is removed.

+  begin_frame_source_->BeginFrame();

[boliu, 2014/10/24 17:42:40]

DCHECK(begin_frame_source_)

+  SetContinuousInvalidate(begin_frame_source_->NeedsBeginFrames());

[boliu, 2014/10/24 17:42:40]

Why?

+}
+
 void SynchronousCompositorImpl::ReturnResources(
     const cc::CompositorFrameAck& frame_ack) {
   DCHECK(CalledOnValidThread());
@@ -147,10 +164,17 @@ void SynchronousCompositorImpl::ReturnResources(
 bool SynchronousCompositorImpl::DemandDrawSw(SkCanvas* canvas) {
   DCHECK(CalledOnValidThread());
   DCHECK(output_surface_);
+  DCHECK(!invoking_composite_);
+  DCHECK(begin_frame_source_);
+  base::AutoReset<bool> invoking_composite_resetter(&invoking_composite_, true);
 
   scoped_ptr<cc::CompositorFrame> frame = output_surface_->DemandDrawSw(canvas);
   if (frame.get())
     UpdateFrameMetaData(frame->metadata);
+
+  begin_frame_source_->BeginFrame();

[boliu, 2014/10/24 17:42:40]

Why?

[simonhong, 2014/10/29 14:47:13]

Removed.

+  SetContinuousInvalidate(begin_frame_source_->NeedsBeginFrames());
+
   return !!frame.get();
 }
 
@@ -227,10 +251,11 @@ void SynchronousCompositorImpl::DidStopFlinging() {
     rwhv->DidStopFlinging();
 }
 
-void SynchronousCompositorImpl::SetContinuousInvalidate(bool enable) {
+void SynchronousCompositorImpl::SetContinuousInvalidate(
+    bool needs_begin_frames) const {
   DCHECK(CalledOnValidThread());
   if (compositor_client_)
-    compositor_client_->SetContinuousInvalidate(enable);
+    compositor_client_->SetContinuousInvalidate(needs_begin_frames);

[boliu, 2014/10/24 17:42:40]

We want to hold off on this if invoking_composite_ is true.

[simonhong, 2014/10/29 14:47:13]

Made early out if invoking_composite_ is true.

 }
 
 InputEventAckState SynchronousCompositorImpl::HandleInputEvent(