❄ Split off RadioButtonLayout from InfoBarControls

❄ Split off RadioButtonLayout from InfoBarControls

Changes to the Infobar spec and how they lay out controls are coming.
Split off radio buttons into a separate layout that InfoBarControls
use and can be added by other layouts to prevent conflating changes
between them.

Also fixes up how the promo dialog (which used the RadioButtons)
assigns widths to some of its children.

Screenshots:
https://drive.google.com/corp/drive/u/0/folders/0B7c8ZkXVwskDYkxiQWNwS1JzR2c

BUG=712836, 712833, 717651
Review-Url: https://codereview.chromium.org/2850063002
Cr-Original-Commit-Position: refs/heads/master@{#468494}
Committed: https://chromium.googlesource.com/chromium/src/+/2883e4221128cdc2b95aa765031ab77b9b4f5794
Review-Url: https://codereview.chromium.org/2850063002
Cr-Commit-Position: refs/heads/master@{#468763}
Committed: https://chromium.googlesource.com/chromium/src/+/3f5e8d10f9ab29fee3f8b02c4b889bec8e3d4845

[gone, 2017-04-30 18:51:09]

Description was changed from

==========
❄ Split off RadioButtonLayout from InfoBarControls

Changes to the Infobar spec and how they lay out controls are coming.
Split off radio buttons into a separate layout that InfoBarControls
use and can be added by other layouts to prevent conflating changes
between them.

Screenshots:
https://drive.google.com/corp/drive/u/0/folders/0B7c8ZkXVwskDYkxiQWNwS1JzR2c

BUG=712836,712833
==========

to

==========
❄ Split off RadioButtonLayout from InfoBarControls

Changes to the Infobar spec and how they lay out controls are coming.
Split off radio buttons into a separate layout that InfoBarControls
use and can be added by other layouts to prevent conflating changes
between them.

Also fixes up how the promo dialog (which used the RadioButtons)
assigns widths to its children so that they always reach across the
whole layout.

Screenshots:
https://drive.google.com/corp/drive/u/0/folders/0B7c8ZkXVwskDYkxiQWNwS1JzR2c

BUG=712836,712833
==========

[gone, 2017-04-30 18:59:08]

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

[gone, 2017-04-30 20:20:28]

Description was changed from

==========
❄ Split off RadioButtonLayout from InfoBarControls

Changes to the Infobar spec and how they lay out controls are coming.
Split off radio buttons into a separate layout that InfoBarControls
use and can be added by other layouts to prevent conflating changes
between them.

Also fixes up how the promo dialog (which used the RadioButtons)
assigns widths to its children so that they always reach across the
whole layout.

Screenshots:
https://drive.google.com/corp/drive/u/0/folders/0B7c8ZkXVwskDYkxiQWNwS1JzR2c

BUG=712836,712833
==========

to

==========
❄ Split off RadioButtonLayout from InfoBarControls

Changes to the Infobar spec and how they lay out controls are coming.
Split off radio buttons into a separate layout that InfoBarControls
use and can be added by other layouts to prevent conflating changes
between them.

Also fixes up how the promo dialog (which used the RadioButtons)
assigns widths to some of its children.

Screenshots:
https://drive.google.com/corp/drive/u/0/folders/0B7c8ZkXVwskDYkxiQWNwS1JzR2c

BUG=712836,712833
==========

[gone, 2017-04-30 20:22:09]

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

[commit-bot: I haz the power, 2017-04-30 20:22:19]

Dry run: 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-04-30 21:25:21]

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

[commit-bot: I haz the power, 2017-04-30 21:25:22]

Dry run: This issue passed the CQ dry run.

[Theresa, 2017-05-01 15:21:09]

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

https://codereview.chromium.org/2850063002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/widget/RadioButtonLayout.java:67:
* Android doesn't provide a way of generating View IDs on the fly before API
level 17, so this
Android's method of generating View IDs could be copied into
ApiCompatibilityUtils. We generate view IDs for LayerTitleCache currently using
a very similar method.

This is okay, too, though if you'd prefer to use indexes over view ID's.

https://codereview.chromium.org/2850063002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/widget/RadioButtonLayout.java:87:
((MarginLayoutParams) child.getLayoutParams()).bottomMargin = margin;
Is it possible to fold this into the loop in addOptions()?

[gone, 2017-05-01 15:29:30]

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

https://codereview.chromium.org/2850063002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/widget/RadioButtonLayout.java:67:
* Android doesn't provide a way of generating View IDs on the fly before API
level 17, so this
On 2017/05/01 15:21:09, Theresa wrote:
> Android's method of generating View IDs could be copied into
> ApiCompatibilityUtils. We generate view IDs for LayerTitleCache currently
using
> a very similar method.
> 
> This is okay, too, though if you'd prefer to use indexes over view ID's.

Figured that kind of logic be used elsewhere, but it's used in at least a few
places now.  You think it's alright to save that for later?

https://codereview.chromium.org/2850063002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/widget/RadioButtonLayout.java:87:
((MarginLayoutParams) child.getLayoutParams()).bottomMargin = margin;
On 2017/05/01 15:21:09, Theresa wrote:
> Is it possible to fold this into the loop in addOptions()?

I had it there originally, but that doesn't work if (for some reason) someone
decides to add more options to the list one-by-one (see
RadioButtonLayoutTest#testMargins()).  If I called it setOptions() it'd work,
though.  WDYT?

[Theresa, 2017-05-01 15:43:08]

lgtm

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

https://codereview.chromium.org/2850063002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/widget/RadioButtonLayout.java:67:
* Android doesn't provide a way of generating View IDs on the fly before API
level 17, so this
On 2017/05/01 15:29:30, slow (dfalcantara) wrote:
> On 2017/05/01 15:21:09, Theresa wrote:
> > Android's method of generating View IDs could be copied into
> > ApiCompatibilityUtils. We generate view IDs for LayerTitleCache currently
> using
> > a very similar method.
> > 
> > This is okay, too, though if you'd prefer to use indexes over view ID's.
> 
> Figured that kind of logic be used elsewhere, but it's used in at least a few
> places now.  You think it's alright to save that for later?

Yes, fine to save for later. I think that we only generate view IDs in
LayerTitleCache -- if we ever generate in more than one place then we need to
centralize the logic.

https://codereview.chromium.org/2850063002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/widget/RadioButtonLayout.java:87:
((MarginLayoutParams) child.getLayoutParams()).bottomMargin = margin;
On 2017/05/01 15:29:30, slow (dfalcantara) wrote:
> On 2017/05/01 15:21:09, Theresa wrote:
> > Is it possible to fold this into the loop in addOptions()?
> 
> I had it there originally, but that doesn't work if (for some reason) someone
> decides to add more options to the list one-by-one (see
> RadioButtonLayoutTest#testMargins()).  If I called it setOptions() it'd work,
> though.  WDYT?

I think updateMargins() is a fine name. I wasn't thinking about that case, but
it makes sense.

[gone, 2017-05-01 22:32:06]

The CQ bit was checked by dfalcantara@chromium.org

[commit-bot: I haz the power, 2017-05-01 22:33:46]

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-05-01 23:53:33]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1493677926209890,
"parent_rev": "7fb9cb4281ac50effd7bdb628e6737fb2d518c4e", "commit_rev":
"2883e4221128cdc2b95aa765031ab77b9b4f5794"}

[commit-bot: I haz the power, 2017-05-01 23:53:44]

Description was changed from

==========
❄ Split off RadioButtonLayout from InfoBarControls

Changes to the Infobar spec and how they lay out controls are coming.
Split off radio buttons into a separate layout that InfoBarControls
use and can be added by other layouts to prevent conflating changes
between them.

Also fixes up how the promo dialog (which used the RadioButtons)
assigns widths to some of its children.

Screenshots:
https://drive.google.com/corp/drive/u/0/folders/0B7c8ZkXVwskDYkxiQWNwS1JzR2c

BUG=712836,712833
==========

to

==========
❄ Split off RadioButtonLayout from InfoBarControls

Changes to the Infobar spec and how they lay out controls are coming.
Split off radio buttons into a separate layout that InfoBarControls
use and can be added by other layouts to prevent conflating changes
between them.

Also fixes up how the promo dialog (which used the RadioButtons)
assigns widths to some of its children.

Screenshots:
https://drive.google.com/corp/drive/u/0/folders/0B7c8ZkXVwskDYkxiQWNwS1JzR2c

BUG=712836,712833

Review-Url: https://codereview.chromium.org/2850063002
Cr-Commit-Position: refs/heads/master@{#468494}
Committed:
https://chromium.googlesource.com/chromium/src/+/2883e4221128cdc2b95aa765031a...
==========

[commit-bot: I haz the power, 2017-05-01 23:53:46]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/2883e4221128cdc2b95aa765031a...

[aelias_OOO_until_Jul13, 2017-05-02 18:48:38]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/2855913002/ by aelias@chromium.org.

The reason for reverting is: Newly introduced test is failing on some builders,
see http://crbug.com/717651

BUG=717651.

[gone, 2017-05-02 19:07:27]

Description was changed from

==========
❄ Split off RadioButtonLayout from InfoBarControls

Changes to the Infobar spec and how they lay out controls are coming.
Split off radio buttons into a separate layout that InfoBarControls
use and can be added by other layouts to prevent conflating changes
between them.

Also fixes up how the promo dialog (which used the RadioButtons)
assigns widths to some of its children.

Screenshots:
https://drive.google.com/corp/drive/u/0/folders/0B7c8ZkXVwskDYkxiQWNwS1JzR2c

BUG=712836,712833

Review-Url: https://codereview.chromium.org/2850063002
Cr-Commit-Position: refs/heads/master@{#468494}
Committed:
https://chromium.googlesource.com/chromium/src/+/2883e4221128cdc2b95aa765031a...
==========

to

==========
❄ Split off RadioButtonLayout from InfoBarControls

Changes to the Infobar spec and how they lay out controls are coming.
Split off radio buttons into a separate layout that InfoBarControls
use and can be added by other layouts to prevent conflating changes
between them.

Also fixes up how the promo dialog (which used the RadioButtons)
assigns widths to some of its children.

Screenshots:
https://drive.google.com/corp/drive/u/0/folders/0B7c8ZkXVwskDYkxiQWNwS1JzR2c

BUG=712836,712833

Review-Url: https://codereview.chromium.org/2850063002
Cr-Commit-Position: refs/heads/master@{#468494}
Committed:
https://chromium.googlesource.com/chromium/src/+/2883e4221128cdc2b95aa765031a...
==========

[gone, 2017-05-02 19:09:49]

Description was changed from

==========
❄ Split off RadioButtonLayout from InfoBarControls

Changes to the Infobar spec and how they lay out controls are coming.
Split off radio buttons into a separate layout that InfoBarControls
use and can be added by other layouts to prevent conflating changes
between them.

Also fixes up how the promo dialog (which used the RadioButtons)
assigns widths to some of its children.

Screenshots:
https://drive.google.com/corp/drive/u/0/folders/0B7c8ZkXVwskDYkxiQWNwS1JzR2c

BUG=712836,712833

Review-Url: https://codereview.chromium.org/2850063002
Cr-Commit-Position: refs/heads/master@{#468494}
Committed:
https://chromium.googlesource.com/chromium/src/+/2883e4221128cdc2b95aa765031a...
==========

to

==========
❄ Split off RadioButtonLayout from InfoBarControls

Changes to the Infobar spec and how they lay out controls are coming.
Split off radio buttons into a separate layout that InfoBarControls
use and can be added by other layouts to prevent conflating changes
between them.

Also fixes up how the promo dialog (which used the RadioButtons)
assigns widths to some of its children.

Screenshots:
https://drive.google.com/corp/drive/u/0/folders/0B7c8ZkXVwskDYkxiQWNwS1JzR2c

BUG=712836,712833,717651

Review-Url: https://codereview.chromium.org/2850063002
Cr-Commit-Position: refs/heads/master@{#468494}
Committed:
https://chromium.googlesource.com/chromium/src/+/2883e4221128cdc2b95aa765031a...
==========

[gone, 2017-05-02 19:10:23]

The CQ bit was checked by dfalcantara@chromium.org

[gone, 2017-05-02 19:10:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
twellington@chromium.org
Link to the patchset: https://codereview.chromium.org/2850063002/#ps80001
(title: "Re-landing")

[commit-bot: I haz the power, 2017-05-02 19:11:10]

CQ is trying da patch.

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

[gone, 2017-05-02 19:26:58]

The CQ bit was checked by dfalcantara@chromium.org

[gone, 2017-05-02 19:26:59]

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

[commit-bot: I haz the power, 2017-05-02 19:27:39]

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-05-02 20:28:39]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1493753218917420,
"parent_rev": "25188be9b29b34ae70d5f380c5eeefeeee3eb845", "commit_rev":
"3f5e8d10f9ab29fee3f8b02c4b889bec8e3d4845"}

[commit-bot: I haz the power, 2017-05-02 20:28:49]

Description was changed from

==========
❄ Split off RadioButtonLayout from InfoBarControls

Changes to the Infobar spec and how they lay out controls are coming.
Split off radio buttons into a separate layout that InfoBarControls
use and can be added by other layouts to prevent conflating changes
between them.

Also fixes up how the promo dialog (which used the RadioButtons)
assigns widths to some of its children.

Screenshots:
https://drive.google.com/corp/drive/u/0/folders/0B7c8ZkXVwskDYkxiQWNwS1JzR2c

BUG=712836,712833,717651

Review-Url: https://codereview.chromium.org/2850063002
Cr-Commit-Position: refs/heads/master@{#468494}
Committed:
https://chromium.googlesource.com/chromium/src/+/2883e4221128cdc2b95aa765031a...
==========

to

==========
❄ Split off RadioButtonLayout from InfoBarControls

Changes to the Infobar spec and how they lay out controls are coming.
Split off radio buttons into a separate layout that InfoBarControls
use and can be added by other layouts to prevent conflating changes
between them.

Also fixes up how the promo dialog (which used the RadioButtons)
assigns widths to some of its children.

Screenshots:
https://drive.google.com/corp/drive/u/0/folders/0B7c8ZkXVwskDYkxiQWNwS1JzR2c

BUG=712836,712833,717651

Review-Url: https://codereview.chromium.org/2850063002
Cr-Original-Commit-Position: refs/heads/master@{#468494}
Committed:
https://chromium.googlesource.com/chromium/src/+/2883e4221128cdc2b95aa765031a...
Review-Url: https://codereview.chromium.org/2850063002
Cr-Commit-Position: refs/heads/master@{#468763}
Committed:
https://chromium.googlesource.com/chromium/src/+/3f5e8d10f9ab29fee3f8b02c4b88...
==========

[commit-bot: I haz the power, 2017-05-02 20:28:50]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/3f5e8d10f9ab29fee3f8b02c4b88...