cc: Make separate interface for BeginFrame ipc from OutputSurface

content/browser/android/in_process/synchronous_compositor_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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"
 #include "content/common/input/did_overscroll_params.h"
 #include "content/public/browser/android/synchronous_compositor_client.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "ui/gl/gl_surface.h"
 
@@ skipping 38 matching lines @@
 }
 
 SynchronousCompositorImpl* SynchronousCompositorImpl::FromRoutingID(
     int routing_id) {
   return FromID(GetInProcessRendererId(), routing_id);
 }
 
 SynchronousCompositorImpl::SynchronousCompositorImpl(WebContents* contents)
     : compositor_client_(NULL),
       output_surface_(NULL),
+      begin_frame_source_(nullptr),
       contents_(contents),
       input_handler_(NULL),
+      invoking_composite_(false),
       weak_ptr_factory_(this) {
   DCHECK(contents);
 }
 
 SynchronousCompositorImpl::~SynchronousCompositorImpl() {
   if (compositor_client_)
     compositor_client_->DidDestroyCompositor(this);
+  if (begin_frame_source_)
+    begin_frame_source_->SetCompositor(nullptr);
   SetInputHandler(NULL);
 }
 
 void SynchronousCompositorImpl::SetClient(
     SynchronousCompositorClient* compositor_client) {
   DCHECK(CalledOnValidThread());
   compositor_client_ = compositor_client;
 }
 
 // static
 void SynchronousCompositor::SetGpuService(
     scoped_refptr<gpu::InProcessCommandBuffer::Service> service) {
   g_factory.Get().SetDeferredGpuService(service);
 }
 
 // static
 void SynchronousCompositor::SetRecordFullDocument(bool record_full_document) {
   g_factory.Get().SetRecordFullDocument(record_full_document);
 }
 
+void SynchronousCompositorImpl::SetExternalBeginFrameSource(
+    SynchronousCompositorExternalBeginFrameSource* begin_frame_source) {
+  if (begin_frame_source)
+    DCHECK(compositor_client_);

[boliu, 2014/10/30 16:21:32]

Why? I don't think this is guaranteed in general. Even if it is, it's a bad
assumption.

[simonhong, 2014/10/31 00:22:08]

Done.

+  begin_frame_source_ = begin_frame_source;

[boliu, 2014/10/30 16:21:33]

You are assuming here that SetExternalBeginFrameSource is called before
DidBindOutputSurface, which is also a bad assumption.

In general, don't assume (SetExternalBeginFrameSource, DidBindOutputSurface,
SetClient) are called in any particular order.

[simonhong, 2014/10/31 00:22:08]

Removed that assumption related code in DidBindOutputSurface.
(removed DCHECK(begin_frame_source_))

Btw, why that assumption is bad?
In current implementation, DidBindOutputSurface is always called after
SetClient().
Also, if SetClient() is not called first than DidBindOutputSurface(),
we can't generate next frame in current code base.

