Make OAuth2TokenService and subclasses take delegate by unique_ptr

Make OAuth2TokenService and subclasses take delegate by unique_ptr

OAuth2TokenService and it's subclasses already take ownership of the delegate
passed to the constructor. Using unique_ptr instead of raw pointer makes
this behavior obvious for callers and also reduces the chance of leaks.

BUG=688387
Review-Url: https://codereview.chromium.org/2685123002
Cr-Commit-Position: refs/heads/master@{#449584}
Committed: https://chromium.googlesource.com/chromium/src/+/a9cae0d0eb1e430848ca10bdbfd22cc1f373610e

[bsazonov, 2017-02-09 14:46:53]

Description was changed from

==========
Revised param passing in the constructor of OAuth2TokenService and derived
classes - now delegate is passed as unique_ptr instead of raw pointer

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

to

==========
Revised param passing in the constructor of OAuth2TokenService and derived
classes - now delegate is passed as unique_ptr instead of raw pointer

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

[bsazonov, 2017-02-09 14:47:42]

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

[commit-bot: I haz the power, 2017-02-09 14:48:01]

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-02-09 14:56:48]

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

[commit-bot: I haz the power, 2017-02-09 14:56:49]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[bsazonov, 2017-02-09 15:48:44]

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

[commit-bot: I haz the power, 2017-02-09 15:49:03]

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

[bsazonov, 2017-02-09 15:49:31]

Patchset #1 (id:1) has been deleted

[bsazonov, 2017-02-09 15:50:25]

Description was changed from

==========
Revised param passing in the constructor of OAuth2TokenService and derived
classes - now delegate is passed as unique_ptr instead of raw pointer

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

to

==========
Make OAuth2TokenService and subclasses take delegate as an unique_ptr

OAuth2TokenService and it's subclasses take ownership of the delegate passed
to the constructor. Using unique_ptr instead of raw pointer makes this obvious
and also improves exception-safety of these constructors.

BUG=688387
==========

[commit-bot: I haz the power, 2017-02-09 15:57:32]

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

[commit-bot: I haz the power, 2017-02-09 15:57:34]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[bsazonov, 2017-02-09 16:31:15]

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

[commit-bot: I haz the power, 2017-02-09 16:31:37]

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

[bsazonov, 2017-02-09 16:32:33]

Patchset #1 (id:20001) has been deleted

[commit-bot: I haz the power, 2017-02-09 16:41:18]

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

[commit-bot: I haz the power, 2017-02-09 16:41:18]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[bsazonov, 2017-02-09 16:50:42]

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

[commit-bot: I haz the power, 2017-02-09 16:51:19]

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

[bsazonov, 2017-02-09 16:51:24]

Patchset #1 (id:40001) has been deleted

[commit-bot: I haz the power, 2017-02-09 17:04:39]

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

[commit-bot: I haz the power, 2017-02-09 17:04:40]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[bsazonov, 2017-02-09 17:14:26]

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

[commit-bot: I haz the power, 2017-02-09 17:15:00]

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-02-09 17:31:30]

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

[commit-bot: I haz the power, 2017-02-09 17:31:32]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[bsazonov, 2017-02-09 17:37:50]

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

[commit-bot: I haz the power, 2017-02-09 17:38:13]

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-02-09 17:53:03]

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

[commit-bot: I haz the power, 2017-02-09 17:53:04]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[bsazonov, 2017-02-09 18:14:46]

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

[commit-bot: I haz the power, 2017-02-09 18:15:30]

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

[bsazonov, 2017-02-09 18:16:01]

Patchset #1 (id:60001) has been deleted

[bsazonov, 2017-02-09 18:17:15]

Patchset #1 (id:80001) has been deleted

[bsazonov, 2017-02-09 18:17:26]

Patchset #1 (id:100001) has been deleted

[bsazonov, 2017-02-09 18:30:38]

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

[bsazonov, 2017-02-09 18:30:51]

Patchset #1 (id:120001) has been deleted

[commit-bot: I haz the power, 2017-02-09 18:31:13]

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

[bsazonov, 2017-02-09 19:01:02]

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

[commit-bot: I haz the power, 2017-02-09 19:01:22]

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

[bsazonov, 2017-02-09 19:02:37]

Patchset #1 (id:140001) has been deleted

[bsazonov, 2017-02-09 19:41:27]

Description was changed from

==========
Make OAuth2TokenService and subclasses take delegate as an unique_ptr

