From 286e8f602d3fa81089eecbe3661cc72f5573a189 Mon Sep 17 00:00:00 2001
From: Gijs Kruitbosch <gijskruitbosch@gmail.com>
Date: Wed, 23 May 2018 18:01:36 +0100
Subject: [PATCH] Bug 1463782 - allow toolkit apps in kinto blocklist,
 r=leplatrem

MozReview-Commit-ID: G1uqNw1Njni

--HG--
extra : rebase_source : 31637d596df1a9e85d0f21cfaced08508139037b
---
 services/common/blocklist-clients.js          | 18 +++++++----
 services/common/remote-settings.js            |  5 +++
 .../tests/unit/test_blocklist_clients.js      |  4 +--
 .../unit/test_blocklist_targetapp_filter.js   | 31 +++++++++++++++++--
 4 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/services/common/blocklist-clients.js b/services/common/blocklist-clients.js
index 616667d1681c..e63bc7bce90c 100644
--- a/services/common/blocklist-clients.js
+++ b/services/common/blocklist-clients.js
@@ -147,14 +147,17 @@ async function targetAppFilter(entry, environment) {
     return entry;
   }
 
-  const { appID, version: appVersion } = environment;
+  const { appID, version: appVersion, toolkitVersion } = environment;
   const { versionRange } = entry;
 
+  // Everywhere in this method, we avoid checking the minVersion, because
+  // we want to retain items whose minVersion is higher than the current
+  // app version, so that we have the items around for app updates.
+
   // Gfx blocklist has a specific versionRange object, which is not a list.
   if (!Array.isArray(versionRange)) {
-    const { minVersion = "0", maxVersion = "*" } = versionRange;
-    const matchesRange = (Services.vc.compare(appVersion, minVersion) >= 0 &&
-                          Services.vc.compare(appVersion, maxVersion) <= 0);
+    const { maxVersion = "*" } = versionRange;
+    const matchesRange = (Services.vc.compare(appVersion, maxVersion) <= 0);
     return matchesRange ? entry : null;
   }
 
@@ -173,12 +176,15 @@ async function targetAppFilter(entry, environment) {
       if (!guid) {
         return entry;
       }
-      const { minVersion = "0", maxVersion = "*" } = ta;
+      const { maxVersion = "*" } = ta;
       if (guid == appID &&
-          Services.vc.compare(appVersion, minVersion) >= 0 &&
           Services.vc.compare(appVersion, maxVersion) <= 0) {
         return entry;
       }
+      if (guid == "toolkit@mozilla.org" &&
+          Services.vc.compare(toolkitVersion, maxVersion) <= 0) {
+        return entry;
+      }
     }
   }
   // Skip this entry.
diff --git a/services/common/remote-settings.js b/services/common/remote-settings.js
index d0220de6b588..e56e8417f2ea 100644
--- a/services/common/remote-settings.js
+++ b/services/common/remote-settings.js
@@ -64,6 +64,11 @@ class ClientEnvironment extends ClientEnvironmentBase {
     Services.appinfo.QueryInterface(Ci.nsIXULAppInfo);
     return Services.appinfo.ID;
   }
+
+  static get toolkitVersion() {
+    Services.appinfo.QueryInterface(Ci.nsIPlatformInfo);
+    return Services.appinfo.platformVersion;
+  }
 }
 
 /**
diff --git a/services/common/tests/unit/test_blocklist_clients.js b/services/common/tests/unit/test_blocklist_clients.js
index aa8323917dc5..88f143020fec 100644
--- a/services/common/tests/unit/test_blocklist_clients.js
+++ b/services/common/tests/unit/test_blocklist_clients.js
@@ -227,7 +227,7 @@ add_task(async function test_sync_event_data_is_filtered_for_target() {
     let called = false;
     client.on("sync", e => called = true);
     await client.maybeSync(timestamp3 + 1, fakeServerTime - 10);
-    equal(called, false, `no sync event for ${client.collectionName}`);
+    equal(called, false, `shouldn't have sync event for ${client.collectionName}`);
 
     // In ?_since=5000 entries, only one entry matches.
     let syncEventData;
@@ -546,7 +546,7 @@ function getSampleResponse(req, port) {
         "versionRange": [{
           "targetApplication": [{
             "guid": "xpcshell@tests.mozilla.org",
-            "minVersion": "99999"
+            "maxVersion": "20"
           }],
         }],
         "id": "86771771-e803-4006-95e9-c9275d58b3d1"
diff --git a/services/common/tests/unit/test_blocklist_targetapp_filter.js b/services/common/tests/unit/test_blocklist_targetapp_filter.js
index f1c3f20467fd..aa6d2330d8a8 100644
--- a/services/common/tests/unit/test_blocklist_targetapp_filter.js
+++ b/services/common/tests/unit/test_blocklist_targetapp_filter.js
@@ -2,6 +2,7 @@ const BlocklistClients = ChromeUtils.import("resource://services-common/blocklis
 const { RemoteSettings } = ChromeUtils.import("resource://services-common/remote-settings.js", {});
 
 const APP_ID = "xpcshell@tests.mozilla.org";
+const TOOLKIT_ID = "toolkit@mozilla.org";
 
 let client;
 
@@ -84,10 +85,17 @@ add_task(async function test_returns_without_guid_or_with_matching_guid() {
         guid: APP_ID,
       }]
     }]
+  }, {
+    willMatch: true,
+    versionRange: [{
+      targetApplication: [{
+        guid: TOOLKIT_ID,
+      }]
+    }]
   }]);
 
   const list = await client.get();
-  equal(list.length, 2);
+  equal(list.length, 3);
   ok(list.every(e => e.willMatch));
 });
 add_task(clear_state);
@@ -126,10 +134,29 @@ add_task(async function test_returns_without_app_version_or_with_matching_versio
         maxVersion: "1",
       }]
     }]
+  }, {
+    willMatch: true,
+    versionRange: [{
+      targetApplication: [{
+        guid: TOOLKIT_ID,
+        minVersion: "0",
+      }]
+    }]
+  }, {
+    willMatch: true,
+    versionRange: [{
+      targetApplication: [{
+        guid: TOOLKIT_ID,
+        minVersion: "0",
+        maxVersion: "9999",
+      }]
+    }]
+    // We can't test the false case with maxVersion for toolkit, because the toolkit version
+    // is 0 in xpcshell.
   }]);
 
   const list = await client.get();
-  equal(list.length, 3);
+  equal(list.length, 5);
   ok(list.every(e => e.willMatch));
 });
 add_task(clear_state);