[Contextual Search] Add close button in maximized state

[Contextual Search] Add close button in maximized state

Previously the close button was only available for Custom Tabs in
the maximized state. Now it is present in the expanded state and in
regular Chrome.

BUG=534680
Committed: https://crrev.com/4fa288fc5b41d03b84d2940cd51fee78f3259db9
Cr-Commit-Position: refs/heads/master@{#351097}

[Theresa, 2015-09-22 01:22:58]

twellington@chromium.org changed reviewers:
+ pedrosimonetti@chromium.org

[Theresa, 2015-09-22 01:22:59]

ptal

This is a really straight-forward change. The only extra thing I slipped in was
adding some touch slop around the 'X'. In Custom Tabs, missing the close button
isn't that bad because we do nothing with touches else where on the panel, but
in regular Chrome missing the close button will promote to a new tab so I think
we should give users some touch slop. I chose 5dp because that sounded good to
me. I'm totally open to changing the value if it seems like too much or not
enough.

[Theresa, 2015-09-23 16:50:58]

pedrosimonetti@, ptal

[pedro (no code reviews), 2015-09-23 23:35:58]

Hey Theresa, sorry for the delay on this one. I got stuck with Perf, and was
lagging behind several code reviews :(

https://codereview.chromium.org/1347023004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanel.java
(right):

https://codereview.chromium.org/1347023004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanel.java:75:
private static final int CLOSE_BUTTON_TOUCH_SLOP_DP = 5;
Nit: For consistency, add JavaDoc

https://codereview.chromium.org/1347023004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelBase.java
(right):

https://codereview.chromium.org/1347023004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelBase.java:1116:
if (isFullscreenSizePanel()) {
As we discussed offline, we should not show the arrow when in expanded state.
This means we should get rid of the rotation code and adjust the cross-fading of
the arrow to "x" when transitioning from peeked to maximized (on a fullscreen
size Panel).

[Theresa, 2015-09-24 01:13:32]

https://codereview.chromium.org/1347023004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanel.java
(right):

https://codereview.chromium.org/1347023004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanel.java:75:
private static final int CLOSE_BUTTON_TOUCH_SLOP_DP = 5;
On 2015/09/23 23:35:58, pedrosimonetti wrote:
> Nit: For consistency, add JavaDoc

Done.

https://codereview.chromium.org/1347023004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelBase.java
(right):

https://codereview.chromium.org/1347023004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelBase.java:1116:
if (isFullscreenSizePanel()) {
On 2015/09/23 23:35:58, pedrosimonetti wrote:
> As we discussed offline, we should not show the arrow when in expanded state.
> This means we should get rid of the rotation code and adjust the cross-fading
of
> the arrow to "x" when transitioning from peeked to maximized (on a fullscreen
> size Panel).

Adjusted the cross-fade to happen between peeked & maximized. I also chagned the
onClick handling to be the same for expanded and maximized.

The arrow png we're using is a sideways arrow >  so I left some of the rotation
code, but removed this code where we're manipulating the rotation value.

https://codereview.chromium.org/1347023004/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelBase.java
(right):

https://codereview.chromium.org/1347023004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelBase.java:50:
private static final float NARROW_EXPANDED_PANEL_HEIGHT_PERCENTAGE = .6f;
I increased this a bit because the arrow was disappearing really quickly before.

[Theresa, 2015-09-25 02:57:24]

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

[Theresa, 2015-09-25 02:57:25]

+dtrainor for OWNERS on
chrome/android/java/src/org/chromium/chrome/browser/compositor/ &
chrome/browser/android/compositor/


I had updated chrome/browser/android/compositor/OWNERS to include:
per-file contextual_search*=donnd@chromium.org
per-file contextual_search*=pedrosimonetti@chromium.org

but git cl upload things I'm still missing owners on those files.. so maybe it
needs to be *contextual_search*? Regardless,
chrome/android/java/src/org/chromium/chrome/browser/compositor/OWNERS doesn't
have donnd@ or pedrosimonetti@ (yet)

[David Trainor- moved to gerrit, 2015-09-25 03:58:53]

*/compositor/* lgtm.

[pedro (no code reviews), 2015-09-25 19:13:16]

lgtm

https://codereview.chromium.org/1347023004/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelBase.java
(right):

https://codereview.chromium.org/1347023004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelBase.java:50:
private static final float NARROW_EXPANDED_PANEL_HEIGHT_PERCENTAGE = .6f;
On 2015/09/24 01:13:32, Theresa Wellington wrote:
> I increased this a bit because the arrow was disappearing really quickly
before.

Acknowledged.

[Theresa, 2015-09-25 20:13:33]

The CQ bit was checked by twellington@chromium.org

[commit-bot: I haz the power, 2015-09-25 20:14:35]

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

[commit-bot: I haz the power, 2015-09-25 20:18:34]

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

[commit-bot: I haz the power, 2015-09-25 20:18:34]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Theresa, 2015-09-28 17:04:01]

The CQ bit was checked by twellington@chromium.org

[Theresa, 2015-09-28 17:04:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org, pedrosimonetti@chromium.org
Link to the patchset: https://codereview.chromium.org/1347023004/#ps60001
(title: "Cleanup rebase")

[commit-bot: I haz the power, 2015-09-28 17:04:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1347023004/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1347023004/60001

[commit-bot: I haz the power, 2015-09-28 19:17:33]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-09-28 19:18:46]

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