Remove invalid "suffix" assumption from CreateSortKey

chrome/browser/ui/passwords/password_manager_presenter_unittest.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/passwords/password_manager_presenter.h"
 
 #include <stddef.h>
 #include <utility>
 
 #include "base/macros.h"
@@ skipping 119 matching lines @@
 void PasswordManagerPresenterTest::SortAndCheckPositions(
     const SortEntry test_entries[],
     size_t number_of_entries,
     PasswordEntryType entry_type) {
   std::vector<std::unique_ptr<autofill::PasswordForm>> list;
   size_t expected_number_of_unique_entries = 0;
   for (size_t i = 0; i < number_of_entries; i++) {
     const SortEntry& entry = test_entries[i];
     std::unique_ptr<autofill::PasswordForm> form(new autofill::PasswordForm());
     form->signon_realm = entry.origin;
-    form->origin = GURL(base::ASCIIToUTF16(entry.origin));
+    form->origin = GURL(entry.origin);
     if (entry_type == PasswordEntryType::SAVED) {
       form->username_value = base::ASCIIToUTF16(entry.username);
       form->password_value = base::ASCIIToUTF16(entry.password);
       if (entry.federation != nullptr)
         form->federation_origin = url::Origin(GURL(entry.federation));
     }
     if (entry.affiliated_web_realm)
       form->affiliated_web_realm = entry.affiliated_web_realm;
     list.push_back(std::move(form));
     if (entry.expected_position >= 0)
       expected_number_of_unique_entries++;
   }
 
   DuplicatesMap duplicates;
   mock_controller_->GetPasswordManagerPresenter()->SortEntriesAndHideDuplicates(
       &list, &duplicates, entry_type);
 
   ASSERT_EQ(expected_number_of_unique_entries, list.size());
   ASSERT_EQ(number_of_entries - expected_number_of_unique_entries,
             duplicates.size());
   for (size_t i = 0; i < number_of_entries; i++) {
     const SortEntry& entry = test_entries[i];
     if (entry.expected_position >= 0) {
       SCOPED_TRACE(testing::Message("position in sorted list: ")
                    << entry.expected_position);
-      EXPECT_EQ(GURL(base::ASCIIToUTF16(entry.origin)),
-                list[entry.expected_position]->origin);
+      EXPECT_EQ(GURL(entry.origin), list[entry.expected_position]->origin);
       if (entry_type == PasswordEntryType::SAVED) {
         EXPECT_EQ(base::ASCIIToUTF16(entry.username),
                   list[entry.expected_position]->username_value);
         EXPECT_EQ(base::ASCIIToUTF16(entry.password),
                   list[entry.expected_position]->password_value);
         if (entry.federation != nullptr)
           EXPECT_EQ(url::Origin(GURL(entry.federation)),
                     list[entry.expected_position]->federation_origin);
       }
     }
@@ skipping 82 matching lines @@
       {"http://example.com", "user_b", "pwd", nullptr, nullptr, 2},
       // Different password.
       {"http://example.com", "user_a", "secret", nullptr, nullptr, 1},
       // Different origin.
       {"http://sub1.example.com", "user_a", "pwd", nullptr, nullptr, 3},
       {"http://example.com", "user_a", "pwd", nullptr, nullptr, -1}  // Hide it.
   };
   SortAndCheckPositions(test_cases, arraysize(test_cases),
                         PasswordEntryType::SAVED);
 }
 

[kolos1, 2016/10/24 03:35:35]

To be sure we sort subdomains in the right way, please add this test here:

TEST_F(PasswordManagerPresenterTest, Sorting_Subdomains) {
  const SortEntry test_cases[] = {
      {"http://example.com", "u", "p", nullptr, nullptr, 0},
      {"http://b.example.com", "u", "p", nullptr, nullptr, 6},
      {"http://a.example.com", "u", "p", nullptr, nullptr, 1},
      {"http://1.a.example.com", "u", "p", nullptr, nullptr, 2},
      {"http://2.a.example.com", "u", "p", nullptr, nullptr, 3},
      {"http://x.2.a.example.com", "u", "p", nullptr, nullptr, 4},
      {"http://y.2.a.example.com", "u", "p", nullptr, nullptr, 5},
  };
  SortAndCheckPositions(test_cases, arraysize(test_cases),
                        PasswordEntryType::SAVED);
}

[vabr (Chromium), 2016/10/24 08:54:02]

Done.

 TEST_F(PasswordManagerPresenterTest, Sorting_PasswordExceptions) {
   const SortEntry test_cases[] = {
       {"http://example-b.com", nullptr, nullptr, nullptr, nullptr, 1},
       {"http://example-a.com", nullptr, nullptr, nullptr, nullptr, 0},
       {"http://example-a.com", nullptr, nullptr, nullptr, nullptr,
        -1},  // Hide it.
       {"http://example-c.com", nullptr, nullptr, nullptr, nullptr, 2}};
   SortAndCheckPositions(test_cases, arraysize(test_cases),
                         PasswordEntryType::BLACKLISTED);
 }
@@ skipping 17 matching lines @@
 TEST_F(PasswordManagerPresenterTest, Sorting_Federations) {
   const SortEntry test_cases[] = {
       {"https://example.com", "user", "secret", nullptr, nullptr, 0},
       {"https://example.com", "user", "secret", nullptr, "https://fed1.com", 1},
       {"https://example.com", "user", "secret", nullptr, "https://fed2.com",
        2}};
   SortAndCheckPositions(test_cases, arraysize(test_cases),
                         PasswordEntryType::SAVED);
 }
 
+TEST_F(PasswordManagerPresenterTest, Sorting_SpecialCharacters) {
+  // URLs with encoded special characters should not cause crash during sorting.
+  const SortEntry test_cases[] = {
+      {"https://xn--bea5m6d.com/", "user_a", "pwd", nullptr, nullptr, 0},

[kolos1, 2016/10/24 03:35:35]

Does this test cause a crash? Shall we use "žřč.com" instead of
"https://xn--bea5m6d.com/"? 

[vabr (Chromium), 2016/10/24 08:54:02]

It does cause crash (without the fix, and on ASAN). xn--bea5m6d gets converted
into žřč during GetShownOriginAndLinkUrl.

+  };
+  SortAndCheckPositions(test_cases, arraysize(test_cases),

[kolos1, 2016/10/24 03:35:35]

Please add more items to |test_cases| to test if the sorting works well on
domains with special characters.

[vabr (Chromium), 2016/10/24 08:54:02]

Done.

+                        PasswordEntryType::SAVED);
+}
+
 }  // namespace