Fix DropdownPopupWindow's position in Android N.

Fix DropdownPopupWindow's position in Android N.

In Android N, the framework will read view.getLeft() and view.getTop() to
determine where to put the drop down, and if the anchor view is not on screen,
the parent view will be scrolled to make it on screen (by means of
requestChildRectangleOnScreen).

setLayoutParams() is not enough to change left and top values of the anchorView
immediately. Show()ing the ListPopupWindow immediately after setLayoutParams()
will show the popup window in the wrong location, and also cause the parent view
to be improperly scrolled.

Call postShow() to put show() after when the layout of the anchor view is
changed.

Initial selection has to be set after show() is actually called,
because getListView() is null until the ListPopupWindow is shown.

BUG=614746
Committed: https://crrev.com/87634341cb230c465a1d2c5cdb09f79c0a97bc08
Cr-Commit-Position: refs/heads/master@{#399274}

[hush (inactive), 2016-06-09 01:07:29]

Description was changed from

==========
Force a layout of the anchor view before showing a <select> drop down.

In Android N, the framework will read view.getLeft() and view.getTop() to
determine where to put the drop down. Calling setLayoutParams() is not enough to
change left and top values immediately. So call view.layout() to force a layout
now.

BUG=614746
==========

to

==========
Force a layout of the anchor view before showing a <select> drop down.

In Android N, the framework will read view.getLeft() and view.getTop() to
determine where to put the drop down, and if the anchor view is not on screen,
the parent view will be scrolled to make it on screen (by means of
requestChildRectangleOnScreen).

setLayoutParams() is not enough to change left and top values of the anchorView
immediately. Show()ing the ListPopupWindow immediately after setLayoutParams()
will show the popup window in the wrong location, and also cause the parent view
to be improperly scrolled.

So call view.layout() to force a layout immediately after setLayoutParams.

BUG=614746
==========

[hush (inactive), 2016-06-09 01:07:29]

hush@chromium.org changed reviewers:
+ dtrainor@chromium.org

[hush (inactive), 2016-06-09 01:07:36]

PTAL

[David Trainor- moved to gerrit, 2016-06-09 02:51:21]

lgtm!

[boliu, 2016-06-09 02:53:28]

On 2016/06/09 02:51:21, David Trainor wrote:
> lgtm!

we came up with maybe a better solution: instead of calling show immediately, do
postOnAnimation({show()}), that should always be correct, and doesn't add nasty
hacks like calling layout or invalidate at random places..

haven't tested it though

[David Trainor- moved to gerrit, 2016-06-09 02:56:16]

On 2016/06/09 02:53:28, boliu wrote:
> On 2016/06/09 02:51:21, David Trainor wrote:
> > lgtm!
> 
> we came up with maybe a better solution: instead of calling show immediately,
do
> postOnAnimation({show()}), that should always be correct, and doesn't add
nasty
> hacks like calling layout or invalidate at random places..
> 
> haven't tested it though

If that works that sounds better :).  Good call!

[boliu, 2016-06-10 01:34:49]

boliu@chromium.org changed reviewers:
+ boliu@chromium.org

[boliu, 2016-06-10 01:34:49]

On 2016/06/09 02:56:16, David Trainor wrote:
> On 2016/06/09 02:53:28, boliu wrote:
> > On 2016/06/09 02:51:21, David Trainor wrote:
> > > lgtm!
> > 
> > we came up with maybe a better solution: instead of calling show
immediately,
> do
> > postOnAnimation({show()}), that should always be correct, and doesn't add
> nasty
> > hacks like calling layout or invalidate at random places..
> > 
> > haven't tested it though
> 
> If that works that sounds better :).  Good call!

Well it doesn't. Because layout happens *after* animate.


Anyway.. the fundamental issue is that setLayoutParams is not synchronous and
requires a layout pass before it's safe to call window.show so that the right
position is used for the window. It just happens that on older android versions,
this looked ok, but on N, they modified things to break this, especially on
webview.

I mean android can definitely be nice and have window.show do the right thing
internally. But I'm expecting too much from android :/

On chat, sounds like hush has a solution to wait for layout before calling show.
But I haven't seen the change yet, not sure how big it is etc. Maybe PS1 is
still the small one that should be merged, but definitely should look at the
"proper" fix first to decide

[hush (inactive), 2016-06-10 18:47:09]

Description was changed from

==========
Force a layout of the anchor view before showing a <select> drop down.

In Android N, the framework will read view.getLeft() and view.getTop() to
determine where to put the drop down, and if the anchor view is not on screen,
the parent view will be scrolled to make it on screen (by means of
requestChildRectangleOnScreen).

setLayoutParams() is not enough to change left and top values of the anchorView
immediately. Show()ing the ListPopupWindow immediately after setLayoutParams()
will show the popup window in the wrong location, and also cause the parent view
to be improperly scrolled.

