Implement overscroll support for the virtual keyboard.

Implement overscroll support for the virtual keyboard.  Instead of resizing windows to prevent occlusion by the virtual keyboard, the inner viewport is resized to extend the vertical scroll range.

BUG=242933
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=266662

[kevers, 2014-03-25 17:20:34]

Hi Sadrul,

Can you please take a look at this CL.

[sadrul, 2014-03-26 17:19:18]

https://codereview.chromium.org/195793004/diff/90001/ui/aura/client/virtual_k...
File ui/aura/client/virtual_keyboard_client.h (right):

https://codereview.chromium.org/195793004/diff/90001/ui/aura/client/virtual_k...
ui/aura/client/virtual_keyboard_client.h:22: class AURA_EXPORT
VirtualKeyboardClient {
Why do we need to introduce this client, and add the observer to this? Why not
continue to use the KeyboardControllerObserver?

[kevers, 2014-03-26 17:43:46]

https://codereview.chromium.org/195793004/diff/90001/ui/aura/client/virtual_k...
File ui/aura/client/virtual_keyboard_client.h (right):

https://codereview.chromium.org/195793004/diff/90001/ui/aura/client/virtual_k...
ui/aura/client/virtual_keyboard_client.h:22: class AURA_EXPORT
VirtualKeyboardClient {
On 2014/03/26 17:19:18, sadrul wrote:
> Why do we need to introduce this client, and add the observer to this? Why not
> continue to use the KeyboardControllerObserver?

RWHVA implements several observers in aura::client.  The refactor is to improve
consistency with other observers. If RWHVA directly implements
KeyboardControllerObserver, we need to include the keyboard code on non-ChromeOS
builds.  Adding the client isolates the keyboard implementation, as the
GetVKClient call will simply return NULL on non-ChromeOS builds.

[sadrul, 2014-03-26 18:26:20]

https://codereview.chromium.org/195793004/diff/90001/ui/aura/client/virtual_k...
File ui/aura/client/virtual_keyboard_client.h (right):

https://codereview.chromium.org/195793004/diff/90001/ui/aura/client/virtual_k...
ui/aura/client/virtual_keyboard_client.h:22: class AURA_EXPORT
VirtualKeyboardClient {
On 2014/03/26 17:43:46, kevers wrote:
> On 2014/03/26 17:19:18, sadrul wrote:
> > Why do we need to introduce this client, and add the observer to this? Why
not
> > continue to use the KeyboardControllerObserver?
> 
> RWHVA implements several observers in aura::client.  The refactor is to
improve
> consistency with other observers. If RWHVA directly implements
> KeyboardControllerObserver, we need to include the keyboard code on
non-ChromeOS
> builds.  Adding the client isolates the keyboard implementation, as the
> GetVKClient call will simply return NULL on non-ChromeOS builds. 

I don't think adding a new aura-client interface is the solution for that. You
can make RWHVA a KCObserver conditionally on OS_CHROMEOS if including keyboard
code in non-chromeos is a concern. But all the v-keyboard code is built on all
aura platforms anyway. Why does this need to be different?

[kevers, 2014-04-09 20:06:27]

https://codereview.chromium.org/195793004/diff/90001/ui/aura/client/virtual_k...
File ui/aura/client/virtual_keyboard_client.h (right):

https://codereview.chromium.org/195793004/diff/90001/ui/aura/client/virtual_k...
ui/aura/client/virtual_keyboard_client.h:22: class AURA_EXPORT
VirtualKeyboardClient {
On 2014/03/26 18:26:21, sadrul wrote:
> On 2014/03/26 17:43:46, kevers wrote:
> > On 2014/03/26 17:19:18, sadrul wrote:
> > > Why do we need to introduce this client, and add the observer to this? Why
> not
> > > continue to use the KeyboardControllerObserver?
> > 
> > RWHVA implements several observers in aura::client.  The refactor is to
> improve
> > consistency with other observers. If RWHVA directly implements
> > KeyboardControllerObserver, we need to include the keyboard code on
> non-ChromeOS
> > builds.  Adding the client isolates the keyboard implementation, as the
> > GetVKClient call will simply return NULL on non-ChromeOS builds. 
> 
> I don't think adding a new aura-client interface is the solution for that. You
> can make RWHVA a KCObserver conditionally on OS_CHROMEOS if including keyboard
> code in non-chromeos is a concern. But all the v-keyboard code is built on all
> aura platforms anyway. Why does this need to be different?

Removed.  Now using singleton keyboard controller.

[sadrul, 2014-04-10 18:27:27]

You should add some unit-tests to verify that the renderer is receiving the
correct IPC messages.

https://codereview.chromium.org/195793004/diff/150001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/195793004/diff/150001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:3239: //
RenderWidgetHostViewAura, aura::client::VirtualKeyboardObserver
keyboard::KeyboardControllerObserver

https://codereview.chromium.org/195793004/diff/150001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.h (right):

https://codereview.chromium.org/195793004/diff/150001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.h:760: 
Remove

[kevers, 2014-04-14 18:47:45]

Added unit test.

https://codereview.chromium.org/195793004/diff/150001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/195793004/diff/150001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:3239: //
RenderWidgetHostViewAura, aura::client::VirtualKeyboardObserver
On 2014/04/10 18:27:27, sadrul wrote:
> keyboard::KeyboardControllerObserver

Done.

https://codereview.chromium.org/195793004/diff/150001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.h (right):

https://codereview.chromium.org/195793004/diff/150001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.h:760: 
On 2014/04/10 18:27:27, sadrul wrote:
> Remove

Done.

[sadrul, 2014-04-15 16:26:42]

Cool. lgtm

Please get a review from jam@ for the .gyp* and DEPS changes in content/

https://codereview.chromium.org/195793004/diff/170001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
(right):

https://codereview.chromium.org/195793004/diff/170001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura_unittest.cc:1357:
virtual void SetUp() {
OVERRIDE

[kevers, 2014-04-15 17:26:43]

+jam@ for content OWNERS
+oshima@ for ash OWNERS

https://codereview.chromium.org/195793004/diff/170001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
(right):

https://codereview.chromium.org/195793004/diff/170001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura_unittest.cc:1357:
virtual void SetUp() {
On 2014/04/15 16:26:43, sadrul wrote:
> OVERRIDE

Done.

[kevers, 2014-04-15 17:28:53]

Oops...forgot to update reviewers.

+jam@ for content OWNERS
+jamescook@ for ash OWNERS

[James Cook, 2014-04-15 17:54:10]

ash/* lgtm

https://codereview.chromium.org/195793004/diff/190001/ui/keyboard/keyboard_ut...
File ui/keyboard/keyboard_util.h (right):

https://codereview.chromium.org/195793004/diff/190001/ui/keyboard/keyboard_ut...
ui/keyboard/keyboard_util.h:67: KEYBOARD_EXPORT bool
IsKeyboardOverscrollEnabled();
Not for this CL but Ash uses functions with names like UseFeatureFoo() instead
of IsFeatureFooEnabled().  See ash_switches.h.  It would be nice to standardize
on one convention or the other.

[kevers, 2014-04-15 19:23:35]

Putting this CL on hold for a bit.  Turns out that keyboard depends on content,
which introduces a cyclic dependency. Investigating a strategy to remove the
dependency of content on ui/keyboard by moving the keyboard observer and
switches.

[jam, 2014-04-15 19:29:07]

sky is an owner in content/browser and ui/, so I defer to him as I'm not
familiar enough with ui/keyboard to know if this is an ok dependency or not

[kevers, 2014-04-22 23:43:15]

Cleaned up dependencies.  The keyboard code directly informs the views of a size
change.

jam and sky, can you please take a look.

[kevers, 2014-04-23 17:05:23]

CCing aelias for android sanity check.

[kevers, 2014-04-23 17:09:12]

https://codereview.chromium.org/195793004/diff/270001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_impl.cc (left):

https://codereview.chromium.org/195793004/diff/270001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_impl.cc:629:
params.overdraw_bottom_height = overdraw_bottom_height_;
Based on discussions with aelias, it appears that overdraw_bottom_height may be
phased out for Android.  Introducing a new parameter to avoid complicating this
cleanup.

[aelias_OOO_until_Jul13, 2014-04-23 19:41:57]

https://codereview.chromium.org/195793004/diff/270001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_impl.cc (left):

https://codereview.chromium.org/195793004/diff/270001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_impl.cc:629:
params.overdraw_bottom_height = overdraw_bottom_height_;
On 2014/04/23 17:09:13, kevers wrote:
> Based on discussions with aelias, it appears that overdraw_bottom_height may
be
> phased out for Android.  Introducing a new parameter to avoid complicating
this
> cleanup.

Hmm, it's ugly to introduce a new parameter for such a similar purpose, but I've
racked my brain for 15 minutes and I don't see a way to use the same parameter
for all platforms owing to the completely different viewport models Android and
ChromeOS have chosen.  overdraw_bottom_height is a delta from
physical_backing_size (and must remain so for complicated reasons relating to
top controls), while visible_viewport_size is an absolute size in the same space
as "size_".  As you say, Android will be able to eventually get rid of
overdraw_bottom_height after some top controls refactorings come through, so I
guess we can duplicate it for now.

But I suggest making it gfx::SizeF visible_viewport_size -- Size because it's
silly to only be able to change the height and not the width, and F because it's
in DIP pixels and therefore fractional values may be needed to represent odd
numbers of physical pixels at dpi scale 2.  (I know the Blink API is in ints so
you can floor it at that time, but let's maintain precision until then.)

[sadrul, 2014-04-23 19:50:21]

ui/keyboard/ slgtm

[sky, 2014-04-23 20:47:07]

https://codereview.chromium.org/195793004/diff/270001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/195793004/diff/270001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:888: void
RenderWidgetHostViewAura::SetKeyboardBounds(const gfx::Rect& bounds) {
Is there a reason you can't set an insets? I'm thinking of an API like:

SetInsets(const gfx::Insets& insets);

I also don't like that the coordinates of this function are screen. That makes
it super easy to get wrong.

[kevers, 2014-04-23 20:57:48]

https://codereview.chromium.org/195793004/diff/270001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/195793004/diff/270001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:888: void
RenderWidgetHostViewAura::SetKeyboardBounds(const gfx::Rect& bounds) {
On 2014/04/23 20:47:08, sky wrote:
> Is there a reason you can't set an insets? I'm thinking of an API like:
> 
> SetInsets(const gfx::Insets& insets);
> 
> I also don't like that the coordinates of this function are screen. That makes
> it super easy to get wrong.

The reason for explicitly using screen coordinates is that the keyboard is only
displayed on the primary root window.  SetInsets would work well if the clipping
applied across all displays.

[sky, 2014-04-23 21:15:30]

https://codereview.chromium.org/195793004/diff/270001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/195793004/diff/270001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:888: void
RenderWidgetHostViewAura::SetKeyboardBounds(const gfx::Rect& bounds) {
On 2014/04/23 20:57:49, kevers wrote:
> On 2014/04/23 20:47:08, sky wrote:
> > Is there a reason you can't set an insets? I'm thinking of an API like:
> > 
> > SetInsets(const gfx::Insets& insets);
> > 
> > I also don't like that the coordinates of this function are screen. That
makes
> > it super easy to get wrong.
> 
> The reason for explicitly using screen coordinates is that the keyboard is
only
> displayed on the primary root window.  SetInsets would work well if the
clipping
> applied across all displays.

Isn't that an implementation detail? Can't the keyboard figure out the insets
and set it?

[kevers, 2014-04-24 02:02:12]

https://codereview.chromium.org/195793004/diff/270001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_impl.cc (left):

https://codereview.chromium.org/195793004/diff/270001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_impl.cc:629:
params.overdraw_bottom_height = overdraw_bottom_height_;
On 2014/04/23 19:41:58, aelias wrote:
> On 2014/04/23 17:09:13, kevers wrote:
> > Based on discussions with aelias, it appears that overdraw_bottom_height may
> be
> > phased out for Android.  Introducing a new parameter to avoid complicating
> this
> > cleanup.
> 
> Hmm, it's ugly to introduce a new parameter for such a similar purpose, but
I've
> racked my brain for 15 minutes and I don't see a way to use the same parameter
> for all platforms owing to the completely different viewport models Android
and
> ChromeOS have chosen.  overdraw_bottom_height is a delta from
> physical_backing_size (and must remain so for complicated reasons relating to
> top controls), while visible_viewport_size is an absolute size in the same
space
> as "size_".  As you say, Android will be able to eventually get rid of
> overdraw_bottom_height after some top controls refactorings come through, so I
> guess we can duplicate it for now.
> 
> But I suggest making it gfx::SizeF visible_viewport_size -- Size because it's
> silly to only be able to change the height and not the width, and F because
it's
> in DIP pixels and therefore fractional values may be needed to represent odd
> numbers of physical pixels at dpi scale 2.  (I know the Blink API is in ints
so
> you can floor it at that time, but let's maintain precision until then.)

Done.

https://codereview.chromium.org/195793004/diff/270001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/195793004/diff/270001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:888: void
RenderWidgetHostViewAura::SetKeyboardBounds(const gfx::Rect& bounds) {
On 2014/04/23 21:15:31, sky wrote:
> On 2014/04/23 20:57:49, kevers wrote:
> > On 2014/04/23 20:47:08, sky wrote:
> > > Is there a reason you can't set an insets? I'm thinking of an API like:
> > > 
> > > SetInsets(const gfx::Insets& insets);
> > > 
> > > I also don't like that the coordinates of this function are screen. That
> makes
> > > it super easy to get wrong.
> > 
> > The reason for explicitly using screen coordinates is that the keyboard is
> only
> > displayed on the primary root window.  SetInsets would work well if the
> clipping
> > applied across all displays.
> 
> Isn't that an implementation detail? Can't the keyboard figure out the insets
> and set it?

Done.

[sky, 2014-04-24 15:55:07]

https://codereview.chromium.org/195793004/diff/290001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_impl.h (right):

https://codereview.chromium.org/195793004/diff/290001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_impl.h:774: gfx::SizeF
visible_viewport_size_;
Same comment about floating point.

https://codereview.chromium.org/195793004/diff/290001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/195793004/diff/290001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:887:
root_window_insets_.right(),
The insets should be relative to window_, not the root.

https://codereview.chromium.org/195793004/diff/290001/content/public/browser/...
File content/public/browser/render_widget_host_view.h (right):

https://codereview.chromium.org/195793004/diff/290001/content/public/browser/...
content/public/browser/render_widget_host_view.h:120: virtual gfx::SizeF
GetVisibleViewportSize() const = 0;
Why is this floating point when nothing else in this class is?

https://codereview.chromium.org/195793004/diff/290001/content/renderer/render...
File content/renderer/render_widget.cc (right):

https://codereview.chromium.org/195793004/diff/290001/content/renderer/render...
content/renderer/render_widget.cc:190: const gfx::SizeF& visible_viewport_size,
gfx::Rect resizer_rect,
nit: one param per line here, and align params with ( on previous line (see 183
for an example of what I'm requesting).

https://codereview.chromium.org/195793004/diff/290001/content/renderer/render...
File content/renderer/render_widget.h (right):

https://codereview.chromium.org/195793004/diff/290001/content/renderer/render...
content/renderer/render_widget.h:571: // The height of the visible viewport in
DPI-adjusted pixels.
height->size

[bokan, 2014-04-24 17:37:46]

https://codereview.chromium.org/195793004/diff/290001/content/renderer/render...
File content/renderer/render_widget.cc (right):

https://codereview.chromium.org/195793004/diff/290001/content/renderer/render...
content/renderer/render_widget.cc:669:
webwidget()->resizePinchViewport(gfx::Size(
This should actually go below the webwidget_->resize() call below (but outside
the 'size_ != new_size' if statement it's in). The reason being that if the
resize() call on the WebWidget isn't a noop, it will size the pinch viewport to
the widget, overriding the value you set here.

[kevers, 2014-04-24 18:38:32]

https://codereview.chromium.org/195793004/diff/270001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_impl.cc (left):

https://codereview.chromium.org/195793004/diff/270001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_impl.cc:629:
params.overdraw_bottom_height = overdraw_bottom_height_;
On 2014/04/24 02:02:13, kevers wrote:
> On 2014/04/23 19:41:58, aelias wrote:
> > On 2014/04/23 17:09:13, kevers wrote:
> > > Based on discussions with aelias, it appears that overdraw_bottom_height
may
> > be
> > > phased out for Android.  Introducing a new parameter to avoid complicating
> > this
> > > cleanup.
> > 
> > Hmm, it's ugly to introduce a new parameter for such a similar purpose, but
> I've
> > racked my brain for 15 minutes and I don't see a way to use the same
parameter
> > for all platforms owing to the completely different viewport models Android
> and
> > ChromeOS have chosen.  overdraw_bottom_height is a delta from
> > physical_backing_size (and must remain so for complicated reasons relating
to
> > top controls), while visible_viewport_size is an absolute size in the same
> space
> > as "size_".  As you say, Android will be able to eventually get rid of
> > overdraw_bottom_height after some top controls refactorings come through, so
I
> > guess we can duplicate it for now.
> > 
> > But I suggest making it gfx::SizeF visible_viewport_size -- Size because
it's
> > silly to only be able to change the height and not the width, and F because
> it's
> > in DIP pixels and therefore fractional values may be needed to represent odd
> > numbers of physical pixels at dpi scale 2.  (I know the Blink API is in ints
> so
> > you can floor it at that time, but let's maintain precision until then.)
> 
> Done.

Using Size instead of SizeF based on sky's feedback.

https://codereview.chromium.org/195793004/diff/290001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_impl.h (right):

https://codereview.chromium.org/195793004/diff/290001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_impl.h:774: gfx::SizeF
visible_viewport_size_;
On 2014/04/24 15:55:09, sky wrote:
> Same comment about floating point.

Done.

https://codereview.chromium.org/195793004/diff/290001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/195793004/diff/290001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:887:
root_window_insets_.right(),
On 2014/04/24 15:55:09, sky wrote:
> The insets should be relative to window_, not the root.

Done.  Added TODOs to keyboard controller to address issues that need to be
fixed if a window is moved while the keyboard is visible.

https://codereview.chromium.org/195793004/diff/290001/content/public/browser/...
File content/public/browser/render_widget_host_view.h (right):

https://codereview.chromium.org/195793004/diff/290001/content/public/browser/...
content/public/browser/render_widget_host_view.h:120: virtual gfx::SizeF
GetVisibleViewportSize() const = 0;
On 2014/04/24 15:55:09, sky wrote:
> Why is this floating point when nothing else in this class is?

Based on suggestion by aelias.  Android makes significant use of floating point
scale factors (e.g. overdraw_bottom_height).  Currently, we have rounding on
both the browser side and Blink. The suggestion to use float is based on the
direction Android seems to be moving. Switching  to integer for now as it can
always be changed to float later as needed.

https://codereview.chromium.org/195793004/diff/290001/content/renderer/render...
File content/renderer/render_widget.cc (right):

https://codereview.chromium.org/195793004/diff/290001/content/renderer/render...
content/renderer/render_widget.cc:190: const gfx::SizeF& visible_viewport_size,
gfx::Rect resizer_rect,
On 2014/04/24 15:55:09, sky wrote:
> nit: one param per line here, and align params with ( on previous line (see
183
> for an example of what I'm requesting).

Done.

https://codereview.chromium.org/195793004/diff/290001/content/renderer/render...
content/renderer/render_widget.cc:669:
webwidget()->resizePinchViewport(gfx::Size(
On 2014/04/24 17:37:47, bokan wrote:
> This should actually go below the webwidget_->resize() call below (but outside
> the 'size_ != new_size' if statement it's in). The reason being that if the
> resize() call on the WebWidget isn't a noop, it will size the pinch viewport
to
> the widget, overriding the value you set here. 

Done.

https://codereview.chromium.org/195793004/diff/290001/content/renderer/render...
File content/renderer/render_widget.h (right):

https://codereview.chromium.org/195793004/diff/290001/content/renderer/render...
content/renderer/render_widget.h:571: // The height of the visible viewport in
DPI-adjusted pixels.
On 2014/04/24 15:55:09, sky wrote:
> height->size

Done.

[sky, 2014-04-24 19:15:00]

https://codereview.chromium.org/195793004/diff/330001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

https://codereview.chromium.org/195793004/diff/330001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_impl.cc:604: gfx::SizeF
old_visible_viewport_size = visible_viewport_size_;
SizeF->Size

https://codereview.chromium.org/195793004/diff/330001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/195793004/diff/330001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:879: gfx::Rect
window_bounds = window_->GetBoundsInScreen();
GetBoundsInScreen()->bounds()

https://codereview.chromium.org/195793004/diff/330001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:880: int
viewport_width = window_bounds.width() - insets_.left() - insets_.right();
std::max(0, ...) for both of these lines?

[kevers, 2014-04-24 20:08:59]

https://codereview.chromium.org/195793004/diff/330001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

https://codereview.chromium.org/195793004/diff/330001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_impl.cc:604: gfx::SizeF
old_visible_viewport_size = visible_viewport_size_;
On 2014/04/24 19:15:01, sky wrote:
> SizeF->Size

Done.

https://codereview.chromium.org/195793004/diff/330001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/195793004/diff/330001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:879: gfx::Rect
window_bounds = window_->GetBoundsInScreen();
On 2014/04/24 19:15:01, sky wrote:
> GetBoundsInScreen()->bounds()

Done.

https://codereview.chromium.org/195793004/diff/330001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:880: int
viewport_width = window_bounds.width() - insets_.left() - insets_.right();
On 2014/04/24 19:15:01, sky wrote:
> std::max(0, ...) for both of these lines?

Done.

[sky, 2014-04-24 22:25:45]

LGTM

[jam, 2014-04-25 17:17:00]

content/common and content/public lgtm (sky reviewed content/browser)

[kevers, 2014-04-25 17:21:06]

The CQ bit was checked by kevers@chromium.org

[commit-bot: I haz the power, 2014-04-25 22:14:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kevers@chromium.org/195793004/350001

[commit-bot: I haz the power, 2014-04-25 23:26:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-25 23:26:15]

Try jobs failed on following builders:
  tryserver.chromium on chromium_presubmit

[kenrb, 2014-04-28 14:32:23]

lgtm for the IPC change

[kevers, 2014-04-28 15:10:49]

The CQ bit was checked by kevers@chromium.org

[commit-bot: I haz the power, 2014-04-28 15:11:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kevers@chromium.org/195793004/370001

[commit-bot: I haz the power, 2014-04-28 15:34:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-28 15:34:51]

Try jobs failed on following builders:
  tryserver.chromium on android_dbg

[kevers, 2014-04-28 18:53:40]

The CQ bit was checked by kevers@chromium.org

[commit-bot: I haz the power, 2014-04-28 18:54:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kevers@chromium.org/195793004/370001

[commit-bot: I haz the power, 2014-04-28 21:19:46]

Change committed as 266662