Add ViewGroup for handling pairs of infobar controls

Add ViewGroup for handling pairs of infobar controls

A recurring pattern in the infobar spec is to have two
controls that are placed horizontally if they fit, but
get stacked vertically otherwise.  This happens for
buttons on the infobar row and for any control where
there is both a label and a value (e.g. spinners or
credit card information).  The only differences
between them are defined by how the two controls are
laid out.

This CL introduces the InfoBarDualControlLayout for
handling these cases, but doesn't change the infobar
classes to use it, yet.  It's meant to be general
enough for all of the cases above to plug into it.

BUG=543205
Committed: https://crrev.com/32f6f55409009f43e0425008bcc20aa57dbeac8e
Cr-Commit-Position: refs/heads/master@{#367590}

[gone, 2015-12-31 00:30:15]

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

[gone, 2015-12-31 00:39:41]

The not-yet-ready follow-up CL is located here:
https://codereview.chromium.org/1558723002

It shows how the Layout is expected to be used.

[newt (away), 2016-01-05 01:36:14]

https://codereview.chromium.org/1553813002/diff/80001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayout.java
(right):

https://codereview.chromium.org/1553813002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayout.java:73:
* @param alignment Alignment to use.  See {@link InfoBarDualControlLayout} for
details.
"See {@link InfoBarDualControlLayout} for details"  -- huh? This is
InfoBarDualControlLayout.

How about just "One of ALIGN_START, ALIGN_APART, ALIGN_END"

https://codereview.chromium.org/1553813002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayout.java:87:
public void addView(View child, int index, ViewGroup.LayoutParams params) {
It might make more sense to override onViewAdded() instead

https://codereview.chromium.org/1553813002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayout.java:104:
int maxWidth = MeasureSpec.getMode(widthMeasureSpec) == MeasureSpec.UNSPECIFIED
I think we should either 1) disallow using an UNSPECIFIED width, or 2) set
maxWidth to Integer.MAX_VALUE if width is UNSPECIFIED.

https://codereview.chromium.org/1553813002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayout.java:122:
int widthSpec = MeasureSpec.makeMeasureSpec(maxWidth, MeasureSpec.EXACTLY);
Should this be EXACTLY or AT_MOST, or does it matter?

https://codereview.chromium.org/1553813002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayout.java:144:
boolean isPrimaryOnRight = (isRtl && mAlignment == ALIGN_START)
!isRtl ^ mAlignment == ALIGN_START