So call view.layout() to force a layout immediately after setLayoutParams.

BUG=614746
==========

to

==========
Fix DropdownPopupWindow's position in Android N.

In Android N, the framework will read view.getLeft() and view.getTop() to
determine where to put the drop down, and if the anchor view is not on screen,
the parent view will be scrolled to make it on screen (by means of
requestChildRectangleOnScreen).

setLayoutParams() is not enough to change left and top values of the anchorView
immediately. Show()ing the ListPopupWindow immediately after setLayoutParams()
will show the popup window in the wrong location, and also cause the parent view
to be improperly scrolled.

Call postShow() to put show() after when the layout of the anchor view is
changed.

BUG=614746
==========

[hush (inactive), 2016-06-10 18:47:43]

On 2016/06/10 01:34:49, boliu wrote:
> On 2016/06/09 02:56:16, David Trainor wrote:
> > On 2016/06/09 02:53:28, boliu wrote:
> > > On 2016/06/09 02:51:21, David Trainor wrote:
> > > > lgtm!
> > > 
> > > we came up with maybe a better solution: instead of calling show
> immediately,
> > do
> > > postOnAnimation({show()}), that should always be correct, and doesn't add
> > nasty
> > > hacks like calling layout or invalidate at random places..
> > > 
> > > haven't tested it though
> > 
> > If that works that sounds better :).  Good call!
> 
> Well it doesn't. Because layout happens *after* animate.
> 
> 
> Anyway.. the fundamental issue is that setLayoutParams is not synchronous and
> requires a layout pass before it's safe to call window.show so that the right
> position is used for the window. It just happens that on older android
versions,
> this looked ok, but on N, they modified things to break this, especially on
> webview.
> 
> I mean android can definitely be nice and have window.show do the right thing
> internally. But I'm expecting too much from android :/
> 
> On chat, sounds like hush has a solution to wait for layout before calling
show.
> But I haven't seen the change yet, not sure how big it is etc. Maybe PS1 is
> still the small one that should be merged, but definitely should look at the
> "proper" fix first to decide

The proper fix is suggested by alanv in the internal bug. PTAL PS2

[boliu, 2016-06-10 19:16:48]

lgtm

also explain why we need setInitialSelection

[hush (inactive), 2016-06-10 19:21:15]

Description was changed from

==========
Fix DropdownPopupWindow's position in Android N.

In Android N, the framework will read view.getLeft() and view.getTop() to
determine where to put the drop down, and if the anchor view is not on screen,
the parent view will be scrolled to make it on screen (by means of
requestChildRectangleOnScreen).

setLayoutParams() is not enough to change left and top values of the anchorView
immediately. Show()ing the ListPopupWindow immediately after setLayoutParams()
will show the popup window in the wrong location, and also cause the parent view
to be improperly scrolled.

Call postShow() to put show() after when the layout of the anchor view is
changed.

BUG=614746
==========

to

==========
Fix DropdownPopupWindow's position in Android N.

In Android N, the framework will read view.getLeft() and view.getTop() to
determine where to put the drop down, and if the anchor view is not on screen,
the parent view will be scrolled to make it on screen (by means of
requestChildRectangleOnScreen).

setLayoutParams() is not enough to change left and top values of the anchorView
immediately. Show()ing the ListPopupWindow immediately after setLayoutParams()
will show the popup window in the wrong location, and also cause the parent view
to be improperly scrolled.

Call postShow() to put show() after when the layout of the anchor view is
changed.

Initial selection has to be set after show() is actually called,
getListView() is null until the ListPopupWindow is shown.

BUG=614746
==========

[hush (inactive), 2016-06-10 19:21:32]

Description was changed from

==========
Fix DropdownPopupWindow's position in Android N.

In Android N, the framework will read view.getLeft() and view.getTop() to
determine where to put the drop down, and if the anchor view is not on screen,
the parent view will be scrolled to make it on screen (by means of
requestChildRectangleOnScreen).

setLayoutParams() is not enough to change left and top values of the anchorView
immediately. Show()ing the ListPopupWindow immediately after setLayoutParams()
will show the popup window in the wrong location, and also cause the parent view
to be improperly scrolled.

Call postShow() to put show() after when the layout of the anchor view is
changed.

Initial selection has to be set after show() is actually called,
getListView() is null until the ListPopupWindow is shown.

BUG=614746
==========

to

==========
Fix DropdownPopupWindow's position in Android N.

In Android N, the framework will read view.getLeft() and view.getTop() to
determine where to put the drop down, and if the anchor view is not on screen,
the parent view will be scrolled to make it on screen (by means of
requestChildRectangleOnScreen).

