Delete PasswordStoreX::Get*LoginsImpl

As pointed out in https://codereview.chromium.org/906973007/diff/80001/chrome/browser/password_manager/password_store_x.cc#newcode157, instead of overriding Get*LoginsImpl in PasswordStoreX, it is enough to rely on the PasswordStoreDefault implementation of those methods to use PasswordStoreX::Fill*Logins.

The only current difference is that in addition to what Fill*Logins do, PasswordStoreX::Get*LoginsImpl also sorted the logins by origin, to match the order of the login database. There are two ways to fix this:
(1) Move the sorting from Get*LoginsImpl to Fill*Logins.
(2) Drop the sorting.

This CL chose (1). This means that now also PasswordSyncableService::ReadFromPasswordStore, which currently gets the logins unsorted, will perform the sorting. This call should be infrequent enough for this not to be noticeable.r

While (2) would also not break the insides of the password manager, it would degrade the UI (try to find a login in an unsorted list of them). For the reference, the CL which added the sorting was https://codereview.chromium.org/6953010.

BUG=324291

Review URL: https://codereview.chromium.org/986983002

Cr-Commit-Position: refs/heads/master@{#319617}

chrome/browser/password_manager/password_store_x.cc

@@ -122,19 +122,20 @@ PasswordStoreChangeList PasswordStoreX::RemoveLoginsSyncedBetweenImpl(
 }
 
 namespace {
+
 struct LoginLessThan {
   bool operator()(const PasswordForm* a, const PasswordForm* b) {
     return a->origin < b->origin;
   }
 };
-}  // anonymous namespace
 
-void PasswordStoreX::SortLoginsByOrigin(
-    std::vector<autofill::PasswordForm*>* list) {
-  // In login_database.cc, the query has ORDER BY origin_url. Simulate that.
+// Sorts |list| by origin, like the ORDER BY clause in login_database.cc.
+void SortLoginsByOrigin(std::vector<autofill::PasswordForm*>* list) {
   std::sort(list->begin(), list->end(), LoginLessThan());
 }
 
+}  // anonymous namespace
+
 ScopedVector<autofill::PasswordForm> PasswordStoreX::FillMatchingLogins(
     const autofill::PasswordForm& form,
     AuthorizationPromptPolicy prompt_policy) {
@@ -154,51 +155,13 @@ ScopedVector<autofill::PasswordForm> PasswordStoreX::FillMatchingLogins(
   return matched_forms.Pass();
 }
 
-void PasswordStoreX::GetAutofillableLoginsImpl(
-    scoped_ptr<PasswordStore::GetLoginsRequest> request) {
-  CheckMigration();
-  ScopedVector<autofill::PasswordForm> obtained_forms;
-  if (use_native_backend() &&
-      backend_->GetAutofillableLogins(&obtained_forms)) {
-    SortLoginsByOrigin(&obtained_forms.get());
-    // See GetLoginsImpl() for why we disallow fallback conditionally here.
-    if (!obtained_forms.empty())
-      allow_fallback_ = false;
-    request->NotifyConsumerWithResults(obtained_forms.Pass());
-    return;
-  } else if (allow_default_store()) {
-    PasswordStoreDefault::GetAutofillableLoginsImpl(request.Pass());
-    return;
-  }
-  // The consumer will be left hanging unless we reply.
-  request->NotifyConsumerWithResults(ScopedVector<autofill::PasswordForm>());
-}
-
-void PasswordStoreX::GetBlacklistLoginsImpl(
-    scoped_ptr<PasswordStore::GetLoginsRequest> request) {
-  CheckMigration();
-  ScopedVector<autofill::PasswordForm> obtained_forms;
-  if (use_native_backend() && backend_->GetBlacklistLogins(&obtained_forms)) {
-    SortLoginsByOrigin(&obtained_forms.get());
-    // See GetLoginsImpl() for why we disallow fallback conditionally here.
-    if (!obtained_forms.empty())
-      allow_fallback_ = false;
-    request->NotifyConsumerWithResults(obtained_forms.Pass());
-    return;
-  } else if (allow_default_store()) {
-    PasswordStoreDefault::GetBlacklistLoginsImpl(request.Pass());
-    return;
-  }
-  // The consumer will be left hanging unless we reply.
-  request->NotifyConsumerWithResults(ScopedVector<autofill::PasswordForm>());
-}
-
 bool PasswordStoreX::FillAutofillableLogins(
     ScopedVector<autofill::PasswordForm>* forms) {
   CheckMigration();
   if (use_native_backend() && backend_->GetAutofillableLogins(forms)) {
+    SortLoginsByOrigin(&forms->get());
     // See GetLoginsImpl() for why we disallow fallback conditionally here.
-    if (forms->size() > 0)
+    if (!forms->empty())
       allow_fallback_ = false;
     return true;
   }
@@ -212,7 +175,8 @@ bool PasswordStoreX::FillBlacklistLogins(
   CheckMigration();
   if (use_native_backend() && backend_->GetBlacklistLogins(forms)) {
     // See GetLoginsImpl() for why we disallow fallback conditionally here.
-    if (forms->size() > 0)
+    SortLoginsByOrigin(&forms->get());
+    if (!forms->empty())
       allow_fallback_ = false;
     return true;
   }

chrome/browser/password_manager/password_store_x.h

@@ -94,18 +94,11 @@ class PasswordStoreX : public password_manager::PasswordStoreDefault {
   ScopedVector<autofill::PasswordForm> FillMatchingLogins(
       const autofill::PasswordForm& form,
       AuthorizationPromptPolicy prompt_policy) override;
-  void GetAutofillableLoginsImpl(
-      scoped_ptr<PasswordStore::GetLoginsRequest> request) override;
-  void GetBlacklistLoginsImpl(
-      scoped_ptr<PasswordStore::GetLoginsRequest> request) override;
   bool FillAutofillableLogins(
       ScopedVector<autofill::PasswordForm>* forms) override;
   bool FillBlacklistLogins(
       ScopedVector<autofill::PasswordForm>* forms) override;
 
-  // Sort logins by origin, like the ORDER BY clause in login_database.cc.
-  void SortLoginsByOrigin(std::vector<autofill::PasswordForm*>* list);
-
   // Check to see whether migration is necessary, and perform it if so.
   void CheckMigration();