OAuth2TokenService and it's subclasses take ownership of the delegate passed
to the constructor. Using unique_ptr instead of raw pointer makes this obvious
and also improves exception-safety of these constructors.

BUG=688387
==========

to

==========
Make OAuth2TokenService and subclasses take delegate by unique_ptr

OAuth2TokenService and it's subclasses already take ownership of the delegate
passed to the constructor. Using unique_ptr instead of raw pointer makes
this behavior obvious for callers and also reduces the chance of leaks.

BUG=688387
==========

[bsazonov, 2017-02-09 19:49:37]

bsazonov@chromium.org changed reviewers:
+ derat@chromium.org, jkrcal@chromium.org, msarda@chromium.org

[bsazonov, 2017-02-09 19:49:38]

jkrcal@chromium.org: Please review changes in
components/ntp_snippets/remote/remote_suggestions_fetcher_unittest.cc

derat@chromium.org: Please review changes in  chrome/browser/chromeos/settings/
and extensions/shell/browser/shell_oauth2_token_service.cc

msarda@chromium.org: Please review changes in 
chrome/browser/signin/, components/signin/core/browser/, 	 google_apis/gaia/ and
 ios/chrome/browser/signin/

[commit-bot: I haz the power, 2017-02-09 20:04:25]

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

[commit-bot: I haz the power, 2017-02-09 20:04:26]

Dry run: This issue passed the CQ dry run.

[jkrcal, 2017-02-09 20:14:19]

components/ntp_snippets/remote/remote_suggestions_fetcher_unittest.cc lgtm

Thanks!

[Daniel Erat, 2017-02-09 22:15:03]

lgtm for c/b/chromeos/settings and extensions/shell/browser/

[msarda, 2017-02-10 09:21:22]

LGTM with an optional nit. Thank you for doing this clean-up.

https://codereview.chromium.org/2685123002/diff/70018/chrome/browser/chromeos...
File chrome/browser/chromeos/settings/device_oauth2_token_service_unittest.cc
(right):

https://codereview.chromium.org/2685123002/diff/70018/chrome/browser/chromeos...
chrome/browser/chromeos/settings/device_oauth2_token_service_unittest.cc:137:
oauth2_service_.reset(new DeviceOAuth2TokenService(std::move(delegate)));
Optional: Consider using MakeUnique.

[bsazonov, 2017-02-10 10:41:50]

https://codereview.chromium.org/2685123002/diff/70018/chrome/browser/chromeos...
File chrome/browser/chromeos/settings/device_oauth2_token_service_unittest.cc
(right):

https://codereview.chromium.org/2685123002/diff/70018/chrome/browser/chromeos...
chrome/browser/chromeos/settings/device_oauth2_token_service_unittest.cc:137:
oauth2_service_.reset(new DeviceOAuth2TokenService(std::move(delegate)));
On 2017/02/10 09:21:22, msarda wrote:
> Optional: Consider using MakeUnique.

The DeviceOAuth2TokenService ctor is private, so MakeUnique won't compile. There
are some workarounds for it, but they are outside of this CL's scope.

[bsazonov, 2017-02-10 10:41:58]

The CQ bit was checked by bsazonov@chromium.org

[commit-bot: I haz the power, 2017-02-10 10:42:22]

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-02-10 10:46:37]

CQ is committing da patch.

Bot data: {"patchset_id": 70018, "attempt_start_ts": 1486723318528760,
"parent_rev": "12a5a4085139903e5fbd11468b8fc77a56e50ad2", "commit_rev":
"a9cae0d0eb1e430848ca10bdbfd22cc1f373610e"}

[commit-bot: I haz the power, 2017-02-10 10:47:09]

Description was changed from

==========
Make OAuth2TokenService and subclasses take delegate by unique_ptr

OAuth2TokenService and it's subclasses already take ownership of the delegate
passed to the constructor. Using unique_ptr instead of raw pointer makes
this behavior obvious for callers and also reduces the chance of leaks.

BUG=688387
==========

to

==========
Make OAuth2TokenService and subclasses take delegate by unique_ptr

OAuth2TokenService and it's subclasses already take ownership of the delegate
passed to the constructor. Using unique_ptr instead of raw pointer makes
this behavior obvious for callers and also reduces the chance of leaks.

BUG=688387

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

[commit-bot: I haz the power, 2017-02-10 10:47:10]

Committed patchset #1 (id:70018) as
https://chromium.googlesource.com/chromium/src/+/a9cae0d0eb1e430848ca10bdbfd2...