Bug 1341350 - Change the hosts prefix algorithm to prefer secure urls, but also to be able to swap back and forth depending on typed entries in history. r=mak
authorMark Banner <standard8@mozilla.com>
Tue, 02 May 2017 18:29:05 +0100
changeset 356547 7e9c97247ba0
parent 356546 4a0e9b01d69d
child 356548 0df833bf2b9f
push id31767
push usercbook@mozilla.com
push date2017-05-05 13:15 +0000
treeherdermozilla-central@8872ad4d52b6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1341350
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1341350 - Change the hosts prefix algorithm to prefer secure urls, but also to be able to swap back and forth depending on typed entries in history. r=mak MozReview-Commit-ID: BuK16J0JUaf
toolkit/components/places/nsPlacesTriggers.h
toolkit/components/places/tests/unifiedcomplete/test_trimming.js
toolkit/components/places/tests/unit/test_hosts_triggers.js
--- a/toolkit/components/places/nsPlacesTriggers.h
+++ b/toolkit/components/places/nsPlacesTriggers.h
@@ -58,38 +58,40 @@
   "OR rev_host = get_unreversed_host(host || '.') || '.www.'"
 
 /**
  * Select the best prefix for a host, based on existing pages registered for it.
  * Prefixes have a priority, from the top to the bottom, so that secure pages
  * have higher priority, and more generically "www." prefixed hosts come before
  * unprefixed ones.
  * Given a host, examine associated pages and:
- *  - if all of the typed pages start with https://www. return https://www.
- *  - if all of the typed pages start with https:// return https://
+ *  - if at least half the typed pages start with https://www. return https://www.
+ *  - if at least half the typed pages start with https:// return https://
  *  - if all of the typed pages start with ftp: return ftp://
- *  - if all of the typed pages start with www. return www.
+ *     - This is because mostly people will want to visit the http version
+ *       of the site.
+ *  - if at least half the typed pages start with www. return www.
  *  - otherwise don't use any prefix
  */
 #define HOSTS_PREFIX_PRIORITY_FRAGMENT \
   "SELECT CASE " \
-    "WHEN 1 = ( " \
-      "SELECT min(substr(url,1,12) = 'https://www.') FROM moz_places h " \
+    "WHEN ( " \
+      "SELECT round(avg(substr(url,1,12) = 'https://www.')) FROM moz_places h " \
       "WHERE (" HOST_TO_REVHOST_PREDICATE ") AND +h.typed = 1 " \
     ") THEN 'https://www.' " \
-    "WHEN 1 = ( " \
-      "SELECT min(substr(url,1,8) = 'https://') FROM moz_places h " \
+    "WHEN ( " \
+      "SELECT round(avg(substr(url,1,8) = 'https://')) FROM moz_places h " \
       "WHERE (" HOST_TO_REVHOST_PREDICATE ") AND +h.typed = 1 " \
     ") THEN 'https://' " \
     "WHEN 1 = ( " \
       "SELECT min(substr(url,1,4) = 'ftp:') FROM moz_places h " \
       "WHERE (" HOST_TO_REVHOST_PREDICATE ") AND +h.typed = 1 " \
     ") THEN 'ftp://' " \
-    "WHEN 1 = ( " \
-      "SELECT min(substr(url,1,11) = 'http://www.') FROM moz_places h " \
+    "WHEN ( " \
+      "SELECT round(avg(substr(url,1,11) = 'http://www.')) FROM moz_places h " \
       "WHERE (" HOST_TO_REVHOST_PREDICATE ") AND +h.typed = 1 " \
     ") THEN 'www.' " \
   "END "
 
 /**
  * These triggers update the hostnames table whenever moz_places changes.
  */
 #define CREATE_PLACES_AFTERINSERT_TRIGGER NS_LITERAL_CSTRING( \
--- a/toolkit/components/places/tests/unifiedcomplete/test_trimming.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_trimming.js
@@ -126,57 +126,57 @@ add_task(function* test_priority_1() {
   yield check_autocomplete({
     search: "mo",
     autofilled: "mozilla.org/",
     completed: "mozilla.org/"
   });
   yield cleanup();
 });
 
