Second try at splitting WebDataService (minus ownership changes)

chrome/browser/webdata/web_data_service.h

Index: chrome/browser/webdata/web_data_service.h
diff --git a/chrome/browser/webdata/web_data_service.h b/chrome/browser/webdata/web_data_service.h
index 3dfe4a20619c72b64a8fc828d6ab7d9813115207..cb990a9e4a0162efee4d4dc16eb5274ff24354ac 100644
--- a/chrome/browser/webdata/web_data_service.h
+++ b/chrome/browser/webdata/web_data_service.h
@@ -28,6 +28,7 @@
 #include "chrome/browser/search_engines/template_url_id.h"
 #include "chrome/browser/webdata/keyword_table.h"
 #include "chrome/browser/webdata/web_data_request_manager.h"
+#include "chrome/browser/webdata/web_database.h"
 #include "content/public/browser/browser_thread.h"
 #include "sql/init_status.h"
 
@@ -42,7 +43,7 @@ struct IE7PasswordInfo;
 class MessageLoop;
 class Profile;
 class SkBitmap;
-class WebDatabase;
+class WebDatabaseService;
 
 namespace base {
 class Thread;
@@ -129,17 +130,16 @@ class WebDataService
   // call.
   virtual void ShutdownOnUIThread() OVERRIDE;
 
-  // Initializes the web data service. Returns false on failure
-  // Takes the path of the profile directory as its argument.
-  bool Init(const base::FilePath& profile_path);
-
-  // Returns false if Shutdown() has been called.
-  bool IsRunning() const;
+  // Initializes the web data service.
+  void Init(const base::FilePath& path);
 
   // Unloads the database without actually shutting down the service.  This can
   // be used to temporarily reduce the browser process' memory footprint.
   void UnloadDatabase();
 
+  // Unloads the database permanently and shuts down service.
+  void ShutdownDatabase();
+
   virtual bool IsDatabaseLoaded();

[dhollowa, 2013/03/15 18:56:21]

Doc please.

[Cait (Slow), 2013/03/15 20:43:59]

Done.

   virtual WebDatabase* GetDatabase();

[dhollowa, 2013/03/15 18:56:21]

Doc please.  When can/should this be called?  Is caller responsible for checking
IsDatabaseLoaded prior to calling this?

[Cait (Slow), 2013/03/15 20:43:59]

Done.

 
@@ -320,11 +320,6 @@ class WebDataService
   virtual AutocompleteSyncableService*
       GetAutocompleteSyncableService() const;
 
-  // Testing
-#ifdef UNIT_TEST
-  void set_failed_init(bool value) { failed_init_ = value; }
-#endif
-
  protected:
   friend class TemplateURLServiceTest;
   friend class TemplateURLServiceTestingProfile;
@@ -333,9 +328,6 @@ class WebDataService
 
   virtual ~WebDataService();
 
-  // This is invoked by the unit test; path is the path of the Web Data file.
-  bool InitWithPath(const base::FilePath& path);
-
   //////////////////////////////////////////////////////////////////////////////
   //
   // The following methods are only invoked in the web data service thread.
@@ -346,59 +338,31 @@ class WebDataService
       content::BrowserThread::UI>;
   friend class base::DeleteHelper<WebDataService>;
 
-  // Invoked on the main thread if initializing the db fails.
-  void DBInitFailed(sql::InitStatus init_status);
-
-  // Initialize the database, if it hasn't already been initialized.
-  void InitializeDatabaseIfNecessary();
-
   // Initialize any syncable services.
   void InitializeSyncableServices();
 
-  // The notification method.
-  void NotifyDatabaseLoadedOnUIThread();
-
-  // Commit any pending transaction and deletes the database.
-  void ShutdownDatabase();
-
   // Deletes the syncable services.
   void ShutdownSyncableServices();
 
-  // Commit the current transaction and creates a new one.
-  void Commit();
-
-  // Schedule a task on our worker thread.
-  void ScheduleTask(const tracked_objects::Location& from_here,
-                    const base::Closure& task);
-
-  void ScheduleDBTask(const tracked_objects::Location& from_here,
-                      const base::Closure& task);
-
-  WebDataService::Handle ScheduleDBTaskWithResult(
-      const tracked_objects::Location& from_here,
-      const ResultTask& task,
-      WebDataServiceConsumer* consumer);
-
-  void DBTaskWrapper(const base::Closure& task,
-                     scoped_ptr<WebDataRequest> request);
-
-  void DBResultTaskWrapper(const ResultTask& task,
-                           scoped_ptr<WebDataRequest> request);
-
-  // Schedule a commit if one is not already pending.
-  void ScheduleCommit();
+  void DBInitFailed(sql::InitStatus sql_status);
+  void NotifyDatabaseLoadedOnUIThread();
+  void OnDatabaseInit(sql::InitStatus status);
 
   //////////////////////////////////////////////////////////////////////////////
   //
   // Keywords.
   //
   //////////////////////////////////////////////////////////////////////////////
-  void AddKeywordImpl(const TemplateURLData& data);
-  void RemoveKeywordImpl(TemplateURLID id);
-  void UpdateKeywordImpl(const TemplateURLData& data);
-  scoped_ptr<WDTypedResult> GetKeywordsImpl();
-  void SetDefaultSearchProviderImpl(TemplateURLID r);
-  void SetBuiltinKeywordVersionImpl(int version);
+  WebDatabase::State AddKeywordImpl(
+      const TemplateURLData& data, WebDatabase* db);
+  WebDatabase::State RemoveKeywordImpl(
+      TemplateURLID id, WebDatabase* db);
+  WebDatabase::State UpdateKeywordImpl(
+      const TemplateURLData& data, WebDatabase* db);
+  scoped_ptr<WDTypedResult> GetKeywordsImpl(WebDatabase* db);
+  WebDatabase::State SetDefaultSearchProviderImpl(
+      TemplateURLID r, WebDatabase* db);
+  WebDatabase::State SetBuiltinKeywordVersionImpl(int version, WebDatabase* db);
 
   //////////////////////////////////////////////////////////////////////////////
   //
@@ -406,10 +370,13 @@ class WebDataService
   //
   //////////////////////////////////////////////////////////////////////////////
 
-  void SetWebAppImageImpl(const GURL& app_url, const SkBitmap& image);
-  void SetWebAppHasAllImagesImpl(const GURL& app_url, bool has_all_images);
-  void RemoveWebAppImpl(const GURL& app_url);
-  scoped_ptr<WDTypedResult> GetWebAppImagesImpl(const GURL& app_url);
+  WebDatabase::State SetWebAppImageImpl(const GURL& app_url,
+      const SkBitmap& image, WebDatabase* db);
+  WebDatabase::State SetWebAppHasAllImagesImpl(const GURL& app_url,
+      bool has_all_images, WebDatabase* db);
+  WebDatabase::State RemoveWebAppImpl(const GURL& app_url, WebDatabase* db);
+  scoped_ptr<WDTypedResult> GetWebAppImagesImpl(
+      const GURL& app_url, WebDatabase* db);
 
 #if defined(ENABLE_WEB_INTENTS)
   //////////////////////////////////////////////////////////////////////////////
@@ -417,18 +384,20 @@ class WebDataService
   // Web Intents.
   //
   //////////////////////////////////////////////////////////////////////////////
-  void AddWebIntentServiceImpl(
+  WebDatabase::State AddWebIntentServiceImpl(
       const webkit_glue::WebIntentServiceData& service);
-  void RemoveWebIntentServiceImpl(
+  WebDatabase::State RemoveWebIntentServiceImpl(
       const webkit_glue::WebIntentServiceData& service);
   scoped_ptr<WDTypedResult> GetWebIntentServicesImpl(const string16& action);
   scoped_ptr<WDTypedResult> GetWebIntentServicesForURLImpl(
       const string16& service_url);
   scoped_ptr<WDTypedResult> GetAllWebIntentServicesImpl();
-  void AddDefaultWebIntentServiceImpl(const DefaultWebIntentService& service);
-  void RemoveDefaultWebIntentServiceImpl(
+  WebDatabase::State AddDefaultWebIntentServiceImpl(
+      const DefaultWebIntentService& service);
+  WebDatabase::State RemoveDefaultWebIntentServiceImpl(
       const DefaultWebIntentService& service);
-  void RemoveWebIntentServiceDefaultsImpl(const GURL& service_url);
+  WebDatabase::State RemoveWebIntentServiceDefaultsImpl(
+      const GURL& service_url);
   scoped_ptr<WDTypedResult> GetDefaultWebIntentServicesForActionImpl(
       const string16& action);
   scoped_ptr<WDTypedResult> GetAllDefaultWebIntentServicesImpl();
@@ -440,10 +409,10 @@ class WebDataService
   //
   //////////////////////////////////////////////////////////////////////////////
 
-  void RemoveAllTokensImpl();
-  void SetTokenForServiceImpl(const std::string& service,
-                              const std::string& token);
-  scoped_ptr<WDTypedResult> GetAllTokensImpl();
+  WebDatabase::State RemoveAllTokensImpl(WebDatabase* db);
+  WebDatabase::State SetTokenForServiceImpl(const std::string& service,
+      const std::string& token, WebDatabase* db);
+  scoped_ptr<WDTypedResult> GetAllTokensImpl(WebDatabase* db);
 
 #if defined(OS_WIN)
   //////////////////////////////////////////////////////////////////////////////
@@ -451,9 +420,12 @@ class WebDataService
   // Password manager.
   //
   //////////////////////////////////////////////////////////////////////////////
-  void AddIE7LoginImpl(const IE7PasswordInfo& info);
-  void RemoveIE7LoginImpl(const IE7PasswordInfo& info);
-  scoped_ptr<WDTypedResult> GetIE7LoginImpl(const IE7PasswordInfo& info);
+  WebDatabase::State AddIE7LoginImpl(
+      const IE7PasswordInfo& info, WebDatabase* db);
+  WebDatabase::State RemoveIE7LoginImpl(
+      const IE7PasswordInfo& info, WebDatabase* db);
+  scoped_ptr<WDTypedResult> GetIE7LoginImpl(
+      const IE7PasswordInfo& info, WebDatabase* db);
 #endif  // defined(OS_WIN)
 
   //////////////////////////////////////////////////////////////////////////////
@@ -461,47 +433,44 @@ class WebDataService
   // Autofill.
   //
   //////////////////////////////////////////////////////////////////////////////
-  void AddFormElementsImpl(const std::vector<FormFieldData>& fields);
+  WebDatabase::State AddFormElementsImpl(
+      const std::vector<FormFieldData>& fields, WebDatabase* db);
   scoped_ptr<WDTypedResult> GetFormValuesForElementNameImpl(
-      const string16& name, const string16& prefix, int limit);
-  void RemoveFormElementsAddedBetweenImpl(
-      const base::Time& delete_begin, const base::Time& delete_end);
-  void RemoveExpiredFormElementsImpl();
-  void RemoveFormValueForElementNameImpl(const string16& name,
-                                         const string16& value);
-  void AddAutofillProfileImpl(const AutofillProfile& profile);
-  void UpdateAutofillProfileImpl(const AutofillProfile& profile);
-  void RemoveAutofillProfileImpl(const std::string& guid);
-  scoped_ptr<WDTypedResult> GetAutofillProfilesImpl();
-  void AddCreditCardImpl(const CreditCard& credit_card);
-  void UpdateCreditCardImpl(const CreditCard& credit_card);
-  void RemoveCreditCardImpl(const std::string& guid);
-  scoped_ptr<WDTypedResult> GetCreditCardsImpl();
-  void RemoveAutofillProfilesAndCreditCardsModifiedBetweenImpl(
-      const base::Time& delete_begin, const base::Time& delete_end);
+      const string16& name, const string16& prefix, int limit, WebDatabase* db);
+  WebDatabase::State RemoveFormElementsAddedBetweenImpl(
+      const base::Time& delete_begin, const base::Time& delete_end,
+      WebDatabase* db);
+  WebDatabase::State RemoveExpiredFormElementsImpl(WebDatabase* db);
+  WebDatabase::State RemoveFormValueForElementNameImpl(
+      const string16& name, const string16& value, WebDatabase* db);
+  WebDatabase::State AddAutofillProfileImpl(
+      const AutofillProfile& profile, WebDatabase* db);
+  WebDatabase::State UpdateAutofillProfileImpl(
+      const AutofillProfile& profile, WebDatabase* db);
+  WebDatabase::State RemoveAutofillProfileImpl(
+      const std::string& guid, WebDatabase* db);
+  scoped_ptr<WDTypedResult> GetAutofillProfilesImpl(WebDatabase* db);
+  WebDatabase::State AddCreditCardImpl(
+      const CreditCard& credit_card, WebDatabase* db);
+  WebDatabase::State UpdateCreditCardImpl(
+      const CreditCard& credit_card, WebDatabase* db);
+  WebDatabase::State RemoveCreditCardImpl(
+      const std::string& guid, WebDatabase* db);
+  scoped_ptr<WDTypedResult> GetCreditCardsImpl(WebDatabase* db);
+  WebDatabase::State RemoveAutofillProfilesAndCreditCardsModifiedBetweenImpl(
+      const base::Time& delete_begin, const base::Time& delete_end,
+      WebDatabase* db);
 
   // Callbacks to ensure that sensitive info is destroyed if request is
   // cancelled.
   void DestroyAutofillProfileResult(const WDTypedResult* result);
   void DestroyAutofillCreditCardResult(const WDTypedResult* result);
 
-  // True once initialization has started.
-  bool is_running_;
+  // Our database service.
+  scoped_ptr<WebDatabaseService> wdbs_;
 
-  // The path with which to initialize the database.
-  base::FilePath path_;
-
-  // Our database.  We own the |db_|, but don't use a |scoped_ptr| because the
-  // |db_| lifetime must be managed on the database thread.
-  WebDatabase* db_;
-
-  // Keeps track of all pending requests made to the db.
-  scoped_refptr<WebDataRequestManager> request_manager_;
-
-  // The application locale.  The locale is needed for some database migrations,
-  // and must be read on the UI thread.  It's cached here so that we can pass it
-  // to the migration code on the DB thread.
-  const std::string app_locale_;
+  // True if we've received a notification that the WebDatabase has loaded.
+  bool db_loaded_;
 
   // Syncable services for the database data.  We own the services, but don't
   // use |scoped_ptr|s because the lifetimes must be managed on the database
@@ -511,16 +480,6 @@ class WebDataService
   AutocompleteSyncableService* autocomplete_syncable_service_;
   AutofillProfileSyncableService* autofill_profile_syncable_service_;
 
-  // Whether the database failed to initialize.  We use this to avoid
-  // continually trying to reinit.
-  bool failed_init_;
-
-  // Whether we should commit the database.
-  bool should_commit_;
-
-  // MessageLoop the WebDataService is created on.
-  MessageLoop* main_loop_;
-
   DISALLOW_COPY_AND_ASSIGN(WebDataService);
 };