Move CT EV white list packaging API from chrome/ to components/.

Move CT EV white list packaging API from chrome/ to components/.

This feature is also useful for projects not based on chrome/
that want to turn on Certificate Transparency for EV certs.

BUG=None

Committed: https://crrev.com/dfdf28fd1a20c4c171360b680ba5647fb04bec43
Cr-Commit-Position: refs/heads/master@{#310595}

[haavardm, 2014-12-18 13:38:47]

haavardm@opera.com changed reviewers:
+ erikwright@chromium.org, eroman@chromium.org

[haavardm, 2014-12-18 13:49:31]

haavardm@opera.com changed reviewers:
+ cpu@chromium.org

[haavardm, 2014-12-18 13:56:46]

haavardm@opera.com changed reviewers:
- cpu@chromium.org

[haavardm, 2014-12-18 14:06:25]

haavardm@opera.com changed reviewers:
+ eranm@chromium.org
- eroman@chromium.org

[haavardm, 2014-12-18 14:07:05]

This looks a lot more than it really is.

There's no real code changes, the API is simply moved as discussed in
https://codereview.chromium.org/547603002/.
(I moved to components rathen than content module, since it seemed more
fitting).

I've added a new namespace to comply with components requirements.

erikwright : please review the move and compliance to the components
requirements.
Eran: FYI. Not sure who to set as owner of this API, but I set you since you
wrote it.

Please reassign if I've gotten the reviewers wrong.

[Eran Messeri, 2014-12-18 14:29:40]

On 2014/12/18 14:07:05, haavardm wrote:
> This looks a lot more than it really is.
> 
> There's no real code changes, the API is simply moved as discussed in
> https://codereview.chromium.org/547603002/.
> (I moved to components rathen than content module, since it seemed more
> fitting).
> 
> I've added a new namespace to comply with components requirements.
> 
> erikwright : please review the move and compliance to the components
> requirements.
> Eran: FYI. Not sure who to set as owner of this API, but I set you since you
> wrote it.
> 
> Please reassign if I've gotten the reviewers wrong.

Overall seems sensible - I'll properly review it as soon as possible.

[erikwright (departed), 2014-12-18 14:44:59]

components/ LGTM

[haavardm, 2015-01-05 12:35:56]

On 2014/12/18 14:29:40, Eran wrote:
> On 2014/12/18 14:07:05, haavardm wrote:
> > This looks a lot more than it really is.
> > 
> > There's no real code changes, the API is simply moved as discussed in
> > https://codereview.chromium.org/547603002/.
> > (I moved to components rathen than content module, since it seemed more
> > fitting).
> > 
> > I've added a new namespace to comply with components requirements.
> > 
> > erikwright : please review the move and compliance to the components
> > requirements.
> > Eran: FYI. Not sure who to set as owner of this API, but I set you since you
> > wrote it.
> > 
> > Please reassign if I've gotten the reviewers wrong.
> 
> Overall seems sensible - I'll properly review it as soon as possible.

Eran: did you also want to review? Erik has so far reviewed compononents, can
you take chrome/ ?

[Eran Messeri, 2015-01-05 12:43:29]

On 2015/01/05 12:35:56, haavardm wrote:
> On 2014/12/18 14:29:40, Eran wrote:
> > On 2014/12/18 14:07:05, haavardm wrote:
> > > This looks a lot more than it really is.
> > > 
> > > There's no real code changes, the API is simply moved as discussed in
> > > https://codereview.chromium.org/547603002/.
> > > (I moved to components rathen than content module, since it seemed more
> > > fitting).
> > > 
> > > I've added a new namespace to comply with components requirements.
> > > 
> > > erikwright : please review the move and compliance to the components
> > > requirements.
> > > Eran: FYI. Not sure who to set as owner of this API, but I set you since
you
> > > wrote it.
> > > 
> > > Please reassign if I've gotten the reviewers wrong.
> > 
> > Overall seems sensible - I'll properly review it as soon as possible.
> 
> Eran: did you also want to review? Erik has so far reviewed compononents, can
> you take chrome/ ?

I will - just back from vacation, will review today.
Note that I'm not an owner in chrome/ so after my review you'll need somebody
else to approve.

[haavardm, 2015-01-05 13:17:25]

On 2015/01/05 12:43:29, Eran wrote:
> On 2015/01/05 12:35:56, haavardm wrote:
> > On 2014/12/18 14:29:40, Eran wrote:
> > > On 2014/12/18 14:07:05, haavardm wrote:
> > > > This looks a lot more than it really is.
> > > > 
> > > > There's no real code changes, the API is simply moved as discussed in
> > > > https://codereview.chromium.org/547603002/.
> > > > (I moved to components rathen than content module, since it seemed more
> > > > fitting).
> > > > 
> > > > I've added a new namespace to comply with components requirements.
> > > > 
> > > > erikwright : please review the move and compliance to the components
> > > > requirements.
> > > > Eran: FYI. Not sure who to set as owner of this API, but I set you since
> you
> > > > wrote it.
> > > > 
> > > > Please reassign if I've gotten the reviewers wrong.
> > > 
> > > Overall seems sensible - I'll properly review it as soon as possible.
> > 
> > Eran: did you also want to review? Erik has so far reviewed compononents,
can
> > you take chrome/ ?
> 
> I will - just back from vacation, will review today.

Great. Just back from vacation my self.

I'll find a reviewer for chrome/

> Note that I'm not an owner in chrome/ so after my review you'll need somebody
> else to approve.

[haavardm, 2015-01-05 13:19:50]

haavardm@opera.com changed reviewers:
+ thestig@chromium.org

[haavardm, 2015-01-05 13:22:45]

On 2015/01/05 12:43:29, Eran wrote:
> Note that I'm not an owner in chrome/ so after my review you'll need somebody
> else to approve.


Lei Zhang: Can you review the rest of chrome/? There's very little real changes,
just moving code to components (components has been approved).

[Eran Messeri, 2015-01-05 15:03:24]

The changes to chrome/ LGTM.

[Lei Zhang, 2015-01-05 21:46:47]

https://codereview.chromium.org/811353002/diff/20001/chrome/browser/component...
File chrome/browser/component_updater/DEPS (right):

https://codereview.chromium.org/811353002/diff/20001/chrome/browser/component...
chrome/browser/component_updater/DEPS:5:
"+components/packed_ct_ev_whitelist/packed_ct_ev_whitelist.h",
alphabetical order please

https://codereview.chromium.org/811353002/diff/20001/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

https://codereview.chromium.org/811353002/diff/20001/chrome/chrome_browser.gy...
chrome/chrome_browser.gypi:2915:
'../components/components.gyp:packed_ct_ev_whitelist',
This should be in the "not iOS" section below around line 3000, since the files
being removed comes from the chrome_browser_net_sources variable, and that
variable is only used in the section below.

https://codereview.chromium.org/811353002/diff/20001/components/packed_ct_ev_...
File components/packed_ct_ev_whitelist/OWNERS (right):

https://codereview.chromium.org/811353002/diff/20001/components/packed_ct_ev_...
components/packed_ct_ev_whitelist/OWNERS:1: eranm@chromium.org
Eran: If possible, it would be good to have a secondary OWNER.

[haavardm, 2015-01-06 13:21:51]

Eran: I'll add a second OWNER when you have decided who that should be.

Lei Zhang:PTAL

https://codereview.chromium.org/811353002/diff/20001/chrome/browser/component...
File chrome/browser/component_updater/DEPS (right):

https://codereview.chromium.org/811353002/diff/20001/chrome/browser/component...
chrome/browser/component_updater/DEPS:5:
"+components/packed_ct_ev_whitelist/packed_ct_ev_whitelist.h",
On 2015/01/05 21:46:47, Lei Zhang wrote:
> alphabetical order please

Done.

https://codereview.chromium.org/811353002/diff/20001/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

https://codereview.chromium.org/811353002/diff/20001/chrome/chrome_browser.gy...
chrome/chrome_browser.gypi:2915:
'../components/components.gyp:packed_ct_ev_whitelist',
On 2015/01/05 21:46:47, Lei Zhang wrote:
> This should be in the "not iOS" section below around line 3000, since the
files
> being removed comes from the chrome_browser_net_sources variable, and that
> variable is only used in the section below.

Done.

https://codereview.chromium.org/811353002/diff/20001/components/packed_ct_ev_...
File components/packed_ct_ev_whitelist/OWNERS (right):

https://codereview.chromium.org/811353002/diff/20001/components/packed_ct_ev_...
components/packed_ct_ev_whitelist/OWNERS:1: eranm@chromium.org
On 2015/01/05 21:46:47, Lei Zhang wrote:
> Eran: If possible, it would be good to have a secondary OWNER.

I'll patch this when Eran has decided who that should be.

[chromium-reviews, 2015-01-06 18:58:17]

I propose rsleevi/agl  as additional owners.
On 6 Jan 2015 1:21 pm, <haavardm@opera.com> wrote:

> Eran: I'll add a second OWNER when you have decided who that should be.
>
> Lei Zhang:PTAL
>
>
> https://codereview.chromium.org/811353002/diff/20001/
> chrome/browser/component_updater/DEPS
> File chrome/browser/component_updater/DEPS (right):
>
> https://codereview.chromium.org/811353002/diff/20001/
> chrome/browser/component_updater/DEPS#newcode5
> chrome/browser/component_updater/DEPS:5:
> "+components/packed_ct_ev_whitelist/packed_ct_ev_whitelist.h",
> On 2015/01/05 21:46:47, Lei Zhang wrote:
>
>> alphabetical order please
>>
>
> Done.
>
> https://codereview.chromium.org/811353002/diff/20001/
> chrome/chrome_browser.gypi
> File chrome/chrome_browser.gypi (right):
>
> https://codereview.chromium.org/811353002/diff/20001/
> chrome/chrome_browser.gypi#newcode2915
> chrome/chrome_browser.gypi:2915:
> '../components/components.gyp:packed_ct_ev_whitelist',
> On 2015/01/05 21:46:47, Lei Zhang wrote:
>
>> This should be in the "not iOS" section below around line 3000, since
>>
> the files
>
>> being removed comes from the chrome_browser_net_sources variable, and
>>
> that
>
>> variable is only used in the section below.
>>
>
> Done.
>
> https://codereview.chromium.org/811353002/diff/20001/
> components/packed_ct_ev_whitelist/OWNERS
> File components/packed_ct_ev_whitelist/OWNERS (right):
>
> https://codereview.chromium.org/811353002/diff/20001/
> components/packed_ct_ev_whitelist/OWNERS#newcode1
> components/packed_ct_ev_whitelist/OWNERS:1: eranm@chromium.org
> On 2015/01/05 21:46:47, Lei Zhang wrote:
>
>> Eran: If possible, it would be good to have a secondary OWNER.
>>
>
> I'll patch this when Eran has decided who that should be.
>
> https://codereview.chromium.org/811353002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Lei Zhang, 2015-01-06 19:34:37]

+nominated owners

[Lei Zhang, 2015-01-06 19:35:28]

lgtm

Feel free to add more owners in a separate CL if you don't want to wait.

[Ryan Sleevi, 2015-01-07 05:01:48]

rsleevi@chromium.org changed reviewers:
+ rsleevi@chromium.org

[Ryan Sleevi, 2015-01-07 05:01:50]

LGTM % nits

However, one question for leiz - do we support embedders using Chromium
extensions (component or otherwise)? My understanding was that the answer was
"no".

This code is not *tightly* coupled to our extension mechanism, just implicitly.
So whatever caveats of thestig apply, but otherwise sounds good.

https://codereview.chromium.org/811353002/diff/40001/components/OWNERS
File components/OWNERS (right):

https://codereview.chromium.org/811353002/diff/40001/components/OWNERS#newcod...
components/OWNERS:139: per-file packed_ct_ev_whitelist.gypi=eranm@chromium.org
please add rsleevi here

https://codereview.chromium.org/811353002/diff/40001/components/packed_ct_ev_...
File components/packed_ct_ev_whitelist/DEPS (right):

https://codereview.chromium.org/811353002/diff/40001/components/packed_ct_ev_...
components/packed_ct_ev_whitelist/DEPS:4: "+net",
Can we be more explicit beyond net here

Is it net/base + net/cert? net/url_request? More specific = better.

https://codereview.chromium.org/811353002/diff/40001/components/packed_ct_ev_...
File components/packed_ct_ev_whitelist/OWNERS (right):

https://codereview.chromium.org/811353002/diff/40001/components/packed_ct_ev_...
components/packed_ct_ev_whitelist/OWNERS:1: eranm@chromium.org
OWNERS files should have 2+ active reviewers

Please feel free to add me.

[Lei Zhang, 2015-01-07 08:08:51]

On 2015/01/07 05:01:50, Ryan Sleevi wrote:
> LGTM % nits
> 
> However, one question for leiz - do we support embedders using Chromium
> extensions (component or otherwise)? My understanding was that the answer was
> "no".
> 
> This code is not *tightly* coupled to our extension mechanism, just
implicitly.
> So whatever caveats of thestig apply, but otherwise sounds good.

extensions/README says:

"This will become a reusable extensions module. It implements the core parts of
Chrome's extension system, and can be used with any host of the 'content'
module."

So one can certainly use the combination of: content + extensions + components +
other_goodies.

[haavardm, 2015-01-07 19:37:44]

PTAL

https://codereview.chromium.org/811353002/diff/40001/components/OWNERS
File components/OWNERS (right):

https://codereview.chromium.org/811353002/diff/40001/components/OWNERS#newcod...
components/OWNERS:139: per-file packed_ct_ev_whitelist.gypi=eranm@chromium.org
On 2015/01/07 05:01:49, Ryan Sleevi wrote:
> please add rsleevi here

Done.

https://codereview.chromium.org/811353002/diff/40001/components/packed_ct_ev_...
File components/packed_ct_ev_whitelist/DEPS (right):

https://codereview.chromium.org/811353002/diff/40001/components/packed_ct_ev_...
components/packed_ct_ev_whitelist/DEPS:4: "+net",
On 2015/01/07 05:01:49, Ryan Sleevi wrote:
> Can we be more explicit beyond net here
> 
> Is it net/base + net/cert? net/url_request? More specific = better.

Done.

Might be I went a bit over board adding the actual .h files. But we only include
one file per directory, so stating the filenames did not add more lines.

https://codereview.chromium.org/811353002/diff/40001/components/packed_ct_ev_...
File components/packed_ct_ev_whitelist/OWNERS (right):

https://codereview.chromium.org/811353002/diff/40001/components/packed_ct_ev_...
components/packed_ct_ev_whitelist/OWNERS:1: eranm@chromium.org
On 2015/01/07 05:01:49, Ryan Sleevi wrote:
> OWNERS files should have 2+ active reviewers
> 
> Please feel free to add me.

Done.

[Ryan Sleevi, 2015-01-07 19:52:15]

LGTM % nit

https://codereview.chromium.org/811353002/diff/60001/components/packed_ct_ev_...
File components/packed_ct_ev_whitelist/DEPS (right):

https://codereview.chromium.org/811353002/diff/60001/components/packed_ct_ev_...
components/packed_ct_ev_whitelist/DEPS:5: "+net/ssl/ssl_config_service.h",
Let's just go with the top-level bits

+content/public/browser
+net/cert
+net/ssl

[haavardm, 2015-01-07 20:44:52]

https://codereview.chromium.org/811353002/diff/60001/components/packed_ct_ev_...
File components/packed_ct_ev_whitelist/DEPS (right):

https://codereview.chromium.org/811353002/diff/60001/components/packed_ct_ev_...
components/packed_ct_ev_whitelist/DEPS:5: "+net/ssl/ssl_config_service.h",
On 2015/01/07 19:52:15, Ryan Sleevi wrote:
> Let's just go with the top-level bits
> 
> +content/public/browser
> +net/cert
> +net/ssl
> 

Done.

[haavardm, 2015-01-07 20:44:59]

The CQ bit was checked by haavardm@opera.com

[commit-bot: I haz the power, 2015-01-07 20:45:39]

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

[commit-bot: I haz the power, 2015-01-07 20:51:13]

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

[commit-bot: I haz the power, 2015-01-07 20:51:15]

Try jobs failed on following builders:
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/10...)
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_chromium_gn_compile_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_compile...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[haavardm, 2015-01-07 21:06:29]

