Hoist SetNeedsBeginFrame messages up to the RWHostImpl

content/browser/frame_host/render_widget_host_view_child_frame.cc

 // Copyright 2014 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/frame_host/render_widget_host_view_child_frame.h"
 
 #include <algorithm>
 #include <utility>
 #include <vector>
 
@@ skipping 30 matching lines @@
 
 RenderWidgetHostViewChildFrame::RenderWidgetHostViewChildFrame(
     RenderWidgetHost* widget_host)
     : host_(RenderWidgetHostImpl::From(widget_host)),
       next_surface_sequence_(1u),
       last_output_surface_id_(0),
       current_surface_scale_factor_(1.f),
       ack_pending_count_(0),
       frame_connector_(nullptr),
       begin_frame_source_(nullptr),
-      observing_begin_frame_source_(false),
       parent_surface_client_id_(0),
       weak_factory_(this) {
   id_allocator_.reset(new cc::SurfaceIdAllocator(AllocateSurfaceClientId()));
   GetSurfaceManager()->RegisterSurfaceClientId(id_allocator_->client_id());
   RegisterSurfaceNamespaceId();
 
   host_->SetView(this);
 }
 
 RenderWidgetHostViewChildFrame::~RenderWidgetHostViewChildFrame() {
@@ skipping 41 matching lines @@
           parent_surface_client_id_, GetSurfaceClientId());
     }
   }
 }
 
 void RenderWidgetHostViewChildFrame::InitAsChild(
     gfx::NativeView parent_view) {
   NOTREACHED();
 }
 
-bool RenderWidgetHostViewChildFrame::OnMessageReceived(
-    const IPC::Message& message) {
-  bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP(RenderWidgetHostViewChildFrame, message)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_SetNeedsBeginFrames, OnSetNeedsBeginFrames)
-    IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-  return handled;
-}
-
 RenderWidgetHost* RenderWidgetHostViewChildFrame::GetRenderWidgetHost() const {
   return host_;
 }
 
 void RenderWidgetHostViewChildFrame::SetSize(const gfx::Size& size) {
   host_->WasResized();
 }
 
 void RenderWidgetHostViewChildFrame::SetBounds(const gfx::Rect& rect) {
   SetSize(rect.size());
@@ skipping 502 matching lines @@
         resources));
     return;
   }
 
   std::copy(resources.begin(), resources.end(),
             std::back_inserter(surface_returned_resources_));
 }
 
 void RenderWidgetHostViewChildFrame::SetBeginFrameSource(
     cc::BeginFrameSource* source) {
-  if (begin_frame_source_ && observing_begin_frame_source_)
+  bool needs_begin_frames = host_->needs_begin_frames();
+  if (begin_frame_source_ && needs_begin_frames)
     begin_frame_source_->RemoveObserver(this);
   begin_frame_source_ = source;
-  if (begin_frame_source_ && observing_begin_frame_source_)
+  if (begin_frame_source_ && needs_begin_frames)
     begin_frame_source_->AddObserver(this);
 }
 
 void RenderWidgetHostViewChildFrame::OnBeginFrame(
     const cc::BeginFrameArgs& args) {
   host_->Send(new ViewMsg_BeginFrame(host_->GetRoutingID(), args));
   last_begin_frame_args_ = args;
 }
 
 const cc::BeginFrameArgs&
 RenderWidgetHostViewChildFrame::LastUsedBeginFrameArgs() const {
   return last_begin_frame_args_;
 }
 
 void RenderWidgetHostViewChildFrame::OnBeginFrameSourcePausedChanged(
     bool paused) {
   // Only used on Android WebView.
 }
 
-void RenderWidgetHostViewChildFrame::OnSetNeedsBeginFrames(
+void RenderWidgetHostViewChildFrame::SetNeedsBeginFrames(
     bool needs_begin_frames) {

[no sievers, 2016/08/22 18:13:56]

I don't quite understand when this codepath is ever used - i.e. when this is not
a RWHVGuest which overrides this (through the generic OnMsgReceived forwarding
to the |platform_view_| before your patch, and the SetNeedsBF() override after),
other than in tests maybe.

But your patch doesn't look like it would change the behavior. Might be worth
following up though if we maybe route the SetNeeds...() vs. actual sending of BF
through different views - it's definitely confusing that the logic exists here
and then SetNeedsBF() (and *only* that is overriden in RWHVGuest).

-  if (observing_begin_frame_source_ == needs_begin_frames)
+  if (!begin_frame_source_)
     return;
 
-  observing_begin_frame_source_ = needs_begin_frames;
-  if (begin_frame_source_) {
-    if (observing_begin_frame_source_)
-      begin_frame_source_->AddObserver(this);
-    else
-      begin_frame_source_->RemoveObserver(this);
-  }
+  if (needs_begin_frames)
+    begin_frame_source_->AddObserver(this);
+  else
+    begin_frame_source_->RemoveObserver(this);
 }
 
 InputEventAckState RenderWidgetHostViewChildFrame::FilterInputEvent(
     const blink::WebInputEvent& input_event) {
   if (input_event.type == blink::WebInputEvent::GestureFlingStart) {
     const blink::WebGestureEvent& gesture_event =
         static_cast<const blink::WebGestureEvent&>(input_event);
     // Zero-velocity touchpad flings are an Aura-specific signal that the
     // touchpad scroll has ended, and should not be forwarded to the renderer.
     if (gesture_event.sourceDevice == blink::WebGestureDeviceTouchpad &&
@@ skipping 29 matching lines @@
 
 bool RenderWidgetHostViewChildFrame::IsChildFrameForTesting() const {
   return true;
 }
 
 cc::SurfaceId RenderWidgetHostViewChildFrame::SurfaceIdForTesting() const {
   return surface_id_;
 };
 
 }  // namespace content