[Cleanup] Used scoped pointers in KeyedServiceFactory's SetTestingFactory functions.

[Cleanup] Used scoped pointers in KeyedServiceFactory's SetTestingFactory functions.

Along the way, clean up some tests' usage of SetTestingFactory().

BUG=NONE
TEST=git cl try

Committed: https://crrev.com/30fa851a798084bd26792079ecf5fd1e317dde2d
Cr-Commit-Position: refs/heads/master@{#333605}

[Ilya Sherman, 2015-06-06 00:10:17]

isherman@chromium.org changed reviewers:
+ erg@chromium.org

[Ilya Sherman, 2015-06-06 00:10:19]

Elliot, PTAL at //components/keyed_service, as well as any other files that
you'd like to spot check or review.

[vabr (Chromium), 2015-06-08 11:13:21]

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

[vabr (Chromium), 2015-06-08 11:13:23]

Drive-by: All in */password_manager/* LGTM.
Thanks for the clean-up!
Vaclav

[vabr (Chromium), 2015-06-08 11:14:48]

On 2015/06/08 11:13:23, vabr (Chromium) wrote:
> Drive-by: All in */password_manager/* LGTM.
> Thanks for the clean-up!
> Vaclav

Also, chrome/browser/profile_resetter/profile_resetter_unittest.cc LGTM.

[Elliot Glaysher, 2015-06-08 17:15:01]

keyed_services lgtm

[Ilya Sherman, 2015-06-08 18:43:03]

isherman@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org, rkc@chromium.org, sky@chromium.org

[Ilya Sherman, 2015-06-08 18:43:05]

Scott, could you please stamp the //chrome changes?
Rahul, could you please stamp the //components/feedback changes?
Devlin, could you please stamp the //extensions changes?

[rkc, 2015-06-08 18:47:46]

lgtm

[Devlin, 2015-06-08 19:29:44]

extensions lgtm.  I noticed a few things that bothered me (mostly
BrowserContexts called profiles) along the way that would be good to change, but
consider them all optional, since they were that way before.

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File
chrome/browser/extensions/api/image_writer_private/operation_manager_unittest.cc
(right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/extensions/api/image_writer_private/operation_manager_unittest.cc:41:
return make_scoped_ptr(new FakeEventRouter(static_cast<Profile*>(context)));
nit: I liked that you updated some of these to be Profile::FromBrowserContext()
(even though it doesn't really matter).  If you don't mind, it'd be good to
update this one to.  If it's a hassle, don't bother (clearly it was already like
this).

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File chrome/browser/extensions/api/mdns/mdns_api_unittest.cc (right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/extensions/api/mdns/mdns_api_unittest.cc:29:
scoped_ptr<KeyedService> BuildEventRouter(content::BrowserContext* profile) {
nit: rename |profile| to |browser_context| or |context|.

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/extensions/api/mdns/mdns_api_unittest.cc:32:
ExtensionPrefsFactory::GetInstance()->GetForBrowserContext((profile))));
nit: Since you're here, could you update this to be simply
ExtensionPrefs::Get()?

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File
chrome/browser/extensions/api/networking_private/networking_private_chromeos_apitest.cc
(right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/extensions/api/networking_private/networking_private_chromeos_apitest.cc:234:
content::BrowserContext* profile) {
nit: rename |profile|.

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File
chrome/browser/extensions/api/networking_private/networking_private_service_client_apitest.cc
(right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/extensions/api/networking_private/networking_private_service_client_apitest.cc:88:
content::BrowserContext* profile) {
nit: rename

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File chrome/browser/extensions/api/sessions/sessions_apitest.cc (right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/extensions/api/sessions/sessions_apitest.cc:125:
content::BrowserContext* profile) {
nit: rename

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File
chrome/browser/extensions/api/signed_in_devices/signed_in_devices_api_unittest.cc
(right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/extensions/api/signed_in_devices/signed_in_devices_api_unittest.cc:149:
content::BrowserContext* profile) {
nit: rename

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File chrome/browser/extensions/extension_garbage_collector_factory.h (right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/extensions/extension_garbage_collector_factory.h:28:
content::BrowserContext* profile);
nit: rename

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File chrome/browser/extensions/menu_manager_unittest.cc (right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/extensions/menu_manager_unittest.cc:488: content::BrowserContext*
profile) {
nit: rename

https://chromiumcodereview.appspot.com/1165913002/diff/100001/extensions/brow...
File extensions/browser/api/storage/storage_api_unittest.cc (right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/extensions/brow...
extensions/browser/api/storage/storage_api_unittest.cc:39:
scoped_ptr<KeyedService> BuildEventRouter(content::BrowserContext* profile) {
nit: rename

[sky, 2015-06-08 20:18:50]

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File chrome/browser/invalidation/profile_invalidation_provider_factory.cc
(right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/invalidation/profile_invalidation_provider_factory.cc:95:
KeyedService* ProfileInvalidationProviderFactory::BuildServiceInstanceFor(
It seems weird not to change this too. Is there a reason you didn't want to
update BuildServiceInfanceFor to return a scoped_ptr too?

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File
chrome/browser/safe_browsing/incident_reporting/last_download_finder_unittest.cc
(right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/safe_browsing/incident_reporting/last_download_finder_unittest.cc:56:
return scoped_ptr<KeyedService>();
I believe return nullptr here and 70 should work.

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File
chrome/browser/safe_browsing/incident_reporting/off_domain_inclusion_detector_unittest.cc
(right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/safe_browsing/incident_reporting/off_domain_inclusion_detector_unittest.cc:69:
return scoped_ptr<KeyedService>();
nullptr (and 83).

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File chrome/browser/signin/easy_unlock_service_unittest_chromeos.cc (right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/signin/easy_unlock_service_unittest_chromeos.cc:177: return
scoped_ptr<KeyedService>();
nullptr (and 183).

[Ilya Sherman, 2015-06-08 22:11:44]

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File
chrome/browser/extensions/api/image_writer_private/operation_manager_unittest.cc
(right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/extensions/api/image_writer_private/operation_manager_unittest.cc:41:
return make_scoped_ptr(new FakeEventRouter(static_cast<Profile*>(context)));
On 2015/06/08 19:29:43, Devlin wrote:
> nit: I liked that you updated some of these to be
Profile::FromBrowserContext()
> (even though it doesn't really matter).  If you don't mind, it'd be good to
> update this one to.  If it's a hassle, don't bother (clearly it was already
like
> this).

I was thinking of making a separate pass to update these across the codebase. 
I'm going to punt on this for now.

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File chrome/browser/extensions/api/mdns/mdns_api_unittest.cc (right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/extensions/api/mdns/mdns_api_unittest.cc:29:
scoped_ptr<KeyedService> BuildEventRouter(content::BrowserContext* profile) {
On 2015/06/08 19:29:43, Devlin wrote:
> nit: rename |profile| to |browser_context| or |context|.

Done.

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/extensions/api/mdns/mdns_api_unittest.cc:32:
ExtensionPrefsFactory::GetInstance()->GetForBrowserContext((profile))));
On 2015/06/08 19:29:43, Devlin wrote:
> nit: Since you're here, could you update this to be simply
> ExtensionPrefs::Get()?

Done.

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File
chrome/browser/extensions/api/networking_private/networking_private_chromeos_apitest.cc
(right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/extensions/api/networking_private/networking_private_chromeos_apitest.cc:234:
content::BrowserContext* profile) {
On 2015/06/08 19:29:43, Devlin wrote:
> nit: rename |profile|.

Done.

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File
chrome/browser/extensions/api/networking_private/networking_private_service_client_apitest.cc
(right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/extensions/api/networking_private/networking_private_service_client_apitest.cc:88:
content::BrowserContext* profile) {
On 2015/06/08 19:29:43, Devlin wrote:
> nit: rename

Done.

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File chrome/browser/extensions/api/sessions/sessions_apitest.cc (right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/extensions/api/sessions/sessions_apitest.cc:125:
content::BrowserContext* profile) {
On 2015/06/08 19:29:43, Devlin wrote:
> nit: rename

Done.

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File
chrome/browser/extensions/api/signed_in_devices/signed_in_devices_api_unittest.cc
(right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/extensions/api/signed_in_devices/signed_in_devices_api_unittest.cc:149:
content::BrowserContext* profile) {
On 2015/06/08 19:29:43, Devlin wrote:
> nit: rename

Done.

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File chrome/browser/extensions/extension_garbage_collector_factory.h (right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/extensions/extension_garbage_collector_factory.h:28:
content::BrowserContext* profile);
On 2015/06/08 19:29:43, Devlin wrote:
> nit: rename

Done.

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File chrome/browser/extensions/menu_manager_unittest.cc (right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/extensions/menu_manager_unittest.cc:488: content::BrowserContext*
profile) {
On 2015/06/08 19:29:43, Devlin wrote:
> nit: rename

Done.

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File chrome/browser/invalidation/profile_invalidation_provider_factory.cc
(right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/invalidation/profile_invalidation_provider_factory.cc:95:
KeyedService* ProfileInvalidationProviderFactory::BuildServiceInstanceFor(
On 2015/06/08 20:18:49, sky wrote:
> It seems weird not to change this too. Is there a reason you didn't want to
> update BuildServiceInfanceFor to return a scoped_ptr too?

I'd like to change it, but would prefer to punt that to a separate CL (which
might take a while to author).  Note that this method is overridden from
BrowserContextKeyedServiceFactory.  There are roughly 400 files that override
the same method, so I figured that it made sense to transition the API over one
step at a time.

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File
chrome/browser/safe_browsing/incident_reporting/last_download_finder_unittest.cc
(right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/safe_browsing/incident_reporting/last_download_finder_unittest.cc:56:
return scoped_ptr<KeyedService>();
On 2015/06/08 20:18:49, sky wrote:
> I believe return nullptr here and 70 should work.

Good to know!  Changed, here and throughout the CL.

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File
chrome/browser/safe_browsing/incident_reporting/off_domain_inclusion_detector_unittest.cc
(right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/safe_browsing/incident_reporting/off_domain_inclusion_detector_unittest.cc:69:
return scoped_ptr<KeyedService>();
On 2015/06/08 20:18:49, sky wrote:
> nullptr (and 83).

Done.

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
File chrome/browser/signin/easy_unlock_service_unittest_chromeos.cc (right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/chrome/browser/...
chrome/browser/signin/easy_unlock_service_unittest_chromeos.cc:177: return
scoped_ptr<KeyedService>();
On 2015/06/08 20:18:49, sky wrote:
> nullptr (and 183).

Done.

https://chromiumcodereview.appspot.com/1165913002/diff/100001/extensions/brow...
File extensions/browser/api/storage/storage_api_unittest.cc (right):

https://chromiumcodereview.appspot.com/1165913002/diff/100001/extensions/brow...
extensions/browser/api/storage/storage_api_unittest.cc:39:
scoped_ptr<KeyedService> BuildEventRouter(content::BrowserContext* profile) {
On 2015/06/08 19:29:43, Devlin wrote:
> nit: rename

Done.

[Kevin M, 2015-06-08 22:15:37]

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

[Kevin M, 2015-06-08 22:15:39]

media/router lgtm

[sdefresne, 2015-06-09 09:40:52]

On 2015/06/06 00:10:19, Ilya Sherman wrote:
> Elliot, PTAL at //components/keyed_service, as well as any other files that
> you'd like to spot check or review.

There's some BSKSF in //ios
(https://code.google.com/p/chromium/codesearch#search/&q=public%5C%20BrowserSt...).
Why are they not ported to return scoped_ptr<KeyedService> as are BCKSF?

[sky, 2015-06-09 13:08:22]

LGTM

[Ilya Sherman, 2015-06-09 19:49:07]

On 2015/06/09 09:40:52, sdefresne wrote:
> On 2015/06/06 00:10:19, Ilya Sherman wrote:
> > Elliot, PTAL at //components/keyed_service, as well as any other files that
> > you'd like to spot check or review.
> 
> There's some BSKSF in //ios
>
(https://code.google.com/p/chromium/codesearch#search/&q=public%5C%20BrowserSt...).
> Why are they not ported to return scoped_ptr<KeyedService> as are BCKSF?

For this CL, I'm trying to do roughly the minimum to clean up the ownership
model for testing factories.  AFAICT, there are no further changes needed for
iOS code to correctly compile with the changes (see trybot runs with patch set
6).  I'd like to clean up the memory ownership semantics further, but to keep CL
sizes somewhat smaller, I'm not aiming to do everything in this particular CL.

[Ilya Sherman, 2015-06-09 20:45:26]

The CQ bit was checked by isherman@chromium.org

[Ilya Sherman, 2015-06-09 20:45:27]

The patchset sent to the CQ was uploaded after l-g-t-m from vabr@chromium.org,
rkc@chromium.org, rdevlin.cronin@chromium.org, erg@chromium.org
Link to the patchset: https://codereview.chromium.org/1165913002/#ps140001
(title: "Rebase")

[commit-bot: I haz the power, 2015-06-09 20:46:57]

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

[commit-bot: I haz the power, 2015-06-09 21:39:14]

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

[commit-bot: I haz the power, 2015-06-09 21:39:17]

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

[Ilya Sherman, 2015-06-09 21:56:51]

The CQ bit was checked by isherman@chromium.org

[Ilya Sherman, 2015-06-09 21:56:52]

The patchset sent to the CQ was uploaded after l-g-t-m from rkc@chromium.org,
rdevlin.cronin@chromium.org, sky@chromium.org, erg@chromium.org,
vabr@chromium.org, kmarshall@chromium.org
Link to the patchset: https://codereview.chromium.org/1165913002/#ps160001
(title: "Finish renaming profile -> context")

[commit-bot: I haz the power, 2015-06-09 21:57:52]

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

[commit-bot: I haz the power, 2015-06-09 23:32:24]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2015-06-09 23:33:15]

Patchset 9 (id:??) landed as
https://crrev.com/30fa851a798084bd26792079ecf5fd1e317dde2d
Cr-Commit-Position: refs/heads/master@{#333605}