Device registration using an enrollment certificate from the PCA.

Device registration using an enrollment certificate from the PCA.

BUG=624187
TEST=unit_tests

Committed: https://crrev.com/59380d4af7c23cae135488b421df2756894b02fe
Cr-Commit-Position: refs/heads/master@{#415148}

[The one and only Dr. Crash, 2016-08-24 07:22:28]

Description was changed from

==========
Device registration using an enrollment certificate from the PCA.

BUG=624187
TEST=none
==========

to

==========
Device registration using an enrollment certificate from the PCA.

BUG=624187
TEST=none
==========

[dkalin, 2016-08-24 18:36:31]

dkalin@google.com changed reviewers:
+ dkalin@google.com

[dkalin, 2016-08-24 18:36:32]

lgtm

[The one and only Dr. Crash, 2016-08-24 20:58:47]

drcrash@chromium.org changed reviewers:
+ pastarmovj@chromium.org

[The one and only Dr. Crash, 2016-08-26 02:39:49]

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

[commit-bot: I haz the power, 2016-08-26 02:40:20]

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, 2016-08-26 03:36:51]

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

[commit-bot: I haz the power, 2016-08-26 03:36:52]

Dry run: This issue passed the CQ dry run.

[The one and only Dr. Crash, 2016-08-26 19:43:28]

+pastarmovj, would you have a look?

[The one and only Dr. Crash, 2016-08-27 16:48:11]

Description was changed from

==========
Device registration using an enrollment certificate from the PCA.

BUG=624187
TEST=none
==========

to

==========
Device registration using an enrollment certificate from the PCA.

BUG=624187
TEST=unit_tests
==========

[The one and only Dr. Crash, 2016-08-28 00:53:40]

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

[commit-bot: I haz the power, 2016-08-28 00:53:49]

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, 2016-08-28 02:11:54]

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

[commit-bot: I haz the power, 2016-08-28 02:11:55]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[The one and only Dr. Crash, 2016-08-28 04:53:04]

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

[commit-bot: I haz the power, 2016-08-28 04:53:12]

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, 2016-08-28 07:37:33]

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

[commit-bot: I haz the power, 2016-08-28 07:37:34]

Dry run: This issue passed the CQ dry run.

[pastarmovj, 2016-08-29 11:51:35]

https://codereview.chromium.org/2276593004/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h (right):

https://codereview.chromium.org/2276593004/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h:151:
std::unique_ptr<chromeos::attestation::AttestationFlow>
Please add a new line and document this function. Also shoudn't the second line
be offset by 4 more spaces?

https://codereview.chromium.org/2276593004/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/device_cloud_policy_initializer.h (right):

