ControlContainer is now a child of CompositorViewHolder

ControlContainer is now a child of CompositorViewHolder

This change is to facilitate the addition of a bottom sheet for the
Chrome Home feature. BottomSheetBehavior will be used to add this
functionality to the toolbar which will need a CoordinatorLayout as
its parent. CompositorViewHolder is a good choice because it is
already the root view for most of Chrome on Android.

This change also cleans up some of the now-unused touch handling
code in the affected classes.

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

[mdjones, 2016-12-08 21:11:39]

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

[mdjones, 2016-12-08 21:11:39]

ptal

[Ian Wen, 2016-12-09 19:03:06]

Looks mostly good!

https://codereview.chromium.org/2560043002/diff/1/chrome/android/java/res/lay...
File chrome/android/java/res/layout/main.xml (right):

https://codereview.chromium.org/2560043002/diff/1/chrome/android/java/res/lay...
chrome/android/java/res/layout/main.xml:33: android:background="#000000"
Nit: use @android:color/black here.

https://codereview.chromium.org/2560043002/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
(right):

https://codereview.chromium.org/2560043002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java:389:
lp.setBehavior(new CompositorViewHolderBehavior());
This can be done in xml. Please follow the example of
omnibox_results_container_stub.

https://codereview.chromium.org/2560043002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolderBehavior.java
(right):

https://codereview.chromium.org/2560043002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolderBehavior.java:59:
return child.dispatchTouchEvent(ev);
This change will make views beneath snackbars to be clickable when snackbar is
on top. (If you click the empty area of the snackbar and the webcontents beneath
it will be clicked). And thus this is not a desirable behavior.

Is there any reason you wanted this?

[mdjones, 2016-12-09 21:48:12]

https://codereview.chromium.org/2560043002/diff/1/chrome/android/java/res/lay...
File chrome/android/java/res/layout/main.xml (right):

https://codereview.chromium.org/2560043002/diff/1/chrome/android/java/res/lay...
chrome/android/java/res/layout/main.xml:33: android:background="#000000"
On 2016/12/09 19:03:06, Ian Wen wrote:
> Nit: use @android:color/black here.

Done.

https://codereview.chromium.org/2560043002/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
(right):

https://codereview.chromium.org/2560043002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java:389:
lp.setBehavior(new CompositorViewHolderBehavior());
On 2016/12/09 19:03:06, Ian Wen wrote:
> This can be done in xml. Please follow the example of
> omnibox_results_container_stub.

Done.

https://codereview.chromium.org/2560043002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolderBehavior.java
(right):

https://codereview.chromium.org/2560043002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolderBehavior.java:59:
return child.dispatchTouchEvent(ev);
On 2016/12/09 19:03:06, Ian Wen wrote:
> This change will make views beneath snackbars to be clickable when snackbar is
> on top. (If you click the empty area of the snackbar and the webcontents
beneath
> it will be clicked). And thus this is not a desirable behavior.
> 
> Is there any reason you wanted this?

Originally I added it to test something related to filtering. It didn't seem to
have an impact and seemed more correct so I left it.

Reverted it to the previous version.

[Ian Wen, 2016-12-14 18:50:36]

lgtm

Thanks for the refactoring!

https://codereview.chromium.org/2560043002/diff/20001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
(right):

https://codereview.chromium.org/2560043002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java:382:
View toolbar = toolbarContainerStub.inflate();
Nit: rename this to controlContainer. Toolbar is a totally different term,
right? Oh God bless Chrome.

[mdjones, 2016-12-16 17:34:57]

https://codereview.chromium.org/2560043002/diff/20001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
(right):

https://codereview.chromium.org/2560043002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java:382:
View toolbar = toolbarContainerStub.inflate();
On 2016/12/14 18:50:36, Ian Wen wrote:
> Nit: rename this to controlContainer. Toolbar is a totally different term,
> right? Oh God bless Chrome.

Hahaha, yes! I renamed this to just "container" because "controlContainer" is
used below. I'm going to hold off on landing this until I know exactly what's
going to happen with the bottom sheet.

[mdjones, 2017-01-12 22:03:20]

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

[mdjones, 2017-01-12 22:03:20]

+dfalcantara

[gone, 2017-01-13 18:31:08]

lgtm

https://codereview.chromium.org/2560043002/diff/60001/chrome/android/java/res...
File chrome/android/java/res/layout/bottom_control_container.xml (right):

https://codereview.chromium.org/2560043002/diff/60001/chrome/android/java/res...
chrome/android/java/res/layout/bottom_control_container.xml:31:
android:inflatedId="@+id/find_toolbar"
nit: shove visibility below the layout lines

https://codereview.chromium.org/2560043002/diff/60001/chrome/android/java/res...
File chrome/android/java/res/layout/main.xml (right):

https://codereview.chromium.org/2560043002/diff/60001/chrome/android/java/res...
chrome/android/java/res/layout/main.xml:43: android:visibility="gone" />
follow file convention and put newlines between these; looks like the originals
should have been doing the same

[mdjones, 2017-01-13 19:56:41]

https://codereview.chromium.org/2560043002/diff/60001/chrome/android/java/res...
File chrome/android/java/res/layout/bottom_control_container.xml (right):

https://codereview.chromium.org/2560043002/diff/60001/chrome/android/java/res...
chrome/android/java/res/layout/bottom_control_container.xml:31:
android:inflatedId="@+id/find_toolbar"
On 2017/01/13 18:31:08, dfalcantara (load balance plz) wrote:
> nit: shove visibility below the layout lines

Done.

https://codereview.chromium.org/2560043002/diff/60001/chrome/android/java/res...
File chrome/android/java/res/layout/main.xml (right):

https://codereview.chromium.org/2560043002/diff/60001/chrome/android/java/res...
chrome/android/java/res/layout/main.xml:43: android:visibility="gone" />
On 2017/01/13 18:31:08, dfalcantara (load balance plz) wrote:
> follow file convention and put newlines between these; looks like the
originals
> should have been doing the same

Done.

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

The CQ bit was checked by mdjones@chromium.org

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

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

[commit-bot: I haz the power, 2017-01-13 19:57:35]

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-13 20:46:33]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1484337410246880,
"parent_rev": "06b3ac918eaff59094279ffcf851435f813a38c1", "commit_rev":
"c9206a8f853ff69c8bab56840d7b0c93058528d6"}

[commit-bot: I haz the power, 2017-01-13 20:47:03]

Description was changed from

==========
ControlContainer is now a child of CompositorViewHolder

This change is to facilitate the addition of a bottom sheet for the
Chrome Home feature. BottomSheetBehavior will be used to add this
functionality to the toolbar which will need a CoordinatorLayout as
its parent. CompositorViewHolder is a good choice because it is
already the root view for most of Chrome on Android.

This change also cleans up some of the now-unused touch handling
code in the affected classes.

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

to

==========
ControlContainer is now a child of CompositorViewHolder

This change is to facilitate the addition of a bottom sheet for the
Chrome Home feature. BottomSheetBehavior will be used to add this
functionality to the toolbar which will need a CoordinatorLayout as
its parent. CompositorViewHolder is a good choice because it is
already the root view for most of Chrome on Android.

This change also cleans up some of the now-unused touch handling
code in the affected classes.

BUG=671361

Review-Url: https://codereview.chromium.org/2560043002
Cr-Commit-Position: refs/heads/master@{#443657}
Committed:
https://chromium.googlesource.com/chromium/src/+/c9206a8f853ff69c8bab56840d7b...
==========

[commit-bot: I haz the power, 2017-01-13 20:47:04]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/c9206a8f853ff69c8bab56840d7b...