Better support for patterns.

chrome/browser/resources/settings/site_settings/site_list.js

Index: chrome/browser/resources/settings/site_settings/site_list.js
diff --git a/chrome/browser/resources/settings/site_settings/site_list.js b/chrome/browser/resources/settings/site_settings/site_list.js
index e724ab81998b8c67b5220030c6c6cfe741077ab3..b06cc8213c412036c31bf10c48f80b037940f01b 100644
--- a/chrome/browser/resources/settings/site_settings/site_list.js
+++ b/chrome/browser/resources/settings/site_settings/site_list.js
@@ -268,14 +268,6 @@ Polymer({
   },
 
   /**
-   * Ensures the URL has a scheme (assumes http if omitted).
-   */
-  ensureUrlHasScheme_: function(url) {
-    if (url.length == 0) return url;
-    return url.indexOf('://') != -1 ? url : 'http://' + url;
-  },
-
-  /**
    * Converts an unordered site list to an ordered array, sorted by site name
    * then protocol and de-duped (by origin).
    * @param {!Array<SiteException>} sites A list of sites to sort and de-dup.
@@ -286,23 +278,30 @@ Polymer({
     sites.sort(function(a, b) {
       // TODO(finnur): Hmm, it would probably be better to ensure scheme on the
       //     JS/C++ boundary.
-      var originA = self.ensureUrlHasScheme_(a.origin);
-      var originB = self.ensureUrlHasScheme_(b.origin);
-      var embeddingOriginA = self.ensureUrlHasScheme_(a.embeddingOrigin);
-      var embeddingOriginB = self.ensureUrlHasScheme_(b.embeddingOrigin);
-      var url1 = new URL(originA);
-      var url2 = new URL(originB);
+      var originA = self.ensureUrlHasScheme(a.origin);
+      var originB = self.ensureUrlHasScheme(b.origin);
+      var embeddingOriginA = self.ensureUrlHasScheme(a.embeddingOrigin);

[michaelpg, 2016/04/11 03:55:35]

you don't use these embedding* vars until 299, so create them down there and
maybe condense them a bit

[Finnur, 2016/04/11 11:45:33]

Done.

+      var embeddingOriginB = self.ensureUrlHasScheme(b.embeddingOrigin);
+      // Sorting needs protocol, port and host, but doesn't care about
+      // wildcards, which cause the generated URL to be invalid.
+      var url1 = new URL(originA.replace('[*.]', ''));
+      var url2 = new URL(originB.replace('[*.]', ''));
       var embeddingUrl1 = embeddingOriginA.length == 0 ? '' :
-          new URL(embeddingOriginA);
+          new URL(embeddingOriginA.replace('[*.]'));

[michaelpg, 2016/04/11 03:55:35]

missing 2nd arg (apparently results in String(undefined) being used)

[Finnur, 2016/04/11 11:45:33]

Good catch. Explains the weirdness I was facing below (fixed now).

       var embeddingUrl2 = embeddingOriginB.length == 0 ? '' :
-          new URL(embeddingOriginB);
+          new URL(embeddingOriginB.replace('[*.]'));
       var comparison = url1.host.localeCompare(url2.host);
       if (comparison == 0) {
         comparison = url1.protocol.localeCompare(url2.protocol);
         if (comparison == 0) {
           comparison = url1.port.localeCompare(url2.port);
-          if (comparison == 0)
-            return embeddingUrl1.host.localeCompare(embeddingUrl2.host);
+          if (comparison == 0) {
+            var host1 = embeddingUrl1.host === undefined ?
+                '' : embeddingUrl1.host;
+            var host2 = embeddingUrl2.host === undefined ?
+                '' : embeddingUrl2.host;
+            return host1.localeCompare(host2);
+          }
         }
       }
       return comparison;
@@ -314,22 +313,26 @@ Polymer({
       var origin = sites[i].origin;
       var embeddingOrigin = sites[i].embeddingOrigin;
 
-      // The All Sites category can contain duplicates (from other categories).
-      if (origin == lastOrigin && embeddingOrigin == lastEmbeddingOrigin)
-        continue;
-
+      var originForDisplay = origin.replace('[*.]', '');
       var embeddingOriginForDisplay = '';
       if (embeddingOrigin != '*' && origin != embeddingOrigin)
         embeddingOriginForDisplay = embeddingOrigin;
 
+      // The All Sites category can contain duplicates (from other categories).
+      if (originForDisplay == lastOrigin &&
+          embeddingOriginForDisplay == lastEmbeddingOrigin) {
+        continue;
+      }
+
       results.push({
          origin: origin,
+         originForDisplay: originForDisplay,
          embeddingOrigin: embeddingOrigin,
          embeddingOriginForDisplay: embeddingOriginForDisplay,
       });
 
-      lastOrigin = origin;
-      lastEmbeddingOrigin = embeddingOrigin;
+      lastOrigin = originForDisplay;
+      lastEmbeddingOrigin = embeddingOriginForDisplay;
     }
     return results;
   },