From e1a8bfff19ae5d453f941994c9ae16157f36db24 Mon Sep 17 00:00:00 2001
From: Ed Lee <edilee@mozilla.com>
Date: Wed, 11 Jan 2017 12:27:03 -0800
Subject: [PATCH] fix(topsites): Combine frecency of the top pages of
 with/without www instead of all pages

---
 addon/PlacesProvider.js     | 11 ++++++-----
 test/test-PlacesProvider.js |  9 +++++++++
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/addon/PlacesProvider.js b/addon/PlacesProvider.js
index 0aadd1cff7..e3df65fa0c 100644
--- a/addon/PlacesProvider.js
+++ b/addon/PlacesProvider.js
@@ -348,7 +348,8 @@ Links.prototype = {
 
     let blockedURLs = ignoreBlocked ? [] : this.blockedURLs.items().map(item => `"${item}"`);
 
-    // this query does "GROUP BY rev_nowww" (rev_host without www) to remove urls from same domain.
+    // GROUP first by rev_host to get the most-frecent page of an exact host
+    // then GROUP by rev_nowww to dedupe between top two pages of nowww host.
     // Note that unlike mysql, sqlite picks the last raw from groupby bucket.
     // Which is why subselect orders frecency and last_visit_date backwards.
     // In general the groupby behavior in the absence of aggregates is not
@@ -357,9 +358,9 @@ Links.prototype = {
     let sqlQuery = `SELECT url, title, ${frecencyScore} frecency, guid, bookmarkGuid,
                           last_visit_date / 1000 as lastVisitDate, favicon, mimeType,
                           "history" as type
-                    FROM
-                    (
+                    FROM (SELECT * FROM (
                       SELECT
+                        rev_host,
                         CASE SUBSTR(rev_host, -5)
                           WHEN ".www." THEN SUBSTR(rev_host, -4, -999)
                           ELSE rev_host
@@ -379,8 +380,8 @@ Links.prototype = {
                       on moz_places.id = moz_bookmarks.fk
                       WHERE hidden = 0 AND last_visit_date NOTNULL
                       AND moz_places.url NOT IN (${blockedURLs})
-                      ORDER BY rev_host, frecency, last_visit_date, moz_places.url DESC
-                    )
+                      ORDER BY frecency, last_visit_date, moz_places.url DESC
+                    ) GROUP BY rev_host)
                     GROUP BY rev_nowww
                     ORDER BY frecency DESC, lastVisitDate DESC, url
                     LIMIT :limit`;
diff --git a/test/test-PlacesProvider.js b/test/test-PlacesProvider.js
index 7c62b436f5..b019f27abf 100644
--- a/test/test-PlacesProvider.js
+++ b/test/test-PlacesProvider.js
@@ -103,6 +103,15 @@ exports.test_Links_getTopFrecentSites_dedupeWWW = function*(assert) {
   links = yield provider.getTopFrecentSites();
   assert.equal(links.length, 1, "adding both www. and no-www. yields one link");
   assert.equal(links[0].frecency, 200, "frecency scores are combined");
+
+  // add another page visit with www and without www
+  testURI = NetUtil.newURI("http://mozilla.com/page");
+  yield PlacesTestUtils.addVisits(testURI);
+  testURI = NetUtil.newURI("http://www.mozilla.com/page");
+  yield PlacesTestUtils.addVisits(testURI);
+  links = yield provider.getTopFrecentSites();
+  assert.equal(links.length, 1, "adding both www. and no-www. yields one link");
+  assert.equal(links[0].frecency, 200, "frecency scores are combined ignoring extra pages");
 };
 
 exports.test_Links_getTopFrecentSites_limit = function*(assert) {