First implementation of chrome.platformKeys.

First implementation of chrome.platformKeys.

This change adds the selectClientCertificates() and subtleCrypto() functions and is missing the getKeyPair() function.
It is also missing the certificate permissions per extension and any UI changes.

BUG=450167
Committed: https://crrev.com/f813693baf707fd30d0e25eebaf0da6c08f74acc
Cr-Commit-Position: refs/heads/master@{#314625}

[pneubeck (no reviews), 2015-01-27 13:43:39]

Patchset #1 (id:1) has been deleted

[pneubeck (no reviews), 2015-01-27 13:51:10]

Patchset #1 (id:20001) has been deleted

[pneubeck (no reviews), 2015-01-27 14:37:30]

Patchset #1 (id:40001) has been deleted

[pneubeck (no reviews), 2015-01-27 15:41:54]

Patchset #2 (id:80001) has been deleted

[pneubeck (no reviews), 2015-01-27 15:43:02]

Patchset #2 (id:100001) has been deleted

[pneubeck (no reviews), 2015-01-27 15:43:09]

Patchset #1 (id:60001) has been deleted

[pneubeck (no reviews), 2015-01-27 15:43:13]

Patchset #1 (id:60001) has been deleted

[pneubeck (no reviews), 2015-01-27 15:50:52]

pneubeck@chromium.org changed reviewers:
+ arv@chromium.org, asvitkine@chromium.org, kalman@chromium.org,
rsleevi@chromium.org

[pneubeck (no reviews), 2015-01-27 15:50:53]

@Ryan, FYI

asvitkine@chromium.org: Please review the histogram change.
  extensions/browser/extension_function_histogram_value.h
  tools/metrics/histograms/histograms.xml

arv@chromium.org: Please review the resource change.
  chrome/renderer/resources/renderer_resources.grd

@Benjamin, please review the extensions// changes.

@Eric, please review the implementation in
  chrome/browser/chromeos/platform_keys/

Thanks!

[Alexei Svitkine (slow), 2015-01-27 16:07:12]

histograms lgtm

[not at google - send to devlin, 2015-01-27 16:41:30]

Extensions infrastructure lgtm (pending discussion on other bug, but I don't
expect that to change). However, I haven't looked deeply at the implementation
of the API. I can review a small change like that, however,
- add an OWNERS file
- ideally, have a 2nd owner who can review patches to this API, as I am not a
platform keys expert.

https://codereview.chromium.org/875373002/diff/140001/chrome/common/extension...
File chrome/common/extensions/api/_permission_features.json (right):

https://codereview.chromium.org/875373002/diff/140001/chrome/common/extension...
chrome/common/extensions/api/_permission_features.json:797: "extension_types":
["extension", "legacy_packaged_app", "platform_app"]
No "legacy_packaged_app"

[pneubeck (no reviews), 2015-01-28 09:29:51]

pneubeck@chromium.org changed reviewers:
+ eroman@chromium.org

[pneubeck (no reviews), 2015-01-28 09:29:52]

@Eric, please take a look at the implementation in
  chrome/browser/chromeos/platform_keys/

[pneubeck (no reviews), 2015-01-28 10:37:36]

Alexei, ptal at the PermissionMessage change which I also had to add in
histograms.xml.

[Alexei Svitkine (slow), 2015-01-28 16:11:34]

lgtm

[pneubeck (no reviews), 2015-01-29 10:37:08]

https://codereview.chromium.org/875373002/diff/140001/chrome/common/extension...
File chrome/common/extensions/api/_permission_features.json (right):

https://codereview.chromium.org/875373002/diff/140001/chrome/common/extension...
chrome/common/extensions/api/_permission_features.json:797: "extension_types":
["extension", "legacy_packaged_app", "platform_app"]
On 2015/01/27 16:41:30, kalman wrote:
> No "legacy_packaged_app"

Done.

[pneubeck (no reviews), 2015-01-29 13:47:48]

On 2015/01/27 16:41:30, kalman wrote:
> Extensions infrastructure lgtm (pending discussion on other bug, but I don't
> expect that to change). However, I haven't looked deeply at the implementation
> of the API. I can review a small change like that, however,
> - add an OWNERS file
I have OWNERS files in chrome/browser/chromeos/platform_keys/ and
chrome/test/data/extensions/api_test/platform_keys/
but in chrome/browser/extensions/api/platform_keys/ .
Should I add one there too?

[not at google - send to devlin, 2015-01-29 16:50:05]

On 2015/01/29 13:47:48, pneubeck wrote:
> On 2015/01/27 16:41:30, kalman wrote:
> > Extensions infrastructure lgtm (pending discussion on other bug, but I don't
> > expect that to change). However, I haven't looked deeply at the
implementation
> > of the API. I can review a small change like that, however,
> > - add an OWNERS file
> I have OWNERS files in chrome/browser/chromeos/platform_keys/ and
> chrome/test/data/extensions/api_test/platform_keys/
> but in chrome/browser/extensions/api/platform_keys/ .
> Should I add one there too?

Yes, you own the implementation.

[pneubeck (no reviews), 2015-02-02 10:10:03]

pneubeck@chromium.org changed reviewers:
+ bartfab@chromium.org

[pneubeck (no reviews), 2015-02-02 10:10:04]

Bartosz, ptal at this CL secondly.
The actual implementation is not fully reviewed yet but the boilerplate around
is.

[pneubeck (no reviews), 2015-02-02 12:37:21]

pneubeck@chromium.org changed reviewers:
+ tnagel@chromium.org
- bartfab@chromium.org

[pneubeck (no reviews), 2015-02-02 12:37:22]

-Bartosz, +Thiemo

[Ryan Sleevi, 2015-02-02 22:42:15]

LGTM.

To confirm, this is whitelisted-only at present, right? Just making sure the
permissions bit won't hold us up.

https://codereview.chromium.org/875373002/diff/280001/chrome/browser/chromeos...
File chrome/browser/chromeos/platform_keys/platform_keys.h (right):

https://codereview.chromium.org/875373002/diff/280001/chrome/browser/chromeos...
chrome/browser/chromeos/platform_keys/platform_keys.h:95: // contain the list of
matching certificates (maybe empty) and |error_message|
comment nit: s/maybe empty/which may be empty/

https://codereview.chromium.org/875373002/diff/280001/chrome/browser/extensio...
File chrome/browser/extensions/api/platform_keys/platform_keys_apitest_nss.cc
(right):

https://codereview.chromium.org/875373002/diff/280001/chrome/browser/extensio...
chrome/browser/extensions/api/platform_keys/platform_keys_apitest_nss.cc:95:
browser()->profile(),
I'm assuming the profile is temporary and created anew for each test?

https://codereview.chromium.org/875373002/diff/280001/chrome/browser/extensio...
chrome/browser/extensions/api/platform_keys/platform_keys_apitest_nss.cc:159:
std::string system_token_availability = "";
drop the = "";, it's cleaner that way :)

[pneubeck (no reviews), 2015-02-03 10:13:11]

Ryan, thanks a lot for reviewing!

On 2015/02/02 22:42:15, Ryan Sleevi wrote:
> To confirm, this is whitelisted-only at present, right? Just making sure the
> permissions bit won't hold us up.
This API is not restricted to a whitelist.
However, it is restricted to ChromeOS & dev-channel only.

[pneubeck (no reviews), 2015-02-03 10:13:58]

https://codereview.chromium.org/875373002/diff/280001/chrome/browser/chromeos...
File chrome/browser/chromeos/platform_keys/platform_keys.h (right):

https://codereview.chromium.org/875373002/diff/280001/chrome/browser/chromeos...
chrome/browser/chromeos/platform_keys/platform_keys.h:95: // contain the list of
matching certificates (maybe empty) and |error_message|
On 2015/02/02 22:42:15, Ryan Sleevi wrote:
> comment nit: s/maybe empty/which may be empty/
> 

Done.

https://codereview.chromium.org/875373002/diff/280001/chrome/browser/extensio...
File chrome/browser/extensions/api/platform_keys/platform_keys_apitest_nss.cc
(right):

https://codereview.chromium.org/875373002/diff/280001/chrome/browser/extensio...
chrome/browser/extensions/api/platform_keys/platform_keys_apitest_nss.cc:95:
browser()->profile(),
On 2015/02/02 22:42:15, Ryan Sleevi wrote:
> I'm assuming the profile is temporary and created anew for each test?

Yes. For each execution of a IN_PROC_BROWSER_TEST_P, a new profile should be
used.  PlatformKeysTest.Basic is executed 3 times that way.

Within one execution of PlatformKeysTest.Basic, a whole bunch of javascript
tests is executed within the _same_ profile.

https://codereview.chromium.org/875373002/diff/280001/chrome/browser/extensio...
chrome/browser/extensions/api/platform_keys/platform_keys_apitest_nss.cc:159:
std::string system_token_availability = "";
On 2015/02/02 22:42:15, Ryan Sleevi wrote:
> drop the = "";, it's cleaner that way :)

Done.

[pneubeck (no reviews), 2015-02-04 14:05:01]

Patchset #12 (id:340001) has been deleted

[pneubeck (no reviews), 2015-02-04 15:36:45]

pneubeck@chromium.org changed reviewers:
+ sky@chromium.org
- arv@chromium.org

[pneubeck (no reviews), 2015-02-04 15:36:47]

sky@chromium.org: ptal at
  chrome/test/BUILD.gn
  chrome/renderer/resources/renderer_resources.grd

[pneubeck (no reviews), 2015-02-04 15:37:34]

pneubeck@chromium.org changed reviewers:
- tnagel@chromium.org

[sky, 2015-02-04 18:52:47]

LGTM

[pneubeck (no reviews), 2015-02-04 19:29:53]

@Ryan, please take a quick look at diff 13-11 which effectively disables
selectClientCertificate() outside of tests until I implemented permissions.

[Ryan Sleevi, 2015-02-04 19:36:12]

Thanks, LG

[pneubeck (no reviews), 2015-02-04 19:51:13]

The CQ bit was checked by pneubeck@chromium.org

[commit-bot: I haz the power, 2015-02-04 19:52:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/875373002/380001

[commit-bot: I haz the power, 2015-02-04 20:44:22]

Committed patchset #13 (id:380001)

[commit-bot: I haz the power, 2015-02-04 20:45:22]

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