Clear the 'has_scrolled_node_into_rect' flag on viewport resize.

Clear the 'has_scrolled_node_into_rect' flag on viewport resize.

This flag prevents redundant auto zoom commands when scrolling an
editable node into view. It had the side effect of making it so that
a keyboard being dismissed using the dismiss button (without losing
focus on the current node) wouldn't scroll the node back into view
if it was tapped on again. This patch clears the flag when the
visible viewport size changes, as happens when the keyboard is shown
or hidden.

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

[bokan, 2014-05-23 17:05:29]

Jared, is it ok to make this Android-only for the time being?

[jdduke (slow), 2014-05-23 17:11:36]

Hmm, if it feels buggy on Aura, it probably feels buggy on Android.  Is there a
hook we could use in render_view_impl to reset the flag if the user dismisses
the keyboard?

[bokan, 2014-05-23 18:11:57]

On 2014/05/23 17:11:36, jdduke wrote:
> Hmm, if it feels buggy on Aura, it probably feels buggy on Android.  Is there
a
> hook we could use in render_view_impl to reset the flag if the user dismisses
> the keyboard?

Kevers@, perhaps you know?

[kevers, 2014-05-23 19:17:10]

On 2014/05/23 18:11:57, bokan wrote:
> On 2014/05/23 17:11:36, jdduke wrote:
> > Hmm, if it feels buggy on Aura, it probably feels buggy on Android.  Is
there
> a
> > hook we could use in render_view_impl to reset the flag if the user
dismisses
> > the keyboard?
> 
> Kevers@, perhaps you know?

Only thing that comes to mind is resetting on a change to visible_viewport_size
in render_widget.

[aelias_OOO_until_Jul13, 2014-05-23 19:34:51]

On 2014/05/23 19:17:10, kevers wrote:
> Only thing that comes to mind is resetting on a change to
visible_viewport_size
> in render_widget.

That sounds OK.  Let's do that.  I can reproduce the bug on Android and this
should fix it there.  It helps that the URL-bar doesn't hide while form fields
are focused on Android, so it's unlikely that a spurious visible_viewport_size
change would occur.

[bokan, 2014-05-23 19:49:08]

On 2014/05/23 19:34:51, aelias wrote:
> On 2014/05/23 19:17:10, kevers wrote:
> > Only thing that comes to mind is resetting on a change to
> visible_viewport_size
> > in render_widget.
> 
> That sounds OK.  Let's do that.  I can reproduce the bug on Android and this
> should fix it there.  It helps that the URL-bar doesn't hide while form fields
> are focused on Android, so it's unlikely that a spurious visible_viewport_size
> change would occur.

That was what I tried first, but the visible_viewport_size will change on
resizing the whole RenderWidget so it'll get cleared if we did an orientation
change. I'm not sure when the suppression is triggered, maybe that's ok?

[bokan, 2014-05-23 19:51:41]

On 2014/05/23 19:49:08, bokan wrote:
> On 2014/05/23 19:34:51, aelias wrote:
> > On 2014/05/23 19:17:10, kevers wrote:
> > > Only thing that comes to mind is resetting on a change to
> > visible_viewport_size
> > > in render_widget.
> > 
> > That sounds OK.  Let's do that.  I can reproduce the bug on Android and this
> > should fix it there.  It helps that the URL-bar doesn't hide while form
fields
> > are focused on Android, so it's unlikely that a spurious
visible_viewport_size
> > change would occur.
> 
> That was what I tried first, but the visible_viewport_size will change on
> resizing the whole RenderWidget so it'll get cleared if we did an orientation
> change. I'm not sure when the suppression is triggered, maybe that's ok?

Though, I guess I can just check if *only* the visible viewport size changed.
I'll try that.

[aelias_OOO_until_Jul13, 2014-05-23 19:53:52]

Android doesn't have visible viewport size only changes, only ChromeOS has that
behavior.  I don't think it's a big deal to clear if orientation changes.

[jdduke (slow), 2014-05-23 19:58:44]

On 2014/05/23 19:53:52, aelias wrote:
> Android doesn't have visible viewport size only changes, only ChromeOS has
that
> behavior.  I don't think it's a big deal to clear if orientation changes.

Yup, in fact I swear there was a recent patch that did just that (cleared on
orientation change), maybe it never landed...

[jdduke (slow), 2014-05-23 20:00:26]

On 2014/05/23 19:58:44, jdduke wrote:
> On 2014/05/23 19:53:52, aelias wrote:
> > Android doesn't have visible viewport size only changes, only ChromeOS has
> that
> > behavior.  I don't think it's a big deal to clear if orientation changes.
> 
> Yup, in fact I swear there was a recent patch that did just that (cleared on
> orientation change), maybe it never landed...

Ah, yes, https://codereview.chromium.org/268753003/, this change should take
care of that.

[bokan, 2014-05-23 21:32:23]

Ok, made the change. PTAL, thanks.

[aelias_OOO_until_Jul13, 2014-05-23 23:26:18]

lgtm modulo nit below.  +piman@ for OWNERS.

https://codereview.chromium.org/289343003/diff/20001/content/renderer/render_...
File content/renderer/render_view_impl.cc (right):

https://codereview.chromium.org/289343003/diff/20001/content/renderer/render_...
content/renderer/render_view_impl.cc:3098: gfx::Size old_visible_viewport_size_
= visible_viewport_size_;
Shouldn't be an underscore on the local variable here.

[piman, 2014-05-23 23:42:33]

LGTM %nit

[bokan, 2014-05-24 11:48:39]

The CQ bit was checked by bokan@chromium.org

[commit-bot: I haz the power, 2014-05-24 11:49:12]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bokan@chromium.org/289343003/40001

[commit-bot: I haz the power, 2014-05-24 13:38:53]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[commit-bot: I haz the power, 2014-05-24 14:02:05]

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

[commit-bot: I haz the power, 2014-05-24 14:02:06]

Try jobs failed on following builders:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)

[bokan, 2014-05-26 13:41:58]

The CQ bit was checked by bokan@chromium.org

[commit-bot: I haz the power, 2014-05-26 13:43:06]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bokan@chromium.org/289343003/40001

[commit-bot: I haz the power, 2014-05-26 14:46:45]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)

[commit-bot: I haz the power, 2014-05-26 15:57:42]

Change committed as 272832