Bug 1350349 - collect data for feed reader usage to evaluate its future, r=mak,data-review=liuche, a=lizzard
☠☠ backed out by 47d222274694 ☠ ☠
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Fri, 16 Feb 2018 19:18:00 +0000
changeset 455051 63366815c258d2be09d71785ab7aeba260245520
parent 455050 7690c7e5ef4a05cf2b4a1985268ec55fda6a15cf
child 455052 01f3f0dc10701aec97254c8973e30726f3e99f14
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak, lizzard
bugs1350349
milestone59.0
Bug 1350349 - collect data for feed reader usage to evaluate its future, r=mak,data-review=liuche, a=lizzard This patch adds data collection for 6 different data points: - number of live bookmarks the user has - number of times the user subscribes to a feed - number of times we attempt to open feed preview - number of times the user opens a feed popup in the UI - number of times the user opens an entry from a feed popup in the UI. MozReview-Commit-ID: DG9JDFXCjai
browser/base/content/browser-feeds.js
browser/components/feeds/FeedWriter.js
browser/components/feeds/moz.build
browser/components/feeds/test/browser/.eslintrc.js
browser/components/feeds/test/browser/browser.ini
browser/components/feeds/test/browser/browser_telemetry_checks.js
browser/components/feeds/test/browser/valid-feed.xml
browser/components/places/PlacesUIUtils.jsm
browser/components/places/content/browserPlacesViews.js
toolkit/components/places/PlacesDBUtils.jsm
toolkit/components/telemetry/Scalars.yaml
--- a/browser/base/content/browser-feeds.js
+++ b/browser/base/content/browser-feeds.js
@@ -588,16 +588,19 @@ var FeedHandler = {
         if (!settings.action || !VALID_ACTIONS.has(settings.action)) {
           LOG(`Invalid action ${settings.action}`);
           return;
         }
         if (!settings.reader || !VALID_READERS.has(settings.reader)) {
           LOG(`Invalid reader ${settings.reader}`);
           return;
         }
+
+        Services.telemetry.scalarAdd("browser.feeds.feed_subscribed", 1);
+
         const actionPref = getPrefActionForType(settings.feedType);
         this._setPref(actionPref, settings.action);
         const readerPref = getPrefReaderForType(settings.feedType);
         this._setPref(readerPref, settings.reader);
         handler = null;
 
         switch (settings.reader) {
           case "web":
--- a/browser/components/feeds/FeedWriter.js
+++ b/browser/components/feeds/FeedWriter.js
@@ -70,16 +70,17 @@ function convertByteUnits(aBytes) {
 }
 
 function FeedWriter() {
   this._selectedApp = undefined;
   this._selectedAppMenuItem = null;
   this._subscribeCallback = null;
   this._defaultHandlerMenuItem = null;
 
+  Services.telemetry.scalarAdd("browser.feeds.preview_loaded", 1);
 
   XPCOMUtils.defineLazyGetter(this, "_mm", () =>
     this._window.QueryInterface(Ci.nsIInterfaceRequestor).
                  getInterface(Ci.nsIDocShell).
                  QueryInterface(Ci.nsIInterfaceRequestor).
                  getInterface(Ci.nsIContentFrameMessageManager));
 }
 
--- a/browser/components/feeds/moz.build
+++ b/browser/components/feeds/moz.build
@@ -1,16 +1,17 @@
 # -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*-
 # vim: set filetype=python:
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 MOCHITEST_CHROME_MANIFESTS += ['test/chrome/chrome.ini']
 MOCHITEST_MANIFESTS += ['test/mochitest.ini']
+BROWSER_CHROME_MANIFESTS += ['test/browser/browser.ini']
 
 JAR_MANIFESTS += ['jar.mn']
 
 XPIDL_SOURCES += [
     'nsIFeedResultService.idl',
     'nsIWebContentConverterRegistrar.idl',
 ]
 
new file mode 100644
--- /dev/null
+++ b/browser/components/feeds/test/browser/.eslintrc.js
@@ -0,0 +1,7 @@
+"use strict";
+
+module.exports = {
+  "extends": [
+    "plugin:mozilla/browser-test"
+  ]
+};
new file mode 100644
--- /dev/null
+++ b/browser/components/feeds/test/browser/browser.ini
@@ -0,0 +1,3 @@
+[browser_telemetry_checks.js]
+support-files =
+  valid-feed.xml
new file mode 100644
--- /dev/null
+++ b/browser/components/feeds/test/browser/browser_telemetry_checks.js
@@ -0,0 +1,97 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+add_task(async function() {
+  function getSnapShot() {
+    return Services.telemetry.snapshotScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTOUT, false);
+  }
+  const TEST_PATH = getRootDirectory(gTestPath).replace("chrome://mochitests/content", "http://example.com");
+  const FEED_URI = TEST_PATH + "valid-feed.xml";
+
+  // Ensure we don't have any pre-existing telemetry that'd mess up counts in here:
+  Services.telemetry.clearScalars();
+  const kScalarPrefix = "browser.feeds.";
+  const kPreviewLoaded = kScalarPrefix + "preview_loaded";
+  const kSubscribed = kScalarPrefix + "feed_subscribed";
+  const kLivemarkCount = kScalarPrefix + "livebookmark_count";
+  const kLivemarkOpened = kScalarPrefix + "livebookmark_opened";
+  const kLivemarkItemOpened = kScalarPrefix + "livebookmark_item_opened";
+
+  let scalarForContent = gMultiProcessBrowser ? "content" : "parent";
+  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, FEED_URI);
+  // Ensure we get telemetry from the content process:
+  let previewCount = await TestUtils.waitForCondition(() => {
+    let snapshot = getSnapShot()[scalarForContent];
+    return snapshot && snapshot[kPreviewLoaded];
+  });
+  Assert.equal(previewCount, 1, "Should register the preview in telemetry.");
+
+  // Now check subscription. We stub out the actual code for adding the live bookmark,
+  // because the dialog creates an initial copy of the bookmark when it's opened and
+  // that's hard to deal with deterministically in tests.
+  let old = PlacesCommandHook.addLiveBookmark;
+  let createBMPromise = new Promise(resolve => {
+    PlacesCommandHook.addLiveBookmark = function(...args) {
+      resolve(args);
+      // Return the promise because Feeds.jsm expects a promise:
+      return createBMPromise;
+    };
+  });
+  registerCleanupFunction(() => PlacesCommandHook.addLiveBookmark = old);
+  await BrowserTestUtils.synthesizeMouseAtCenter("#subscribeButton", {}, tab.linkedBrowser);
+  let bmArgs = await createBMPromise;
+  Assert.deepEqual(bmArgs, [FEED_URI, "Example Feed", ""], "Should have been trying to subscribe");
+  let snapshot = getSnapShot();
+  Assert.equal(snapshot.parent[kSubscribed], 1, "Should have subscribed once.");
+
+  // Now manually add a livemark in the menu and one in the bookmarks toolbar:
+  let livemarks = await Promise.all([
+    PlacesUtils.livemarks.addLivemark({
+      parentGuid: PlacesUtils.bookmarks.menuGuid,
+      feedURI: Services.io.newURI(FEED_URI),
+    }),
+    PlacesUtils.livemarks.addLivemark({
+      parentGuid: PlacesUtils.bookmarks.toolbarGuid,
+      feedURI: Services.io.newURI(FEED_URI),
+    }),
+  ]);
+  registerCleanupFunction(async () => {
+    for (let mark of livemarks) {
+      await PlacesUtils.livemarks.removeLivemark(mark);
+    }
+  });
+
+  if (document.getElementById("PersonalToolbar").getAttribute("collapsed") == "true") {
+    CustomizableUI.setToolbarVisibility("PersonalToolbar", true);
+    registerCleanupFunction(() => CustomizableUI.setToolbarVisibility("PersonalToolbar", false));
+  }
+
+  // Force updating telemetry:
+  let {PlacesDBUtils} = ChromeUtils.import("resource://gre/modules/PlacesDBUtils.jsm", {});
+  await PlacesDBUtils._telemetryForFeeds();
+  Assert.equal(getSnapShot().parent[kLivemarkCount], 2,
+    "Should have created two livemarks and counted them.");
+
+  info("Waiting for livemark");
+  // Check we count opening the livemark popup:
+  let livemarkOnToolbar = await TestUtils.waitForCondition(
+    () => document.querySelector("#PersonalToolbar .bookmark-item[livemark]"));
+  let popup = livemarkOnToolbar.querySelector("menupopup");
+  let popupShownPromise = BrowserTestUtils.waitForEvent(popup, "popupshown");
+  info("Clicking on livemark");
+  // Using .click() or .doCommand() doesn't seem to work.
+  EventUtils.synthesizeMouseAtCenter(livemarkOnToolbar, {});
+  await popupShownPromise;
+  Assert.equal(getSnapShot().parent[kLivemarkOpened], 1, "Should count livemark opening");
+
+  // And opening an item in the popup:
+  let item = await TestUtils.waitForCondition(
+    () => popup.querySelector("menuitem.bookmark-item"));
+  item.doCommand();
+  Assert.equal(getSnapShot().parent[kLivemarkItemOpened], 1, "Should count livemark item opening");
+  popup.hidePopup();
+  await BrowserTestUtils.removeTab(tab);
+});
+
copy from browser/components/feeds/test/valid-feed.xml
copy to browser/components/feeds/test/browser/valid-feed.xml
--- a/browser/components/places/PlacesUIUtils.jsm
+++ b/browser/components/places/PlacesUIUtils.jsm
@@ -996,16 +996,20 @@ this.PlacesUIUtils = {
 
     let where = window.whereToOpenLink(aEvent, false, true);
     if (where == "current" && this.loadBookmarksInTabs &&
         PlacesUtils.nodeIsBookmark(aNode) && !aNode.uri.startsWith("javascript:")) {
       where = "tab";
     }
 
     this._openNodeIn(aNode, where, window);
+    let view = this.getViewForNode(aEvent.target);
+    if (view && view.controller.hasCachedLivemarkInfo(aNode.parent)) {
+      Services.telemetry.scalarAdd("browser.feeds.livebookmark_item_opened", 1);
+    }
   },
 
   /**
    * Loads the node's URL in the appropriate tab or window or as a
    * web panel.
    * see also openUILinkIn
    */
   openNodeIn: function PUIU_openNodeIn(aNode, aWhere, aView, aPrivate) {
--- a/browser/components/places/content/browserPlacesViews.js
+++ b/browser/components/places/content/browserPlacesViews.js
@@ -684,16 +684,20 @@ PlacesViewBase.prototype = {
       if (PlacesUtils.nodeIsFolder(aPlacesNode)) {
         let queryOptions = PlacesUtils.asQuery(this._result.root).queryOptions;
         if (queryOptions.excludeItems) {
           return;
         }
 
         PlacesUtils.livemarks.getLivemark({ id: aPlacesNode.itemId })
           .then(aLivemark => {
+            if (!this.controller) {
+              // We might have been destroyed in the interim...
+              return;
+            }
             let shouldInvalidate =
               !this.controller.hasCachedLivemarkInfo(aPlacesNode);
             this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark);
             if (aNewState == Ci.nsINavHistoryContainerResultNode.STATE_OPENED) {
               aLivemark.registerForUpdates(aPlacesNode, this);
               // Prioritize the current livemark.
               aLivemark.reload();
               PlacesUtils.livemarks.reloadLivemarks();
@@ -928,16 +932,19 @@ PlacesViewBase.prototype = {
     // Remove any delayed element, see _cleanPopup for details.
     if ("_delayedRemovals" in popup) {
       while (popup._delayedRemovals.length > 0) {
         popup.removeChild(popup._delayedRemovals.shift());
       }
     }
 
     if (popup._placesNode && PlacesUIUtils.getViewForNode(popup) == this) {
+      if (this.controller.hasCachedLivemarkInfo(popup._placesNode)) {
+        Services.telemetry.scalarAdd("browser.feeds.livebookmark_opened", 1);
+      }
       if (!popup._placesNode.containerOpen)
         popup._placesNode.containerOpen = true;
       if (!popup._built)
         this._rebuildPopup(popup);
 
       this._mayAddCommandsItems(popup);
     }
   },
--- a/toolkit/components/places/PlacesDBUtils.jsm
+++ b/toolkit/components/places/PlacesDBUtils.jsm
@@ -917,16 +917,19 @@ this.PlacesDBUtils = {
   /**
    * Collects telemetry data and reports it to Telemetry.
    *
    * Note: although this function isn't actually async, we keep it async to
    * allow us to maintain a simple, consistent API for the tasks within this object.
    *
    */
   async telemetry() {
+    // First deal with some scalars for feeds:
+    await this._telemetryForFeeds();
+
     // This will be populated with one integer property for each probe result,
     // using the histogram name as key.
     let probeValues = {};
 
     // The following array contains an ordered list of entries that are
     // processed to collect telemetry data.  Each entry has these properties:
     //
     //  histogram: Name of the telemetry histogram to update.
@@ -1060,16 +1063,26 @@ this.PlacesDBUtils = {
           value = aProbe.callback(value);
         }
         probeValues[aProbe.histogram] = value;
         Services.telemetry.getHistogramById(aProbe.histogram).add(value);
       }).catch(Cu.reportError);
     }
   },
 
+  async _telemetryForFeeds() {
+    let db = await PlacesUtils.promiseDBConnection();
+    let livebookmarkCount = await db.execute(
+      `SELECT count(*) FROM moz_items_annos a
+                       JOIN moz_anno_attributes aa ON a.anno_attribute_id = aa.id
+                       WHERE aa.name = 'livemark/feedURI'`);
+    livebookmarkCount = livebookmarkCount[0].getResultByIndex(0);
+    Services.telemetry.scalarSet("browser.feeds.livebookmark_count", livebookmarkCount);
+  },
+
   /**
    * Runs a list of tasks, returning a Map when done.
    *
    * @param tasks
    *        Array of tasks to be executed, in form of pointers to methods in
    *        this module.
    * @return {Promise}
    *        A promise that resolves with a Map[taskName(String) -> Object].
--- a/toolkit/components/telemetry/Scalars.yaml
+++ b/toolkit/components/telemetry/Scalars.yaml
@@ -257,16 +257,84 @@ browser.engagement.navigation:
     kind: uint
     keyed: true
     notification_emails:
       - bcolloran@mozilla.com
     release_channel_collection: opt-out
     record_in_processes:
       - 'main'
 
+# The following section contains (RSS/Atom) feed usage
+browser.feeds:
+  preview_loaded:
+    bug_numbers:
+      - 1350349
+    description: >
+      The number of times a feed preview page is loaded.
+    expires: "65"
+    kind: uint
+    notification_emails:
+      - gijs@mozilla.com
+    release_channel_collection: opt-out
+    record_in_processes:
+      - 'main'
+      - 'content'
+
+  feed_subscribed:
+    bug_numbers:
+      - 1350349
+    description: >
+      The number of times a user subscribes to a feed.
+    expires: "65"
+    kind: uint
+    notification_emails:
+      - gijs@mozilla.com
+    release_channel_collection: opt-out
+    record_in_processes:
+      - 'main'
+
+  livebookmark_opened:
+    bug_numbers:
+      - 1350349
+    description: >
+      The number of times a user opens a live bookmark dropdown.
+    expires: "65"
+    kind: uint
+    notification_emails:
+      - gijs@mozilla.com
+    release_channel_collection: opt-out
+    record_in_processes:
+      - 'main'
+
+  livebookmark_item_opened:
+    bug_numbers:
+      - 1350349
+    description: >
+      The number of times a user opens an item that is part of a live bookmark.
+    expires: "65"
+    kind: uint
+    notification_emails:
+      - gijs@mozilla.com
+    release_channel_collection: opt-out
+    record_in_processes:
+      - 'main'
+
+  livebookmark_count:
+    bug_numbers:
+      - 1350349
+    description: >
+      The number of live bookmark folders the user has (ie not individual items).
+    expires: "65"
+    kind: uint
+    notification_emails:
+      - gijs@mozilla.com
+    release_channel_collection: opt-out
+    record_in_processes:
+      - 'main'
+
 # The following section contains the browser usage scalars.
 browser.usage:
   graphite:
     bug_numbers:
       - 1331915
     description: >
       The number of times a graphite2 font has been loaded.
     expires: "65"