setLayoutParams() is not enough to change left and top values of the anchorView
immediately. Show()ing the ListPopupWindow immediately after setLayoutParams()
will show the popup window in the wrong location, and also cause the parent view
to be improperly scrolled.

Call postShow() to put show() after when the layout of the anchor view is
changed.

Initial selection has to be set after show() is actually called,
because getListView() is null until the ListPopupWindow is shown.

BUG=614746
==========

[hush (inactive), 2016-06-10 19:21:42]

The CQ bit was checked by hush@chromium.org

[hush (inactive), 2016-06-10 19:21:43]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/2049523005/#ps20001
(title: "fix with "postShow"")

[commit-bot: I haz the power, 2016-06-10 19:22:09]

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

[boliu, 2016-06-10 19:24:35]

The CQ bit was unchecked by boliu@chromium.org

[boliu, 2016-06-10 19:24:54]

On 2016/06/10 19:22:09, commit-bot: I haz the power wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/2049523005/20001

wait for dtrainor

[David Trainor- moved to gerrit, 2016-06-10 21:27:34]

lgtm thanks!

[hush (inactive), 2016-06-10 21:28:01]

The CQ bit was checked by hush@chromium.org

[commit-bot: I haz the power, 2016-06-10 21:29:31]

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

[commit-bot: I haz the power, 2016-06-10 21:35:49]

Description was changed from

==========
Fix DropdownPopupWindow's position in Android N.

In Android N, the framework will read view.getLeft() and view.getTop() to
determine where to put the drop down, and if the anchor view is not on screen,
the parent view will be scrolled to make it on screen (by means of
requestChildRectangleOnScreen).

setLayoutParams() is not enough to change left and top values of the anchorView
immediately. Show()ing the ListPopupWindow immediately after setLayoutParams()
will show the popup window in the wrong location, and also cause the parent view
to be improperly scrolled.

Call postShow() to put show() after when the layout of the anchor view is
changed.

Initial selection has to be set after show() is actually called,
because getListView() is null until the ListPopupWindow is shown.

BUG=614746
==========

to

==========
Fix DropdownPopupWindow's position in Android N.

In Android N, the framework will read view.getLeft() and view.getTop() to
determine where to put the drop down, and if the anchor view is not on screen,
the parent view will be scrolled to make it on screen (by means of
requestChildRectangleOnScreen).

setLayoutParams() is not enough to change left and top values of the anchorView
immediately. Show()ing the ListPopupWindow immediately after setLayoutParams()
will show the popup window in the wrong location, and also cause the parent view
to be improperly scrolled.

Call postShow() to put show() after when the layout of the anchor view is
changed.

Initial selection has to be set after show() is actually called,
because getListView() is null until the ListPopupWindow is shown.

BUG=614746
==========

[commit-bot: I haz the power, 2016-06-10 21:35:50]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-10 21:36:20]

CQ bit was unchecked

[commit-bot: I haz the power, 2016-06-10 21:37:15]

Description was changed from

==========
Fix DropdownPopupWindow's position in Android N.

In Android N, the framework will read view.getLeft() and view.getTop() to
determine where to put the drop down, and if the anchor view is not on screen,
the parent view will be scrolled to make it on screen (by means of
requestChildRectangleOnScreen).

setLayoutParams() is not enough to change left and top values of the anchorView
immediately. Show()ing the ListPopupWindow immediately after setLayoutParams()
will show the popup window in the wrong location, and also cause the parent view
to be improperly scrolled.

Call postShow() to put show() after when the layout of the anchor view is
changed.

Initial selection has to be set after show() is actually called,
because getListView() is null until the ListPopupWindow is shown.

BUG=614746
==========

to

==========
Fix DropdownPopupWindow's position in Android N.

In Android N, the framework will read view.getLeft() and view.getTop() to
determine where to put the drop down, and if the anchor view is not on screen,
the parent view will be scrolled to make it on screen (by means of
requestChildRectangleOnScreen).

setLayoutParams() is not enough to change left and top values of the anchorView
immediately. Show()ing the ListPopupWindow immediately after setLayoutParams()
will show the popup window in the wrong location, and also cause the parent view
to be improperly scrolled.

Call postShow() to put show() after when the layout of the anchor view is
changed.

Initial selection has to be set after show() is actually called,
because getListView() is null until the ListPopupWindow is shown.

BUG=614746

Committed: https://crrev.com/87634341cb230c465a1d2c5cdb09f79c0a97bc08
Cr-Commit-Position: refs/heads/master@{#399274}
==========

[commit-bot: I haz the power, 2016-06-10 21:37:16]

Patchset 2 (id:??) landed as
https://crrev.com/87634341cb230c465a1d2c5cdb09f79c0a97bc08
Cr-Commit-Position: refs/heads/master@{#399274}