[Android] Coordinate Infobars and Snackbars

[Android] Coordinate Infobars and Snackbars

CoordinatorLayout in android design support library makes it possible to
coordinate views' animation. This CL makes CompositorViewHolder a
CoordinatorLayout, and adds several behaviors to control the interaction
between snackbars and infobars.

After this change, CompositorViewHolder will first check if a touch
event is handled by behaviors in the children views, then give the
touchevent to fullscreen manager and layout manager afterwards.

BUG=581227
Committed: https://crrev.com/87ee877992e41f3037a23a164abf4689c1ba603c
Cr-Commit-Position: refs/heads/master@{#395210}

[Ian Wen, 2016-05-17 23:16:02]

ianwen@chromium.org changed reviewers:
+ dfalcantara@chromium.org, jbudorick@chromium.org

[Ian Wen, 2016-05-17 23:16:02]

PTAL. This CL will only be landed after M52 branch point.

[gone, 2016-05-17 23:48:25]

l-g-t-m from a cursory look.  Not terribly happy about the need to instanceof
everywhere, but we may as well be consistent when we do it.

https://codereview.chromium.org/1983353002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java
(right):

https://codereview.chromium.org/1983353002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:69:
private Behavior<View> mBehavior = new Behavior<View>() {
final?

https://codereview.chromium.org/1983353002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:86:
* Creates an instance of the {@link SnackbarView}.
Need the @param activity

https://codereview.chromium.org/1983353002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:94:
mParent = (ViewGroup) activity.findViewById(R.id.compositor_view_holder);
This logic is kind of gross because it's directly searching for an
Activity-specific View.  What do you think about containing the grossness like
this:

mParent = findParentView(activity);

/**
 * Comment explaining why this is gross.
 */
private void findParentView(Activity activity) {
    if (activity instanceof ChromeActivity) {
        return ((ChromeActivity) activity).getCompositorViewHolder();
    } else {
        return (ViewGroup) activity.findViewById(android.R.id.content);
    }
}

https://codereview.chromium.org/1983353002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:98:
mView = (ViewGroup) LayoutInflater.from(activity).inflate(R.layout.snackbar,
mParent,
indent right before R.layout.snackbar

https://codereview.chromium.org/1983353002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:240:
* Starts the animation with {@link SurfaceView} optimization disabled.
This comment only applies to the top of the conditional, doesn't it?

https://codereview.chromium.org/1983353002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/tab/TabContentViewParent.java
(right):

https://codereview.chromium.org/1983353002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/tab/TabContentViewParent.java:24:
private Behavior<?> mBehavior = new SnackbarAwareBehavior();
private final for both?

[Ian Wen, 2016-05-18 00:23:46]

Thanks for the quick review! All addressed.

https://codereview.chromium.org/1983353002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java
(right):

https://codereview.chromium.org/1983353002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:69:
private Behavior<View> mBehavior = new Behavior<View>() {
On 2016/05/17 23:48:24, dfalcantara wrote:
> final?

Done.

https://codereview.chromium.org/1983353002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:86:
* Creates an instance of the {@link SnackbarView}.
On 2016/05/17 23:48:24, dfalcantara wrote:
> Need the @param activity

Done.

https://codereview.chromium.org/1983353002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:94:
mParent = (ViewGroup) activity.findViewById(R.id.compositor_view_holder);
On 2016/05/17 23:48:24, dfalcantara wrote:
> This logic is kind of gross because it's directly searching for an
> Activity-specific View.  What do you think about containing the grossness like
> this:
> 
> mParent = findParentView(activity);
> 
> /**
>  * Comment explaining why this is gross.
>  */
> private void findParentView(Activity activity) {
>     if (activity instanceof ChromeActivity) {
>         return ((ChromeActivity) activity).getCompositorViewHolder();
>     } else {
>         return (ViewGroup) activity.findViewById(android.R.id.content);
>     }
> }

Good suggestion. Done.

https://codereview.chromium.org/1983353002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:98:
mView = (ViewGroup) LayoutInflater.from(activity).inflate(R.layout.snackbar,
mParent,
On 2016/05/17 23:48:24, dfalcantara wrote:
> indent right before R.layout.snackbar

Done.

https://codereview.chromium.org/1983353002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:240:
* Starts the animation with {@link SurfaceView} optimization disabled.
On 2016/05/17 23:48:24, dfalcantara wrote:
> This comment only applies to the top of the conditional, doesn't it?

Done.

https://codereview.chromium.org/1983353002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/tab/TabContentViewParent.java
(right):

https://codereview.chromium.org/1983353002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/tab/TabContentViewParent.java:24:
private Behavior<?> mBehavior = new SnackbarAwareBehavior();
On 2016/05/17 23:48:25, dfalcantara wrote:
> private final for both?

Done.

[gone, 2016-05-18 00:30:28]

lgtm

[jbudorick, 2016-05-18 00:32:22]

build/ lgtm

[Ian Wen, 2016-05-20 20:57:00]

The CQ bit was checked by ianwen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-20 20:57:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1983353002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1983353002/40001

[commit-bot: I haz the power, 2016-05-20 21:17:04]

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

[commit-bot: I haz the power, 2016-05-20 21:17:06]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Ian Wen, 2016-05-20 21:31:12]

The CQ bit was checked by ianwen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-20 21:32:12]

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

[commit-bot: I haz the power, 2016-05-20 21:35:20]

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

[commit-bot: I haz the power, 2016-05-20 21:35:21]

Dry run: Try jobs failed on following builders:
  ios-device-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  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_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Ian Wen, 2016-05-20 21:51:38]

The CQ bit was checked by ianwen@chromium.org

[Ian Wen, 2016-05-20 21:51:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
jbudorick@chromium.org, dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/1983353002/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2016-05-20 21:52:10]

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

[commit-bot: I haz the power, 2016-05-20 23:55:31]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-05-20 23:56:32]

Description was changed from

==========
[Android] Coordinate Infobars and Snackbars

CoordinatorLayout in android design support library makes it possible to
coordinate views' animation. This CL makes CompositorViewHolder a
CoordinatorLayout, and adds several behaviors to control the interaction
between snackbars and infobars.

After this change, CompositorViewHolder will first check if a touch
event is handled by behaviors in the children views, then give the
touchevent to fullscreen manager and layout manager afterwards.

BUG=581227
==========

to

==========
[Android] Coordinate Infobars and Snackbars

CoordinatorLayout in android design support library makes it possible to
coordinate views' animation. This CL makes CompositorViewHolder a
CoordinatorLayout, and adds several behaviors to control the interaction
between snackbars and infobars.

After this change, CompositorViewHolder will first check if a touch
event is handled by behaviors in the children views, then give the
touchevent to fullscreen manager and layout manager afterwards.

BUG=581227

Committed: https://crrev.com/87ee877992e41f3037a23a164abf4689c1ba603c
Cr-Commit-Position: refs/heads/master@{#395210}
==========

[commit-bot: I haz the power, 2016-05-20 23:56:34]

Patchset 5 (id:??) landed as
https://crrev.com/87ee877992e41f3037a23a164abf4689c1ba603c
Cr-Commit-Position: refs/heads/master@{#395210}