Clean up RenderWidgetHostView(ChildFrame and Guest) compositing code

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>
 
 #include "base/location.h"
 #include "base/memory/ptr_util.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "cc/output/copy_output_request.h"
 #include "cc/output/copy_output_result.h"
 #include "cc/surfaces/surface.h"
 #include "cc/surfaces/surface_factory.h"
 #include "cc/surfaces/surface_manager.h"
-#include "cc/surfaces/surface_sequence.h"
 #include "content/browser/accessibility/browser_accessibility_manager.h"
 #include "content/browser/browser_plugin/browser_plugin_guest.h"
 #include "content/browser/compositor/surface_utils.h"
 #include "content/browser/frame_host/cross_process_frame_connector.h"
 #include "content/browser/gpu/compositor_util.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/renderer_host/render_widget_host_delegate.h"
 #include "content/browser/renderer_host/render_widget_host_impl.h"
 #include "content/browser/renderer_host/render_widget_host_input_event_router.h"
 #include "content/common/text_input_state.h"
@@ skipping 335 matching lines @@
   DCHECK_GT(ack_pending_count_, 0U);
   if (host_) {
     host_->Send(new ViewMsg_ReclaimCompositorResources(
         host_->GetRoutingID(), compositor_frame_sink_id, true /* is_swap_ack */,
         surface_returned_resources_));
     surface_returned_resources_.clear();
   }
   ack_pending_count_--;
 }
 
