Create FormatUrlForSecurityDisplay helper function.

chrome/browser/ui/elide_url.cc

Index: chrome/browser/ui/elide_url.cc
diff --git a/chrome/browser/ui/elide_url.cc b/chrome/browser/ui/elide_url.cc
index 88b393c4165555de6cbc73e7e4021ae319e65916..aca9231c133b58a28e3d8c59c2697cd2e6841f42 100644
--- a/chrome/browser/ui/elide_url.cc
+++ b/chrome/browser/ui/elide_url.cc
@@ -13,6 +13,7 @@
 #include "ui/gfx/text_elider.h"
 #include "ui/gfx/text_utils.h"
 #include "url/gurl.h"
+#include "url/url_constants.h"
 
 using base::UTF8ToUTF16;
 using gfx::ElideText;
@@ -301,3 +302,47 @@ base::string16 ElideHost(const GURL& url,
       url_subdomain, font_list, subdomain_width, gfx::ELIDE_HEAD);
   return elided_subdomain + url_domain;
 }
+
+base::string16 FormatOriginForDisplay(const GURL& url,
+                                      const std::string& languages) {
+  if (!url.is_valid() || url.is_empty() || !url.IsStandard())
+    return net::FormatUrl(url, languages);
+
+  const base::string16 colon(base::ASCIIToUTF16(":"));
+  const base::string16 scheme_separator(
+      (base::ASCIIToUTF16(url::kStandardSchemeSeparator)));
+
+  if (url.SchemeIsFile()) {
+    return (base::ASCIIToUTF16(url::kFileScheme)) + scheme_separator +

[Ryan Sleevi, 2015/05/29 21:13:51]

nit: You don't need the wrapping ()

[palmer, 2015/05/29 21:51:36]

Done.

+           base::UTF8ToUTF16(url.path());
+  }
+
+  if (url.SchemeIsFileSystem()) {
+    const GURL* inner_url = url.inner_url();
+    const GURL& inner_origin = inner_url->GetOrigin();
+    if (inner_origin.SchemeIsFile()) {
+      return base::ASCIIToUTF16(url::kFileSystemScheme) + colon +
+             base::ASCIIToUTF16(url::kFileScheme) + scheme_separator +
+             base::UTF8ToUTF16(inner_url->path()) +
+             base::UTF8ToUTF16(url.path());

[Ryan Sleevi, 2015/05/29 21:13:50]

I'm not sure I fully grokked your reply about this.

That is, it seems like it could be

return base::ASCIIToUTF16(url::kFileSystemScheme) + colon +
FormatOriginForDisplay(inner_url) + base::UTF8ToUTF16(url.path());

You're doing string manipulation either way, but I think the type of string
manipulation you're doing is fine for this purpose. The old code was more akin
to GURL::GetWithEmptyPath(), which not as safe as GWEP

Alternatively, it seems like you could also replace this (lines 323 - 329) with

return base::ASCIIToUTf16(url::kFileSystemScheme) + colon +
       FormatOriginForDisplay(inner_origin, languages) +
       (inner_origin.SchemeIsFile() ? base::UTF8ToUTF16(url.path()) :
string16());

[palmer, 2015/05/29 21:51:36]

Ahh, yes, thanks. Done.

+    }
+    return base::ASCIIToUTF16(url::kFileSystemScheme) + colon +
+           FormatOriginForDisplay(inner_origin, languages);
+  }
+
+  const GURL origin = url.GetOrigin();
+  const std::string& scheme = origin.scheme();
+  const std::string& host = origin.host();
+
+  base::string16 result = base::UTF8ToUTF16(scheme) + scheme_separator;

[Ryan Sleevi, 2015/05/29 21:13:51]

EXTREME PEDANTRY: This does result in an extra copy in a naive form compared to

base::string16 result = base::UTF8ToUTF16(scheme);
result += scheme_separator;
result += base::UTF8ToUTF16(host);

[this form allows for R-V-O to be used for the 'result', and then appends the
scheme_separator using +=; your current form penalizes RVO by forcing a
temporary to be constructed to perform the "+ scheme_separator" prior to
initializing |result|. Technically, the compiler is allowed to optimize it to be
equivalent to the code I suggested, but I haven't really seen any do it]

[palmer, 2015/05/29 21:51:36]

Done.

+
+  result += base::UTF8ToUTF16(host);
+
+  const int port = origin.IntPort();
+  const int default_port = url::DefaultPortForScheme(origin.scheme().c_str(),
+                                                     origin.scheme().length());
+  if (port != url::PORT_UNSPECIFIED && port != default_port)
+    result += base::ASCIIToUTF16(":") + base::UTF8ToUTF16(origin.port());
+
+  return result;
+}