Create net::FormatOriginForDisplay helper function.

net/base/net_util_icu.cc

Index: net/base/net_util_icu.cc
diff --git a/net/base/net_util_icu.cc b/net/base/net_util_icu.cc
index 94e1a0d7232a65a7c517e714fa128fc571b47aa2..bdbe2701351b46d54e695b25e323e78daf90940e 100644
--- a/net/base/net_util_icu.cc
+++ b/net/base/net_util_icu.cc
@@ -797,7 +797,7 @@ base::string16 FormatUrlWithAdjustments(
     // after stripping the prefix.  The only thing necessary is to add an
     // adjustment to reflect the stripped prefix.
     adjustments->insert(adjustments->begin(),
-        base::OffsetAdjuster::Adjustment(0, kHTTPSize, 0));
+                        base::OffsetAdjuster::Adjustment(0, kHTTPSize, 0));
 
     if (prefix_end)
       *prefix_end -= kHTTPSize;
@@ -829,4 +829,46 @@ base::string16 FormatUrl(const GURL& url,
   return result;
 }
 
+base::string16 FormatOriginForDisplay(const GURL& url,
+                                      const std::string& languages,
+                                      bool omit_scheme) {
+  if (!url.IsStandard())

[Ryan Sleevi, 2015/05/16 01:26:04]

BUG: You should be checking how invalid GURLs provided here work (w/r/t
is_valid()

[palmer, 2015/05/16 01:37:18]

Can you formulate a unit test that triggers a failure in this code? Note that
the unit tests include several wacky URLs.

[Ryan Sleevi, 2015/05/16 01:43:01]

Sure, easy - FormatOriginForDisplay(GURL(), ...)

+    return FormatUrl(url, languages);
+
+  if (url.SchemeIsFile()) {
+    // TODO(palmer): Determine whether to encode this policy in GURL::GetOrigin.
+    return (omit_scheme ? base::ASCIIToUTF16("")

[Ryan Sleevi, 2015/05/16 01:26:04]

Performance: You should just be using base::string16() here, rather than forcing
such a conversion

[palmer, 2015/05/16 01:37:18]

OK.

+                        : base::ASCIIToUTF16("file://")) +
+           base::UTF8ToUTF16(url.path());
+  }
+
+  if (url.SchemeIsFileSystem()) {
+    // TODO(palmer): Determine whether to encode this policy in GURL::GetOrigin.
+    const GURL inner_url(url.spec().substr(strlen("filesystem:")));
+    return base::ASCIIToUTF16("filesystem:") +
+           FormatOriginForDisplay(inner_url, languages, omit_scheme);

[Ryan Sleevi, 2015/05/16 01:26:04]

BUG: This is not the right way to create sub-GURLs. You should use the
::Replacements to format the inner_url, getting the appropriate scheme & such.

[palmer, 2015/05/16 01:37:18]

Look at previous patchsets, in which we tried that approach. It created new
intertwinglings between this function and FormatUrl, which caused code
complications and bugs.

Can you formulate a unit test that triggers a failure in this code?

[Ryan Sleevi, 2015/05/16 01:43:01]

I did before making that comment, I didn't see a single patch where you tried to
use url::Replacements
I know this was well intentioned push-back, but as the CL submitter, it's your
responsibility to adhere to the API contract.

It's not my role to provide you counter-tests. It's my role to tell you "Hey,
you're not adhering to the supported API, and you're making it our problem to
hopefully find and fix before it becomes an issue". That's a hidden technical
debt time-bomb.

+  }
+
+  const GURL origin = url.GetOrigin();
+  const std::string& scheme = origin.scheme();
+  const std::string& host = origin.host();
+  if (scheme.empty() || host.empty())
+    return FormatUrl(url, languages);
+
+  base::string16 result;
+
+  if (!omit_scheme)
+    result = base::UTF8ToUTF16(scheme) + base::ASCIIToUTF16("://");
+
+  result += base::UTF8ToUTF16(host);
+
+  const int port = origin.IntPort();
+  const int default_port = url::DefaultPortForScheme(origin.scheme().c_str(),
+                                                     origin.scheme().length());
+  if (origin.port().length() > 0 && port != 0 && port != default_port)
+    result += base::ASCIIToUTF16(":") + base::UTF8ToUTF16(origin.port());
+
+  return result;
+}
+
 }  // namespace net