Add url::Origin::GetURL() to convert Origins to URLs without reparsing

url/scheme_host_port.cc

 // Copyright 2015 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 "url/scheme_host_port.h"
 
 #include <stdint.h>
 #include <string.h>
 
 #include <tuple>
 
 #include "base/logging.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/strings/string_number_conversions.h"
 #include "url/gurl.h"
+#include "url/third_party/mozilla/url_parse.h"
 #include "url/url_canon.h"
 #include "url/url_canon_stdstring.h"
 #include "url/url_constants.h"
 #include "url/url_util.h"
 
 namespace url {
 
 namespace {
 
 bool IsCanonicalHost(const base::StringPiece& host) {
@@ skipping 134 matching lines @@
   if (default_port == PORT_UNSPECIFIED)
     return result;
   if (port_ != default_port) {
     result.push_back(':');
     result.append(base::UintToString(port_));
   }
 
   return result;
 }
 
+GURL SchemeHostPort::GetURL() const {
+  std::string serialized = Serialize();

[brettw, 2016/09/30 20:18:48]

I'm a little nervous about how this depends on the internals of how Serialize
works but is separate from it.

What about making a new private internal serialize function that returns a
string and has a url::Parsed structure as an out param. Then have Serialize call
into that and discard the Parsed structure (I wouldn't make this optional since
I think null checking will be slower and uglier than just filling it in) and
GetURL makes a GURL out of it.

[Charlie Harrison, 2016/10/03 17:28:47]

Great idea. Done. The only problem I can see is that appending the "/" is
different between the two versions. RFC 6454 forbids adding this "/" so I had to
special case the GetURL() case.

+  if (serialized.empty())
+    return GURL();
+
+  static const int schemeSeparatorLength = strlen(kStandardSchemeSeparator);

[brettw, 2016/09/30 20:18:48]

style nit: use underscores and not camelCase (although I think if you take the
previous comment's advice this can be deleted).

[Charlie Harrison, 2016/10/03 17:28:47]

oops, been working too long in Blink :P

+
+  Parsed parsed;
+  parsed.scheme = Component(0, scheme().length());
+
+  int host_begin = scheme().length() + schemeSeparatorLength;
+  parsed.host = Component(host_begin, host().length());
+
+  if (serialized.length() > host_begin + host().length()) {
+    int port_begin = host_begin + host().length() + 1;
+    parsed.port = Component(port_begin, serialized.length() - port_begin);
+  }
+
+  // Append a / on the end of the URL.
+  parsed.path = Component(serialized.length(), 1);
+  serialized.append("/");
+
+  return GURL(serialized.data(), serialized.length(), parsed, true);

[brettw, 2016/09/30 20:18:48]

I told you to use this constructor, but I realize now there's a better one. Do:
  return GURL(std::move(serialized), parsed, true);
which will prevent the allocation of a new string.

[Charlie Harrison, 2016/10/03 17:28:47]

:) was hoping there was a better way here. Done!

+}
+
 bool SchemeHostPort::Equals(const SchemeHostPort& other) const {
   return port_ == other.port() && scheme_ == other.scheme() &&
          host_ == other.host();
 }
 
 bool SchemeHostPort::operator<(const SchemeHostPort& other) const {
   return std::tie(port_, scheme_, host_) <
          std::tie(other.port_, other.scheme_, other.host_);
 }
 
 }  // namespace url