Skeleton mechanical impl. for the RecoveryImprovedComponent.

Skeleton mechanical impl. for the RecoveryImprovedComponent.

* defines a command line argument --enable-features=ImprovedRecoveryComponent" and a Finch experiment.
* uses the default component installer
* does nothing otherwise.

BUG=687231
Review-Url: https://codereview.chromium.org/2661113003
Cr-Commit-Position: refs/heads/master@{#449139}
Committed: https://chromium.googlesource.com/chromium/src/+/6ddc3bbb1b2744d8fc8760767f33df763e6b77a0

[Sorin Jianu, 2017-01-31 17:44:30]

The CQ bit was checked by [email protected] to run a CQ dry run

[commit-bot: I haz the power, 2017-01-31 17:44:56]

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-01-31 17:53:20]

The CQ bit was unchecked by [email protected]

[commit-bot: I haz the power, 2017-01-31 17:53:20]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[Sorin Jianu, 2017-01-31 18:17:21]

The CQ bit was checked by [email protected] to run a CQ dry run

[commit-bot: I haz the power, 2017-01-31 18:18:07]

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-01-31 19:20:59]

The CQ bit was unchecked by [email protected]

[commit-bot: I haz the power, 2017-01-31 19:20:59]

Dry run: This issue passed the CQ dry run.

[Sorin Jianu, 2017-01-31 20:59:51]

The CQ bit was checked by [email protected] to run a CQ dry run

[commit-bot: I haz the power, 2017-01-31 21:00:44]

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

[Sorin Jianu, 2017-01-31 21:02:21]

The CQ bit was checked by [email protected] to run a CQ dry run

[commit-bot: I haz the power, 2017-01-31 21:03:10]

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

[Sorin Jianu, 2017-01-31 21:16:34]

The CQ bit was checked by [email protected] to run a CQ dry run

[Sorin Jianu, 2017-01-31 21:17:14]

[email protected] changed reviewers:
+ [email protected]

[Sorin Jianu, 2017-01-31 21:17:14]

PTAL thank you!

[commit-bot: I haz the power, 2017-01-31 21:17:16]

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

[waffles, 2017-01-31 21:34:40]

lgtm

https://codereview.chromium.org/2661113003/diff/60001/chrome/browser/componen...
File chrome/browser/component_updater/recovery_improved_component_installer.cc
(right):

https://codereview.chromium.org/2661113003/diff/60001/chrome/browser/componen...
chrome/browser/component_updater/recovery_improved_component_installer.cc:19:
const char kRecoveryImprovedManifestName[] = "ChromeRecovery";
A glance at chrome://components indicates most components are using "Capitals
with Spaces" format.

[Sorin Jianu, 2017-01-31 21:44:16]

The CQ bit was checked by [email protected] to run a CQ dry run

[commit-bot: I haz the power, 2017-01-31 21:45:12]

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

[Sorin Jianu, 2017-01-31 21:50:44]

[email protected] changed reviewers:
+ [email protected], [email protected]

[Sorin Jianu, 2017-01-31 21:50:44]

hi, 
Sylvain, please I need an approval for the ios changes.

Devlin, I need an approval for extensions changes too, thank you!

[Devlin, 2017-01-31 22:14:22]

On 2017/01/31 21:50:44, Sorin Jianu wrote:
> hi, 
> Sylvain, please I need an approval for the ios changes.
> 
> Devlin, I need an approval for extensions changes too, thank you!

Can we add a bit more context in the bug?  e.g. a description, summary, or
design doc?  Right now, there's very little information available.

[Sorin Jianu, 2017-01-31 22:19:49]

On 2017/01/31 22:14:22, Devlin wrote:
> On 2017/01/31 21:50:44, Sorin Jianu wrote:
> > hi, 
> > Sylvain, please I need an approval for the ios changes.
> > 
> > Devlin, I need an approval for extensions changes too, thank you!
> 
> Can we add a bit more context in the bug?  e.g. a description, summary, or
> design doc?  Right now, there's very little information available.

Thank you, details added.

[Devlin, 2017-01-31 23:48:09]

extensions lgtm

[commit-bot: I haz the power, 2017-02-01 00:00:08]

The CQ bit was unchecked by [email protected]

[commit-bot: I haz the power, 2017-02-01 00:00:14]

Dry run: This issue passed the CQ dry run.

[sdefresne, 2017-02-01 10:42:21]

lgtm for ios/

[Sorin Jianu, 2017-02-07 23:32:33]

The CQ bit was checked by [email protected] to run a CQ dry run

[commit-bot: I haz the power, 2017-02-07 23:33:56]

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-02-08 00:46:57]

The CQ bit was unchecked by [email protected]

[commit-bot: I haz the power, 2017-02-08 00:46:58]

Dry run: This issue passed the CQ dry run.

[Sorin Jianu, 2017-02-08 01:19:39]

Description was changed from

==========
Skeleton mechanical impl. for the RecoveryImprovedComponent.

* defines a command line argument "enable-improved-recovery"
* uses the default component installer
* does nothing otherwise.

BUG=687231
==========

to

==========
Skeleton mechanical impl. for the RecoveryImprovedComponent.

* defines a command line argument "enable-improved-recovery"
* uses the default component installer
* does nothing otherwise.

BUG=687231
==========

[Sorin Jianu, 2017-02-08 01:19:39]

[email protected] changed reviewers:
- [email protected], [email protected]

[Sorin Jianu, 2017-02-08 01:33:48]

PTAL, thank you!

[waffles, 2017-02-08 01:34:55]

lgtm

[Sorin Jianu, 2017-02-08 01:35:54]

