DO NOT LAND

url/origin.cc

Index: url/origin.cc
diff --git a/url/origin.cc b/url/origin.cc
index 8aaa173fa5e0d98728067daca5af80b349e0ba28..aa62453f429355e84e021e8031e11581a236f80a 100644
--- a/url/origin.cc
+++ b/url/origin.cc
@@ -4,16 +4,75 @@
 
 #include "url/origin.h"
 
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 
 namespace url {
 
-Origin::Origin() : string_("null") {}
+Origin::Origin() {
+  Init(GURL());
+}
+
+Origin::Origin(const GURL& url) {
+  Init(url);
+}
+
+Origin::Origin(const std::string& origin) {
+  Init(GURL(origin));
+}
+
+void Origin::Init(const GURL& url) {
+  // Start with a unique origin, parse from there:
+  scheme_ = "";
+  host_ = "";

[Ryan Sleevi, 2015/05/22 02:50:03]

scheme_.clear()
host_.clear()

+  port_ = 0;
+  unique_ = true;
+
+  GURL origin(url.GetOrigin());
+  if (!origin.is_valid() || !origin.IsStandard())
+    return;
+
+  // Ok. We have an origin.

[Ryan Sleevi, 2015/05/22 02:50:03]

No pronouns in comments or the boogey-sleevi will get you. Not to be confused
with the boogy-sleevi, who will get down.

+  unique_ = false;
+  if (origin.SchemeIsFile()) {
+    scheme_ = "file";
+    return;
+  }

[Ryan Sleevi, 2015/05/22 02:50:03]

API wise, I don't think this is right. You're encoding assumptions here re:
standard URLs.

More importantly, why can't you just use origin.scheme()?

I suspect what you really want is something like

scheme_ = origin.scheme()

if (!origin.SchemeIsStandard() || origin.SchemeIsFile())
  return;

host_ = origin.HostNoBrackets();  <-- I'm not sure how I feel about this, for
the record.

+
+  unique_ = false;
+  scheme_ = origin.scheme();
+  host_ = origin.HostNoBrackets();
+  port_ = origin.EffectiveIntPort();
+  is_default_port_ = origin.EffectiveIntPort() != origin.IntPort();

[Ryan Sleevi, 2015/05/22 02:50:03]

BUG: I don't think this does what you think it does

If the GURL didn't specify a port, origin.IntPort() == PORT_UNSPECIFIED
If the scheme has a default port, origin.EffectiveIntPort != PORT_UNSPECIFIED

+}
+
+std::string Origin::serialize() const {
+  if (unique_)
+    return "null";
+
+  if (scheme_ == "file")
+    return "file://";
+
+  return scheme_ + "://" + host_ +
+         (is_default_port_ ? "" : ":" + base::IntToString(port_));
+}

[Ryan Sleevi, 2015/05/22 02:50:03]

url code uses the URL constants -
https://code.google.com/p/chromium/codesearch#chromium/src/url/url_constants....

+
+bool Origin::SchemeIs(const std::string& scheme) const {
+  return scheme_ == scheme;
+}
+
+bool Origin::operator==(const Origin& other) const {
+  return !unique_ && !other.unique_ && scheme_ == other.scheme_ &&
+         host_ == other.host_ && port_ == other.port_;
+}
+
+bool Origin::operator!=(const Origin& other) const {
+  return unique_ || other.unique_ || scheme_ != other.scheme_ ||
+         host_ != other.host_ || port_ != other.port_;
+}
 
-Origin::Origin(const std::string& origin) : string_(origin) {
-  DCHECK(origin == "null" || MatchPattern(origin, "?*://?*"));
-  DCHECK_GT(origin.size(), 0u);
-  DCHECK(origin == "file://" || origin[origin.size() - 1] != '/');
+std::ostream& operator<<(std::ostream& out, const url::Origin& url) {
+  return out << url.serialize();
 }
 
 }  // namespace url