Remove c/b/api/sync dependency from AutofillManager using delegate.

BUG=140037


Review URL: https://chromiumcodereview.appspot.com/12382017

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@186348 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 95 insertions, 46 deletions

diff --git a/chrome/browser/autofill/DEPS b/chrome/browser/autofill/DEPS
index e5a94c0..dd73f68 100644
--- a/chrome/browser/autofill/DEPS
+++ b/chrome/browser/autofill/DEPS
@@ -1,25 +1,27 @@
 include_rules = [
-  # Autofill is being made into a Browser Component, so we have these basic
-  # rules followed by temporary exceptions.  Please don't add to the list of
-  # exceptions!
+  # Autofill is being made into a component (it will end up at
+  # src/components/autofill and not depend on src/chrome), so we have
+  # these basic rules followed by temporary exceptions.  Please don't
+  # add to the list of exceptions!
   "-chrome/browser",
-  "+chrome/browser/api",
   "+chrome/browser/autofill",
-  "+chrome/browser/common",
 
   # Permanently-allowed DEPS beyond the standard Browser
   # Components-like DEPS above go here.
   "+third_party/libphonenumber",  # For phone number i18n.
 
-  # TODO(joi): May be able to remove this if PKS is moved to c/b/api.
-  "!chrome/browser/profiles/profile_keyed_service.h",
+  # TODO(joi, kaiwang): Bring this list to zero.
+  "!chrome/browser/api/infobars",
+  "!chrome/browser/api/webdata",
+  "!chrome/browser/common",
+  "!chrome/browser/ui/autofill",
 
   # TODO(akalin): Remove this dependency.
   "!sync/util/data_encryption_win.h",
 ]
 
 specific_include_rules = {
-  # TODO(joi): Bring this list to zero.
+  # TODO(joi, kaiwang): Bring this list to zero.
   #
   # Do not add to the list of temporarily-allowed dependencies below,
   # and please do not introduce more #includes of these files.
@@ -47,12 +49,18 @@ specific_include_rules = {
     "!chrome/browser/browser_process.h",
   ],
 
-  # TODO(joi): May be able to get rid of the need for this by moving
-  # PersonalDataManagerFactory to chrome/browser/profiles.
+  # TODO(joi): These files will stay in chrome/browser/autofill when
+  # the rest move to components/autofill. Fix DEPS once that happens.
   'personal_data_manager_factory\.(h|cc)': [
     "!chrome/browser/profiles/profile_dependency_manager.h",
+    "!chrome/browser/profiles/profile_keyed_service.h",
     "!chrome/browser/profiles/profile_keyed_service_factory.h",
     "!chrome/browser/profiles/profile.h",
     "!chrome/browser/webdata/web_data_service_factory.h",
   ],
+
+  'personal_data_manager\.(h|cc)': [
+    # TODO(joi): Remove.
+    "!chrome/browser/api/sync",
+  ],
 }
diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc
index 8747671..5cff9bc 100644
--- a/chrome/browser/autofill/autofill_manager.cc
+++ b/chrome/browser/autofill/autofill_manager.cc
@@ -21,7 +21,6 @@
 #include "base/threading/sequenced_worker_pool.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/api/infobars/infobar_service.h"
-#include "chrome/browser/api/sync/profile_sync_service_base.h"
 #include "chrome/browser/autofill/autocheckout/whitelist_manager.h"
 #include "chrome/browser/autofill/autocheckout_manager.h"
 #include "chrome/browser/autofill/autocomplete_history_manager.h"
