Allow the component updater framework to control Origin Trials.

Allow the component updater framework to control Origin Trials.

This patch adds support for updating the public key for verifying origin trial token signatures through the component updater. Future patches are planned to add support for disabling individual experimental APIs and for revoking individual trial tokens. This patch lays the groundwork for these, but does not actually provide those extra features.

The component is not required to be present in order for origin trials to work (the default key will be used in that case), but if it is, it should include the public key in its manifest as a base64-encoded string representing 32 bytes, like this:

	"origin-trials": {
		"public-key": "abcdefghjijklmnopqrstuvwxabcdefghjijklmnopq="
	}

BUG=603588
Committed: https://crrev.com/99c766f3508de3e9b3c629aedb2b95a97afa3880
Cr-Commit-Position: refs/heads/master@{#390636}

[iclelland, 2016-04-13 20:36:56]

Description was changed from

==========
Allow the component updater framework to control Origin Trials.

BUG=
==========

to

==========
Allow the component updater framework to control Origin Trials.

BUG=
==========

[iclelland, 2016-04-13 20:36:56]

iclelland@chromium.org changed reviewers:
+ sorin@chromium.org, waffles@chromium.org

[iclelland, 2016-04-13 20:37:47]

On 2016/04/13 20:36:56, iclelland wrote:
> mailto:iclelland@chromium.org changed reviewers:
> + mailto:sorin@chromium.org, mailto:waffles@chromium.org

+r waffles, sorin -- I've taken a first stab at integrating origin trials with
Omaha; can you take a look and tell me if I'm going in the right direction here?

Thanks!

[Sorin Jianu, 2016-04-13 21:17:47]

lgtm

Thank you for sending this out!

Please create a crbug and associate it with this change.

https://codereview.chromium.org/1887743003/diff/1/chrome/browser/component_up...
File chrome/browser/component_updater/origin_trials_component_installer.cc
(right):

https://codereview.chromium.org/1887743003/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/origin_trials_component_installer.cc:63: return
true;  // nothing to do!
Comment is not needed. Please start with capital letter if the decision is to
keep the comment anyway.

https://codereview.chromium.org/1887743003/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/origin_trials_component_installer.cc:98: // No
additional parameters required; return empty string.
Comment is not needed, especially the part that follows after ;.

https://codereview.chromium.org/1887743003/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/origin_trials_component_installer.cc:104:
VLOG(1) << "Registering origin trials component.";
Maybe use a DVLOG if this is needed for debugging, otherwise, our opinion is
that the VLOG here provides little value.

https://codereview.chromium.org/1887743003/diff/1/chrome/browser/component_up...
File chrome/browser/component_updater/origin_trials_component_installer.h
(right):

https://codereview.chromium.org/1887743003/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/origin_trials_component_installer.h:17: class
OriginTrialsComponentInstallerTraits : public ComponentInstallerTraits {
use private to declare the appropriate access specifier instead of relying on
default.

https://codereview.chromium.org/1887743003/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/origin_trials_component_installer.h:39: }  //
namespace component_updater
most likely, it needs an empty line after.

Also, if not already doing so, please use git cl lint and git cl format for this
CL.

[iclelland, 2016-04-14 14:30:08]

Thanks, Sorin!

https://codereview.chromium.org/1887743003/diff/1/chrome/browser/component_up...
File chrome/browser/component_updater/origin_trials_component_installer.cc
(right):

https://codereview.chromium.org/1887743003/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/origin_trials_component_installer.cc:63: return
true;  // nothing to do!
On 2016/04/13 21:17:47, Sorin Jianu wrote:
> Comment is not needed. Please start with capital letter if the decision is to
> keep the comment anyway.

Removed.

https://codereview.chromium.org/1887743003/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/origin_trials_component_installer.cc:98: // No
additional parameters required; return empty string.
On 2016/04/13 21:17:47, Sorin Jianu wrote:
> Comment is not needed, especially the part that follows after ;.

Removed.

https://codereview.chromium.org/1887743003/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/origin_trials_component_installer.cc:104:
VLOG(1) << "Registering origin trials component.";
On 2016/04/13 21:17:47, Sorin Jianu wrote:
> Maybe use a DVLOG if this is needed for debugging, otherwise, our opinion is
> that the VLOG here provides little value.

Also removed.

https://codereview.chromium.org/1887743003/diff/1/chrome/browser/component_up...
File chrome/browser/component_updater/origin_trials_component_installer.h
(right):

https://codereview.chromium.org/1887743003/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/origin_trials_component_installer.h:17: class
OriginTrialsComponentInstallerTraits : public ComponentInstallerTraits {
On 2016/04/13 21:17:47, Sorin Jianu wrote:
> use private to declare the appropriate access specifier instead of relying on
> default.

Yes, done -- thanks. The code was in a pretty rough shape, but I appreciate all
the early feedback!

https://codereview.chromium.org/1887743003/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/origin_trials_component_installer.h:39: }  //
namespace component_updater
On 2016/04/13 21:17:47, Sorin Jianu wrote:
> most likely, it needs an empty line after.
> 
> Also, if not already doing so, please use git cl lint and git cl format for
this
> CL.

Done -- though, oddly, neither lint nor format picked up on that.

[iclelland, 2016-04-14 15:31:46]

Description was changed from

==========
Allow the component updater framework to control Origin Trials.

BUG=
==========

to

==========
Allow the component updater framework to control Origin Trials.

BUG=603588
==========

[Sorin Jianu, 2016-04-14 16:23:57]

lgtm

Thank you!

[iclelland, 2016-04-15 19:35:27]

iclelland@chromium.org changed reviewers:
+ chasej@chromium.org, cpu@chromium.org

[iclelland, 2016-04-15 19:35:27]

+r chasej for all of it
+r cpu for chrome/browser/chrome_browser_main.cc specifically, though I see you
have ownership in component_updater as well.


Thanks!

[waffles, 2016-04-15 19:42:21]

lgtm

As we discussed offline, there might be a change to kSha256Hash depending on how
exactly you work on the key management story on the server.

[chasej, 2016-04-28 13:44:55]

lgtm

[iclelland, 2016-04-28 14:19:07]

Description was changed from

==========
Allow the component updater framework to control Origin Trials.

BUG=603588
==========

to

==========
Allow the component updater framework to control Origin Trials.

This patch adds support for updating the public key for verifying origin trial
token signatures through the component updater. Future patches are planned to
add support for disabling individual experimental APIs and for revoking
individual trial tokens. This patch lays the groundwork for these, but does not
actually provide those extra features.

The component is not required to be present in order for origin trials to work
(the default key will be used in that case), but if it is, it should include the
public key as a base64-encoded string representing 32 bytes, like this:

	"origin-trials": {
		"public-key": "abcdefghjijklmnopqrstuvwxabcdefghjijklmnopq="
	}

BUG=603588
==========

[iclelland, 2016-04-28 14:37:48]

Description was changed from

==========
Allow the component updater framework to control Origin Trials.

This patch adds support for updating the public key for verifying origin trial
token signatures through the component updater. Future patches are planned to
add support for disabling individual experimental APIs and for revoking
individual trial tokens. This patch lays the groundwork for these, but does not
actually provide those extra features.

The component is not required to be present in order for origin trials to work
(the default key will be used in that case), but if it is, it should include the
public key as a base64-encoded string representing 32 bytes, like this:

	"origin-trials": {
		"public-key": "abcdefghjijklmnopqrstuvwxabcdefghjijklmnopq="
	}

BUG=603588
==========

to

==========
Allow the component updater framework to control Origin Trials.

This patch adds support for updating the public key for verifying origin trial
token signatures through the component updater. Future patches are planned to
add support for disabling individual experimental APIs and for revoking
individual trial tokens. This patch lays the groundwork for these, but does not
actually provide those extra features.

The component is not required to be present in order for origin trials to work
(the default key will be used in that case), but if it is, it should include the
public key in its manifest as a base64-encoded string representing 32 bytes,
like this:

	"origin-trials": {
		"public-key": "abcdefghjijklmnopqrstuvwxabcdefghjijklmnopq="
	}

BUG=603588
==========

[iclelland, 2016-04-28 14:39:07]

cpu -- ping :)

This still needs an OWNERS stamp for chrome_browser_main.cc, thanks!

[Sorin Jianu, 2016-04-28 16:49:10]

sorin@chromium.org changed reviewers:
+ sky@chromium.org
- cpu@chromium.org

[Sorin Jianu, 2016-04-28 16:49:11]

sky@, could you please review the changes under chrome/ ?

cpu@ is not really with the project at the moment.

[iclelland, 2016-04-28 17:35:06]

On 2016/04/28 16:49:11, Sorin Jianu wrote:
> sky@, could you please review the changes under chrome/ ?
> 
> cpu@ is not really with the project at the moment.

Thanks, Sorin -- OWNERS led me to believe that he might be particularly
appropriate for this CL, but these things can be misleading :)

[sky, 2016-04-29 00:26:18]

chrome LGTM

[iclelland, 2016-04-29 13:20:45]

The CQ bit was checked by iclelland@chromium.org

[iclelland, 2016-04-29 13:20:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
waffles@chromium.org, sorin@chromium.org, chasej@chromium.org
Link to the patchset: https://codereview.chromium.org/1887743003/#ps40001
(title: "Rebase and fix missed scoped_ptr -> unique_ptr conversion")

[commit-bot: I haz the power, 2016-04-29 13:20:57]

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

[commit-bot: I haz the power, 2016-04-29 13:24:53]

Description was changed from

==========
Allow the component updater framework to control Origin Trials.

This patch adds support for updating the public key for verifying origin trial
token signatures through the component updater. Future patches are planned to
add support for disabling individual experimental APIs and for revoking
individual trial tokens. This patch lays the groundwork for these, but does not
actually provide those extra features.

The component is not required to be present in order for origin trials to work
(the default key will be used in that case), but if it is, it should include the
public key in its manifest as a base64-encoded string representing 32 bytes,
like this:

	"origin-trials": {
		"public-key": "abcdefghjijklmnopqrstuvwxabcdefghjijklmnopq="
	}

BUG=603588
==========

to

==========
Allow the component updater framework to control Origin Trials.

This patch adds support for updating the public key for verifying origin trial
token signatures through the component updater. Future patches are planned to
add support for disabling individual experimental APIs and for revoking
individual trial tokens. This patch lays the groundwork for these, but does not
actually provide those extra features.

The component is not required to be present in order for origin trials to work
(the default key will be used in that case), but if it is, it should include the
public key in its manifest as a base64-encoded string representing 32 bytes,
like this:

	"origin-trials": {
		"public-key": "abcdefghjijklmnopqrstuvwxabcdefghjijklmnopq="
	}

BUG=603588
==========

[commit-bot: I haz the power, 2016-04-29 13:24:54]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-30 17:25:31]

Patchset 3 (id:??) landed as
https://crrev.com/99c766f3508de3e9b3c629aedb2b95a97afa3880
Cr-Commit-Position: refs/heads/master@{#390636}

[commit-bot: I haz the power, 2016-04-30 18:00:26]

Description was changed from

==========
Allow the component updater framework to control Origin Trials.

This patch adds support for updating the public key for verifying origin trial
token signatures through the component updater. Future patches are planned to
add support for disabling individual experimental APIs and for revoking
individual trial tokens. This patch lays the groundwork for these, but does not
actually provide those extra features.

The component is not required to be present in order for origin trials to work
(the default key will be used in that case), but if it is, it should include the
public key in its manifest as a base64-encoded string representing 32 bytes,
like this:

	"origin-trials": {
		"public-key": "abcdefghjijklmnopqrstuvwxabcdefghjijklmnopq="
	}

BUG=603588
==========

to

==========
Allow the component updater framework to control Origin Trials.

This patch adds support for updating the public key for verifying origin trial
token signatures through the component updater. Future patches are planned to
add support for disabling individual experimental APIs and for revoking
individual trial tokens. This patch lays the groundwork for these, but does not
actually provide those extra features.

The component is not required to be present in order for origin trials to work
(the default key will be used in that case), but if it is, it should include the
public key in its manifest as a base64-encoded string representing 32 bytes,
like this:

	"origin-trials": {
		"public-key": "abcdefghjijklmnopqrstuvwxabcdefghjijklmnopq="
	}

BUG=603588

Committed: https://crrev.com/99c766f3508de3e9b3c629aedb2b95a97afa3880
Cr-Commit-Position: refs/heads/master@{#390636}
==========