Bug 708134 - Rewrite add-on URL to note it was installed by Sync; r=rnewman, Unfocused

--HG--
extra : rebase_source : cbcdcf2f85d8b68c57accef1b1e2db887df4925f

services/sync/modules/engines/addons.js

@@ -649,13 +649,12 @@ AddonsStore.prototype = {
    *        Function to be called with result of operation.
    */
   getInstallFromSearchResult: function getInstallFromSearchResult(addon, cb) {
-    // If we have an install, use it.
-    if (addon.install) {
-      cb(null, addon.install);
-      return;
-    }
-
-    this._log.debug("Manually obtaining install for " + addon.id);
+    // We should theoretically be able to obtain (and use) addon.install if
+    // it is available. However, the addon.sourceURI rewriting won't be
+    // reflected in the AddonInstall, so we can't use it. If we ever get rid
+    // of sourceURI rewriting, we can avoid having to reconstruct the
+    // AddonInstall.
+    this._log.debug("Obtaining install for " + addon.id);
 
     // Verify that the source URI uses TLS. We don't allow installs from
     // insecure sources for security reasons. The Addon Manager ensures that
@@ -957,6 +956,36 @@ AddonsStore.prototype = {
           }
         }.bind(this);
 
+        // Rewrite the "src" query string parameter of the source URI to note
+        // that the add-on was installed by Sync and not something else so
+        // server-side metrics aren't skewed (bug 708134). The server should
+        // ideally send proper URLs, but this solution was deemed too
+        // complicated at the time the functionality was implemented.
+        for each (let addon in addons) {
+          // We should always be able to QI the nsIURI to nsIURL. If not, we
+          // still try to install the add-on, but we don't rewrite the URL,
+          // potentially skewing metrics.
+          try {
+            addon.sourceURI.QueryInterface(Ci.nsIURL);
+          } catch (ex) {
+            this._log.warn("Unable to QI sourceURI to nsIURL: " +
+                           addon.sourceURI.spec);
+            continue;
+          }
+
+          let params = addon.sourceURI.query.split("&").map(
+            function rewrite(param) {
+
+            if (param.indexOf("src=") == 0) {
+              return "src=sync";
+            } else {
+              return param;
+            }
+          });
+
+          addon.sourceURI.query = params.join("&");
+        }
+
         // Start all the installs asynchronously. They will report back to us
         // as they finish, eventually triggering the global callback.
         for (let i = 0; i < addonsLength; i++) {

services/sync/tests/unit/rewrite-search.xml

@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<searchresults total_results="1">
+  <addon id="5617">
+  <name>Rewrite Test Extension</name>
+  <type id="1">Extension</type>
+  <guid>rewrite@tests.mozilla.org</guid>
+  <slug>rewrite</slug>
+  <version>1.0</version>
+
+  <compatible_applications><application>
+      <name>Firefox</name>
+      <application_id>1</application_id>
+      <min_version>3.6</min_version>
+      <max_version>*</max_version>
+      <appID>xpcshell@tests.mozilla.org</appID>
+    </application></compatible_applications>
+  <all_compatible_os><os>ALL</os></all_compatible_os>
+
+  <install os="ALL" size="485">http://127.0.0.1:8888/require.xpi?src=api</install>
+    <created epoch="1252903662">
+      2009-09-14T04:47:42Z
+    </created>
+    <last_updated epoch="1315255329">
+      2011-09-05T20:42:09Z
+    </last_updated>
+    </addon>
+</searchresults>

services/sync/tests/unit/test_addons_store.js

@@ -53,6 +53,9 @@ function createAndStartHTTPServer(port) {
     server.registerFile("/search/guid:missing-xpi%40tests.mozilla.org",
                         do_get_file("missing-xpi-search.xml"));
 
+    server.registerFile("/search/guid:rewrite%40tests.mozilla.org",
+                        do_get_file("rewrite-search.xml"));
+
     server.start(port);
 
     return server;
@@ -434,3 +437,44 @@ add_test(function test_wipe() {
 
   run_next_test();
 });
+
+add_test(function test_source_uri_rewrite() {
+  _("Ensure that a 'src=api' query string is rewritten to 'src=sync'");
+
+  // This tests for conformance with bug 708134 so server-side metrics aren't
+  // skewed.
+
+  Svc.Prefs.set("addons.ignoreRepositoryChecking", true);
+
+  // We resort to monkeypatching because of the API design.
+  let oldFunction = store.__proto__.installAddonFromSearchResult;
+
+  let installCalled = false;
+  store.__proto__.installAddonFromSearchResult =
+    function testInstallAddon(addon, cb) {
+
+    do_check_eq("http://127.0.0.1:8888/require.xpi?src=sync",
+                addon.sourceURI.spec);
+
+    installCalled = true;
+
+    store.getInstallFromSearchResult(addon, function (error, install) {
+      do_check_eq("http://127.0.0.1:8888/require.xpi?src=sync",
+                  install.sourceURI.spec);
+
+      cb(null, {id: addon.id, addon: addon, install: install});
+    });
+  };
+
+  let server = createAndStartHTTPServer(HTTP_PORT);
+
+  let installCallback = Async.makeSpinningCallback();
+  store.installAddonsFromIDs(["rewrite@tests.mozilla.org"], installCallback);
+
+  installCallback.wait();
+  do_check_true(installCalled);
+  store.__proto__.installAddonFromSearchResult = oldFunction;
+
+  Svc.Prefs.reset("addons.ignoreRepositoryChecking");
+  server.stop(run_next_test);
+});