@@ -248,9 +247,13 @@ void AutofillManager::RegisterUserPrefs(PrefRegistrySyncable* registry) {
 }
 
 void AutofillManager::RegisterWithSyncService() {
-  ProfileSyncServiceBase* service = manager_delegate_->GetProfileSyncService();
-  if (service)
-    service->AddObserver(this);
+  // TODO(joi): If/when SupportsWebData supports structured
+  // destruction ordering, we could use a base::Unretained here and
+  // just unsubscribe in our destructor. As is, we can't guarantee
+  // that the delegate doesn't get destroyed (by WebContent's
+  // SupportsUserData) right before the AutofillManager.
+  manager_delegate_->SetSyncStateChangedCallback(base::Bind(
+      &AutofillManager::OnSyncStateChanged, weak_ptr_factory_.GetWeakPtr()));
 }
 
 void AutofillManager::SendPasswordGenerationStateToRenderer(
@@ -266,21 +269,12 @@ void AutofillManager::SendPasswordGenerationStateToRenderer(
 void AutofillManager::UpdatePasswordGenerationState(
     content::RenderViewHost* host,
     bool new_renderer) {
-  ProfileSyncServiceBase* service = manager_delegate_->GetProfileSyncService();
-
-  bool password_sync_enabled = false;
-  if (service) {
-    syncer::ModelTypeSet sync_set = service->GetPreferredDataTypes();
-    password_sync_enabled =
-      service->HasSyncSetupCompleted() && sync_set.Has(syncer::PASSWORDS);
-  }
-
   bool saving_passwords_enabled = manager_delegate_->IsSavingPasswordsEnabled();
   bool preference_checked = manager_delegate_->GetPrefs()->GetBoolean(
       prefs::kPasswordGenerationEnabled);
 
   bool new_password_generation_enabled =
-      password_sync_enabled &&
+      manager_delegate_->IsPasswordSyncEnabled() &&
       saving_passwords_enabled &&
       preference_checked;
 
@@ -300,7 +294,7 @@ void AutofillManager::OnPasswordGenerationEnabledChanged() {
   UpdatePasswordGenerationState(web_contents()->GetRenderViewHost(), false);
 }
 
-void AutofillManager::OnStateChanged() {
+void AutofillManager::OnSyncStateChanged() {
   // It is possible for sync state to change during tab contents destruction.
   // In this case, we don't need to update the renderer since it's going away.
   if (web_contents() && web_contents()->GetRenderViewHost()) {
@@ -369,9 +363,8 @@ bool AutofillManager::OnMessageReceived(const IPC::Message& message) {
 }
 
 void AutofillManager::WebContentsDestroyed(content::WebContents* web_contents) {
-  ProfileSyncServiceBase* service = manager_delegate_->GetProfileSyncService();
-  if (service && service->HasObserver(this))
-    service->RemoveObserver(this);
+  // Unsubscribe.
+  manager_delegate_->SetSyncStateChangedCallback(base::Closure());
 }
 
 bool AutofillManager::OnFormSubmitted(const FormData& form,
diff --git a/chrome/browser/autofill/autofill_manager.h b/chrome/browser/autofill/autofill_manager.h
index b071265..e2605a8 100644
--- a/chrome/browser/autofill/autofill_manager.h
+++ b/chrome/browser/autofill/autofill_manager.h
@@ -21,7 +21,6 @@
 #include "base/string16.h"
 #include "base/supports_user_data.h"
 #include "base/time.h"
-#include "chrome/browser/api/sync/profile_sync_service_observer.h"
 #include "chrome/browser/autofill/autocheckout_manager.h"
 #include "chrome/browser/autofill/autocomplete_history_manager.h"
 #include "chrome/browser/autofill/autofill_download.h"
@@ -75,7 +74,6 @@ class Message;
 // forms.
 class AutofillManager : public content::WebContentsObserver,
                         public AutofillDownloadManager::Observer,
-                        public ProfileSyncServiceObserver,
                         public base::SupportsUserData::Data {
  public:
   static void CreateForWebContentsAndDelegate(
@@ -220,8 +218,7 @@ class AutofillManager : public content::WebContentsObserver,
   virtual void OnLoadedServerPredictions(
       const std::string& response_xml) OVERRIDE;
 
-  // ProfileSyncServiceObserver:
-  virtual void OnStateChanged() OVERRIDE;
+  void OnSyncStateChanged();
 
   // Register as an observer with the sync service.
   void RegisterWithSyncService();
diff --git a/chrome/browser/autofill/autofill_manager_delegate.h b/chrome/browser/autofill/autofill_manager_delegate.h
index f908352..5547d3b 100644
--- a/chrome/browser/autofill/autofill_manager_delegate.h
+++ b/chrome/browser/autofill/autofill_manager_delegate.h
@@ -66,10 +66,6 @@ class AutofillManagerDelegate {
   // Gets the preferences associated with the delegate.
   virtual PrefService* GetPrefs() = 0;
 
-  // Gets the profile sync service associated with the delegate.  Will
-  // be NULL if sync is not enabled.
-  virtual ProfileSyncServiceBase* GetProfileSyncService() = 0;
-
   // Hides the associated request autocomplete dialog (if it exists).
   virtual void HideRequestAutocompleteDialog() = 0;
 
@@ -77,6 +73,16 @@ class AutofillManagerDelegate {
   // delegate.
   virtual bool IsSavingPasswordsEnabled() const = 0;
 
+  // Returns true if Sync is enabled for the passwords datatype.
+  virtual bool IsPasswordSyncEnabled() const = 0;
+
+  // Sets a callback that will be called when sync state changes.
+  //
+  // Set the callback to an object for which |is_null()| evaluates to
+  // true to stop receiving notifications
+  // (e.g. SetSyncStateChangedCallback(base::Closure())).
+  virtual void SetSyncStateChangedCallback(const base::Closure& callback) = 0;
+
   // Causes an error explaining that Autocheckout has failed to be displayed to
   // the user.
   virtual void OnAutocheckoutError() = 0;
diff --git a/chrome/browser/autofill/test_autofill_manager_delegate.cc b/chrome/browser/autofill/test_autofill_manager_delegate.cc
index 169db1f..6c68f2c 100644
--- a/chrome/browser/autofill/test_autofill_manager_delegate.cc
+++ b/chrome/browser/autofill/test_autofill_manager_delegate.cc
@@ -21,16 +21,19 @@ PrefService* TestAutofillManagerDelegate::GetPrefs() {
   return NULL;
 }
 
-ProfileSyncServiceBase* TestAutofillManagerDelegate::GetProfileSyncService() {
-  return NULL;
-}
-
 void TestAutofillManagerDelegate::HideRequestAutocompleteDialog() {}
 
 bool TestAutofillManagerDelegate::IsSavingPasswordsEnabled() const {
   return false;
 }
 
+bool TestAutofillManagerDelegate::IsPasswordSyncEnabled() const {
+  return false;
+}
+
+void TestAutofillManagerDelegate::SetSyncStateChangedCallback(
+    const base::Closure& callback) { }
+
 void TestAutofillManagerDelegate::OnAutocheckoutError() {}
 
 void TestAutofillManagerDelegate::ShowAutofillSettings() {}
diff --git a/chrome/browser/autofill/test_autofill_manager_delegate.h b/chrome/browser/autofill/test_autofill_manager_delegate.h
index 86ab904..95550ab 100644
--- a/chrome/browser/autofill/test_autofill_manager_delegate.h
+++ b/chrome/browser/autofill/test_autofill_manager_delegate.h
@@ -21,9 +21,11 @@ class TestAutofillManagerDelegate : public AutofillManagerDelegate {
   virtual InfoBarService* GetInfoBarService() OVERRIDE;
   virtual PersonalDataManager* GetPersonalDataManager() OVERRIDE;
   virtual PrefService* GetPrefs() OVERRIDE;
-  virtual ProfileSyncServiceBase* GetProfileSyncService() OVERRIDE;
   virtual void HideRequestAutocompleteDialog() OVERRIDE;
   virtual bool IsSavingPasswordsEnabled() const OVERRIDE;
+  virtual bool IsPasswordSyncEnabled() const OVERRIDE;
+  virtual void SetSyncStateChangedCallback(
+      const base::Closure& callback) OVERRIDE;
   virtual void OnAutocheckoutError() OVERRIDE;
   virtual void ShowAutofillSettings() OVERRIDE;
   virtual void ShowPasswordGenerationBubble(
diff --git a/chrome/browser/ui/autofill/tab_autofill_manager_delegate.cc b/chrome/browser/ui/autofill/tab_autofill_manager_delegate.cc
index 4265af0..33b20f4 100644
--- a/chrome/browser/ui/autofill/tab_autofill_manager_delegate.cc
+++ b/chrome/browser/ui/autofill/tab_autofill_manager_delegate.cc
@@ -56,13 +56,39 @@ PrefService* TabAutofillManagerDelegate::GetPrefs() {
       GetPrefs();
 }
 
-ProfileSyncServiceBase* TabAutofillManagerDelegate::GetProfileSyncService() {
-  return ProfileSyncServiceFactory::GetForProfile(
+bool TabAutofillManagerDelegate::IsSavingPasswordsEnabled() const {
+  return PasswordManager::FromWebContents(web_contents_)->IsSavingEnabled();
+}
+
+bool TabAutofillManagerDelegate::IsPasswordSyncEnabled() const {
+  ProfileSyncServiceBase* service = ProfileSyncServiceFactory::GetForProfile(
       Profile::FromBrowserContext(web_contents_->GetBrowserContext()));
+  if (!service)
+    return false;
+
+  syncer::ModelTypeSet sync_set = service->GetPreferredDataTypes();
+  return service->HasSyncSetupCompleted() && sync_set.Has(syncer::PASSWORDS);
 }
 
-bool TabAutofillManagerDelegate::IsSavingPasswordsEnabled() const {
-  return PasswordManager::FromWebContents(web_contents_)->IsSavingEnabled();
+void TabAutofillManagerDelegate::SetSyncStateChangedCallback(
+    const base::Closure& callback) {
+  ProfileSyncServiceBase* service = ProfileSyncServiceFactory::GetForProfile(
+      Profile::FromBrowserContext(web_contents_->GetBrowserContext()));
+  if (!service)
+    return;
+
+  if (sync_state_changed_callback_.is_null() && !callback.is_null())
+    service->AddObserver(this);
+  else if (!sync_state_changed_callback_.is_null() && callback.is_null())
+    service->RemoveObserver(this);
+
+  sync_state_changed_callback_ = callback;
+
+  // Invariant: Either sync_state_changed_callback_.is_null() is true
+  // and this object is not subscribed as a
+  // ProfileSyncServiceObserver, or
+  // sync_state_changed_callback_.is_null() is false and this object
+  // is subscribed as a ProfileSyncServiceObserver.
 }
 
 void TabAutofillManagerDelegate::OnAutocheckoutError() {
@@ -161,6 +187,11 @@ void TabAutofillManagerDelegate::HideRequestAutocompleteDialog() {
   }
 }
 
+void TabAutofillManagerDelegate::OnStateChanged() {
+  if (!sync_state_changed_callback_.is_null())
+    sync_state_changed_callback_.Run();
+}
+
 void TabAutofillManagerDelegate::DidNavigateMainFrame(
     const content::LoadCommittedDetails& details,
     const content::FrameNavigateParams& params) {
diff --git a/chrome/browser/ui/autofill/tab_autofill_manager_delegate.h b/chrome/browser/ui/autofill/tab_autofill_manager_delegate.h
index ed53ff0..6f702e0 100644
--- a/chrome/browser/ui/autofill/tab_autofill_manager_delegate.h
+++ b/chrome/browser/ui/autofill/tab_autofill_manager_delegate.h
@@ -5,8 +5,10 @@
 #ifndef CHROME_BROWSER_UI_AUTOFILL_TAB_AUTOFILL_MANAGER_DELEGATE_H_
 #define CHROME_BROWSER_UI_AUTOFILL_TAB_AUTOFILL_MANAGER_DELEGATE_H_
 
+#include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/memory/weak_ptr.h"
+#include "chrome/browser/api/sync/profile_sync_service_observer.h"
 #include "chrome/browser/autofill/autofill_manager_delegate.h"
 #include "chrome/browser/ui/autofill/autofill_dialog_types.h"
 #include "content/public/browser/web_contents_observer.h"
@@ -27,6 +29,7 @@ class AutofillDialogControllerImpl;
 // Chrome implementation of AutofillManagerDelegate.
 class TabAutofillManagerDelegate
     : public AutofillManagerDelegate,
+      public ProfileSyncServiceObserver,
       public content::WebContentsUserData<TabAutofillManagerDelegate>,
       public content::WebContentsObserver {
  public:
@@ -36,9 +39,11 @@ class TabAutofillManagerDelegate
   virtual InfoBarService* GetInfoBarService() OVERRIDE;
   virtual PersonalDataManager* GetPersonalDataManager() OVERRIDE;
   virtual PrefService* GetPrefs() OVERRIDE;
-  virtual ProfileSyncServiceBase* GetProfileSyncService() OVERRIDE;
   virtual void HideRequestAutocompleteDialog() OVERRIDE;
   virtual bool IsSavingPasswordsEnabled() const OVERRIDE;
+  virtual bool IsPasswordSyncEnabled() const OVERRIDE;
+  virtual void SetSyncStateChangedCallback(
+      const base::Closure& callback) OVERRIDE;
   virtual void OnAutocheckoutError() OVERRIDE;
   virtual void ShowAutofillSettings() OVERRIDE;
   virtual void ShowPasswordGenerationBubble(
@@ -66,6 +71,9 @@ class TabAutofillManagerDelegate
   virtual void HideAutofillPopup() OVERRIDE;
   virtual void UpdateProgressBar(double value) OVERRIDE;
 
+  // ProfileSyncServiceObserver implementation.
+  virtual void OnStateChanged() OVERRIDE;
+
   // content::WebContentsObserver implementation.
   virtual void DidNavigateMainFrame(
       const content::LoadCommittedDetails& details,
@@ -75,6 +83,7 @@ class TabAutofillManagerDelegate
   explicit TabAutofillManagerDelegate(content::WebContents* web_contents);
   friend class content::WebContentsUserData<TabAutofillManagerDelegate>;
 
+  base::Closure sync_state_changed_callback_;
   content::WebContents* const web_contents_;
   AutofillDialogControllerImpl* dialog_controller_;  // weak.
   base::WeakPtr<AutofillPopupControllerImpl> popup_controller_;