On 2015/01/07 20:51:15, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
Seems like I need to rebase before committing this.

[haavardm, 2015-01-08 10:34:46]

Patchset #6 (id:100001) has been deleted

[haavardm, 2015-01-08 10:39:43]

Patchset #6 (id:120001) has been deleted

[haavardm, 2015-01-08 11:34:34]

The CQ bit was checked by haavardm@opera.com

[commit-bot: I haz the power, 2015-01-08 11:35:43]

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

[commit-bot: I haz the power, 2015-01-08 11:43:40]

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

[commit-bot: I haz the power, 2015-01-08 11:43:41]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[haavardm, 2015-01-08 15:48:57]

haavardm@opera.com changed reviewers:
+ creis@chromium.org

[haavardm, 2015-01-08 15:48:58]

Charlie, can you please review the dependency added to
src/components/packed_ct_ev_whitelist/DEPS?

This CL simply moves the EV whitelist API from Chrome/ to Components/ so this
dependency has been approved previously. I need a formal approval to commit
though:

** Presubmit ERRORS **
Missing LGTM from OWNERS of dependencies added to DEPS:
    '+content/public/browser',

[Charlie Reis, 2015-01-08 17:53:04]

DEPS LGTM.

https://codereview.chromium.org/811353002/diff/140001/components/components_t...
File components/components_tests.gyp (right):

