Made Blink aware of top controls offset (Chromium-side)

content/renderer/render_widget.cc

Index: content/renderer/render_widget.cc
diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc
index 3d693b95f9230d20494738dc0faa56e4d1d3af9e..1d23854e9b2a20ac16ba4e73590753c292106543 100644
--- a/content/renderer/render_widget.cc
+++ b/content/renderer/render_widget.cc
@@ -665,7 +665,7 @@ void RenderWidget::Resize(const gfx::Size& new_size,
 
   if (compositor_) {
     compositor_->setViewportSize(new_size, physical_backing_size);
-    compositor_->SetTopControlsLayoutHeight(top_controls_layout_height);
+    compositor_->SetTopControlsContentOffset(top_controls_layout_height);

[aelias_OOO_until_Jul13, 2014/08/29 04:37:01]

This isn't quite what I had in mind and it's impossible to build main-thread top
controls manipulation on top of this data model.  Blink should be pushing this
value to the compositor, not RenderWidget.  I think switching to that model will
also unblock pushing a delta instead of an absolute value in ScrollAndScaleSet.

My proposed change will raise the question of how does Blink know what direction
to resize the outer viewport in.  That's where the browser's
top_controls_layout_height remains relevant: it can be passed into Blink instead
of into CC.  I propose maintaining the current name "top_controls_layout_height"
for this particular purpose.  So in summary I propose:

- Blink receives top_controls_layout_height value from RenderWidget
- Blink receives top_controls_content_offset_delta from CC
- CC receives top_controls_content_offset from Blink
(also needed for main-thread-manipulation in future: Blink receives top control
size from RenderWidget so it can clamp properly)

In addition I think this means the sketchy FrameView resize you're currently
doing in the Blink-side patch becomes unnecessary since CC receives sufficient
outer viewport size delta information via top_controls_content_offset instead. 
(That is, Blink will *know* about top controls position increments but it won't
*do* anything with them aside from informing CC.)

[bokan, 2014/08/29 18:02:14]

Ack...I think I introduced some confusion here by accidentally abusing the
compositor interface. This method is part of RenderWidgetCompositor's
LayerTreeHostClient interface so this is setting the offset in *Blink*. This
should actually be webwidget_->setTopControlsContentOffset(), which
(confusingly) gets called through compositor_->SetTopControlsContentOffset(). I
believe this is currently doing what you describe below but masquerading as
updating the compositor. That's a mistake on my part, I'll fix this to be clear.
Maybe I'm missing something, but I think this still has the problem I described
in my earlier e-mail. There's a race between the Browser and the Compositor as
to who updates Blink first. The compositor sends its total TC offset (including
delta) to the Browser but the delta it would send to Blink is the delta from the
last committed offset. If the Browser decides to resize Blink before the
compositor commits then the delta would be applied twice since it's already
included in the Browser's offset.
Yup, will add.
Could you elaborate on how this would work? The problem I see is that the
compositor can't apply the top_controls_content_offset against the viewport
container since the viewport bounds may or may not already account for it. In
the case of a RenderWidget::Resize, the viewport container's size is set so that
no adjustment is needed. When a compositor commit happens, only the TC offset is
changed so the viewport's bounds need an adjustment. The way I currently get
around this is making the viewport adjustment in Blink (it could also be done in
LayerTreeHost) and during RenderWidget::Resize the TopControls get resized prior
to the widget resize so it will clobber the viewport adjustment.

   }
 
   physical_backing_size_ = physical_backing_size;