[Password manager] Human readable origins for Android credentials on chrome://settings/passwords

components/password_manager/core/browser/password_store.cc

Index: components/password_manager/core/browser/password_store.cc
diff --git a/components/password_manager/core/browser/password_store.cc b/components/password_manager/core/browser/password_store.cc
index 5789aee09e6a6fbc61dadfdec4a160a722ab7c03..7170db96d86295a04492c89fa44565944a07eba7 100644
--- a/components/password_manager/core/browser/password_store.cc
+++ b/components/password_manager/core/browser/password_store.cc
@@ -189,6 +189,12 @@ void PasswordStore::GetAutofillableLogins(PasswordStoreConsumer* consumer) {
   Schedule(&PasswordStore::GetAutofillableLoginsImpl, consumer);
 }
 
+void PasswordStore::GetAutofillableLoginsWithAffiliatedRealms(
+    PasswordStoreConsumer* consumer) {
+  Schedule(&PasswordStore::GetAutofillableLoginsWithAffiliatedRealmsImpl,
+           consumer);
+}
+
 void PasswordStore::GetBlacklistLogins(PasswordStoreConsumer* consumer) {
   Schedule(&PasswordStore::GetBlacklistLoginsImpl, consumer);
 }
@@ -402,6 +408,38 @@ void PasswordStore::GetAutofillableLoginsImpl(
   request->NotifyConsumerWithResults(std::move(obtained_forms));
 }
 
+void PasswordStore::GetAutofillableLoginsWithAffiliatedRealmsImpl(
+    scoped_ptr<GetLoginsRequest> request) {
+  scoped_ptr<ScopedVector<PasswordForm>> obtained_forms(

[engedy, 2016/03/02 14:22:10]

Note that ScopedVector is already a move-only type, so you should be able to use
it in std::move() and base::Passed(), and not need to wrap it in a scoped_ptr.

[kolos1, 2016/03/07 10:46:59]

Yes, it's is possible to pass ScopedVector, but I need a wrapped vector in
PasswordStore::FillAffiliatedWebRealms(InjectAffiliatedWebRealms). I need to
provide a pointer to the vector into
affiliated_match_helper_->FillAffiliatedWebRealms(InjectAffiliatedWebRealms),
but ownership of the vector should be passed to the callback
(PasswordStore::NotifyLoginsWithAffiliatedRealms). It is why I wrapped
ScopedVector. 

+      new ScopedVector<PasswordForm>());
+  if (!FillAutofillableLogins(obtained_forms.get()))
+    obtained_forms->clear();
+  // Since AffiliatedMatchHelper's requests should be sent from UI thread,
+  // post a request to UI thread.
+  main_thread_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&PasswordStore::FillAffiliatedWebRealms, this,
+                 base::Passed(&obtained_forms), base::Passed(&request)));
+}
+
+void PasswordStore::FillAffiliatedWebRealms(

[engedy, 2016/03/02 14:22:10]

Naming nit: At least in the scope of the PasswordStore, "fill" refers to
synchronous operations that do the actual work, and are called from the
asynchronous wrapper methods. 

How about incorporate, include, inject? 

[kolos1, 2016/03/07 10:47:00]

Renamed to Inject...

+    scoped_ptr<ScopedVector<PasswordForm>> forms,
+    scoped_ptr<GetLoginsRequest> request) {
+  if (affiliated_match_helper_) {
+    affiliated_match_helper_->FillAffiliatedWebRealms(
+        *forms, base::Bind(&PasswordStore::NotifyLoginsWithAffiliatedRealms,
+                           this, base::Passed(&forms), base::Passed(&request)));
+  } else {
+    NotifyLoginsWithAffiliatedRealms(std::move(forms), std::move(request));
+  }
+}
+
+void PasswordStore::NotifyLoginsWithAffiliatedRealms(

[engedy, 2016/03/02 14:22:10]

I think we could make this a non-member function defined in an anonymous
namespace in this CC file. Alternatively, I think you could write:

base::Bind(&PS::GLR::NotifyConsumerWithResults, base::Owned(request.release()),
...)

[kolos1, 2016/03/07 10:47:00]

I would prefer to left it here since we have similar methods in PasswordStore
(NotifySiteStats, NotifyLoginsChanged).
Yes, it would be nice to get rid of NotifyLoginsWithAffiliatedRealms, but I need
to keep the wrapper of ScopedVector (i.e. scoped_ptr). Otherwise,
affiliated_match_helper_->FillAffiliatedWebRealms wouldn't receive the content
of ScopedVector (i.e. |*forms|). So, I need some method to unwrap ScopedVector. 

+    scoped_ptr<ScopedVector<PasswordForm>> obtained_forms,
+    scoped_ptr<GetLoginsRequest> request) {
+  request->NotifyConsumerWithResults(std::move(*(obtained_forms.release())));
+}
+
 void PasswordStore::GetBlacklistLoginsImpl(
     scoped_ptr<GetLoginsRequest> request) {
   ScopedVector<PasswordForm> obtained_forms;