🤝 Remove trailing slash from Origin.

GURLUtils.getOrigin returns an origin formatted with a trailing slash
because it calls GURL::GetOrigin which returns a GURL, then serializes
this URL. This seems to be incorrect behaviour as more generally an
origin is serialized without a trailing slash.

GURLUtils.getOrigin is used by AwGeolocationPermissions.java where the
returned value is saved to the user's Android Preferences. This means
that changing its behaviour would be tricky.

For now, I'm updating Origin.java to serialize without a trailing
slash and documenting the deviance in behaviour between the two.

Bug: 827161
Change-Id: Ia70f8a60263742b28477f969cc075dcb97062f48
Reviewed-on: https://chromium-review.googlesource.com/1013483Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Peter Conn <peconn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551351}

chrome/android/java/src/org/chromium/chrome/browser/browserservices/Origin.java

@@ -7,8 +7,15 @@ package org.chromium.chrome.browser.browserservices;
 import android.net.Uri;
 
 /**
- * A class to canonically represent a web origin in Java. It intends to mirror the behaviour of
- * GURLUtils.getOrigin, but needs to work without native being loaded.
+ * A class to canonically represent a web origin in Java. In comparison to
+ * {@link org.chromium.net.GURLUtils#getOrigin} it can be used before native is loaded and lets us
+ * ensure conversion to an origin has been done with the type system.
+ *
+ * {@link #toString()} does <b>not</b> match {@link org.chromium.net.GURLUtils#getOrigin}. The
+ * latter will return a String with a trailing "/". Not having a trailing slash matches RFC
+ * behaviour (https://tools.ietf.org/html/rfc6454), it seems that
+ * {@link org.chromium.net.GURLUtils#getOrigin} adds it as a bug, but as its result is saved to
+ * user's Android Preferences, it is not trivial to change.
  */
 public class Origin {
     private static final int HTTP_DEFAULT_PORT = 80;
@@ -46,7 +53,7 @@ public class Origin {
                     .buildUpon()
                     .opaquePart("")
                     .fragment("")
-                    .path("/")
+                    .path("")
                     .encodedAuthority(authority)
                     .clearQuery()
                     .build();
@@ -57,6 +64,11 @@ public class Origin {
         mOrigin = origin;
     }
 
+    /**
+     * Returns whether the Origin is valid.
+     */
+    public boolean isValid() { return !mOrigin.equals(Uri.EMPTY); }
+
     /**
      * Returns a Uri representing the Origin.
      */

chrome/android/javatests/src/org/chromium/chrome/browser/browserservices/OriginTest.java

@@ -64,18 +64,15 @@ public class OriginTest {
         Assert.assertEquals(host, origin.uri().getHost());
         Assert.assertEquals(port, origin.uri().getPort());
 
-        if (port == -1) {
-            Assert.assertEquals(origin.toString(), scheme + "://" + host + "/");
-        } else {
-            Assert.assertEquals(origin.toString(), scheme + "://" + host + ":" + port + "/");
-        }
+        Assert.assertEquals(origin.toString(),
+                scheme + "://" + host + (port == -1 ? "" : ":" + port));
     }
 
     @Test
     @SmallTest
     public void testConstruction() {
         Origin origin = new Origin("http://www.example.com/path/to/page.html");
-        Assert.assertEquals("http://www.example.com/", origin.toString());
+        Assert.assertEquals("http://www.example.com", origin.toString());
     }
 
     @Test
@@ -95,7 +92,7 @@ public class OriginTest {
     @SmallTest
     public void testToUri() {
         Origin origin = new Origin(Uri.parse("http://www.example.com/page.html"));
-        Uri uri = Uri.parse("http://www.example.com/");
+        Uri uri = Uri.parse("http://www.example.com");
         Assert.assertEquals(uri, origin.uri());
     }
 
@@ -103,6 +100,14 @@ public class OriginTest {
     @SmallTest
     public void testToString() {
         Origin origin = new Origin("http://www.example.com/page.html");
-        Assert.assertEquals("http://www.example.com/", origin.toString());
+        Assert.assertEquals("http://www.example.com", origin.toString());
+    }
+
+    @Test
+    @SmallTest
+    public void testValidity() {
+        Assert.assertTrue(new Origin("http://www.example.com").isValid());
+        Assert.assertFalse(new Origin("null").isValid());
+        Assert.assertFalse(new Origin("www.example.com").isValid());
     }
 }

net/android/java/src/org/chromium/net/GURLUtils.java

@@ -14,7 +14,7 @@ public final class GURLUtils {
 
     /**
      * Get the origin of an url: Ex getOrigin("http://www.example.com:8080/index.html?bar=foo")
-     * would return "http://www.example.com:8080". It will return an empty string for an
+     * would return "http://www.example.com:8080/". It will return an empty string for an
      * invalid url.
      *
      * @return The origin of the url