-void RenderWidgetHostViewChildFrame::OnSwapCompositorFrame(
+bool RenderWidgetHostViewChildFrame::ShouldRecreateSurface(

[Fady Samuel, 2017/01/18 01:03:43]

I think ShouldCreateNewSurfaceId better captures what it's doing.

[Saman Sami, 2017/01/18 18:15:53]

Done.

+    uint32_t compositor_frame_sink_id,
+    const cc::CompositorFrame& frame) {
+  gfx::Size new_frame_size = frame.render_pass_list.back()->output_rect.size();
+  float new_scale_factor = frame.metadata.device_scale_factor;
+
+  return compositor_frame_sink_id != last_compositor_frame_sink_id_ ||
+         new_frame_size != current_surface_size_ ||
+         new_scale_factor != current_surface_scale_factor_;
+}
+
+void RenderWidgetHostViewChildFrame::ProcessCompositorFrame(
     uint32_t compositor_frame_sink_id,
     cc::CompositorFrame frame) {
-  TRACE_EVENT0("content",
-               "RenderWidgetHostViewChildFrame::OnSwapCompositorFrame");
-
-  last_scroll_offset_ = frame.metadata.root_scroll_offset;
-
-  if (!frame_connector_)
-    return;
-
-  cc::RenderPass* root_pass = frame.render_pass_list.back().get();
-
-  gfx::Size frame_size = root_pass->output_rect.size();
-  float scale_factor = frame.metadata.device_scale_factor;
-
-  // Check whether we need to recreate the cc::Surface, which means the child
-  // frame renderer has changed its frame sink, or size, or scale factor.
-  if (compositor_frame_sink_id != last_compositor_frame_sink_id_ ||
-      frame_size != current_surface_size_ ||
-      scale_factor != current_surface_scale_factor_) {
+  if (ShouldRecreateSurface(compositor_frame_sink_id, frame)) {
     ClearCompositorSurfaceIfNecessary();
     // If the renderer changed its frame sink, reset the surface factory to
     // avoid returning stale resources.
     if (compositor_frame_sink_id != last_compositor_frame_sink_id_)
       surface_factory_->Reset();
     last_compositor_frame_sink_id_ = compositor_frame_sink_id;
-    current_surface_size_ = frame_size;
-    current_surface_scale_factor_ = scale_factor;
+    current_surface_size_ = frame.render_pass_list.back()->output_rect.size();
+    current_surface_scale_factor_ = frame.metadata.device_scale_factor;
   }
 
   bool allocated_new_local_frame_id = false;
   if (!local_frame_id_.is_valid()) {
     local_frame_id_ = id_allocator_->GenerateId();
     allocated_new_local_frame_id = true;
   }
 
   cc::SurfaceFactory::DrawCallback ack_callback =
       base::Bind(&RenderWidgetHostViewChildFrame::SurfaceDrawn, AsWeakPtr(),
                  compositor_frame_sink_id);
   ack_pending_count_++;
   // If this value grows very large, something is going wrong.
   DCHECK_LT(ack_pending_count_, 1000U);
   surface_factory_->SubmitCompositorFrame(local_frame_id_, std::move(frame),
                                           ack_callback);
+  if (allocated_new_local_frame_id)
+    SendSurfaceInfoToChild();
+  ProcessFrameSwappedCallbacks();
+}
 
-  if (allocated_new_local_frame_id) {
-    cc::SurfaceSequence sequence =
-        cc::SurfaceSequence(frame_sink_id_, next_surface_sequence_++);
-    // The renderer process will satisfy this dependency when it creates a
-    // SurfaceLayer.
-    cc::SurfaceManager* manager = GetSurfaceManager();
-    manager->GetSurfaceForId(cc::SurfaceId(frame_sink_id_, local_frame_id_))
-        ->AddDestructionDependency(sequence);
-    frame_connector_->SetChildFrameSurface(
-        cc::SurfaceId(frame_sink_id_, local_frame_id_), frame_size,
-        scale_factor, sequence);
-  }
-  ProcessFrameSwappedCallbacks();
+void RenderWidgetHostViewChildFrame::SendSurfaceInfoToChild() {

[Fady Samuel, 2017/01/18 01:03:43]

This should be called SendSurfaceInfoToEmbedder

[Saman Sami, 2017/01/18 18:15:53]

Done.

+  cc::SurfaceSequence sequence =
+      cc::SurfaceSequence(frame_sink_id_, next_surface_sequence_++);
+  cc::SurfaceManager* manager = GetSurfaceManager();
+  cc::SurfaceId surface_id(frame_sink_id_, local_frame_id_);
+  // The renderer process will satisfy this dependency when it creates a
+  // SurfaceLayer.
+  manager->GetSurfaceForId(surface_id)->AddDestructionDependency(sequence);
+  cc::SurfaceInfo surface_info(surface_id, current_surface_scale_factor_,
+                               current_surface_size_);
+  SendSurfaceInfoToChild(surface_info, sequence);
+}
+
+void RenderWidgetHostViewChildFrame::SendSurfaceInfoToChild(

[Fady Samuel, 2017/01/18 01:03:43]

SendSurfaceInfoToEmbedder

[Saman Sami, 2017/01/18 18:15:53]

Done.

+    const cc::SurfaceInfo& surface_info,
+    const cc::SurfaceSequence& sequence) {
+  frame_connector_->SetChildFrameSurface(surface_info, sequence);
+}
+
+void RenderWidgetHostViewChildFrame::OnSwapCompositorFrame(
+    uint32_t compositor_frame_sink_id,
+    cc::CompositorFrame frame) {
+  TRACE_EVENT0("content",
+               "RenderWidgetHostViewChildFrame::OnSwapCompositorFrame");
+  last_scroll_offset_ = frame.metadata.root_scroll_offset;
+  if (!frame_connector_)
+    return;
+  ProcessCompositorFrame(compositor_frame_sink_id, std::move(frame));
 }
 
 void RenderWidgetHostViewChildFrame::ProcessFrameSwappedCallbacks() {
   // We only use callbacks once, therefore we make a new list for registration
   // before we start, and discard the old list entries when we are done.
   FrameSwappedCallbackList process_callbacks;
   process_callbacks.swap(frame_swapped_callbacks_);
   for (std::unique_ptr<base::Closure>& callback : process_callbacks)
     callback->Run();
 }
@@ skipping 298 matching lines @@
 
 bool RenderWidgetHostViewChildFrame::IsChildFrameForTesting() const {
   return true;
 }
 
 cc::SurfaceId RenderWidgetHostViewChildFrame::SurfaceIdForTesting() const {
   return cc::SurfaceId(frame_sink_id_, local_frame_id_);
 };
 
 }  // namespace content