https://codereview.chromium.org/1553813002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayout.java:197:
void overrideRtlForTest(boolean isRtl) {
just use View.setLayoutDirection() instead of adding this method and isRtl()
above.

https://codereview.chromium.org/1553813002/diff/80001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayoutTest.java
(right):

https://codereview.chromium.org/1553813002/diff/80001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayoutTest.java:46:
public void testAlignStart() {
for efficiency's sake (i.e. to keep John happy and employed), maybe combine
these three tests into one? You could even use nested for loops.

https://codereview.chromium.org/1553813002/diff/80001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayoutTest.java:71:
/** Lays out out two controls that fit on the same line. */
s/out out/out/

[gone, 2016-01-05 02:26:01]

https://codereview.chromium.org/1553813002/diff/80001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayout.java
(right):

https://codereview.chromium.org/1553813002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayout.java:73:
* @param alignment Alignment to use.  See {@link InfoBarDualControlLayout} for
details.
On 2016/01/05 01:36:14, newt wrote:
> "See {@link InfoBarDualControlLayout} for details"  -- huh? This is
> InfoBarDualControlLayout.
> 
> How about just "One of ALIGN_START, ALIGN_APART, ALIGN_END"

Done.

https://codereview.chromium.org/1553813002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayout.java:87:
public void addView(View child, int index, ViewGroup.LayoutParams params) {
On 2016/01/05 01:36:14, newt wrote:
> It might make more sense to override onViewAdded() instead

Done.

https://codereview.chromium.org/1553813002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayout.java:104:
int maxWidth = MeasureSpec.getMode(widthMeasureSpec) == MeasureSpec.UNSPECIFIED
On 2016/01/05 01:36:14, newt wrote:
> I think we should either 1) disallow using an UNSPECIFIED width, or 2) set
> maxWidth to Integer.MAX_VALUE if width is UNSPECIFIED.

Went with Integer.MAX_VALUE since we're more likely to get into situations where
UNSPECIFIED is actually used for infobars.

https://codereview.chromium.org/1553813002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayout.java:122:
int widthSpec = MeasureSpec.makeMeasureSpec(maxWidth, MeasureSpec.EXACTLY);
On 2016/01/05 01:36:14, newt wrote:
> Should this be EXACTLY or AT_MOST, or does it matter?

Should be EXACTLY to occupy the full width and provide the maximum area for a
touch target.

https://codereview.chromium.org/1553813002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayout.java:144:
boolean isPrimaryOnRight = (isRtl && mAlignment == ALIGN_START)
On 2016/01/05 01:36:13, newt wrote:
> !isRtl ^ mAlignment == ALIGN_START

Acknowledged.

https://codereview.chromium.org/1553813002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayout.java:197:
void overrideRtlForTest(boolean isRtl) {
On 2016/01/05 01:36:13, newt wrote:
> just use View.setLayoutDirection() instead of adding this method and isRtl()
> above.

Done.

https://codereview.chromium.org/1553813002/diff/80001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayoutTest.java
(right):

https://codereview.chromium.org/1553813002/diff/80001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayoutTest.java:46:
public void testAlignStart() {
On 2016/01/05 01:36:14, newt wrote:
> for efficiency's sake (i.e. to keep John happy and employed), maybe combine
> these three tests into one? You could even use nested for loops.

Combined into a single test, but didn't do the for loop thing because it makes
it harder to tell where the test fails.

https://codereview.chromium.org/1553813002/diff/80001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayoutTest.java:71:
/** Lays out out two controls that fit on the same line. */
On 2016/01/05 01:36:14, newt wrote:
> s/out out/out/

Done.

[newt (away), 2016-01-05 02:27:46]

lgtm with comment

https://codereview.chromium.org/1553813002/diff/100001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayoutTest.java
(right):

https://codereview.chromium.org/1553813002/diff/100001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayoutTest.java:46:
public void testBasic() {
"testAlignSideBySide"?

[gone, 2016-01-05 17:58:43]

https://codereview.chromium.org/1553813002/diff/100001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayoutTest.java
(right):

https://codereview.chromium.org/1553813002/diff/100001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/infobar/InfoBarDualControlLayoutTest.java:46:
public void testBasic() {
On 2016/01/05 02:27:46, newt wrote:
> "testAlignSideBySide"?

Done.

[gone, 2016-01-05 17:58:48]

The CQ bit was checked by dfalcantara@chromium.org

[gone, 2016-01-05 17:58:48]

The patchset sent to the CQ was uploaded after l-g-t-m from newt@chromium.org
Link to the patchset: https://codereview.chromium.org/1553813002/#ps120001
(title: "Comments")

[commit-bot: I haz the power, 2016-01-05 18:00:02]

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

[commit-bot: I haz the power, 2016-01-05 18:37:39]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-01-05 18:38:54]

Description was changed from

==========
Add ViewGroup for handling pairs of infobar controls

A recurring pattern in the infobar spec is to have two
controls that are placed horizontally if they fit, but
get stacked vertically otherwise.  This happens for
buttons on the infobar row and for any control where
there is both a label and a value (e.g. spinners or
credit card information).  The only differences
between them are defined by how the two controls are
laid out.

This CL introduces the InfoBarDualControlLayout for
handling these cases, but doesn't change the infobar
classes to use it, yet.  It's meant to be general
enough for all of the cases above to plug into it.

BUG=543205
==========

to

==========
Add ViewGroup for handling pairs of infobar controls

A recurring pattern in the infobar spec is to have two
controls that are placed horizontally if they fit, but
get stacked vertically otherwise.  This happens for
buttons on the infobar row and for any control where
there is both a label and a value (e.g. spinners or
credit card information).  The only differences
between them are defined by how the two controls are
laid out.

This CL introduces the InfoBarDualControlLayout for
handling these cases, but doesn't change the infobar
classes to use it, yet.  It's meant to be general
enough for all of the cases above to plug into it.

BUG=543205

Committed: https://crrev.com/32f6f55409009f43e0425008bcc20aa57dbeac8e
Cr-Commit-Position: refs/heads/master@{#367590}
==========

[commit-bot: I haz the power, 2016-01-05 18:38:55]

Patchset 7 (id:??) landed as
https://crrev.com/32f6f55409009f43e0425008bcc20aa57dbeac8e
Cr-Commit-Position: refs/heads/master@{#367590}