Move NetworkingPrivateDelegate::CryptoVerify to networking_cast_private

Move NetworkingPrivateDelegate::CryptoVerify to networking_cast_private

Retires NetworkingPrivateDelegate::CryptoVerify and moves verification
logic to ChromeNetworkingCastPrivateDelegate. Moves networkingPrivate
API to use the NetworkingCastPrivateDelegate interface to delegate
verification methods to chrome.

Along the way, done some minor cleanup:
- Make NetworkingCastPrivateDelegate take API agnostic argument for
  verification properties - Credentials, which used to be part of the
  //chrome CryptoVerify implementation, this CL moves the declaration 
  to the interface - the object is now created by the delegate users
  before calling verification methods (it can be created from both
  networkingPrivate and networking.castPrivate API verification 
  properties)
- Make Credentials non-copyable - instead of copying the whole class. 
  the credentials are now passed to a worker thread using a unique_ptr
- Clean up the way DecodeAndVerifyCredentials helperr method returns 
  results - instead of combination of bool return value and bool ptr
  argument, it now returns a 3-value enum.

Next: move networking_private credentials getters and crypto util
to networking_cast_private

BUG=678737
Review-Url: https://codereview.chromium.org/2751423004
Cr-Commit-Position: refs/heads/master@{#459180}
Committed: https://chromium.googlesource.com/chromium/src/+/a0d155d336a59388f84140f7b4c4f79cb1f36e40

[tbarzic, 2017-03-17 00:54:18]

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

[commit-bot: I haz the power, 2017-03-17 00:54:53]

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-03-17 01:06:06]

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

[commit-bot: I haz the power, 2017-03-17 01:06:07]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tbarzic, 2017-03-17 03:43:47]

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

[commit-bot: I haz the power, 2017-03-17 03:44:10]

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-03-17 04:15:30]

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

[commit-bot: I haz the power, 2017-03-17 04:15:31]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tbarzic, 2017-03-17 04:39:21]

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

[commit-bot: I haz the power, 2017-03-17 04:39:42]

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-03-17 04:57:58]

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

[commit-bot: I haz the power, 2017-03-17 04:57:58]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[tbarzic, 2017-03-17 05:10:38]

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

[commit-bot: I haz the power, 2017-03-17 05:10:55]

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-03-17 05:42:42]

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

[commit-bot: I haz the power, 2017-03-17 05:42:43]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[tbarzic, 2017-03-17 16:28:33]

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

[commit-bot: I haz the power, 2017-03-17 16:28:49]

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

[tbarzic, 2017-03-20 17:33:03]

Description was changed from

==========
Move NetworkingPrivateDelegate::CryptoVerify to networking.castPrivate

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

to

==========
Move NetworkingPrivateDelegate::CryptoVerify to networking_cast_private

Retires NetworkingPrivateDelegate::CryptoVerify and moves verification
logic to ChromeNetworkingCastPrivateDelegate. Moves networkingPrivate
API to use the NetworkingCastPrivateDelegate interface to delegate
verification methods to chrome.

Along the way, done some minor cleanup:
- Make NetworkingCastPrivateDelegate take API agnostic argument for
  verification properties - Credentials, which used to be part of the
  //chrome CryptoVerify implementation, this CL moves the declaration 
  to the interface - the object is now created by the delegate users
  before calling verification methods (it can be created from both
  networkingPrivate and networking.castPrivate API verification 
  properties)
- Make Credentials non-copyable - instead of copying the whole class. 
  the credentials are now passed to a worker thread using a unique_ptr
- Clean up the way DecodeAndVerifyCredentials helperr method returns 
  results - instead of combination of bool return value and bool ptr
  argument, it now returns a 3-value enum.

Next: move networking_private credentials getters and crypto util
to networking_cast_private

BUG=678737
==========

[tbarzic, 2017-03-22 02:59:35]

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

[commit-bot: I haz the power, 2017-03-22 03:00:33]

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

[tbarzic, 2017-03-22 04:23:15]

