Merge branch 'master' into metrics-privacy-intro

.eslintrc.js

@@ -34,6 +34,7 @@ module.exports = {
     "react/jsx-curly-spacing": [2, "never"],
     "react/jsx-equals-spacing": [2, "never"],
     "react/jsx-key": 2,
+    "react/jsx-no-bind": 2,
     "react/jsx-no-comment-textnodes": 2,
     "react/jsx-no-duplicate-props": 2,
     "react/jsx-no-target-blank": 2,

addon/ActivityStreams.js

@@ -107,7 +107,7 @@ ActivityStreams.prototype = {
     if (!this.options.shield_variant) {
       this._experimentProvider.init();
     }
-    this._tabTracker.init(this.appURLs, this._experimentProvider.experimentId, this._store);
+    this._tabTracker.init(this.appURLs, this._experimentProvider, this._store);
     this._searchProvider.init();
     this._initializePreviewProvider(this._metadataStore, this._tabTracker, this._store);
     this._initializePageScraper(this._previewProvider, this._tabTracker);
@@ -225,7 +225,12 @@ ActivityStreams.prototype = {
   _respondToPlacesRequests({msg, worker}) {
     switch (msg.type) {
       case am.type("NOTIFY_BOOKMARK_ADD"):
-        PlacesProvider.links.asyncAddBookmark(msg.data);
+        PlacesProvider.links.asyncAddBookmark(msg.data.url);
+
+        // Request metadata for the new bookmark if it has been added from PocketStories.
+        if (msg.data.source === "RECOMMENDED") {
+          this._pageScraper.asyncFetchLinks([{url: msg.data.url}], msg.type, true);
+        }
         break;
       case am.type("NOTIFY_BOOKMARK_DELETE"):
         PlacesProvider.links.asyncDeleteBookmark(msg.data);
@@ -307,11 +312,6 @@ ActivityStreams.prototype = {
     this._tabTracker.handleImpressionStats(msg.data);
   },
 
-  _handleExperimentChange(prefName) {
-    this._tabTracker.experimentId = this._experimentProvider.experimentID;
-    this.broadcast(am.actions.Response("EXPERIMENTS_RESPONSE", this._experimentProvider.data));
-  },
-
   _respondToUIChanges(args) {
     const {msg} = args;
     switch (msg.type) {
@@ -341,9 +341,6 @@ ActivityStreams.prototype = {
     this._handleCurrentEngineChanges = this._handleCurrentEngineChanges.bind(this);
     this._searchProvider.on("browser-search-engine-modified", this._handleCurrentEngineChanges);
 
-    this._handleExperimentChange = this._handleExperimentChange.bind(this);
-    this._experimentProvider.on("change", this._handleExperimentChange);
-
     // This is a collection of handlers that receive messages from content
     this._contentToAddonHandlers = (msgName, args) => {
       // Log requests first so that the requests are logged before responses
@@ -370,7 +367,6 @@ ActivityStreams.prototype = {
   _removeListeners() {
     PLACES_CHANGES_EVENTS.forEach(event => PlacesProvider.links.off(event, this._handlePlacesChanges));
     this._searchProvider.off("browser-search-engine-modified", this._handleCurrentEngineChanges);
-    this._experimentProvider.off("change", this._handleExperimentChange);
     this.off(CONTENT_TO_ADDON, this._contentToAddonHandlers);
   },
 

addon/ExperimentProvider.js

@@ -1,10 +1,13 @@
 const {Cu} = require("chrome");
 const prefService = require("sdk/preferences/service");
 const simplePrefs = require("sdk/simple-prefs");
-const {PrefsTarget} = require("sdk/preferences/event-target");
+const {setTimeout} = require("sdk/timers");
 const {preferencesBranch} = require("sdk/self");
 const PREF_PREFIX = `extensions.${preferencesBranch}.experiments.`;
+const EXPERIMENTS_ENDPOINT = "experiments.endpoint";
+const EXPERIMENTS_REFRESH_TIMEOUT = 60 * 60 * 1000; // Refresh once per hour
 
+Cu.importGlobalProperties(["fetch"]);
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 XPCOMUtils.defineLazyGetter(this, "EventEmitter", () => {
@@ -13,76 +16,85 @@ XPCOMUtils.defineLazyGetter(this, "EventEmitter", () => {
 });
 
 exports.ExperimentProvider = class ExperimentProvider {
-  constructor(experiments = require("../experiments.json"), rng) {
-    this._experiments = experiments;
+  constructor(experiments = null, rng) {
+    this._testMode = false;
+    this._experiments = {};
     this._rng = rng || Math.random;
     this._data = {};
     this._experimentId = null;
-    this._target = PrefsTarget();
-    EventEmitter.decorate(this);
 
-    this._onPrefChange = this._onPrefChange.bind(this);
+    if (experiments) {
+      this._testMode = true;
+      this._experiments = experiments;
+    }
+    EventEmitter.decorate(this);
   }
 
   init() {
+    if (this._testMode) {
+      this.setupExperiments();
+    } else {
+      const experimentsEndpoint = simplePrefs.prefs[EXPERIMENTS_ENDPOINT];
+      if (experimentsEndpoint) {
+        fetch(simplePrefs.prefs[EXPERIMENTS_ENDPOINT])
+          .then(response => response.json())
+          .then(experiments => {
+            this._experiments = {};
+            this._data = {};
+            experiments.forEach(experiment => {
+              this._experiments[experiment.slug] = experiment;
+            });
+            this.setupExperiments();
+            setTimeout(this.init.bind(this), EXPERIMENTS_REFRESH_TIMEOUT);
+          })
+          .catch(e => {
+            Cu.reportError(e);
+          });
+      }
+    }
+  }
+
+  setupExperiments() {
     this.setValues();
-    Object.keys(this._experiments).forEach(experimentName => {
-      this._target.on(PREF_PREFIX + experimentName, this._onPrefChange);
-      Object.defineProperty(this._data, experimentName, {
+    Object.keys(this._experiments).forEach(experimentId => {
+      Object.defineProperty(this._data, experimentId, {
         get() {
-          return prefService.get(PREF_PREFIX + experimentName);
+          return prefService.get(PREF_PREFIX + experimentId);
         },
         enumerable: true
       });
     });
   }
 
-  _onPrefChange(prefName) {
-    this.overrideExperimentPrefs(prefName);
-    this.emit("change", prefName);
-  }
-
-  /**
-   * This is called when experiment prefs are changed so
-   * that users are pulled out of all experiment reporting.
-   */
-  overrideExperimentPrefs(prefName) {
-    simplePrefs.prefs.experimentsOverridden = true;
-    this._experimentId = null;
-  }
-
   /**
    * This is used to disable all experiments, i.e set all their
    * values to their original control value.
    */
   disableAllExperiments() {
-    Object.keys(this._experiments).forEach(key => {
-      const experiment = this._experiments[key];
+    Object.keys(this._experiments).forEach(experimentId => {
+      const experiment = this._experiments[experimentId];
       const {active, control} = experiment;
       if (active) {
-        prefService.set(PREF_PREFIX + key, control.value);
+        prefService.set(PREF_PREFIX + experimentId, control.value);
       }
     });
   }
 
   enroll(experimentId, variant) {
-    this._experimentId = variant.id;
+    this._experimentId = experimentId;
     prefService.set(PREF_PREFIX + experimentId, variant.value);
-    if (experimentId === "topSitesTwoRowsDefault") {
-      simplePrefs.prefs.showMoreTopSites = true;
-    }
     this.emit("experimentEnrolled", {id: experimentId, variant});
   }
 
   setValues() {
     if (simplePrefs.prefs.experimentsOverridden) {
       console.log(`The following experiments were turned on via overrides:\n`); // eslint-disable-line no-console
-      Object.keys(this._experiments).forEach(experimentName => {
-        const {variant, control} = this._experiments[experimentName];
-        if (prefService.get(PREF_PREFIX + experimentName) === variant.value) {
-          console.log(`- ${experimentName} - \n`); // eslint-disable-line no-console
+      Object.keys(this._experiments).forEach(experimentId => {
+        const {variant, control} = this._experiments[experimentId];
+        if (prefService.get(PREF_PREFIX + experimentId) === variant.value) {
+          console.log(`- ${experimentId} - \n`); // eslint-disable-line no-console
         } else {
-          prefService.set(PREF_PREFIX + experimentName, control.value);
+          prefService.set(PREF_PREFIX + experimentId, control.value);
         }
       });
       return;
@@ -98,36 +110,36 @@ exports.ExperimentProvider = class ExperimentProvider {
     let floor = 0;
     let inExperiment;
 
-    Object.keys(this._experiments).forEach(key => {
-      const experiment = this._experiments[key];
+    Object.keys(this._experiments).forEach(experimentId => {
+      const experiment = this._experiments[experimentId];
       const {variant, control} = experiment;
 
-      if (prefService.get(PREF_PREFIX + key) === variant.value) {
+      if (prefService.get(PREF_PREFIX + experimentId) === variant.value) {
         if (experiment.active) {
           // If the user is already part of an active experiment, set the experiment id.
-          this._experimentId = variant.id;
+          this._experimentId = experimentId;
         } else {
           // If the user is part of an inactive experiment,
           // reset that experiment's pref.
-          prefService.set(PREF_PREFIX + key, control.value);
+          prefService.set(PREF_PREFIX + experimentId, control.value);
           this._experimentId = null;
         }
       }
     });
 
-    Object.keys(this._experiments).forEach(key => {
-      const experiment = this._experiments[key];
+    Object.keys(this._experiments).forEach(experimentId => {
+      const experiment = this._experiments[experimentId];
       const {variant, control} = experiment;
       const ceiling = variant.threshold + floor;
 
       // If the experiment is not new or not active you will not be assigned to it.
-      if (prefService.has(PREF_PREFIX + key) || !experiment.active) {
+      if (prefService.has(PREF_PREFIX + experimentId) || !experiment.active) {
         return;
       }
 
       // If the experiment pref is undefined, it's a new experiment. Start
       // by assuming the user will not be in it.
-      prefService.set(PREF_PREFIX + key, control.value);
+      prefService.set(PREF_PREFIX + experimentId, control.value);
 
       if (ceiling > 1) {
         throw new Error("Your variant cohort sizes should add up to less than 1.");
@@ -142,7 +154,7 @@ exports.ExperimentProvider = class ExperimentProvider {
       // randomly assign them to a variant (or control)
       inExperiment = randomNumber >= floor && randomNumber < ceiling;
       if (inExperiment) {
-        this.enroll(key, variant);
+        this.enroll(experimentId, variant);
       }
       floor = ceiling;
     });
@@ -161,14 +173,11 @@ exports.ExperimentProvider = class ExperimentProvider {
 
   destroy() {
     this._experimentId = null;
-    Object.keys(this._experiments).forEach(experimentName => {
-      this._target.removeListener(PREF_PREFIX + experimentName, this._onPrefChange);
-    });
   }
 
   clearPrefs() {
-    Object.keys(this._experiments).forEach(experimentName => {
-      prefService.reset(PREF_PREFIX + experimentName);
+    Object.keys(this._experiments).forEach(experimentId => {
+      prefService.reset(PREF_PREFIX + experimentId);
     });
     simplePrefs.prefs.experimentsOverridden = false;
   }

addon/Feeds/BookmarksFeed.js

@@ -1,9 +1,10 @@
 const {Cu} = require("chrome");
 const {PlacesProvider} = require("addon/PlacesProvider");
 const Feed = require("addon/lib/Feed");
-const {BOOKMARKS_LENGTH} = require("common/constants");
+const {BOOKMARKS_LENGTH, BOOKMARKS_THRESHOLD} = require("common/constants");
 const am = require("common/action-manager");
 const getScreenshot = require("addon/lib/getScreenshot");
+const simplePrefs = require("sdk/simple-prefs");
 
 Cu.import("resource://gre/modules/Task.jsm");
 
@@ -16,18 +17,36 @@ module.exports = class BookmarksFeed extends Feed {
     this.missingData = false;
   }
 
+  /**
+   * Get the timestamp of the oldest bookmark. Used to filter out default bookmarks.
+   * Tries to fetch from prefs first otherwise goes out and does a query in PlacesProvider for it.
+   *
+   * @returns Integer Timestamp with dateAdded of oldest bookmark.
+   * @private
+   */
+  _getDefaultBookmarksAge() {
+    return Task.spawn(function*() {
+      const prefValue = parseInt(simplePrefs.prefs.defaultBookmarksAge, 10);
+
+      if (prefValue && prefValue !== 0) {
+        return prefValue;
+      }
+
+      const result = yield PlacesProvider.links.getDefaultBookmarksAge();
+      simplePrefs.prefs.defaultBookmarksAge = result.toString();
+      return result;
+    });
+  }
+
   /**
    * shouldGetScreenshot - Returns true if the link/site provided meets the following:
    * - is a bookmark
-   * - has metadata
    * - doesn't have any images
    *
    * @return bool
    */
   shouldGetScreenshot(link) {
-    return link.bookmarkGuid &&
-      link.hasMetadata &&
-      (!link.images || link.images.length === 0);
+    return link.bookmarkGuid && (!link.images || link.images.length === 0);
   }
 
   /**
@@ -38,8 +57,13 @@ module.exports = class BookmarksFeed extends Feed {
   getData() {
     return Task.spawn(function*() {
       let links;
+      const defaultBookmarksAge = yield this._getDefaultBookmarksAge();
+
       // Get links from places
-      links = yield PlacesProvider.links.getBookmarks({limit: BOOKMARKS_LENGTH});
+      links = yield PlacesProvider.links.getBookmarks({
+        limit: BOOKMARKS_LENGTH,
+        ageMin: defaultBookmarksAge + BOOKMARKS_THRESHOLD
+      });
 
       // Get metadata from PreviewProvider
       links = yield this.options.getCachedMetadata(links, "BOOKMARKS_RESPONSE");
@@ -62,6 +86,9 @@ module.exports = class BookmarksFeed extends Feed {
         }
       }
 
+      // Filter out bookmarks that don't have title.
+      links = links.filter(l => l.title);
+
       return am.actions.Response("BOOKMARKS_RESPONSE", links);
     }.bind(this));
   }