Integration of PasswordReuseDetector into PasswordStore.

components/password_manager/core/browser/password_store.h

Index: components/password_manager/core/browser/password_store.h
diff --git a/components/password_manager/core/browser/password_store.h b/components/password_manager/core/browser/password_store.h
index 28a7046526cb8a086dae46c17f6c107322424379..0ba89b1744d7750fdd744849b7dffc432f76905b 100644
--- a/components/password_manager/core/browser/password_store.h
+++ b/components/password_manager/core/browser/password_store.h
@@ -17,6 +17,7 @@
 #include "base/single_thread_task_runner.h"
 #include "base/time/time.h"
 #include "components/keyed_service/core/refcounted_keyed_service.h"
+#include "components/password_manager/core/browser/password_reuse_detector.h"
 #include "components/password_manager/core/browser/password_store_change.h"
 #include "components/password_manager/core/browser/password_store_sync.h"
 #include "components/sync/model/syncable_service.h"
@@ -221,6 +222,15 @@ class PasswordStore : protected PasswordStoreSync,
 
   base::WeakPtr<syncer::SyncableService> GetPasswordSyncableService();
 
+  // Checks that some suffix of |input| equals to a password saved on another
+  // registry controlled domain than |domain|.
+  // If such suffix is found, |consumer|->OnReuseFound() is called on the same
+  // thread on which this method is called.
+  // |consumer| should not be null.

[vabr (Chromium), 2016/12/20 18:11:44]

nit: Let's be clear about |consumer|: it must not be null. Writing "should"
makes the consequences unclear.

[dvadym, 2016/12/21 12:15:35]

Done.

+  void CheckReuse(const base::string16& input,
+                  const std::string& domain,
+                  PasswordReuseDetectorConsumer* consumer);
+
  protected:
   friend class base::RefCountedThreadSafe<PasswordStore>;
 
@@ -256,6 +266,23 @@ class PasswordStore : protected PasswordStoreSync,
     DISALLOW_COPY_AND_ASSIGN(GetLoginsRequest);
   };
 
+  // Represents a single CheckReuse() request. Implements functionality to
+  // listen to reuse events and propagate them to |consumer| on the thread on
+  // which CheckReuseRequest is created.
+  class CheckReuseRequest : public PasswordReuseDetectorConsumer {
+   public:
+    explicit CheckReuseRequest(PasswordReuseDetectorConsumer* consumer);

[vabr (Chromium), 2016/12/20 18:11:44]

nit: Please mention that |consumer| must not be null.

[dvadym, 2016/12/21 12:15:35]

Done.

+    ~CheckReuseRequest() override;
+
+    // PasswordReuseDetectorConsumer
+    void OnReuseFound(const base::string16& password,
+                      const std::string& saved_domain) override;
+
+   private:
+    scoped_refptr<base::SingleThreadTaskRunner> origin_task_runner_;

[vabr (Chromium), 2016/12/20 18:11:44]

const scoped_refptr<base::SingleThreadTaskRunner> origin_task_runner_
to indicate that it is not changed during the lifetime of the request?

[dvadym, 2016/12/21 12:15:35]

Done.

+    PasswordReuseDetectorConsumer* consumer_;

[vabr (Chromium), 2016/12/20 18:11:44]

PasswordReuseDetectorConsumer* const consumer_
to indicate that it is not changed during the lifetime of the request?

[dvadym, 2016/12/21 12:15:35]

Done.

+  };

[vabr (Chromium), 2016/12/20 18:11:44]

DISALLOW_COPY_AND_ASSIGN ?

[dvadym, 2016/12/21 12:15:35]

Yeah, thanks

+
   ~PasswordStore() override;
 
   // Get the TaskRunner to use for PasswordStore background tasks.
@@ -348,6 +375,11 @@ class PasswordStore : protected PasswordStoreSync,
   // may have been changed.
   void NotifyLoginsChanged(const PasswordStoreChangeList& changes) override;
 
+  // Synchronous implementation of CheckReuse().
+  void CheckReuseImpl(std::unique_ptr<CheckReuseRequest> request,
+                      const base::string16& input,
+                      const std::string& domain);
+
   // TaskRunner for tasks that run on the main thread (usually the UI thread).
   scoped_refptr<base::SingleThreadTaskRunner> main_thread_runner_;
 
@@ -494,6 +526,7 @@ class PasswordStore : protected PasswordStoreSync,
 
   std::unique_ptr<PasswordSyncableService> syncable_service_;
   std::unique_ptr<AffiliatedMatchHelper> affiliated_match_helper_;
+  std::unique_ptr<PasswordReuseDetector> reuse_detector_;

[vabr (Chromium), 2016/12/20 18:11:44]

Why is this a unique_ptr? Could it be just aggregated into PasswordStore?
PasswordReuseDetector reuse_detector_;

[dvadym, 2016/12/21 12:15:35]

Sure, it could be. Done

[dvadym, 2016/12/21 14:03:20]

I've returned back to unique_ptr. It should be destroyed on UI thread.

   bool is_propagating_password_changes_to_web_credentials_enabled_;
 
   bool shutdown_called_;