[Android] Add an about flag for Download Manager UI

[Android] Add an about flag for Download Manager UI

Download manager should be configured via about flags. This CL
implements it. Also it makes download manager a feature, which can be
controlled for launch purposes.

BUG=616324
Committed: https://crrev.com/c63ffb9ee105792edfa8a876c0f5de03c4e6db35
Cr-Commit-Position: refs/heads/master@{#411183}

[Ian Wen, 2016-08-09 20:58:16]

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

[Ian Wen, 2016-08-09 20:58:17]

dfalcantara@chromium.org: Please review changes in everything

isherman@chromium.org: Please review changes in field trial

[gone, 2016-08-09 21:00:30]

lgtm

https://codereview.chromium.org/2226733004/diff/40001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2226733004/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:15110: +      Enable downloads manager ui in
the app menu.
nit: UI

https://codereview.chromium.org/2226733004/diff/40001/chrome/browser/about_fl...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/2226733004/diff/40001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:2055: // NOTE: Adding new command-line switches
requires adding corresponding
nit: Remove extra space

https://codereview.chromium.org/2226733004/diff/40001/chrome/browser/android/...
File chrome/browser/android/chrome_feature_list.cc (right):

https://codereview.chromium.org/2226733004/diff/40001/chrome/browser/android/...
chrome/browser/android/chrome_feature_list.cc:63:
base::FEATURE_DISABLED_BY_DEFAULT};
ENABLED_BY_DEFAULT?

[Ian Wen, 2016-08-09 21:27:50]

Thanks for the fast review! All addressed.

isherman@ ptal :)

[Ilya Sherman, 2016-08-10 05:59:51]

https://codereview.chromium.org/2226733004/diff/60001/testing/variations/fiel...
File testing/variations/fieldtrial_testing_config_android.json (right):

https://codereview.chromium.org/2226733004/diff/60001/testing/variations/fiel...
testing/variations/fieldtrial_testing_config_android.json:136: ],
Hmm, could you please email/IM me with a link to the google3 config for this
field trial?  If you are not currently controlling this field trial server-side,
I don't think there's a need to add this kind of test coverage for the field
trial control.

FWIW, if you do need to add test coverage, you'll need to specify
disable_features.

[Ian Wen, 2016-08-10 07:01:05]

On 2016/08/10 05:59:51, Ilya Sherman wrote:
>
https://codereview.chromium.org/2226733004/diff/60001/testing/variations/fiel...
> File testing/variations/fieldtrial_testing_config_android.json (right):
> 
>
https://codereview.chromium.org/2226733004/diff/60001/testing/variations/fiel...
> testing/variations/fieldtrial_testing_config_android.json:136: ],
> Hmm, could you please email/IM me with a link to the google3 config for this
> field trial?  If you are not currently controlling this field trial
server-side,
> I don't think there's a need to add this kind of test coverage for the field
> trial control.
> 
> FWIW, if you do need to add test coverage, you'll need to specify
> disable_features.

Done.

Ah good catch.

We will run this experiment on M54, so I think I have to include this in the
test config anyway. Otherwise in the server side CL the presubmit bots will
complain. I haven't drafted a server side CL yet.

[Ilya Sherman, 2016-08-10 19:42:57]

On 2016/08/10 07:01:05, Ian Wen wrote:
> We will run this experiment on M54, so I think I have to include this in the
> test config anyway. Otherwise in the server side CL the presubmit bots will
> complain. I haven't drafted a server side CL yet.

Okay, so: What configuration of the feature do you plan to ship -- enabled or
disabled?  Is this a new feature, or the deprecation of an existing feature?  If
this is a new feature that you're planning to experiment with using Finch, then
you probably want to make the feature disabled by default, and specify a test
config for the Enabled group.

[Ian Wen, 2016-08-10 20:14:36]

On 2016/08/10 19:42:57, Ilya Sherman wrote:
> On 2016/08/10 07:01:05, Ian Wen wrote:
> > We will run this experiment on M54, so I think I have to include this in the
> > test config anyway. Otherwise in the server side CL the presubmit bots will
> > complain. I haven't drafted a server side CL yet.
> 
> Okay, so: What configuration of the feature do you plan to ship -- enabled or
> disabled?  Is this a new feature, or the deprecation of an existing feature? 
If
> this is a new feature that you're planning to experiment with using Finch,
then
> you probably want to make the feature disabled by default, and specify a test
> config for the Enabled group.

Agreed. I flipped the enable/disable bit. PTAL :)

[Ian Wen, 2016-08-10 20:16:10]

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

[commit-bot: I haz the power, 2016-08-10 20:17:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ilya Sherman, 2016-08-10 20:49:37]

field trials lgtm, thanks

[commit-bot: I haz the power, 2016-08-10 23:13:41]

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

[commit-bot: I haz the power, 2016-08-10 23:13:42]

Dry run: This issue passed the CQ dry run.

[Ian Wen, 2016-08-10 23:14:03]

The CQ bit was checked by ianwen@chromium.org

[Ian Wen, 2016-08-10 23:14:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org
Link to the patchset:
https://chromiumcodereview.appspot.com/2226733004/#ps100001 (title:
"enable_by_default -> disable_by_default")

[commit-bot: I haz the power, 2016-08-10 23:15:06]

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, 2016-08-10 23:35:28]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-08-10 23:39:52]

Description was changed from

==========
[Android] Add an about flag for Download Manager UI

Download manager should be configured via about flags. This CL
implements it. Also it makes download manager a feature, which can be
controlled for launch purposes.

BUG=616324
==========

to

==========
[Android] Add an about flag for Download Manager UI

Download manager should be configured via about flags. This CL
implements it. Also it makes download manager a feature, which can be
controlled for launch purposes.

BUG=616324

Committed: https://crrev.com/c63ffb9ee105792edfa8a876c0f5de03c4e6db35
Cr-Commit-Position: refs/heads/master@{#411183}
==========

[commit-bot: I haz the power, 2016-08-10 23:39:53]

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