https://codereview.chromium.org/2276593004/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_cloud_policy_initializer.h:109: void
SignData(const std::string& data,
Please add a comment : policy::SigningService implementation or something like
that for the functions that are implementing the interface.

https://codereview.chromium.org/2276593004/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_cloud_policy_initializer.h:154:
base::WeakPtrFactory<DeviceCloudPolicyInitializer> weak_ptr_factory_;
Do you need this weak pointer? I think you only need the one in the inner class?

[The one and only Dr. Crash, 2016-08-29 16:26:23]

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

[commit-bot: I haz the power, 2016-08-29 16:35:26]

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

[The one and only Dr. Crash, 2016-08-29 19:55:32]

https://codereview.chromium.org/2276593004/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h (right):

https://codereview.chromium.org/2276593004/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h:151:
std::unique_ptr<chromeos::attestation::AttestationFlow>
On 2016/08/29 11:51:35, pastarmovj wrote:
> Please add a new line and document this function. Also shoudn't the second
line
> be offset by 4 more spaces?

Done, except for the second line because 'git cl format' puts it back there, so
I guess our style guide says there should not be an indent in that case.

https://codereview.chromium.org/2276593004/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/device_cloud_policy_initializer.h (right):

https://codereview.chromium.org/2276593004/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_cloud_policy_initializer.h:109: void
SignData(const std::string& data,
On 2016/08/29 11:51:35, pastarmovj wrote:
> Please add a comment : policy::SigningService implementation or something like
> that for the functions that are implementing the interface.

That is what the override keyword is for IMO. I improved the comment to mention
the interface. I considered calling the interface
policy::SigningServiceInterface but it would be an odd name considering the
enrollment/login code has plenty of interfaces that are not called *Interface at
all.

https://codereview.chromium.org/2276593004/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_cloud_policy_initializer.h:154:
base::WeakPtrFactory<DeviceCloudPolicyInitializer> weak_ptr_factory_;
On 2016/08/29 11:51:35, pastarmovj wrote:
> Do you need this weak pointer? I think you only need the one in the inner
class?

You are right, I do not need it anymore. Removed.

[The one and only Dr. Crash, 2016-08-29 21:45:08]

Description was changed from

==========
Device registration using an enrollment certificate from the PCA.

BUG=624187
TEST=unit_tests
==========

to

==========
Device registration using an enrollment certificate from the PCA.

BUG=624187,641447
TEST=unit_tests
==========

[The one and only Dr. Crash, 2016-08-29 21:48:17]

Description was changed from

==========
Device registration using an enrollment certificate from the PCA.

BUG=624187,641447
TEST=unit_tests
==========

to

==========
Device registration using an enrollment certificate from the PCA.

BUG=624187
TEST=unit_tests
==========

[pastarmovj, 2016-08-29 22:08:51]

lgtm

https://codereview.chromium.org/2276593004/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h (right):

https://codereview.chromium.org/2276593004/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h:151:
std::unique_ptr<chromeos::attestation::AttestationFlow>
On 2016/08/29 19:55:32, The one and only Dr. Crash wrote:
> On 2016/08/29 11:51:35, pastarmovj wrote:
> > Please add a new line and document this function. Also shoudn't the second
> line
> > be offset by 4 more spaces?
> 
> Done, except for the second line because 'git cl format' puts it back there,
so
> I guess our style guide says there should not be an indent in that case.

Sure whatever the formatter says. Who are we to argue ;)

[The one and only Dr. Crash, 2016-08-29 22:36:27]

The CQ bit was unchecked by drcrash@chromium.org

[The one and only Dr. Crash, 2016-08-29 22:36:28]

The CQ bit was checked by drcrash@chromium.org

[The one and only Dr. Crash, 2016-08-29 22:36:29]

The patchset sent to the CQ was uploaded after l-g-t-m from dkalin@google.com
Link to the patchset: https://codereview.chromium.org/2276593004/#ps100001
(title: "Adressed review feedback.")

[The one and only Dr. Crash, 2016-08-29 22:45:30]

The CQ bit was unchecked by drcrash@chromium.org

[The one and only Dr. Crash, 2016-08-29 22:45:37]

The CQ bit was checked by drcrash@chromium.org

[The one and only Dr. Crash, 2016-08-29 23:08:47]

The CQ bit was checked by drcrash@chromium.org

[The one and only Dr. Crash, 2016-08-29 23:08:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
pastarmovj@chromium.org, dkalin@google.com
Link to the patchset: https://codereview.chromium.org/2276593004/#ps120001
(title: "Fixed constant.")

[commit-bot: I haz the power, 2016-08-29 23:21:13]

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, 2016-08-30 03:50:05]

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

[commit-bot: I haz the power, 2016-08-30 03:50:06]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[The one and only Dr. Crash, 2016-08-30 04:06:05]

The CQ bit was checked by drcrash@chromium.org

[commit-bot: I haz the power, 2016-08-30 04:08:53]

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, 2016-08-30 05:21:06]

Description was changed from

==========
Device registration using an enrollment certificate from the PCA.

BUG=624187
TEST=unit_tests
==========

to

==========
Device registration using an enrollment certificate from the PCA.

BUG=624187
TEST=unit_tests
==========

[commit-bot: I haz the power, 2016-08-30 05:21:08]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-08-30 05:22:23]

Description was changed from

==========
Device registration using an enrollment certificate from the PCA.

BUG=624187
TEST=unit_tests
==========

to

==========
Device registration using an enrollment certificate from the PCA.

BUG=624187
TEST=unit_tests

Committed: https://crrev.com/59380d4af7c23cae135488b421df2756894b02fe
Cr-Commit-Position: refs/heads/master@{#415148}
==========

[commit-bot: I haz the power, 2016-08-30 05:22:24]

Patchset 7 (id:??) landed as
https://crrev.com/59380d4af7c23cae135488b421df2756894b02fe
Cr-Commit-Position: refs/heads/master@{#415148}