RenderWidgetCompositor should be decoupled from RenderWidget.

content/renderer/gpu/render_widget_compositor.cc

Index: content/renderer/gpu/render_widget_compositor.cc
diff --git a/content/renderer/gpu/render_widget_compositor.cc b/content/renderer/gpu/render_widget_compositor.cc
index 4048e3b6ffddd6a697ba655bed0f423f2a982f92..a809d30f2584ccd8c4f87bbd7318244aa81a72b9 100644
--- a/content/renderer/gpu/render_widget_compositor.cc
+++ b/content/renderer/gpu/render_widget_compositor.cc
@@ -45,6 +45,7 @@
 #include "content/common/content_switches_internal.h"
 #include "content/common/gpu/client/context_provider_command_buffer.h"
 #include "content/public/common/content_switches.h"
+#include "content/renderer/gpu/render_widget_compositor_delegate.h"
 #include "content/renderer/input/input_handler_manager.h"
 #include "gpu/command_buffer/client/gles2_interface.h"
 #include "gpu/command_buffer/service/gpu_switches.h"
@@ -54,7 +55,6 @@
 #include "third_party/WebKit/public/web/WebKit.h"
 #include "third_party/WebKit/public/web/WebRuntimeFeatures.h"
 #include "third_party/WebKit/public/web/WebSelection.h"
-#include "third_party/WebKit/public/web/WebWidget.h"
 #include "ui/gl/gl_switches.h"
 #include "ui/native_theme/native_theme_switches.h"
 
@@ -135,7 +135,7 @@ cc::LayerSelection ConvertWebSelection(const WebSelection& web_selection) {
   return cc_selection;
 }
 