-add_task(function* test_periority_2() {
+add_task(function* test_priority_2() {
   do_print( "Ensuring correct priority 2");
   yield PlacesTestUtils.addVisits([
     { uri: NetUtil.newURI("https://mozilla.org/test/"), transition: TRANSITION_TYPED },
     { uri: NetUtil.newURI("ftp://mozilla.org/test/"), transition: TRANSITION_TYPED },
     { uri: NetUtil.newURI("http://www.mozilla.org/test/"), transition: TRANSITION_TYPED },
     { uri: NetUtil.newURI("http://mozilla.org/test/"), transition: TRANSITION_TYPED }
   ]);
   yield check_autocomplete({
     search: "mo",
     autofilled: "mozilla.org/",
     completed: "mozilla.org/"
   });
   yield cleanup();
 });
 
-add_task(function* test_periority_3() {
+add_task(function* test_priority_3() {
   do_print("Ensuring correct priority 3");
   yield PlacesTestUtils.addVisits([
     { uri: NetUtil.newURI("ftp://mozilla.org/test/"), transition: TRANSITION_TYPED },
     { uri: NetUtil.newURI("http://www.mozilla.org/test/"), transition: TRANSITION_TYPED },
     { uri: NetUtil.newURI("http://mozilla.org/test/"), transition: TRANSITION_TYPED }
   ]);
   yield check_autocomplete({
     search: "mo",
     autofilled: "mozilla.org/",
     completed: "mozilla.org/"
   });
   yield cleanup();
 });
 
-add_task(function* test_periority_4() {
+add_task(function* test_priority_4() {
   do_print("Ensuring correct priority 4");
   yield PlacesTestUtils.addVisits([
     { uri: NetUtil.newURI("http://www.mozilla.org/test/"), transition: TRANSITION_TYPED },
     { uri: NetUtil.newURI("http://mozilla.org/test/"), transition: TRANSITION_TYPED }
   ]);
   yield check_autocomplete({
     search: "mo",
     autofilled: "mozilla.org/",
-    completed: "mozilla.org/"
+    completed: "www.mozilla.org/"
   });
   yield cleanup();
 });
 
 add_task(function* test_priority_5() {
   do_print("Ensuring correct priority 5");
   yield PlacesTestUtils.addVisits([
     { uri: NetUtil.newURI("ftp://mozilla.org/test/"), transition: TRANSITION_TYPED },
--- a/toolkit/components/places/tests/unit/test_hosts_triggers.js
+++ b/toolkit/components/places/tests/unit/test_hosts_triggers.js
@@ -214,60 +214,146 @@ add_task(function* test_moz_hosts_update
                 },
                 { uri: TEST_URI_2
                 }];
   yield PlacesTestUtils.addVisits(places);
 
   checkHostInMozHosts(TEST_URI_1, true, "https://www.");
 });
 
-function getTestSection(baseURL1, baseURL2, extra) {
+function getTestSection(baseURL1, baseURL2, baseURL2Prefix, extra) {
   let extraStr = "";
   let expectedSimplePrefix = null;
-  let expectedSecurePrefix = "https://";
+  let expectedUpgradePrefix = baseURL2Prefix;
   if (extra) {
     extraStr = ` (${extra})`;
     expectedSimplePrefix = `${extra}.`;
-    expectedSecurePrefix = `https://${extra}.`;
+    expectedUpgradePrefix = `${baseURL2Prefix}${extra}.`;
   }
   return [{
-    title: `Test simple insecure${extraStr}`,
+    title: `Test simple url${extraStr}`,
     visits: [{ uri: baseURL1, transition: TRANSITION_TYPED }],
-    expect: [baseURL1, true, expectedSimplePrefix ]
+    expect: [baseURL1, true, expectedSimplePrefix]
   }, {
-    title: `Test upgrade secure${extraStr}`,
+    title: `Test upgrade url${extraStr}`,
     visits: [{ uri: baseURL2, transition: TRANSITION_TYPED }],
-    expect: [baseURL2, true, null]
+    expect: [baseURL2, true, expectedUpgradePrefix]
   }, {
-    title: `Test remove insecure completely${extraStr}`,
+    title: `Test remove simple completely${extraStr}`,
     remove: baseURL1,
-    expect: [baseURL2, true, expectedSecurePrefix]
+    expect: [baseURL2, true, expectedUpgradePrefix]
   }, {
     title: `Test add more visits${extraStr}`,
     visits: [
       { uri: baseURL2, transition: TRANSITION_TYPED },
       { uri: baseURL1, transition: TRANSITION_TYPED },
     ],
-    expect: [baseURL2, true, null]
+    expect: [baseURL2, true, expectedUpgradePrefix]
   }, {
-    title: `Test switch to insecure${extraStr}`,
-    visits: [{ uri: baseURL1, transition: TRANSITION_TYPED }],
-    expect: [baseURL2, true, null]
+    title: `Test remove upgrade url${extraStr}`,
+    remove: baseURL2,
+    expect: [baseURL2, true, expectedSimplePrefix]
   }];
 }
 
-const updateTestURL1 = "http://example.com/";
-const updateTestURL2 = "https://example.com/";
-
 const hostsUpdateTests = [{
   title: "Upgrade Secure/Downgrade Insecure",
-  tests: getTestSection("http://example.com", "https://example.com")
+  tests: getTestSection("http://example.com", "https://example.com", "https://")
 }, {
   title: "Upgrade Secure/Downgrade Insecure (www)",
-  tests: getTestSection("http://www.example1.com", "https://www.example1.com", "www")
+  tests: getTestSection("http://www.example1.com", "https://www.example1.com", "https://", "www")
+}, {
+  title: "Upgrade Secure/Downgrade non-www to www",
+  tests: getTestSection("http://example3.com", "http://www.example3.com", "www.")
+}, {
+  title: "Switch to/from ftp",
+  tests: [{
+    title: `Test normal url`,
+    visits: [{ uri: "http://example4.com", transition: TRANSITION_TYPED }],
+    expect: ["http://example4.com", true, null]
+  }, {
+    title: `Test switch to ftp`,
+    visits: [{ uri: "ftp://example4.com", transition: TRANSITION_TYPED }],
+    // ftp is only switched to if all pages are ftp://
+    remove: ["http://example4.com"],
+    expect: ["ftp://example4.com", true, "ftp://"]
+  }, {
+    title: `Test visit http`,
+    visits: [{ uri: "http://example4.com", transition: TRANSITION_TYPED }],
+    expect: ["ftp://example4.com", true, null]
+  }]
+}, {
+  title: "Multiple URLs for source",
+  tests: [{
+    title: `Test simple insecure`,
+    visits: [{ uri: "http://example2.com", transition: TRANSITION_TYPED }],
+    expect: ["http://example2.com", true, null]
+  }, {
+    title: `Test upgrade secure`,
+    visits: [{ uri: "https://example2.com", transition: TRANSITION_TYPED }],
+    expect: ["https://example2.com", true, "https://"]
+  }, {
+    title: `Test extra insecure visit`,
+    visits: [{ uri: "http://example2.com/fake", transition: TRANSITION_TYPED }],
+    expect: ["https://example2.com", true, null]
+  }, {
+    title: `Test extra secure visits`,
+    visits: [
+      { uri: "https://example2.com/foo", transition: TRANSITION_TYPED },
+      { uri: "https://example2.com/bar", transition: TRANSITION_TYPED },
+    ],
+    expect: ["https://example2.com", true, "https://"]
+  }, {
+    title: `Test remove secure`,
+    remove: ["https://example2.com", "https://example2.com/foo", "https://example2.com/bar"],
+    expect: ["https://example2.com", true, null]
+  }]
+}, {
+  title: "Test upgrade tree",
+  tests: [{
+    title: `Add ftp`,
+    visits: [{ uri: "ftp://example5.com", transition: TRANSITION_TYPED }],
+    expect: ["http://example5.com", true, "ftp://"]
+  }, {
+    title: `Add basic http`,
+    visits: [{ uri: "http://example5.com", transition: TRANSITION_TYPED }],
+    expect: ["http://example5.com", true, null]
+  }, {
+    title: `Add basic www`,
+    visits: [
+      // Add multiples to exceed the average.
+      { uri: "http://www.example5.com", transition: TRANSITION_TYPED },
+      { uri: "http://www.example5.com/past", transition: TRANSITION_TYPED }
+    ],
+    expect: ["http://example5.com", true, "www."]
+  }, {
+    title: `Add https`,
+    visits: [
+      // Add multiples to exceed the average.
+      { uri: "https://example5.com", transition: TRANSITION_TYPED },
+      { uri: "https://example5.com/past", transition: TRANSITION_TYPED },
+      { uri: "https://example5.com/mak", transition: TRANSITION_TYPED },
+      { uri: "https://example5.com/standard8", transition: TRANSITION_TYPED }
+    ],
+    expect: ["https://example5.com", true, "https://"]
+  }, {
+    title: `Add https www`,
+    visits: [
+      // Add multiples to exceed the average.
+      { uri: "https://www.example5.com", transition: TRANSITION_TYPED },
+      { uri: "https://www.example5.com/quantum", transition: TRANSITION_TYPED },
+      { uri: "https://www.example5.com/photon", transition: TRANSITION_TYPED },
+      { uri: "https://www.example5.com/dash", transition: TRANSITION_TYPED },
+      { uri: "https://www.example5.com/flow", transition: TRANSITION_TYPED },
+      { uri: "https://www.example5.com/persona", transition: TRANSITION_TYPED },
+      { uri: "https://www.example5.com/ff_fx", transition: TRANSITION_TYPED },
+      { uri: "https://www.example5.com/search", transition: TRANSITION_TYPED }
+    ],
+    expect: ["https://example5.com", true, "https://www."]
+  }]
 }];
 
 add_task(function* test_moz_hosts_update() {
   for (const section of hostsUpdateTests) {
     do_print(section.title);
 
     for (const test of section.tests) {
       do_print(test.title);