tbarzic@chromium.org changed reviewers:
+ erg@chromium.org, rdevlin.cronin@chromium.org, stevenjb@chromium.org

[tbarzic, 2017-03-22 04:23:16]

Owners approvals I require:
rdevlin.cronin: chrome/browser/extensions/BUILD.gn
erg: chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc

[commit-bot: I haz the power, 2017-03-22 05:03:29]

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

[commit-bot: I haz the power, 2017-03-22 05:03:30]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't
started yet, builder probably lacks capacity)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_TIMED_OUT,
build hasn't started yet, builder probably lacks capacity)

[Elliot Glaysher, 2017-03-22 17:27:19]

profiles lgtm

[Devlin, 2017-03-22 20:17:17]

This is fine so far, but would you remind re-pinging once stevenjb@ signs off,
just in case something changes in the interim?

[tbarzic, 2017-03-22 20:18:20]

On 2017/03/22 20:17:17, Devlin wrote:
> This is fine so far, but would you remind re-pinging once stevenjb@ signs off,
> just in case something changes in the interim?

sure, will do

[stevenjb, 2017-03-22 20:26:01]

lgtm

https://codereview.chromium.org/2751423004/diff/140001/chrome/browser/extensi...
File
chrome/browser/extensions/api/networking_cast_private/chrome_networking_cast_private_delegate.cc
(right):

https://codereview.chromium.org/2751423004/diff/140001/chrome/browser/extensi...
chrome/browser/extensions/api/networking_cast_private/chrome_networking_cast_private_delegate.cc:194:
sequence_token, base::SequencedWorkerPool::SKIP_ON_SHUTDOWN);
I think that we are supposed to be using base::PostTaskWithTraits instead, see
crbug.com/667892.

https://codereview.chromium.org/2751423004/diff/140001/chrome/browser/extensi...
File
chrome/browser/extensions/api/networking_private/networking_private_apitest.cc
(right):

https://codereview.chromium.org/2751423004/diff/140001/chrome/browser/extensi...
chrome/browser/extensions/api/networking_private/networking_private_apitest.cc:394:
return std::move(delegate);
nit: I didn't think std::move was needed for a return result?

[Devlin, 2017-03-22 21:18:35]

chrome/browser/extensions/BUILD.gn lgtm

[tbarzic, 2017-03-23 01:29:40]

https://codereview.chromium.org/2751423004/diff/140001/chrome/browser/extensi...
File
chrome/browser/extensions/api/networking_cast_private/chrome_networking_cast_private_delegate.cc
(right):

https://codereview.chromium.org/2751423004/diff/140001/chrome/browser/extensi...
chrome/browser/extensions/api/networking_cast_private/chrome_networking_cast_private_delegate.cc:194:
sequence_token, base::SequencedWorkerPool::SKIP_ON_SHUTDOWN);
On 2017/03/22 20:26:01, stevenjb wrote:
> I think that we are supposed to be using base::PostTaskWithTraits instead, see
> crbug.com/667892.

Oh yeah.. missed that.

It doesn't seem that the task ordering has to be guaranteed either (I can't see
the reason kCryptoVerifySequenceTokenName was used), so removing that, too.

https://codereview.chromium.org/2751423004/diff/140001/chrome/browser/extensi...
File
chrome/browser/extensions/api/networking_private/networking_private_apitest.cc
(right):

https://codereview.chromium.org/2751423004/diff/140001/chrome/browser/extensi...
chrome/browser/extensions/api/networking_private/networking_private_apitest.cc:394:
return std::move(delegate);
On 2017/03/22 20:26:01, stevenjb wrote:
> nit: I didn't think std::move was needed for a return result?

Done.

[stevenjb, 2017-03-23 18:18:34]

lgtm ++

https://codereview.chromium.org/2751423004/diff/140001/chrome/browser/extensi...
File
chrome/browser/extensions/api/networking_cast_private/chrome_networking_cast_private_delegate.cc
(right):

