Expand bottom sheet when URL bar is focused

Expand bottom sheet when URL bar is focused

This change adds some plumbing to allow the bottom sheet to expand
when a user focuses the URL bar. Unfocusing the URL bar will return
the sheet to the state it was in before being focused.

BUG=671361
Review-Url: https://codereview.chromium.org/2630513003
Cr-Commit-Position: refs/heads/master@{#445849}
Committed: https://chromium.googlesource.com/chromium/src/+/3eadcad51fce2b1b25470357e2cd942bb07bb3e4

[mdjones, 2017-01-13 18:26:38]

Description was changed from

==========
Expand bottom sheet when URL bar is focused

This change adds some plumbing to allow the bottom sheet to expand
when a user focuses the URL bar. Unfocusing the URL bar will return
the sheet to the state it was in before being focused.

Some flickering is visible when the animation is occuring due to
animation conflicts.

BUG=671361
==========

to

==========
Expand bottom sheet when URL bar is focused

This change adds some plumbing to allow the bottom sheet to expand
when a user focuses the URL bar. Unfocusing the URL bar will return
the sheet to the state it was in before being focused.

BUG=671361
==========

[mdjones, 2017-01-13 18:28:10]

mdjones@chromium.org changed reviewers:
+ ianwen@chromium.org

[mdjones, 2017-01-13 18:28:10]

ptal

[Ian Wen, 2017-01-17 19:23:59]

https://codereview.chromium.org/2630513003/diff/60001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/widget/BottomSheet.java
(right):

https://codereview.chromium.org/2630513003/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/widget/BottomSheet.java:227:
if (!isRunningSettleAnimation()) setSheetState(mCurrentState, false);
I think you could cancel the current animation, and then call setSheetState().

If you simply check if animation is running, I bet you will encounter some edge
case where after an orientation change the sheet is in wrong state or so.

[mdjones, 2017-01-17 21:59:32]

https://codereview.chromium.org/2630513003/diff/60001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/widget/BottomSheet.java
(right):

https://codereview.chromium.org/2630513003/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/widget/BottomSheet.java:227:
if (!isRunningSettleAnimation()) setSheetState(mCurrentState, false);
On 2017/01/17 19:23:59, Ian Wen wrote:
> I think you could cancel the current animation, and then call setSheetState().
> 
> If you simply check if animation is running, I bet you will encounter some
edge
> case where after an orientation change the sheet is in wrong state or so.

This was for when the keyboard shows, onLayoutChange is called without the
dimensions actually changing. I have updated the code to reflect this and
address your comment.

[mdjones, 2017-01-19 19:01:50]

mdjones@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[mdjones, 2017-01-19 19:01:51]

+dfalcantara

[gone, 2017-01-20 19:04:08]

https://codereview.chromium.org/2630513003/diff/100001/chrome/android/java/sr...
File chrome/android/java/src/org/chromium/chrome/browser/toolbar/Toolbar.java
(right):

https://codereview.chromium.org/2630513003/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/toolbar/Toolbar.java:48: *
@param sheet The bottom sheet.
Should probably just use {@link BottomSheet} rather than saying "bottom sheet".

https://codereview.chromium.org/2630513003/diff/100001/chrome/android/java/sr...
File chrome/android/java/src/org/chromium/chrome/browser/widget/BottomSheet.java
(right):

https://codereview.chromium.org/2630513003/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/widget/BottomSheet.java:235:
// Make sure the size of the layout actually changed.
Can't you just do equality checks?  Subtraction of two ints and comparing to
zero implies they're the same value, no?

[mdjones, 2017-01-20 20:11:47]

https://codereview.chromium.org/2630513003/diff/100001/chrome/android/java/sr...
File chrome/android/java/src/org/chromium/chrome/browser/toolbar/Toolbar.java
(right):

https://codereview.chromium.org/2630513003/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/toolbar/Toolbar.java:48: *
@param sheet The bottom sheet.
On 2017/01/20 19:04:08, dfalcantara (load balance plz) wrote:
> Should probably just use {@link BottomSheet} rather than saying "bottom
sheet".

Done.

https://codereview.chromium.org/2630513003/diff/100001/chrome/android/java/sr...
File chrome/android/java/src/org/chromium/chrome/browser/widget/BottomSheet.java
(right):

https://codereview.chromium.org/2630513003/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/widget/BottomSheet.java:235:
// Make sure the size of the layout actually changed.
On 2017/01/20 19:04:08, dfalcantara (load balance plz) wrote:
> Can't you just do equality checks?  Subtraction of two ints and comparing to
> zero implies they're the same value, no?

Yeah, not really sure what I was thinking here. Fixed.

[gone, 2017-01-20 20:14:11]

lgtm % ian's comments

https://codereview.chromium.org/2630513003/diff/120001/chrome/android/java/sr...
File chrome/android/java/src/org/chromium/chrome/browser/widget/BottomSheet.java
(right):

https://codereview.chromium.org/2630513003/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/widget/BottomSheet.java:237:
// Make sure the size of the layout actually changed.
Also this doesn't actually check for size changes; it's checking if all four
coordinates are the same.  Might want to update the comment.

[mdjones, 2017-01-20 21:03:44]

https://codereview.chromium.org/2630513003/diff/120001/chrome/android/java/sr...
File chrome/android/java/src/org/chromium/chrome/browser/widget/BottomSheet.java
(right):

https://codereview.chromium.org/2630513003/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/widget/BottomSheet.java:237:
// Make sure the size of the layout actually changed.
On 2017/01/20 20:14:10, dfalcantara (load balance plz) wrote:
> Also this doesn't actually check for size changes; it's checking if all four
> coordinates are the same.  Might want to update the comment.

It does now.

[gone, 2017-01-20 21:07:59]

lgtm

[Ian Wen, 2017-01-24 22:03:35]

The CQ bit was checked by ianwen@chromium.org

[Ian Wen, 2017-01-24 22:03:36]

lgtm

lgtm

[commit-bot: I haz the power, 2017-01-24 22:04:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 22:50:07]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1485295415507510,
"parent_rev": "eab94cd8af6d359162f74d21c8d7634ea3dce58a", "commit_rev":
"3eadcad51fce2b1b25470357e2cd942bb07bb3e4"}

[commit-bot: I haz the power, 2017-01-24 22:50:48]

Description was changed from

==========
Expand bottom sheet when URL bar is focused

This change adds some plumbing to allow the bottom sheet to expand
when a user focuses the URL bar. Unfocusing the URL bar will return
the sheet to the state it was in before being focused.

BUG=671361
==========

to

==========
Expand bottom sheet when URL bar is focused

This change adds some plumbing to allow the bottom sheet to expand
when a user focuses the URL bar. Unfocusing the URL bar will return
the sheet to the state it was in before being focused.

BUG=671361

Review-Url: https://codereview.chromium.org/2630513003
Cr-Commit-Position: refs/heads/master@{#445849}
Committed:
https://chromium.googlesource.com/chromium/src/+/3eadcad51fce2b1b25470357e2cd...
==========

[commit-bot: I haz the power, 2017-01-24 22:50:49]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/3eadcad51fce2b1b25470357e2cd...