Show password sync status in CBD

components/browsing_data/core/counters/passwords_counter.cc

Index: components/browsing_data/core/counters/passwords_counter.cc
diff --git a/components/browsing_data/core/counters/passwords_counter.cc b/components/browsing_data/core/counters/passwords_counter.cc
index 8585158b83981f8184fd1c8d7bfdcd214df9af1b..9f64858c074fb7994cb454aa0545e79fd7fd77d2 100644
--- a/components/browsing_data/core/counters/passwords_counter.cc
+++ b/components/browsing_data/core/counters/passwords_counter.cc
@@ -5,20 +5,38 @@
 #include "components/browsing_data/core/counters/passwords_counter.h"
 
 #include "components/browsing_data/core/pref_names.h"
+#include "components/password_manager/core/browser/password_manager_util.h"
 #include "components/password_manager/core/browser/password_store.h"
+#include "components/sync/driver/sync_service.h"
+
+namespace {
+bool IsSyncEnabled(const syncer::SyncService* sync_service) {

[msramek, 2017/04/07 12:17:40]

nit: one line padding between the method and the namespace on both sides

nit: IsPasswordsSyncEnabled() to avoid confusion

[dullweber, 2017/04/11 15:12:48]

Done.

+  DCHECK(sync_service);
+  return password_manager_util::GetPasswordSyncState(sync_service) !=
+         password_manager::PasswordSyncState::NOT_SYNCING_PASSWORDS;
+}
+}  // namespace
 
 namespace browsing_data {
 
 PasswordsCounter::PasswordsCounter(
-    scoped_refptr<password_manager::PasswordStore> store) : store_(store) {}
+    scoped_refptr<password_manager::PasswordStore> store,
+    syncer::SyncService* sync_service)
+    : store_(store), sync_service_(sync_service), password_sync_enabled_() {}
 
 PasswordsCounter::~PasswordsCounter() {
-  store_->RemoveObserver(this);
+  if (store_.get())

[msramek, 2017/04/07 12:17:40]

Hmm... either we're sure that the store must exist, or we're not. So either we
DCHECK everywhere, or we guard it everywhere, but not both.

[dullweber, 2017/04/11 15:12:48]

It is now expected to always be passed to the counter. Thanks

+    store_->RemoveObserver(this);
+  if (sync_service_)

[msramek, 2017/04/07 12:17:41]

Ditto here. AFAIK SyncService doesn't have to exist, so the DCHECK on line 36 is
not correct.

[dullweber, 2017/04/11 15:12:48]

Done.

+    sync_service_->RemoveObserver(this);
 }
 
 void PasswordsCounter::OnInitialized() {
   DCHECK(store_);
+  DCHECK(sync_service_);
   store_->AddObserver(this);
+  sync_service_->AddObserver(this);
+  password_sync_enabled_ = IsSyncEnabled(sync_service_);
 }
 
 const char* PasswordsCounter::GetPrefName() const {
@@ -26,6 +44,7 @@ const char* PasswordsCounter::GetPrefName() const {
 }
 
 void PasswordsCounter::Count() {
+  DCHECK(store_);
   cancelable_task_tracker()->TryCancelAll();
   // TODO(msramek): We don't actually need the logins themselves, just their
   // count. Consider implementing |PasswordStore::CountAutofillableLogins|.
@@ -37,11 +56,13 @@ void PasswordsCounter::Count() {
 void PasswordsCounter::OnGetPasswordStoreResults(
     std::vector<std::unique_ptr<autofill::PasswordForm>> results) {
   base::Time start = GetPeriodStart();
-  ReportResult(std::count_if(
+  int num_passwords = std::count_if(
       results.begin(), results.end(),
       [start](const std::unique_ptr<autofill::PasswordForm>& form) {
         return form->date_created >= start;
-      }));
+      });
+  ReportResult(base::MakeUnique<PasswordResult>(this, num_passwords,
+                                                password_sync_enabled_));
 }
 
 void PasswordsCounter::OnLoginsChanged(
@@ -49,4 +70,21 @@ void PasswordsCounter::OnLoginsChanged(
   Restart();
 }
 
+PasswordsCounter::PasswordResult::PasswordResult(const PasswordsCounter* source,
+                                                 ResultInt value,
+                                                 bool has_synced_passwords)
+    : FinishedResult(source, value),
+      has_synced_passwords_(has_synced_passwords) {}
+
+PasswordsCounter::PasswordResult::~PasswordResult() {}
+
+void PasswordsCounter::OnStateChanged(syncer::SyncService* sync) {

[msramek, 2017/04/07 12:17:40]

style: Ordering. You have a few PasswordsCounter methods, then PasswordResult,
then PasswordsCounter again.

[dullweber, 2017/04/11 15:12:48]

Done.

+  bool enabled = IsSyncEnabled(sync_service_);
+
+  if (password_sync_enabled_ != enabled) {
+    password_sync_enabled_ = enabled;
+    Restart();
+  }
+}
+
 }  // namespace browsing_data