https://codereview.chromium.org/2751423004/diff/140001/chrome/browser/extensi...
chrome/browser/extensions/api/networking_cast_private/chrome_networking_cast_private_delegate.cc:194:
sequence_token, base::SequencedWorkerPool::SKIP_ON_SHUTDOWN);
On 2017/03/23 01:29:40, tbarzic wrote:
> On 2017/03/22 20:26:01, stevenjb wrote:
> > I think that we are supposed to be using base::PostTaskWithTraits instead,
see
> > crbug.com/667892.
> 
> Oh yeah.. missed that.
> 
> It doesn't seem that the task ordering has to be guaranteed either (I can't
see
> the reason kCryptoVerifySequenceTokenName was used), so removing that, too.

Agreed.

[tbarzic, 2017-03-23 18:26:00]

The CQ bit was checked by tbarzic@chromium.org

[tbarzic, 2017-03-23 18:26:01]

The patchset sent to the CQ was uploaded after l-g-t-m from erg@chromium.org,
rdevlin.cronin@chromium.org
Link to the patchset: https://codereview.chromium.org/2751423004/#ps160001
(title: "blocking pool removed")

[commit-bot: I haz the power, 2017-03-23 18:26:51]

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-03-23 19:35:49]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1490293560871600,
"parent_rev": "53a9a9ad5cc4d43822d530eab0aa951b1a12a503", "commit_rev":
"a0d155d336a59388f84140f7b4c4f79cb1f36e40"}

[commit-bot: I haz the power, 2017-03-23 19:36:37]

Description was changed from

==========
Move NetworkingPrivateDelegate::CryptoVerify to networking_cast_private

Retires NetworkingPrivateDelegate::CryptoVerify and moves verification
logic to ChromeNetworkingCastPrivateDelegate. Moves networkingPrivate
API to use the NetworkingCastPrivateDelegate interface to delegate
verification methods to chrome.

Along the way, done some minor cleanup:
- Make NetworkingCastPrivateDelegate take API agnostic argument for
  verification properties - Credentials, which used to be part of the
  //chrome CryptoVerify implementation, this CL moves the declaration 
  to the interface - the object is now created by the delegate users
  before calling verification methods (it can be created from both
  networkingPrivate and networking.castPrivate API verification 
  properties)
- Make Credentials non-copyable - instead of copying the whole class. 
  the credentials are now passed to a worker thread using a unique_ptr
- Clean up the way DecodeAndVerifyCredentials helperr method returns 
  results - instead of combination of bool return value and bool ptr
  argument, it now returns a 3-value enum.

Next: move networking_private credentials getters and crypto util
to networking_cast_private

BUG=678737
==========

to

==========
Move NetworkingPrivateDelegate::CryptoVerify to networking_cast_private

Retires NetworkingPrivateDelegate::CryptoVerify and moves verification
logic to ChromeNetworkingCastPrivateDelegate. Moves networkingPrivate
API to use the NetworkingCastPrivateDelegate interface to delegate
verification methods to chrome.

Along the way, done some minor cleanup:
- Make NetworkingCastPrivateDelegate take API agnostic argument for
  verification properties - Credentials, which used to be part of the
  //chrome CryptoVerify implementation, this CL moves the declaration 
  to the interface - the object is now created by the delegate users
  before calling verification methods (it can be created from both
  networkingPrivate and networking.castPrivate API verification 
  properties)
- Make Credentials non-copyable - instead of copying the whole class. 
  the credentials are now passed to a worker thread using a unique_ptr
- Clean up the way DecodeAndVerifyCredentials helperr method returns 
  results - instead of combination of bool return value and bool ptr
  argument, it now returns a 3-value enum.

Next: move networking_private credentials getters and crypto util
to networking_cast_private

BUG=678737

Review-Url: https://codereview.chromium.org/2751423004
Cr-Commit-Position: refs/heads/master@{#459180}
Committed:
https://chromium.googlesource.com/chromium/src/+/a0d155d336a59388f84140f7b4c4...
==========

[commit-bot: I haz the power, 2017-03-23 19:36:39]

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