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..313f4a4a0a5a749ee84b6a13eaa56d7471314736 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,46 @@ 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);

[Ryan Sleevi, 2015/06/01 21:18:46]

In writing my comment about the function comment, it made me realize that we
don't account for Blob URLs here.

The effective security origin of a Blob URL is similar to that of a Filesystem
URL. However, //url doesn't have awareness of Blob.

Separately, Mike and I have been talking about whether or not it's "right" for
//url to know about Filesystem URLs and their parsing, or whether //url should
be doing something like Blink's Security Origin does, which is allow a way for a
higher layer to add parsing knowledge.

Given that
1) //url doesn't know that blob's share an inner_url like filesystem does
2) Consumers of //url (excepting Blink's security origin) are coded against
SchemeIsFilesystem rather than something more generic like SchemeHasInnerUrl()
or something weird like that
3) It's not clear how much //url should even know about (blob, filesystem)

I think it's fine to punt on that problem, but I leave that up to you on which
is more pressing for you.

[Note I think it'd be weird to solve 1/2 without first having an idea about what
to do for 3, since you'd end up with twice as much codechurn if it's decided
//url shouldn't know about these sorts of 'web platform-y' URLs. RFC 6454 is
already woefully obsolete in this respect, and the URL Standard a more accurate
reflection of reality, but very much a "web platform" thing and not a "generic
standard URI parser" that //url originally started out as]

Just highlighting this issue.

[palmer, 2015/06/01 23:30:34]

Most pressing for me right now is fixing bugs in Chrome's UX, for which the fix
depends on a function like FormatOriginForDisplay.

AFAIK, blob: URLs are not currently the most likely to surface those bugs. But
clearly we ultimately will need to handle them as gracefully as possible.

The current code does handle blob: URLs; they immediately fall through to
FormatUrl on line 309. I have added a test case to illustrate this.

+
+  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 +
+           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 +
+             FormatOriginForDisplay(*inner_url, languages) +
+             base::UTF8ToUTF16(url.path());
+    }
+    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));

[Ryan Sleevi, 2015/06/01 21:18:46]

EXTREME PEDANTRY REDUX:

I suggested the "=" initialization over this form because this form is
explicitly copy-ctor initialization, and at least MSVC in the past ha some
stupidity around here.

Both forms

base::string16 result = base::UTF8ToUTF16(scheme)
base::string16 result(base::UTF8ToUTF16(scheme))

follow the same "initializer" syntax in C++03 8.5, and thus both should be
subject to the "as-if" rule allowing for RVO, but again, past experience (and
mind you, it's been nearly 8 years since I took the time to dig into the
assembly - this stuff may be fixed by now) was that the " = " syntax got
optimized, when the () syntax didn't.

That's why I prefer the " = "

(Yes, I realize I'm discussing about a microoptimization that for this case is
likely entirely lost in the noise; but like the "" vs std::string(), it's one of
those 'useful' things to know/realize, and generally has no side-effect on
readability but can add up)

Like I said, EXTREME PEDANTRY

[palmer, 2015/06/01 23:30:34]

Done.

+  result += scheme_separator;
+  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;
+}