Create net::FormatOriginForDisplay helper function.

net/base/net_util_icu.cc

 // Copyright (c) 2012 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 "net/base/net_util.h"
 
 #include <map>
 #include <vector>
 
 #include "base/i18n/time_formatting.h"
@@ skipping 779 matching lines @@
 
   // If we need to strip out http do it after the fact.
   if (omit_http && StartsWith(url_string, base::ASCIIToUTF16(kHTTP), true)) {
     const size_t kHTTPSize = arraysize(kHTTP) - 1;
     url_string = url_string.substr(kHTTPSize);
     // Because offsets in the |adjustments| are already calculated with respect
     // to the string with the http:// prefix in it, those offsets remain correct
     // 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;
 
     // Adjust new_parsed.
     DCHECK(new_parsed->scheme.is_valid());
     int delta = -(new_parsed->scheme.len + 3);  // +3 for ://.
     new_parsed->scheme.reset();
     AdjustAllComponentsButScheme(delta, new_parsed);
   }
@@ skipping 11 matching lines @@
   Offsets offsets;
   if (offset_for_adjustment)
     offsets.push_back(*offset_for_adjustment);
   base::string16 result = FormatUrlWithOffsets(url, languages, format_types,
       unescape_rules, new_parsed, prefix_end, &offsets);
   if (offset_for_adjustment)
     *offset_for_adjustment = offsets[0];
   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