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

chrome/browser/ui/webui/options/password_manager_handler.cc

Index: chrome/browser/ui/webui/options/password_manager_handler.cc
diff --git a/chrome/browser/ui/webui/options/password_manager_handler.cc b/chrome/browser/ui/webui/options/password_manager_handler.cc
index 374e6e58cbb0ef2d542f7ce6e16df26bc80b1d5d..d153e36f1ccaa81d81749c4120e8847b94de59b8 100644
--- a/chrome/browser/ui/webui/options/password_manager_handler.cc
+++ b/chrome/browser/ui/webui/options/password_manager_handler.cc
@@ -45,9 +45,10 @@ namespace options {
 namespace {
 // The following constants should be synchronized with the constants in
 // chrome/browser/resources/options/password_manager_list.js.
-const char kOriginField[] = "origin";
-const char kShownUrlField[] = "shownUrl";
+const char kUrlField[] = "url";
+const char kShownOriginField[] = "shownOrigin";
 const char kIsAndroidUriField[] = "isAndroidUri";
+const char kIsClickable[] = "isClickable";
 const char kIsSecureField[] = "isSecure";
 const char kUsernameField[] = "username";
 const char kPasswordField[] = "password";
@@ -58,16 +59,21 @@ const char kFederationField[] = "federation";
 void CopyOriginInfoOfPasswordForm(const autofill::PasswordForm& form,
                                   const std::string& languages,
                                   base::DictionaryValue* entry) {
-  entry->SetString(
-      kOriginField,
-      url_formatter::FormatUrl(
-          form.origin, languages, url_formatter::kFormatUrlOmitNothing,
-          net::UnescapeRule::SPACES, nullptr, nullptr, nullptr));
   bool is_android_uri = false;
-  entry->SetString(kShownUrlField, password_manager::GetShownOrigin(
-                                       form, languages, &is_android_uri));
+  bool origin_is_clickable = false;
+  GURL link_url;
+  entry->SetString(
+      kShownOriginField,
+      password_manager::GetShownOriginAndLinkUrl(
+          form, languages, &is_android_uri, &link_url, &origin_is_clickable));
+  DCHECK(link_url.is_valid());

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

Hang on, how does this not fail for empty |link_url| for Android credentials
without matching web realms?

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

It is not always PasswordForm.affiliated_web_realm. See into
GetShownOriginAndLinkUrl:
a) For non-Android forms: link_url is PasswordForm.origin.
b) For Android forms with non-empty affiliated_web_realm: it is
affiliated_web_realm. 
c) For Android forms with empty affiliated_web_realm: it is
PasswordForm.signon_realm. Could signon_realm be invalid URL? 

[engedy, 2016/03/08 17:36:37]

Please see my other comment. GetShownOriginAndLinkUrl() should return an empty
GURL as |link_url| in case (c).

+  entry->SetString(
+      kUrlField, url_formatter::FormatUrl(
+                     link_url, languages, url_formatter::kFormatUrlOmitNothing,
+                     net::UnescapeRule::SPACES, nullptr, nullptr, nullptr));
   entry->SetBoolean(kIsAndroidUriField, is_android_uri);
-  entry->SetBoolean(kIsSecureField, content::IsOriginSecure(form.origin));
+  entry->SetBoolean(kIsClickable, origin_is_clickable);
+  entry->SetBoolean(kIsSecureField, content::IsOriginSecure(link_url));

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

Nit: FWIW, Android credentials are always secure too, even if we don't know the
affiliated web realms.

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

Thanks. 

Is it possible that the scheme of affiliated web realm is http? 

link_url is the URL where user will go to if he clicks on the origin on password
page. If we are going to show where the ulr is secure, let just check it. But
don't consider various cases. 

[engedy, 2016/03/08 17:36:37]

No, HTTP URLs are not valid facet URIs.
Let's discuss this offline tomorrow.

 }
 
 }  // namespace