Autofill - Stop listening to OrientationChangeEvent.

Autofill - Stop listening to OrientationChangeEvent.

The other option would be to switch to using a DisplayObserver --- see https://codereview.chromium.org/802833004/ --- but I don't think that's necessary, because an OrientationChangeEvent will almost always result in a resize (especially on mobile devices).

BUG=433486
Committed: https://crrev.com/db4981a558d08869afa5ff8687d8f86b1fcdb470
Cr-Commit-Position: refs/heads/master@{#308914}

[Evan Stade, 2014-12-16 19:59:14]

estade@chromium.org changed reviewers:
+ mlamouri@chromium.org

[please use gerrit instead, 2014-12-16 20:37:42]

FYI, https://codereview.chromium.org/715733002/ plans to change behavior to hide
the autofill popup only when the bounds of the focused element changed after
viewport resize. If the viewport resize did not change bounds of the focused
element (e.g., when keyboard slightly increased or decreased its own size), then
the autofill popup will not hide itself. If both
https://codereview.chromium.org/715733002/ and your patch land, then orientation
changes sometimes will not hide the popup. One example is the first input field
on https://form870.appspot.com/smoke.html. (Poke me for a demo.) If you always
want to hide the popup on orientation change, then I recommend that you don't
land this patch.

[Evan Stade, 2014-12-16 20:42:58]

On 2014/12/16 20:37:42, Rouslan Solomakhin wrote:
> FYI, https://codereview.chromium.org/715733002/ plans to change behavior to
hide
> the autofill popup only when the bounds of the focused element changed after
> viewport resize. If the viewport resize did not change bounds of the focused
> element (e.g., when keyboard slightly increased or decreased its own size),
then
> the autofill popup will not hide itself. If both
> https://codereview.chromium.org/715733002/ and your patch land, then
orientation
> changes sometimes will not hide the popup. One example is the first input
field
> on https://form870.appspot.com/smoke.html. (Poke me for a demo.) If you always
> want to hide the popup on orientation change, then I recommend that you don't
> land this patch.

if the bounds of the focus element don't change, does that mean we can leave the
popup up?

[please use gerrit instead, 2014-12-16 20:55:56]

On 2014/12/16 20:42:58, Evan Stade wrote:
> if the bounds of the focus element don't change, does that mean we can leave
the
> popup up?

Yes, that would be OK.

[mlamouri (slow - plz ping), 2014-12-17 17:16:07]

The change itself lgtm. Obviously, I can't judge whether it's what autofill
wants.

It seems to be the last use of OrientationChangeEvent. Thanks for removing it!
:) (I will gladly write a follow-up CL that delete the method from the
RVObserver interface.)

[Evan Stade, 2014-12-17 23:32:58]

So Rouslan, do you think your concern is addressed? If not I can switch to
https://codereview.chromium.org/802833004/

[please use gerrit instead, 2014-12-17 23:39:47]

My concern is addressed. LGTM

[please use gerrit instead, 2014-12-17 23:39:49]

My concern is addressed. LGTM

[Evan Stade, 2014-12-17 23:45:41]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2014-12-17 23:47:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/809623005/1

[commit-bot: I haz the power, 2014-12-18 01:39:11]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2014-12-18 01:40:09]

Patchset 1 (id:??) landed as
https://crrev.com/db4981a558d08869afa5ff8687d8f86b1fcdb470
Cr-Commit-Position: refs/heads/master@{#308914}