https://codereview.chromium.org/811353002/diff/140001/components/components_t...
components/components_tests.gyp:194:
'packed_ct_ev_whitelist/bit_stream_reader_unittest.cc',
nit: Alphabetize

https://codereview.chromium.org/811353002/diff/140001/components/components_t...
components/components_tests.gyp:357: # Dependencies packet CT EV white list
nit: Dependencies of packed

[haavardm, 2015-01-08 18:16:25]

The CQ bit was checked by haavardm@opera.com

[commit-bot: I haz the power, 2015-01-08 18:17:26]

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

[Charlie Reis, 2015-01-08 18:28:34]

You only fixed one of my two nits, though.  Lines 193 and 194 of
components_tests.gyp need to be swapped.

[haavardm, 2015-01-08 19:04:28]

The CQ bit was unchecked by haavardm@opera.com

[haavardm, 2015-01-08 19:20:53]

On 2015/01/08 18:28:34, Charlie Reis wrote:
> You only fixed one of my two nits, though.  Lines 193 and 194 of
> components_tests.gyp need to be swapped.

Ah, sorry, missed that one.

[haavardm, 2015-01-08 19:21:02]

https://codereview.chromium.org/811353002/diff/140001/components/components_t...
File components/components_tests.gyp (right):

https://codereview.chromium.org/811353002/diff/140001/components/components_t...
components/components_tests.gyp:194:
'packed_ct_ev_whitelist/bit_stream_reader_unittest.cc',
On 2015/01/08 17:53:04, Charlie Reis wrote:
> nit: Alphabetize

Done.

https://codereview.chromium.org/811353002/diff/140001/components/components_t...
components/components_tests.gyp:357: # Dependencies packet CT EV white list
On 2015/01/08 17:53:04, Charlie Reis wrote:
> nit: Dependencies of packed

Done.

[haavardm, 2015-01-08 19:21:13]

The CQ bit was checked by haavardm@opera.com

[commit-bot: I haz the power, 2015-01-08 19:22:15]

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

[commit-bot: I haz the power, 2015-01-08 21:05:38]

Committed patchset #8 (id:180001)

[commit-bot: I haz the power, 2015-01-08 21:06:42]

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