Description was changed from

==========
Skeleton mechanical impl. for the RecoveryImprovedComponent.

* defines a command line argument "enable-improved-recovery"
* uses the default component installer
* does nothing otherwise.

BUG=687231
==========

to

==========
Skeleton mechanical impl. for the RecoveryImprovedComponent.

* defines a command line argument --enable-features=ImprovedRecoveryComponent"
and a Finch experiment.
* uses the default component installer
* does nothing otherwise.

BUG=687231
==========

[Sorin Jianu, 2017-02-08 01:38:33]

The CQ bit was checked by [email protected] to run a CQ dry run

[commit-bot: I haz the power, 2017-02-08 01:39:52]

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

[Sorin Jianu, 2017-02-08 01:55:00]

[email protected] changed reviewers:
+ [email protected], [email protected]

[Sorin Jianu, 2017-02-08 01:57:10]

Robert, can I please have an owners review for the changes to
testing/variations/fieldtrial_testing_config.json ?

Scott, can I please have an owners approval for the changes under chrome/common
?

Thank you!

[commit-bot: I haz the power, 2017-02-08 03:03:43]

The CQ bit was unchecked by [email protected]

[commit-bot: I haz the power, 2017-02-08 03:03:44]

Dry run: This issue passed the CQ dry run.

[rkaplow, 2017-02-08 15:30:54]

lgtm

[sky, 2017-02-08 17:12:47]

Naming suggesiton.

https://codereview.chromium.org/2661113003/diff/160001/chrome/common/chrome_f...
File chrome/common/chrome_features.cc (right):

https://codereview.chromium.org/2661113003/diff/160001/chrome/common/chrome_f...
chrome/common/chrome_features.cc:112: "ImprovedRecoveryComponent",
base::FEATURE_DISABLED_BY_DEFAULT};
Did you consider naming this "FixUpdater"? "ImprovedRecoveryComponent" is
confusing and doesn't convey what it does.

[Sorin Jianu, 2017-02-08 17:33:33]

On 2017/02/08 17:12:47, sky wrote:
> Naming suggesiton.
> 
>
https://codereview.chromium.org/2661113003/diff/160001/chrome/common/chrome_f...
> File chrome/common/chrome_features.cc (right):
> 
>
https://codereview.chromium.org/2661113003/diff/160001/chrome/common/chrome_f...
> chrome/common/chrome_features.cc:112: "ImprovedRecoveryComponent",
> base::FEATURE_DISABLED_BY_DEFAULT};
> Did you consider naming this "FixUpdater"? "ImprovedRecoveryComponent" is
> confusing and doesn't convey what it does.

Thank you for the suggestion. There is already a recovery component in place, it
has been there since M16.

The plan here is to create a new recovery component, do an A/B, then remove the
old recovery component code when we are confident, and rename
RecoveryImprovedComponent to RecoveryComponent.

[Sorin Jianu, 2017-02-08 21:16:08]

On 2017/02/08 17:33:33, Sorin Jianu wrote:
> On 2017/02/08 17:12:47, sky wrote:
> > Naming suggesiton.
> > 
> >
>
https://codereview.chromium.org/2661113003/diff/160001/chrome/common/chrome_f...
> > File chrome/common/chrome_features.cc (right):
> > 
> >
>
https://codereview.chromium.org/2661113003/diff/160001/chrome/common/chrome_f...
> > chrome/common/chrome_features.cc:112: "ImprovedRecoveryComponent",
> > base::FEATURE_DISABLED_BY_DEFAULT};
> > Did you consider naming this "FixUpdater"? "ImprovedRecoveryComponent" is
> > confusing and doesn't convey what it does.
> 
> Thank you for the suggestion. There is already a recovery component in place,
it
> has been there since M16.
> 
> The plan here is to create a new recovery component, do an A/B, then remove
the
> old recovery component code when we are confident, and rename
> RecoveryImprovedComponent to RecoveryComponent.

Scott, ok with landing like this? How strong is the suggestion to rename the cmd
line argument in this case?

[sky, 2017-02-08 23:22:58]

I'm ok with landing, LGTM

[Sorin Jianu, 2017-02-08 23:25:32]

The CQ bit was checked by [email protected]

[Sorin Jianu, 2017-02-08 23:25:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
[email protected], [email protected]
Link to the patchset: https://codereview.chromium.org/2661113003/#ps160001
(title: "wip")

[commit-bot: I haz the power, 2017-02-08 23:25:59]

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-02-08 23:35:24]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1486596332478290,
"parent_rev": "96b843574c741967951a158ccf924b453c3f301a", "commit_rev":
"6ddc3bbb1b2744d8fc8760767f33df763e6b77a0"}

[commit-bot: I haz the power, 2017-02-08 23:36:41]

Description was changed from

==========
Skeleton mechanical impl. for the RecoveryImprovedComponent.

* defines a command line argument --enable-features=ImprovedRecoveryComponent"
and a Finch experiment.
* uses the default component installer
* does nothing otherwise.

BUG=687231
==========

to

==========
Skeleton mechanical impl. for the RecoveryImprovedComponent.

* defines a command line argument --enable-features=ImprovedRecoveryComponent"
and a Finch experiment.
* uses the default component installer
* does nothing otherwise.

BUG=687231

Review-Url: https://codereview.chromium.org/2661113003
Cr-Commit-Position: refs/heads/master@{#449139}
Committed:
https://chromium.googlesource.com/chromium/src/+/6ddc3bbb1b2744d8fc8760767f33...
==========

[commit-bot: I haz the power, 2017-02-08 23:36:43]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/6ddc3bbb1b2744d8fc8760767f33...