+}
+
 bool SynchronousCompositorImpl::InitializeHwDraw() {
   DCHECK(CalledOnValidThread());
   DCHECK(output_surface_);
 
   scoped_refptr<cc::ContextProvider> onscreen_context =
       g_factory.Get().CreateOnscreenContextProviderForCompositorThread();
 
   bool success = output_surface_->InitializeHwDraw(onscreen_context);
 
   if (success)
@@ skipping 10 matching lines @@
 
 scoped_ptr<cc::CompositorFrame> SynchronousCompositorImpl::DemandDrawHw(
     gfx::Size surface_size,
     const gfx::Transform& transform,
     gfx::Rect viewport,
     gfx::Rect clip,
     gfx::Rect viewport_rect_for_tile_priority,
     const gfx::Transform& transform_for_tile_priority) {
   DCHECK(CalledOnValidThread());
   DCHECK(output_surface_);
+  DCHECK(!invoking_composite_);
+  DCHECK(compositor_client_);
 
+  base::AutoReset<bool> invoking_composite_resetter(&invoking_composite_,
+                                                    true);
   scoped_ptr<cc::CompositorFrame> frame =
       output_surface_->DemandDrawHw(surface_size,
                                     transform,
                                     viewport,
                                     clip,
                                     viewport_rect_for_tile_priority,
                                     transform_for_tile_priority);
   if (frame.get())
     UpdateFrameMetaData(frame->metadata);
 
+  bool needs_begin_frames = false;
+  if (begin_frame_source_)

[boliu, 2014/10/30 16:21:32]

This should be a DCHECK like output_surface_.

compositor_client_ will only call DemandDrawHw/Sw between
DidInitializeCompositor and DidDestroyCompositor. So don't call
DidInitializeCompositor until both OS and BFS are ready, and call
DidDestroyCompositor as soon as one of them goes away.

Then you can make this a DCHECK.

[simonhong, 2014/10/31 00:22:08]

yep, right. It should be always not null.

+    needs_begin_frames = begin_frame_source_->NeedsBeginFrames();
+  compositor_client_->SetContinuousInvalidate(needs_begin_frames);
+
   return frame.Pass();
 }
 
 void SynchronousCompositorImpl::ReturnResources(
     const cc::CompositorFrameAck& frame_ack) {
   DCHECK(CalledOnValidThread());
   output_surface_->ReturnResources(frame_ack);
 }
 
 bool SynchronousCompositorImpl::DemandDrawSw(SkCanvas* canvas) {
   DCHECK(CalledOnValidThread());
   DCHECK(output_surface_);
+  DCHECK(!invoking_composite_);
+  DCHECK(compositor_client_);
 
-  scoped_ptr<cc::CompositorFrame> frame = output_surface_->DemandDrawSw(canvas);
+  base::AutoReset<bool> invoking_composite_resetter(&invoking_composite_,
+                                                    true);
+  scoped_ptr<cc::CompositorFrame> frame =
+      output_surface_->DemandDrawSw(canvas);
   if (frame.get())
     UpdateFrameMetaData(frame->metadata);
+
+  bool needs_begin_frames = false;
+  if (begin_frame_source_)
+    needs_begin_frames = begin_frame_source_->NeedsBeginFrames();
+  compositor_client_->SetContinuousInvalidate(needs_begin_frames);
+
   return !!frame.get();
 }
 
 void SynchronousCompositorImpl::UpdateFrameMetaData(
     const cc::CompositorFrameMetadata& frame_metadata) {
   RenderWidgetHostViewAndroid* rwhv = static_cast<RenderWidgetHostViewAndroid*>(
       contents_->GetRenderWidgetHostView());
   if (rwhv)
     rwhv->SynchronousFrameMetadata(frame_metadata);
   DeliverMessages();
 }
 
 void SynchronousCompositorImpl::SetMemoryPolicy(size_t bytes_limit) {
   DCHECK(CalledOnValidThread());
   DCHECK(output_surface_);
 
   output_surface_->SetMemoryPolicy(bytes_limit);
 }
 
 void SynchronousCompositorImpl::DidChangeRootLayerScrollOffset() {
   if (input_handler_)
     input_handler_->OnRootLayerDelegatedScrollOffsetChanged();
 }
 
 void SynchronousCompositorImpl::DidBindOutputSurface(
-      SynchronousCompositorOutputSurface* output_surface) {
+    SynchronousCompositorOutputSurface* output_surface) {
   DCHECK(CalledOnValidThread());
   output_surface_ = output_surface;
   if (compositor_client_)
     compositor_client_->DidInitializeCompositor(this);
+
+  DCHECK(begin_frame_source_);
+  output_surface_->SetBeginFrameSource(begin_frame_source_);
 }
 
 void SynchronousCompositorImpl::DidDestroySynchronousOutputSurface(
-       SynchronousCompositorOutputSurface* output_surface) {
+    SynchronousCompositorOutputSurface* output_surface) {
   DCHECK(CalledOnValidThread());
 
   // Allow for transient hand-over when two output surfaces may refer to
   // a single delegate.
   if (output_surface_ == output_surface) {
     output_surface_ = NULL;
     if (compositor_client_)
       compositor_client_->DidDestroyCompositor(this);
     compositor_client_ = NULL;
+
+    DCHECK(begin_frame_source_);
+    begin_frame_source_->SetCompositor(nullptr);
+    begin_frame_source_ = nullptr;
   }
 }
 
 void SynchronousCompositorImpl::SetInputHandler(
     cc::InputHandler* input_handler) {
   DCHECK(CalledOnValidThread());
 
   if (input_handler_)
     input_handler_->SetRootLayerScrollOffsetDelegate(NULL);
 
@@ skipping 12 matching lines @@
   }
 }
 
 void SynchronousCompositorImpl::DidStopFlinging() {
   RenderWidgetHostViewAndroid* rwhv = static_cast<RenderWidgetHostViewAndroid*>(
       contents_->GetRenderWidgetHostView());
   if (rwhv)
     rwhv->DidStopFlinging();
 }
 
-void SynchronousCompositorImpl::SetContinuousInvalidate(bool enable) {
+void SynchronousCompositorImpl::SetContinuousInvalidate(
+    bool needs_begin_frames) const {
   DCHECK(CalledOnValidThread());
+  if (invoking_composite_)
+    return;
+
   if (compositor_client_)
-    compositor_client_->SetContinuousInvalidate(enable);
+    compositor_client_->SetContinuousInvalidate(needs_begin_frames);
 }
 
 InputEventAckState SynchronousCompositorImpl::HandleInputEvent(
     const blink::WebInputEvent& input_event) {
   DCHECK(CalledOnValidThread());
   return g_factory.Get().synchronous_input_event_filter()->HandleInputEvent(
       contents_->GetRoutingID(), input_event);
 }
 
 void SynchronousCompositorImpl::DeliverMessages() {
@@ skipping 66 matching lines @@
     g_factory.Get();  // Ensure it's initialized.
     SynchronousCompositorImpl::CreateForWebContents(contents);
   }
   if (SynchronousCompositorImpl* instance =
       SynchronousCompositorImpl::FromWebContents(contents)) {
     instance->SetClient(client);
   }
 }
 
 }  // namespace content