-gfx::Size CalculateDefaultTileSize(RenderWidget* widget) {
+gfx::Size CalculateDefaultTileSize(float device_scale_factor) {
   int default_tile_size = 256;
 #if defined(OS_ANDROID)
   // TODO(epenner): unify this for all platforms if it
@@ -180,7 +180,7 @@ gfx::Size CalculateDefaultTileSize(RenderWidget* widget) {
   }
 #elif defined(OS_CHROMEOS)
   // Use 512 for high DPI (dsf=2.0f) devices.
-  if (widget->screen_info().deviceScaleFactor >= 2.0f)
+  if (device_scale_factor >= 2.0f)
     default_tile_size = 512;
 #endif
 
@@ -205,27 +205,27 @@ static cc::TopControlsState ConvertTopControlsState(
 
 // static
 scoped_ptr<RenderWidgetCompositor> RenderWidgetCompositor::Create(
-    RenderWidget* widget,
+    RenderWidgetCompositorDelegate* delegate,
+    float device_scale_factor,

[piman, 2016/01/13 22:09:30]

Minor nit, this makes it sound like the device_scale_factor is fixed in time,
whereas in truth it can change if a tab is moved to a different screen. I guess
we're only using the initial DSF for the tile size computation, maybe that one
should move to RenderWidget, and explicitly pass the default tile size here?
(dunno if mus would use different heuristics there)

[Fady Samuel, 2016/01/13 23:48:13]

Hmm, tile size seems like a compositor detail. I'd prefer not to put it in
RenderWidget. Perhaps I can add a GetDeviceScaleFactor() to the delegate?

[piman, 2016/01/14 00:13:23]

There's already RWC::setDeviceScaleFactor, I wouldn't want 3 ways of passing the
same info.
Maybe simply naming it initial_device_scale_factor to make it clear that it's
not set in stone?

     CompositorDependencies* compositor_deps) {
   scoped_ptr<RenderWidgetCompositor> compositor(
-      new RenderWidgetCompositor(widget, compositor_deps));
-  compositor->Initialize();
+      new RenderWidgetCompositor(delegate, compositor_deps));
+  compositor->Initialize(device_scale_factor);
   return compositor;
 }
 
 RenderWidgetCompositor::RenderWidgetCompositor(
-    RenderWidget* widget,
+    RenderWidgetCompositorDelegate* delegate,
     CompositorDependencies* compositor_deps)
     : num_failed_recreate_attempts_(0),
-      widget_(widget),
+      delegate_(delegate),
       compositor_deps_(compositor_deps),
       never_visible_(false),
       layout_and_paint_async_callback_(nullptr),
       remote_proto_channel_receiver_(nullptr),
-      weak_factory_(this) {
-}
+      weak_factory_(this) {}
 
-void RenderWidgetCompositor::Initialize() {
+void RenderWidgetCompositor::Initialize(float device_scale_factor) {
   base::CommandLine* cmd = base::CommandLine::ForCurrentProcess();
 
   cc::LayerTreeSettings settings;
@@ -257,7 +257,7 @@ void RenderWidgetCompositor::Initialize() {
   blink::WebRuntimeFeatures::enableCompositorAnimationTimelines(
       settings.use_compositor_animation_timelines);
 
-  settings.default_tile_size = CalculateDefaultTileSize(widget_);
+  settings.default_tile_size = CalculateDefaultTileSize(device_scale_factor);
   if (cmd->HasSwitch(switches::kDefaultTileWidth)) {
     int tile_width = 0;
     GetSwitchValueAsInt(*cmd,
@@ -456,7 +456,7 @@ void RenderWidgetCompositor::Initialize() {
   settings.renderer_settings.use_rgba_4444_textures &=
       !cmd->HasSwitch(switches::kDisableRGBA4444Textures);
 
-  if (widget_->for_oopif()) {
+  if (delegate_->ForOOPIF()) {
     // TODO(simonhong): Apply BeginFrame scheduling for OOPIF.
     // See crbug.com/471411.
     settings.use_external_begin_frame_source = false;
@@ -489,8 +489,7 @@ void RenderWidgetCompositor::Initialize() {
 
   scoped_ptr<cc::BeginFrameSource> external_begin_frame_source;
   if (settings.use_external_begin_frame_source) {
-    external_begin_frame_source =
-        compositor_deps_->CreateExternalBeginFrameSource(widget_->routing_id());
+    external_begin_frame_source = delegate_->CreateExternalBeginFrameSource();
   }
 
   cc::LayerTreeHost::InitParams params;
@@ -865,7 +864,7 @@ void RenderWidgetCompositor::setTopControlsShownRatio(float ratio) {
 }
 
 void RenderWidgetCompositor::WillBeginMainFrame() {
-  widget_->WillBeginCompositorFrame();
+  delegate_->WillBeginCompositorFrame();
 }
 
 void RenderWidgetCompositor::DidBeginMainFrame() {
@@ -874,7 +873,7 @@ void RenderWidgetCompositor::DidBeginMainFrame() {
 void RenderWidgetCompositor::BeginMainFrame(const cc::BeginFrameArgs& args) {
   compositor_deps_->GetRendererScheduler()->WillBeginFrame(args);
   double frame_time_sec = (args.frame_time - base::TimeTicks()).InSecondsF();
-  widget_->webwidget()->beginFrame(frame_time_sec);
+  delegate_->BeginMainFrame(frame_time_sec);
 }
 
 void RenderWidgetCompositor::BeginMainFrameNotExpectedSoon() {
@@ -882,8 +881,6 @@ void RenderWidgetCompositor::BeginMainFrameNotExpectedSoon() {
 }
 
 void RenderWidgetCompositor::UpdateLayerTreeHost() {
-  widget_->webwidget()->updateAllLifecyclePhases();

[piman, 2016/01/13 22:09:30]

So if I understand correctly, this moved from here to the end of BeginMainFrame.
Relative to the code in LayerTreeHost, it means it moves before AnimateLayers.
Not knowing the details of updateAllLifecyclePhases, I don't know if it's ok or
not. +danakj/chrishtr, do you know?

[danakj, 2016/01/13 22:24:33]

Take a look here, the time when things run matters:
https://code.google.com/p/chromium/codesearch#chromium/src/cc/trees/proxy_mai...

During the "animate step" requests for new frames will in fact cause another new
frame.

During the "update step" which is the UpdateLayerTreeHost() method here,
properties can be changed on layers without causing another frame to happen.

BeginMainFrame() happens before the animate and has properties more like the
animate step.

Also UpdateLayers does not always happen unless it's requested, so moving this
work out of this function changes it to run always instead of obeying the logic
in proxy_main there.

[Fady Samuel, 2016/01/13 23:48:13]

I don't want to change semantics in this CL so I've added a UpdateVisualState
call out.

-
   if (temporary_copy_output_request_) {
     // For WebViewImpl, this will always have a root layer.  For other widgets,
     // the widget may be closed before servicing this request, so ignore it.
@@ -903,24 +900,16 @@ void RenderWidgetCompositor::ApplyViewportDeltas(
     const gfx::Vector2dF& elastic_overscroll_delta,
     float page_scale,
     float top_controls_delta) {
-  widget_->webwidget()->applyViewportDeltas(
-      inner_delta,
-      outer_delta,
-      elastic_overscroll_delta,
-      page_scale,
-      top_controls_delta);
+  delegate_->ApplyViewportDeltas(inner_delta, outer_delta,
+                                 elastic_overscroll_delta, page_scale,
+                                 top_controls_delta);
 }
 
 void RenderWidgetCompositor::RequestNewOutputSurface() {
-  // If the host is closing, then no more compositing is possible.  This
-  // prevents shutdown races between handling the close message and
-  // the CreateOutputSurface task.
-  if (widget_->host_closing())
-    return;
-
   bool fallback =
       num_failed_recreate_attempts_ >= OUTPUT_SURFACE_RETRIES_BEFORE_FALLBACK;
-  scoped_ptr<cc::OutputSurface> surface(widget_->CreateOutputSurface(fallback));
+  scoped_ptr<cc::OutputSurface> surface(
+      delegate_->CreateOutputSurface(fallback));
 
   if (!surface) {
     DidFailToInitializeOutputSurface();
@@ -954,35 +943,35 @@ void RenderWidgetCompositor::WillCommit() {
 
 void RenderWidgetCompositor::DidCommit() {
   DCHECK(!temporary_copy_output_request_);
-  widget_->DidCommitCompositorFrame();
+  delegate_->DidCommitCompositorFrame();
   compositor_deps_->GetRendererScheduler()->DidCommitFrameToCompositor();
 }
 
 void RenderWidgetCompositor::DidCommitAndDrawFrame() {
-  widget_->DidCommitAndDrawCompositorFrame();
+  delegate_->DidCommitAndDrawCompositorFrame();
 }
 
 void RenderWidgetCompositor::DidCompleteSwapBuffers() {
-  widget_->DidCompleteSwapBuffers();
+  delegate_->DidCompleteSwapBuffers();
   bool threaded = !!compositor_deps_->GetCompositorImplThreadTaskRunner().get();
   if (!threaded)
-    widget_->OnSwapBuffersComplete();
+    delegate_->OnSwapBuffersComplete();
 }
 
 void RenderWidgetCompositor::DidCompletePageScaleAnimation() {
-  widget_->DidCompletePageScaleAnimation();
+  delegate_->DidCompletePageScaleAnimation();
 }
 
 void RenderWidgetCompositor::ScheduleAnimation() {
-  widget_->scheduleAnimation();
+  delegate_->ScheduleAnimation();
 }
 
 void RenderWidgetCompositor::DidPostSwapBuffers() {
-  widget_->OnSwapBuffersPosted();
+  delegate_->OnSwapBuffersPosted();
 }
 
 void RenderWidgetCompositor::DidAbortSwapBuffers() {
-  widget_->OnSwapBuffersAborted();
+  delegate_->OnSwapBuffersAborted();
 }
 
 void RenderWidgetCompositor::SetProtoReceiver(ProtoReceiver* receiver) {
@@ -995,39 +984,14 @@ void RenderWidgetCompositor::SendCompositorProto(
   size_t unsigned_size = base::checked_cast<size_t>(signed_size);
   std::vector<uint8_t> serialized(unsigned_size);
   proto.SerializeToArray(serialized.data(), signed_size);
-  widget_->ForwardCompositorProto(serialized);
+  delegate_->ForwardCompositorProto(serialized);
 }
 
 void RenderWidgetCompositor::RecordFrameTimingEvents(
     scoped_ptr<cc::FrameTimingTracker::CompositeTimingSet> composite_events,
     scoped_ptr<cc::FrameTimingTracker::MainFrameTimingSet> main_frame_events) {
-  for (const auto& composite_event : *composite_events ) {
-    int64_t frameId = composite_event.first;
-    const std::vector<cc::FrameTimingTracker::CompositeTimingEvent>& events =
-        composite_event.second;
-    std::vector<blink::WebFrameTimingEvent> webEvents;
-    for (size_t i = 0; i < events.size(); ++i) {
-      webEvents.push_back(blink::WebFrameTimingEvent(
-          events[i].frame_id,
-          (events[i].timestamp - base::TimeTicks()).InSecondsF()));
-    }
-    widget_->webwidget()->recordFrameTimingEvent(
-        blink::WebWidget::CompositeEvent, frameId, webEvents);
-  }
-  for (const auto& main_frame_event : *main_frame_events ) {
-    int64_t frameId = main_frame_event.first;
-    const std::vector<cc::FrameTimingTracker::MainFrameTimingEvent>& events =
-        main_frame_event.second;
-    std::vector<blink::WebFrameTimingEvent> webEvents;
-    for (size_t i = 0; i < events.size(); ++i) {
-      webEvents.push_back(blink::WebFrameTimingEvent(
-          events[i].frame_id,
-          (events[i].timestamp - base::TimeTicks()).InSecondsF(),
-          (events[i].end_time - base::TimeTicks()).InSecondsF()));
-    }
-    widget_->webwidget()->recordFrameTimingEvent(
-        blink::WebWidget::RenderEvent, frameId, webEvents);
-  }
+  delegate_->RecordFrameTimingEvents(std::move(composite_events),
+                                     std::move(main_frame_events));
 }
 
 void RenderWidgetCompositor::SetSurfaceIdNamespace(