Eliminate AutofillWebDataService::FromBrowserContext().

chrome/browser/webdata/web_data_service_factory.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_WEBDATA_WEB_DATA_SERVICE_FACTORY_H__
 #define CHROME_BROWSER_WEBDATA_WEB_DATA_SERVICE_FACTORY_H__
 
 #include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/singleton.h"
@@ skipping 35 matching lines @@
   scoped_refptr<TokenWebData> token_web_data_;
   scoped_refptr<WebDataService> web_data_;
 
   DISALLOW_COPY_AND_ASSIGN(WebDataServiceWrapper);
 };
 
 // Singleton that owns all WebDataServiceWrappers and associates them with
 // Profiles.
 class WebDataServiceFactory : public BrowserContextKeyedServiceFactory {
  public:
   // Returns the |WebDataServiceWrapper| associated with the |profile|.

[Peter Kasting, 2013/11/19 19:14:15]

Nit: While here: remove pipes around type name (these are only used for variable
names).  (2 places)

[blundell, 2013/12/02 16:11:00]

Done.

   // |access_type| is either EXPLICIT_ACCESS or IMPLICIT_ACCESS
   // (see its definition).

[Peter Kasting, 2013/11/19 19:14:15]

Nit: While here: Remove this last sentence enumerating the values of the enum. 
If someone doesn't understand access types they can figure out how to look up
the declaration.  (2 places)

[blundell, 2013/12/02 16:11:00]

Done.

   static WebDataServiceWrapper* GetForProfile(
       Profile* profile, Profile::ServiceAccessType access_type);

[Peter Kasting, 2013/11/19 19:14:15]

Nit: While here: This is legal, but I tend to prefer one arg per line if the
whole function declaration doesn't fit on one line.  (2 places)

[blundell, 2013/12/02 16:11:00]

Done.

 
   static WebDataServiceWrapper* GetForProfileIfExists(
       Profile* profile, Profile::ServiceAccessType access_type);
 
+  // Returns the |AutofillWebDataService| associated with the |profile| (as
+  // |AutofillWebDataService| is shared with iOS, it cannot know directly about
+  // BrowserContext/Profile and thus cannot define this getter itself).

[Peter Kasting, 2013/11/19 19:14:15]

Nit: Just:

  // Returns the AutofillWebDataService associated with the |profile|.

Once AutofillWebDataService is a layered component it won't even be able to
compile if someone adds a browser context dependency, so there's no need for the
rest of the comment.

[blundell, 2013/12/02 16:11:00]

Done.

+  // |access_type| is either EXPLICIT_ACCESS or IMPLICIT_ACCESS
+  // (see its definition).
+  static scoped_refptr<autofill::AutofillWebDataService>

[Peter Kasting, 2013/11/19 19:14:15]

I thought our current recommendation on scoped_refptr was to return it by raw
pointer until we'd eliminated the implicit refptr->raw pointer conversion.

[blundell, 2013/12/02 16:11:00]

Yes, I filed crbug.com/321059. I'd rather keep those changes separate from this
refactoring to help ensure that this CL doesn't introduce any functional changes
(note that the API I'm moving away from is returning a scoped_refptr).

On 2013/11/19 19:14:15, Peter Kasting wrote:

+      GetAutofillWebDataForProfile(Profile* profile,
+                                   Profile::ServiceAccessType access_type);
+
   static WebDataServiceFactory* GetInstance();
 
  private:
   friend struct DefaultSingletonTraits<WebDataServiceFactory>;
 
   WebDataServiceFactory();
   virtual ~WebDataServiceFactory();
 
   // |BrowserContextKeyedBaseFactory| methods:
   virtual content::BrowserContext* GetBrowserContextToUse(
       content::BrowserContext* context) const OVERRIDE;
   virtual BrowserContextKeyedService* BuildServiceInstanceFor(
       content::BrowserContext* profile) const OVERRIDE;
   virtual bool ServiceIsNULLWhileTesting() const OVERRIDE;
 
   DISALLOW_COPY_AND_ASSIGN(WebDataServiceFactory);
 };
 
 #endif  // CHROME_BROWSER_WEBDATA_WEB_DATA_SERVICE_FACTORY_H__