Merge mozilla-central to mozilla-inbound. a=merge

browser/actors/SearchTelemetryChild.jsm

@@ -0,0 +1,165 @@
+/* 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/. */
+"use strict";
+
+var EXPORTED_SYMBOLS = ["SearchTelemetryChild"];
+
+ChromeUtils.import("resource://gre/modules/ActorChild.jsm");
+ChromeUtils.import("resource://gre/modules/Services.jsm");
+
+const SHARED_DATA_KEY = "SearchTelemetry:ProviderInfo";
+
+/**
+ * SearchProviders looks after keeping track of the search provider information
+ * received from the main process.
+ *
+ * It is separate to SearchTelemetryChild so that it is not constructed for each
+ * tab, but once per process.
+ */
+class SearchProviders {
+  constructor() {
+    this._searchProviderInfo = null;
+    Services.cpmm.sharedData.addEventListener("change", this);
+  }
+
+  /**
+   * Gets the search provider information for any provider with advert information.
+   * If there is nothing in the cache, it will obtain it from shared data.
+   *
+   * @returns {object} Returns the search provider information. @see SearchTelemetry.jsm
+   */
+  get info() {
+    if (this._searchProviderInfo) {
+      return this._searchProviderInfo;
+    }
+
+    this._searchProviderInfo = Services.cpmm.sharedData.get(SHARED_DATA_KEY);
+
+    if (!this._searchProviderInfo) {
+      return null;
+    }
+
+    // Filter-out non-ad providers so that we're not trying to match against
+    // those unnecessarily.
+    for (let [providerName, info] of Object.entries(this._searchProviderInfo)) {
+      if (!("extraAdServersRegexps" in info)) {
+        delete this._searchProviderInfo[providerName];
+      }
+    }
+
+    return this._searchProviderInfo;
+  }
+
+  /**
+   * Handles events received from sharedData notifications.
+   *
+   * @param {object} event The event details.
+   */
+  handleEvent(event) {
+    switch (event.type) {
+      case "change": {
+        if (event.changedKeys.includes(SHARED_DATA_KEY)) {
+          // Just null out the provider information for now, we'll fetch it next
+          // time we need it.
+          this._searchProviderInfo = null;
+        }
+        break;
+      }
+    }
+  }
+}
+
+const searchProviders = new SearchProviders();
+
+/**
+ * SearchTelemetryChild monitors for pages that are partner searches, and
+ * looks through them to find links which looks like adverts and sends back
+ * a notification to SearchTelemetry for possible telemetry reporting.
+ *
+ * Only the partner details and the fact that at least one ad was found on the
+ * page are returned to SearchTelemetry. If no ads are found, no notification is
+ * given.
+ */
+class SearchTelemetryChild extends ActorChild {
+  /**
+   * Determines if there is a provider that matches the supplied URL and returns
+   * the information associated with that provider.
+   *
+   * @param {string} url The url to check
+   * @returns {array|null} Returns null if there's no match, otherwise an array
+   *   of provider name and the provider information.
+   */
+  _getProviderInfoForUrl(url) {
+    return Object.entries(searchProviders.info || []).find(
+      ([_, info]) => info.regexp.test(url)
+    );
+  }
+
+  /**
+   * Checks to see if the page is a partner and has an ad link within it. If so,
+   * it will notify SearchTelemetry.
+   *
+   * @param {object} doc The document object to check.
+   */
+  _checkForAdLink(doc) {
+    let providerInfo = this._getProviderInfoForUrl(doc.documentURI);
+    if (!providerInfo) {
+      return;
+    }
+
+    let regexps = providerInfo[1].extraAdServersRegexps;
+
+    let anchors = doc.getElementsByTagName("a");
+    let hasAds = false;
+    for (let anchor of anchors) {
+      if (!anchor.href) {
+        continue;
+      }
+      for (let regexp of regexps) {
+        if (regexp.test(anchor.href)) {
+          hasAds = true;
+          break;
+        }
+      }
+      if (hasAds) {
+        break;
+      }
+    }
+    if (hasAds) {
+      this.sendAsyncMessage("SearchTelemetry:PageInfo", {
+        hasAds: true,
+        url: doc.documentURI,
+      });
+    }
+  }
+
+  /**
+   * Handles events received from the actor child notifications.
+   *
+   * @param {object} event The event details.
+   */
+  handleEvent(event) {
+    // We are only interested in the top-level frame.
+    if (event.target.ownerGlobal != this.content) {
+      return;
+    }
+
+    switch (event.type) {
+      case "pageshow": {
+        // If a page is loaded from the bfcache, we won't get a "DOMContentLoaded"
+        // event, so we need to rely on "pageshow" in this case. Note: we do this
+        // so that we remain consistent with the *.in-content:sap* count for the
+        // SEARCH_COUNTS histogram.
+        if (event.persisted) {
+          this._checkForAdLink(this.content.document);
+        }
+        break;
+      }
+      case "DOMContentLoaded": {
+        this._checkForAdLink(this.content.document);
+        break;
+      }
+    }
+  }
+}

browser/actors/moz.build

@@ -43,6 +43,7 @@ FINAL_TARGET_FILES.actors += [
     'PageMetadataChild.jsm',
     'PageStyleChild.jsm',
     'PluginChild.jsm',
+    'SearchTelemetryChild.jsm',
     'URIFixupChild.jsm',
     'WebRTCChild.jsm',
 ]

browser/app/profile/firefox.js

@@ -1275,13 +1275,18 @@ pref("browser.newtabpage.activity-stream.improvesearch.topSiteSearchShortcuts",
 
 // ASRouter provider configuration
 #if defined(NIGHTLY_BUILD)
-pref("browser.newtabpage.activity-stream.asrouter.providers.snippets", "{\"id\":\"snippets\",\"enabled\":true,\"type\":\"remote\",\"url\":\"https://snippets.cdn.mozilla.net/%STARTPAGE_VERSION%/%NAME%/%VERSION%/%APPBUILDID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/\",\"updateCycleInMs\":14400000}");
 pref("browser.newtabpage.activity-stream.asrouter.providers.cfr", "{\"id\":\"cfr\",\"enabled\":true,\"type\":\"local\",\"localProvider\":\"CFRMessageProvider\",\"frequency\":{\"custom\":[{\"period\":\"daily\",\"cap\":1}]}}");
 #else
-pref("browser.newtabpage.activity-stream.asrouter.providers.snippets", "{\"id\":\"snippets\",\"enabled\":false,\"type\":\"remote\",\"url\":\"https://snippets.cdn.mozilla.net/%STARTPAGE_VERSION%/%NAME%/%VERSION%/%APPBUILDID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/\",\"updateCycleInMs\":14400000}");
 pref("browser.newtabpage.activity-stream.asrouter.providers.cfr", "{\"id\":\"cfr\",\"enabled\":false,\"type\":\"local\",\"localProvider\":\"CFRMessageProvider\",\"frequency\":{\"custom\":[{\"period\":\"daily\",\"cap\":1}]}}");
 #endif
 
+#ifdef EARLY_BETA_OR_EARLIER
+pref("browser.newtabpage.activity-stream.asrouter.providers.snippets", "{\"id\":\"snippets\",\"enabled\":true,\"type\":\"remote\",\"url\":\"https://snippets.cdn.mozilla.net/%STARTPAGE_VERSION%/%NAME%/%VERSION%/%APPBUILDID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/\",\"updateCycleInMs\":14400000}");
+#else
+pref("browser.newtabpage.activity-stream.asrouter.providers.snippets", "{\"id\":\"snippets\",\"enabled\":false,\"type\":\"remote\",\"url\":\"https://snippets.cdn.mozilla.net/%STARTPAGE_VERSION%/%NAME%/%VERSION%/%APPBUILDID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/\",\"updateCycleInMs\":14400000}");
+#endif
+
+
 
 // Enable the DOM fullscreen API.
 pref("full-screen-api.enabled", true);

browser/base/content/test/performance/browser_startup_content.js

@@ -53,6 +53,7 @@ const whitelist = {
     "resource:///modules/ContentMetaHandler.jsm",
     "resource:///actors/LinkHandlerChild.jsm",
     "resource:///actors/PageStyleChild.jsm",
+    "resource:///actors/SearchTelemetryChild.jsm",
     "resource://gre/modules/ActorChild.jsm",
     "resource://gre/modules/ActorManagerChild.jsm",
     "resource://gre/modules/E10SUtils.jsm",

browser/components/nsBrowserGlue.js

@@ -252,6 +252,16 @@ let ACTORS = {
     },
   },
 
+  SearchTelemetry: {
+    child: {
+      module: "resource:///actors/SearchTelemetryChild.jsm",
+      events: {
+        DOMContentLoaded: {},
+        "pageshow": {mozSystemGroup: true},
+      },
+    },
+  },
+
   ShieldFrame: {
     child: {
       module: "resource://normandy-content/ShieldFrameChild.jsm",
@@ -426,6 +436,7 @@ XPCOMUtils.defineLazyModuleGetters(this, {
   SafeBrowsing: "resource://gre/modules/SafeBrowsing.jsm",
   Sanitizer: "resource:///modules/Sanitizer.jsm",
   SaveToPocket: "chrome://pocket/content/SaveToPocket.jsm",
+  SearchTelemetry: "resource:///modules/SearchTelemetry.jsm",
   SessionStartup: "resource:///modules/sessionstore/SessionStartup.jsm",
   SessionStore: "resource:///modules/sessionstore/SessionStore.jsm",
   ShellService: "resource:///modules/ShellService.jsm",
@@ -1444,6 +1455,7 @@ BrowserGlue.prototype = {
     }
 
     BrowserUsageTelemetry.uninit();
+    SearchTelemetry.uninit();
     // Only uninit PingCentre if the getter has initialized it
     if (Object.prototype.hasOwnProperty.call(this, "pingCentre")) {
       this.pingCentre.uninit();
@@ -1511,6 +1523,7 @@ BrowserGlue.prototype = {
     }
 
     BrowserUsageTelemetry.init();
+    SearchTelemetry.init();
 
     // Show update notification, if needed.
     if (Services.prefs.prefHasUserValue("app.update.postupdate"))

browser/components/search/SearchTelemetry.jsm

@@ -12,36 +12,59 @@ XPCOMUtils.defineLazyModuleGetters(this, {
   Services: "resource://gre/modules/Services.jsm",
 });
 
+// The various histograms and scalars that we report to.
 const SEARCH_COUNTS_HISTOGRAM_KEY = "SEARCH_COUNTS";
+const SEARCH_WITH_ADS_SCALAR = "browser.search.with_ads";
+const SEARCH_AD_CLICKS_SCALAR = "browser.search.ad_clicks";
 
-// Used to identify various parameters (query, code, etc.) in search URLS.
+/**
+ * Used to identify various parameters used with partner search providers. This
+ * consists of the following structure:
+ * - {<string>} name
+ *     Details for a particular provider with the string name.
+ * - {regexp} <string>.regexp
+ *     The regular expression used to match the url for the search providers main page.
+ * - {string} <string>.queryParam
+ *     The query parameter name that indicates a search has been made.
+ * - {string} [<string>.codeParam]
+ *     The query parameter name that indicates a search provider's code.
+ * - {array} [<string>.codePrefixes]
+ *     An array of the possible string prefixes for a codeParam, indicating a
+ *     partner search.
+ * - {array} [<string>.followOnParams]
+ *     An array of parameters name that indicates this is a follow-on search.
+ * - {array} [<string>.extraAdServersRegexps]
+ *     An array of regular expressions used to determine if a link on a search
+ *     page mightbe an advert.
+ */
 const SEARCH_PROVIDER_INFO = {
   "google": {
-    "regexp": /^https:\/\/www\.(google)\.(?:.+)\/search/,
+    "regexp": /^https:\/\/www\.google\.(?:.+)\/search/,
     "queryParam": "q",
     "codeParam": "client",
     "codePrefixes": ["firefox"],
     "followonParams": ["oq", "ved", "ei"],
+    "extraAdServersRegexps": [/^https:\/\/www\.googleadservices\.com\/(?:pagead\/)?aclk/],
   },
   "duckduckgo": {
-    "regexp": /^https:\/\/(duckduckgo)\.com\//,
+    "regexp": /^https:\/\/duckduckgo\.com\//,
     "queryParam": "q",
     "codeParam": "t",
     "codePrefixes": ["ff"],
   },
   "yahoo": {
-    "regexp": /^https:\/\/(?:.*)search\.(yahoo)\.com\/search/,
+    "regexp": /^https:\/\/(?:.*)search\.yahoo\.com\/search/,
     "queryParam": "p",
   },
   "baidu": {
-    "regexp": /^https:\/\/www\.(baidu)\.com\/(?:s|baidu)/,
+    "regexp": /^https:\/\/www\.baidu\.com\/(?:s|baidu)/,
     "queryParam": "wd",
     "codeParam": "tn",
     "codePrefixes": ["monline_dg"],
     "followonParams": ["oq"],
   },
   "bing": {
-    "regexp": /^https:\/\/www\.(bing)\.com\/search/,
+    "regexp": /^https:\/\/www\.bing\.com\/search/,
     "queryParam": "q",
     "codeParam": "pc",
     "codePrefixes": ["MOZ", "MZ"],
@@ -52,11 +75,81 @@ const BROWSER_SEARCH_PREF = "browser.search.";
 
 XPCOMUtils.defineLazyPreferenceGetter(this, "loggingEnabled", BROWSER_SEARCH_PREF + "log", false);
 
+/**
+ * TelemetryHandler is the main class handling search telemetry. It primarily
+ * deals with tracking of what pages are loaded into tabs.
+ *
+ * It handles the *in-content:sap* keys of the SEARCH_COUNTS histogram.
+ */
 class TelemetryHandler {
   constructor() {
+    this._browserInfoByUrl = new Map();
+    this._initialized = false;
     this.__searchProviderInfo = null;
+    this._contentHandler = new ContentHandler({
+      browserInfoByUrl: this._browserInfoByUrl,
+      getProviderInfoForUrl: this._getProviderInfoForUrl.bind(this),
+    });
   }
 
+  /**
+   * Initializes the TelemetryHandler and its ContentHandler. It will add
+   * appropriate listeners to the window so that window opening and closing
+   * can be tracked.
+   */
+  init() {
+    if (this._initialized) {
+      return;
+    }
+
+    this._contentHandler.init();
+
+    for (let win of Services.wm.getEnumerator("navigator:browser")) {
+      this._registerWindow(win);
+    }
+    Services.wm.addListener(this);
+
+    this._initialized = true;
+  }
+
+  /**
+   * Uninitializes the TelemetryHandler and its ContentHandler.
+   */
+  uninit() {
+    if (!this._initialized) {
+      return;
+    }
+
+    this._contentHandler.uninit();
+
+    for (let win of Services.wm.getEnumerator("navigator:browser")) {
+      this._unregisterWindow(win);
+    }
+    Services.wm.removeListener(this);
+
+    this._initialized = false;
+  }
+
+  /**
+   * Handles the TabClose event received from the listeners.
+   *
+   * @param {object} event
+   */
+  handleEvent(event) {
+    if (event.type != "TabClose") {
+      Cu.reportError(`Received unexpected event type ${event.type}`);
+      return;
+    }
+
+    this.stopTrackingBrowser(event.target.linkedBrowser);
+  }
+
+  /**
+   * Test-only function, used to override the provider information, so that
+   * unit tests can set it to easy to test values.
+   *
+   * @param {object} infoByProvider @see SEARCH_PROVIDER_INFO for type information.
+   */
   overrideSearchTelemetryForTests(infoByProvider) {
     if (infoByProvider) {
       for (let info of Object.values(infoByProvider)) {
@@ -66,19 +159,162 @@ class TelemetryHandler {
     } else {
       this.__searchProviderInfo = SEARCH_PROVIDER_INFO;
     }
+    this._contentHandler.overrideSearchTelemetryForTests(this.__searchProviderInfo);
   }
 
-  recordSearchURLTelemetry(url) {
-    let entry = Object.entries(this._searchProviderInfo).find(
-      ([_, info]) => info.regexp.test(url)
-    );
-    if (!entry) {
+  /**
+   * This may start tracking a tab based on the URL. If the URL matches a search
+   * partner, and it has a code, then we'll start tracking it. This will aid
+   * determining if it is a page we should be tracking for adverts.
+   *
+   * @param {object} browser The browser associated with the page.
+   * @param {string} url The url that was loaded in the browser.
+   */
+  updateTrackingStatus(browser, url) {
+    let info = this._checkURLForSerpMatch(url);
+    if (!info) {
+      this.stopTrackingBrowser(browser);
       return;
     }
-    let [provider, searchProviderInfo] = entry;
+
+    this._reportSerpPage(info, url);
+
+    // If we have a code, then we also track this for potential ad clicks.
+    if (info.code) {
+      let item = this._browserInfoByUrl.get(url);
+      if (item) {
+        item.browsers.add(browser);
+      } else {
+        this._browserInfoByUrl.set(url, {
+          browser: new WeakSet([browser]),
+          info,
+        });
+      }
+    }
+  }
+
+  /**
+   * Stops tracking of a tab, for example the tab has loaded a different URL.
+   *
+   * @param {object} browser The browser associated with the tab to stop being
+   *                         tracked.
+   */
+  stopTrackingBrowser(browser) {
+    for (let [url, item] of this._browserInfoByUrl) {
+      item.browser.delete(browser);
+
+      if (!item.browser.length) {
+        this._browserInfoByUrl.delete(url);
+      }
+    }
+  }
+
+  // nsIWindowMediatorListener
+
+  /**
+   * This is called when a new window is opened, and handles registration of
+   * that window if it is a browser window.
+   *
+   * @param {nsIXULWindow} xulWin The xul window that was opened.
+   */
+  onOpenWindow(xulWin) {
+    let win = xulWin.docShell.domWindow;
+    win.addEventListener("load", () => {
+      if (win.document.documentElement.getAttribute("windowtype") != "navigator:browser") {
+        return;
+      }
+
+      this._registerWindow(win);
+    }, {once: true});
+  }
+
+  /**
+   * Listener that is called when a window is closed, and handles deregistration of
+   * that window if it is a browser window.
+   *
+   * @param {nsIXULWindow} xulWin The xul window that was closed.
+   */
+  onCloseWindow(xulWin) {
+    let win = xulWin.docShell.domWindow;
+
+    if (win.document.documentElement.getAttribute("windowtype") != "navigator:browser") {
+      return;
+    }
+
+    this._unregisterWindow(win);
+  }
+
+  /**
+   * Adds event listeners for the window and registers it with the content handler.
+   *
+   * @param {object} win The window to register.
+   */
+  _registerWindow(win) {
+    this._contentHandler.registerWindow(win);
+    win.gBrowser.tabContainer.addEventListener("TabClose", this);
+  }
+
+  /**
+   * Removes event listeners for the window and unregisters it with the content
+   * handler.
+   *
+   * @param {object} win The window to unregister.
+   */
+  _unregisterWindow(win) {
+    for (let tab of win.gBrowser.tabs) {
+      this.stopTrackingBrowser(tab);
+    }
+
+    this._contentHandler.unregisterWindow(win);
+    win.gBrowser.tabContainer.removeEventListener("TabClose", this);
+  }
+
+  /**
+   * Searches for provider information for a given url.
+   *
+   * @param {string} url The url to match for a provider.
+   * @param {boolean} useOnlyExtraAdServers If true, this will use the extra
+   *   ad server regexp to match instead of the main regexp.
+   * @returns {array|null} Returns an array of provider name and the provider information.
+   */
+  _getProviderInfoForUrl(url, useOnlyExtraAdServers = false) {
+    if (useOnlyExtraAdServers) {
+      return Object.entries(this._searchProviderInfo).find(
+        ([_, info]) => {
+          if (info.extraAdServersRegexps) {
+            for (let regexp of info.extraAdServersRegexps) {
+              if (regexp.test(url)) {
+                return true;
+              }
+            }
+          }
+          return false;
+        }
+      );
+    }
+
+    return Object.entries(this._searchProviderInfo).find(
+      ([_, info]) => info.regexp.test(url)
+    );
+  }
+
+  /**
+   * Checks to see if a url is a search partner location, and determines the
+   * provider and codes used.
+   *
+   * @param {string} url The url to match.
+   * @returns {null|object} Returns null if there is no match found. Otherwise,
+   *   returns an object of strings for provider, code and type.
+   */
+  _checkURLForSerpMatch(url) {
+    let info = this._getProviderInfoForUrl(url);
+    if (!info) {
+      return null;
+    }
+    let [provider, searchProviderInfo] = info;
     let queries = new URLSearchParams(url.split("#")[0].split("?")[1]);
     if (!queries.get(searchProviderInfo.queryParam)) {
-      return;
+      return null;
     }
     // Default to organic to simplify things.
     // We override type in the sap cases.
@@ -114,13 +350,29 @@ class TelemetryHandler {
         }
       }
     }
-
-    let payload = `${provider}.in-content:${type}:${code || "none"}`;
-    let histogram = Services.telemetry.getKeyedHistogramById(SEARCH_COUNTS_HISTOGRAM_KEY);
-    histogram.add(payload);
-    LOG("recordSearchURLTelemetry: " + payload);
+    return {provider, type, code};
   }
 
+  /**
+   * Logs telemetry for a search provider visit.
+   *
+   * @param {object} info
+   * @param {string} info.provider The name of the provider.
+   * @param {string} info.type The type of search.
+   * @param {string} [info.code] The code for the provider.
+   * @param {string} url The url that was matched (for debug logging only).
+   */
+  _reportSerpPage(info, url) {
+    let payload = `${info.provider}.in-content:${info.type}:${info.code || "none"}`;
+    let histogram = Services.telemetry.getKeyedHistogramById(SEARCH_COUNTS_HISTOGRAM_KEY);
+    histogram.add(payload);
+    LOG(`SearchTelemetry::recordSearchURLTelemetry: ${payload} for ${url}`);
+  }
+
+  /**
+   * Returns the current search provider information in use.
+   * @see SEARCH_PROVIDER_INFO
+   */
   get _searchProviderInfo() {
     if (!this.__searchProviderInfo) {
       this.__searchProviderInfo = SEARCH_PROVIDER_INFO;
@@ -130,12 +382,168 @@ class TelemetryHandler {
 }
 
 /**
- * Outputs aText to the JavaScript console as well as to stdout.
+ * ContentHandler deals with handling telemetry of the content within a tab -
+ * when ads detected and when they are selected.
+ *
+ * It handles the "browser.search.with_ads" and "browser.search.ad_clicks"
+ * scalars.
  */
-function LOG(aText) {
+class ContentHandler {
+  /**
+   * Constructor.
+   *
+   * @param {object} options
+   * @param {Map} options.browserInfoByUrl The  map of urls from TelemetryHandler.
+   * @param {function} options.getProviderInfoForUrl A function that obtains
+   *   the provider information for a url.
+   */
+  constructor(options) {
+    this._browserInfoByUrl = options.browserInfoByUrl;
+    this._getProviderInfoForUrl = options.getProviderInfoForUrl;
+  }
+
+  /**
+   * Initializes the content handler. This will also set up the shared data that is
+   * shared with the SearchTelemetryChild actor.
+   */
+  init() {
+    Services.ppmm.sharedData.set("SearchTelemetry:ProviderInfo", SEARCH_PROVIDER_INFO);
+
+    Cc["@mozilla.org/network/http-activity-distributor;1"]
+      .getService(Ci.nsIHttpActivityDistributor)
+      .addObserver(this);
+  }
+
+  /**
+   * Uninitializes the content handler.
+   */
+  uninit() {
+    Cc["@mozilla.org/network/http-activity-distributor;1"]
+      .getService(Ci.nsIHttpActivityDistributor)
+      .removeObserver(this);
+  }
+
+  /**
+   * Receives a message from the SearchTelemetryChild actor.
+   *
+   * @param {object} msg
+   */
+  receiveMessage(msg) {
+    if (msg.name != "SearchTelemetry:PageInfo") {
+      LOG(`"Received unexpected message: ${msg.name}`);
+      return;
+    }
+
+    this._reportPageWithAds(msg.data);
+  }
+
+  /**
+   * Test-only function to override the search provider information for use
+   * with tests. Passes it to the SearchTelemetryChild actor.
+   *
+   * @param {object} providerInfo @see SEARCH_PROVIDER_INFO for type information.
+   */
+  overrideSearchTelemetryForTests(providerInfo) {
+    Services.ppmm.sharedData.set("SearchTelemetry:ProviderInfo", providerInfo);
+  }
+
+  /**
+   * Listener that observes network activity, so that we can determine if a link
+   * from a search provider page was followed, and if then if that link was an
+   * ad click or not.
+   *
+   * @param {nsISupports} httpChannel The channel that generated the activity.
+   * @param {number} activityType The type of activity.
+   * @param {number} activitySubtype The subtype for the activity.
+   * @param {PRTime} timestamp The time of the activity.
+   * @param {number} [extraSizeData] Any size data available for the activity.
+   * @param {string} [extraStringData] Any extra string data available for the
+   *   activity.
+   */
+  observeActivity(httpChannel, activityType, activitySubtype, timestamp, extraSizeData, extraStringData) {
+    if (!this._browserInfoByUrl.size ||
+        activityType != Ci.nsIHttpActivityObserver.ACTIVITY_TYPE_HTTP_TRANSACTION ||
+        activitySubtype != Ci.nsIHttpActivityObserver.ACTIVITY_SUBTYPE_TRANSACTION_CLOSE) {
+      return;
+    }
+
+    let channel = httpChannel.QueryInterface(Ci.nsIHttpChannel);
+    let loadInfo;
+    try {
+      loadInfo = channel.loadInfo;
+    } catch (e) {
+      // Channels without a loadInfo are not pertinent.
+      return;
+    }
+
+    try {
+      let uri = channel.URI;
+      let triggerURI = loadInfo.triggeringPrincipal.URI;
+
+      if (!triggerURI || !this._browserInfoByUrl.has(triggerURI.spec)) {
+        return;
+      }
+
+      let info = this._getProviderInfoForUrl(uri.spec, true);
+      if (!info) {
+        return;
+      }
+
+      Services.telemetry.keyedScalarAdd(SEARCH_AD_CLICKS_SCALAR, info[0], 1);
+      LOG(`SearchTelemetry::recordSearchURLTelemetry: Counting ad click in page for ${info[0]} ${triggerURI.spec}`);
+    } catch (e) {
+      Cu.reportError(e);
+    }
+  }
+
+  /**
+   * Adds a message listener for the window being registered to receive messages
+   * from SearchTelemetryChild.
+   *
+   * @param {object} win The window to register.
+   */
+  registerWindow(win) {
+    win.messageManager.addMessageListener("SearchTelemetry:PageInfo", this);
+  }
+
+  /**
+   * Removes the message listener for the window.
+   *
+   * @param {object} win The window to unregister.
+   */
+  unregisterWindow(win) {
+    win.messageManager.removeMessageListener("SearchTelemetry:PageInfo", this);
+  }
+
+  /**
+   * Logs telemetry for a page with adverts, if it is one of the partner search
+   * provider pages that we're tracking.
+   *
+   * @param {object} info
+   * @param {boolean} info.hasAds Whether or not the page has adverts.
+   * @param {string} info.url The url of the page.
+   */
+  _reportPageWithAds(info) {
+    let item = this._browserInfoByUrl.get(info.url);
+    if (!item) {
+      LOG(`Expected to report URI with ads but couldn't find the information`);
+      return;
+    }
+
+    Services.telemetry.keyedScalarAdd(SEARCH_WITH_ADS_SCALAR, item.info.provider, 1);
+    LOG(`SearchTelemetry::recordSearchURLTelemetry: Counting ads in page for ${item.info.provider} ${info.url}`);
+  }
+}
+
+/**
+ * Outputs the message to the JavaScript console as well as to stdout.
+ *
+ * @param {string} msg The message to output.
+ */
+function LOG(msg) {
   if (loggingEnabled) {
-    dump(`*** SearchTelemetry: ${aText}\n"`);
-    Services.console.logStringMessage(aText);
+    dump(`*** SearchTelemetry: ${msg}\n"`);
+    Services.console.logStringMessage(msg);
   }
 }
 

browser/components/search/test/browser/browser.ini

@@ -46,5 +46,9 @@ skip-if = os == "linux" # Linux has different focus behaviours.
 [browser_searchbar_smallpanel_keyboard_navigation.js]
 [browser_searchEngine_behaviors.js]
 skip-if = artifact # bug 1315953
+[browser_searchTelemetry.js]
+support-files =
+  searchTelemetry.html
+  searchTelemetryAd.html
 [browser_webapi.js]
 [browser_tooManyEnginesOffered.js]

browser/components/search/test/browser/browser_searchTelemetry.js

@@ -0,0 +1,221 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/*
+ * Main tests for SearchTelemetry - general engine visiting and link clicking.
+ */
+
+"use strict";
+
+const {SearchTelemetry} = ChromeUtils.import("resource:///modules/SearchTelemetry.jsm", null);
+
+const TEST_PROVIDER_INFO = {
+  "example": {
+    "regexp": /^http:\/\/mochi.test:.+\/browser\/browser\/components\/search\/test\/browser\/searchTelemetry(?:Ad)?.html/,
+    "queryParam": "s",
+    "codeParam": "abc",
+    "codePrefixes": ["ff"],
+    "followonParams": ["a"],
+    "extraAdServersRegexps": [/^https:\/\/example\.com\/ad2?/],
+  },
+};
+
+const SEARCH_AD_CLICK_SCALARS = [
+  "browser.search.with_ads",
+  "browser.search.ad_clicks",
+];
+
+function getPageUrl(useExample = false, useAdPage = false) {
+  let server = useExample ? "example.com" : "mochi.test:8888";
+  let page = useAdPage ? "searchTelemetryAd.html" : "searchTelemetry.html";
+  return `http://${server}/browser/browser/components/search/test/browser/${page}`;
+}
+
+function getSERPUrl(page) {
+  return page + "?s=test&abc=ff";
+}
+
+function getSERPFollowOnUrl(page) {
+  return page + "?s=test&abc=ff&a=foo";
+}
+
+const searchCounts = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS");
+
+async function assertTelemetry(expectedHistograms, expectedScalars) {
+  let histSnapshot = {};
+  let scalars = {};
+
+  await TestUtils.waitForCondition(() => {
+    histSnapshot = searchCounts.snapshot();
+    return Object.getOwnPropertyNames(histSnapshot).length ==
+      Object.getOwnPropertyNames(expectedHistograms).length;
+  });
+
+  if (Object.entries(expectedScalars).length > 0) {
+    await TestUtils.waitForCondition(() => {
+      scalars = Services.telemetry.getSnapshotForKeyedScalars(
+        "main", false).parent || {};
+      return Object.getOwnPropertyNames(expectedScalars)[0] in
+        scalars;
+    });
+  }
+
+  Assert.equal(Object.getOwnPropertyNames(histSnapshot).length,
+    Object.getOwnPropertyNames(expectedHistograms).length,
+    "Should only have one key");
+
+  for (let [key, value] of Object.entries(expectedHistograms)) {
+    Assert.ok(key in histSnapshot,
+      `Histogram should have the expected key: ${key}`);
+    Assert.equal(histSnapshot[key].sum, value,
+      `Should have counted the correct number of visits for ${key}`);
+  }
+
+  for (let [name, value] of Object.entries(expectedScalars)) {
+    Assert.ok(name in scalars,
+      `Scalar ${name} should have been added.`);
+    Assert.deepEqual(scalars[name], value,
+      `Should have counted the correct number of visits for ${name}`);
+  }
+
+  for (let name of SEARCH_AD_CLICK_SCALARS) {
+    Assert.equal(name in scalars, name in expectedScalars,
+      `Should have matched ${name} in scalars and expectedScalars`);
+  }
+}
+
+add_task(async function setup() {
+  SearchTelemetry.overrideSearchTelemetryForTests(TEST_PROVIDER_INFO);
+  // Enable local telemetry recording for the duration of the tests.
+  let oldCanRecord = Services.telemetry.canRecordExtended;
+  Services.telemetry.canRecordExtended = true;
+  Services.prefs.setBoolPref("browser.search.log", true);
+
+  registerCleanupFunction(async () => {
+    SearchTelemetry.overrideSearchTelemetryForTests();
+    Services.telemetry.canRecordExtended = oldCanRecord;
+    Services.telemetry.clearScalars();
+  });
+});
+
+add_task(async function test_simple_search_page_visit() {
+  searchCounts.clear();
+
+  await BrowserTestUtils.withNewTab({
+    gBrowser,
+    url: getSERPUrl(getPageUrl()),
+  }, async () => {
+    await assertTelemetry({"example.in-content:sap:ff": 1}, {});
+  });
+});
+
+add_task(async function test_follow_on_visit() {
+  await BrowserTestUtils.withNewTab({
+    gBrowser,
+    url: getSERPFollowOnUrl(getPageUrl()),
+  }, async () => {
+    await assertTelemetry({
+      "example.in-content:sap:ff": 1,
+      "example.in-content:sap-follow-on:ff": 1,
+    }, {});
+  });
+});
+
+add_task(async function test_track_ad() {
+  searchCounts.clear();
+
+  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser,
+    getSERPUrl(getPageUrl(false, true)));
+
+  await assertTelemetry({"example.in-content:sap:ff": 1}, {
+    "browser.search.with_ads": {"example": 1},
+  });
+
+  BrowserTestUtils.removeTab(tab);
+});
+
+add_task(async function test_track_ad_new_window() {
+  searchCounts.clear();
+  Services.telemetry.clearScalars();
+
+  let win = await BrowserTestUtils.openNewBrowserWindow();
+
+  let url = getSERPUrl(getPageUrl(false, true));
+  await BrowserTestUtils.loadURI(win.gBrowser.selectedBrowser, url);
+  await BrowserTestUtils.browserLoaded(win.gBrowser.selectedBrowser, false, url);
+
+  await assertTelemetry({"example.in-content:sap:ff": 1}, {
+    "browser.search.with_ads": {"example": 1},
+  });
+
+  await BrowserTestUtils.closeWindow(win);
+});
+
+add_task(async function test_track_ad_pages_without_ads() {
+  // Note: the above tests have already checked a page with no ad-urls.
+  searchCounts.clear();
+  Services.telemetry.clearScalars();
+
+  let tabs = [];
+
+  tabs.push(await BrowserTestUtils.openNewForegroundTab(gBrowser,
+    getSERPUrl(getPageUrl(false, false))));
+  tabs.push(await BrowserTestUtils.openNewForegroundTab(gBrowser,
+    getSERPUrl(getPageUrl(false, true))));
+
+  await assertTelemetry({"example.in-content:sap:ff": 2}, {
+    "browser.search.with_ads": {"example": 1},
+  });
+
+  for (let tab of tabs) {
+    BrowserTestUtils.removeTab(tab);
+  }
+});
+
+add_task(async function test_track_ad_click() {
+  // Note: the above tests have already checked a page with no ad-urls.
+  searchCounts.clear();
+  Services.telemetry.clearScalars();
+
+  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser,
+    getSERPUrl(getPageUrl(false, true)));
+
+  await assertTelemetry({"example.in-content:sap:ff": 1}, {
+    "browser.search.with_ads": {"example": 1},
+  });
+
+  let pageLoadPromise = BrowserTestUtils.waitForLocationChange(gBrowser);
+  await ContentTask.spawn(tab.linkedBrowser, {}, () => {
+    content.document.getElementById("ad1").click();
+  });
+  await pageLoadPromise;
+
+  await assertTelemetry({"example.in-content:sap:ff": 1}, {
+    "browser.search.with_ads": {"example": 1},
+    "browser.search.ad_clicks": {"example": 1},
+  });
+
+  // Now go back, and click again.
+  pageLoadPromise = BrowserTestUtils.waitForLocationChange(gBrowser);
+  gBrowser.goBack();
+  await pageLoadPromise;
+
+  // We've gone back, so we register an extra display & if it is with ads or not.
+  await assertTelemetry({"example.in-content:sap:ff": 2}, {
+    "browser.search.with_ads": {"example": 2},
+    "browser.search.ad_clicks": {"example": 1},
+  });
+
+  pageLoadPromise = BrowserTestUtils.waitForLocationChange(gBrowser);
+  await ContentTask.spawn(tab.linkedBrowser, {}, () => {
+    content.document.getElementById("ad1").click();
+  });
+  await pageLoadPromise;
+
+  await assertTelemetry({"example.in-content:sap:ff": 2}, {
+    "browser.search.with_ads": {"example": 2},
+    "browser.search.ad_clicks": {"example": 2},
+  });
+
+  BrowserTestUtils.removeTab(tab);
+});

browser/components/search/test/browser/searchTelemetry.html

@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+  <a href="https://example.com/otherpage">Non ad link</a>
+  <a href="https://example1.com/ad">Matching path prefix, different server</a>
+  <a href="https://mochi.test:8888/otherpage">Non ad link</a>
+</body>
+</html>

browser/components/search/test/browser/searchTelemetryAd.html

@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+  <a id="ad1" href="https://example.com/ad">Ad link</a>
+  <a id="ad2" href="https://example.com/ad2">Second Ad link</a>
+</body>
+</html>

browser/components/search/test/unit/test_urlTelemetry.js

@@ -7,79 +7,79 @@ ChromeUtils.import("resource:///modules/SearchTelemetry.jsm");
 add_task(async function test_parsing_search_urls() {
   let hs;
   // Google search access point.
-  SearchTelemetry.recordSearchURLTelemetry("https://www.google.com/search?q=test&ie=utf-8&oe=utf-8&client=firefox-b-1-ab");
+  SearchTelemetry.updateTrackingStatus({}, "https://www.google.com/search?q=test&ie=utf-8&oe=utf-8&client=firefox-b-1-ab");
   hs = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS").snapshot();
   Assert.ok(hs);
   Assert.ok("google.in-content:sap:firefox-b-1-ab" in hs, "The histogram must contain the correct key");
 
   // Google search access point follow-on.
-  SearchTelemetry.recordSearchURLTelemetry("https://www.google.com/search?client=firefox-b-1-ab&ei=EI_VALUE&q=test2&oq=test2&gs_l=GS_L_VALUE");
+  SearchTelemetry.updateTrackingStatus({}, "https://www.google.com/search?client=firefox-b-1-ab&ei=EI_VALUE&q=test2&oq=test2&gs_l=GS_L_VALUE");
   hs = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS").snapshot();
   Assert.ok(hs);
   Assert.ok("google.in-content:sap-follow-on:firefox-b-1-ab" in hs, "The histogram must contain the correct key");
 
   // Google organic.
-  SearchTelemetry.recordSearchURLTelemetry("https://www.google.com/search?source=hp&ei=EI_VALUE&q=test&oq=test&gs_l=GS_L_VALUE");
+  SearchTelemetry.updateTrackingStatus({}, "https://www.google.com/search?source=hp&ei=EI_VALUE&q=test&oq=test&gs_l=GS_L_VALUE");
   hs = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS").snapshot();
   Assert.ok(hs);
   Assert.ok("google.in-content:organic:none" in hs, "The histogram must contain the correct key");
 
   // Google organic UK.
-  SearchTelemetry.recordSearchURLTelemetry("https://www.google.co.uk/search?source=hp&ei=EI_VALUE&q=test&oq=test&gs_l=GS_L_VALUE");
+  SearchTelemetry.updateTrackingStatus({}, "https://www.google.co.uk/search?source=hp&ei=EI_VALUE&q=test&oq=test&gs_l=GS_L_VALUE");
   hs = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS").snapshot();
   Assert.ok(hs);
   Assert.ok("google.in-content:organic:none" in hs, "The histogram must contain the correct key");
 
   // Yahoo organic.
-  SearchTelemetry.recordSearchURLTelemetry("https://search.yahoo.com/search?p=test&fr=yfp-t&fp=1&toggle=1&cop=mss&ei=UTF-8");
+  SearchTelemetry.updateTrackingStatus({}, "https://search.yahoo.com/search?p=test&fr=yfp-t&fp=1&toggle=1&cop=mss&ei=UTF-8");
   hs = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS").snapshot();
   Assert.ok(hs);
   Assert.ok("yahoo.in-content:organic:none" in hs, "The histogram must contain the correct key");
 
   // Yahoo organic UK.
-  SearchTelemetry.recordSearchURLTelemetry("https://uk.search.yahoo.com/search?p=test&fr=yfp-t&fp=1&toggle=1&cop=mss&ei=UTF-8");
+  SearchTelemetry.updateTrackingStatus({}, "https://uk.search.yahoo.com/search?p=test&fr=yfp-t&fp=1&toggle=1&cop=mss&ei=UTF-8");
   hs = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS").snapshot();
   Assert.ok(hs);
   Assert.ok("yahoo.in-content:organic:none" in hs, "The histogram must contain the correct key");
 
   // Bing search access point.
-  SearchTelemetry.recordSearchURLTelemetry("https://www.bing.com/search?q=test&pc=MOZI&form=MOZLBR");
+  SearchTelemetry.updateTrackingStatus({}, "https://www.bing.com/search?q=test&pc=MOZI&form=MOZLBR");
   hs = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS").snapshot();
   Assert.ok(hs);
   Assert.ok("bing.in-content:sap:MOZI" in hs, "The histogram must contain the correct key");
 
   // Bing organic.
-  SearchTelemetry.recordSearchURLTelemetry("https://www.bing.com/search?q=test&qs=n&form=QBLH&sp=-1&pq=&sc=0-0&sk=&cvid=CVID_VALUE");
+  SearchTelemetry.updateTrackingStatus({}, "https://www.bing.com/search?q=test&qs=n&form=QBLH&sp=-1&pq=&sc=0-0&sk=&cvid=CVID_VALUE");
   hs = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS").snapshot();
   Assert.ok(hs);
   Assert.ok("bing.in-content:organic:none" in hs, "The histogram must contain the correct key");
 
   // DuckDuckGo search access point.
-  SearchTelemetry.recordSearchURLTelemetry("https://duckduckgo.com/?q=test&t=ffab");
+  SearchTelemetry.updateTrackingStatus({}, "https://duckduckgo.com/?q=test&t=ffab");
   hs = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS").snapshot();
   Assert.ok(hs);
   Assert.ok("duckduckgo.in-content:sap:ffab" in hs, "The histogram must contain the correct key");
 
   // DuckDuckGo organic.
-  SearchTelemetry.recordSearchURLTelemetry("https://duckduckgo.com/?q=test&t=hi&ia=news");
+  SearchTelemetry.updateTrackingStatus({}, "https://duckduckgo.com/?q=test&t=hi&ia=news");
   hs = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS").snapshot();
   Assert.ok(hs);
   Assert.ok("duckduckgo.in-content:organic:hi" in hs, "The histogram must contain the correct key");
 
   // Baidu search access point.
-  SearchTelemetry.recordSearchURLTelemetry("https://www.baidu.com/baidu?wd=test&tn=monline_dg&ie=utf-8");
+  SearchTelemetry.updateTrackingStatus({}, "https://www.baidu.com/baidu?wd=test&tn=monline_dg&ie=utf-8");
   hs = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS").snapshot();
   Assert.ok(hs);
   Assert.ok("baidu.in-content:sap:monline_dg" in hs, "The histogram must contain the correct key");
 
   // Baidu search access point follow-on.
-  SearchTelemetry.recordSearchURLTelemetry("https://www.baidu.com/s?ie=utf-8&f=8&rsv_bp=1&tn=monline_dg&wd=test2&oq=test&rsv_pq=RSV_PQ_VALUE&rsv_t=RSV_T_VALUE&rqlang=cn&rsv_enter=1&rsv_sug3=2&rsv_sug2=0&inputT=227&rsv_sug4=397");
+  SearchTelemetry.updateTrackingStatus({}, "https://www.baidu.com/s?ie=utf-8&f=8&rsv_bp=1&tn=monline_dg&wd=test2&oq=test&rsv_pq=RSV_PQ_VALUE&rsv_t=RSV_T_VALUE&rqlang=cn&rsv_enter=1&rsv_sug3=2&rsv_sug2=0&inputT=227&rsv_sug4=397");
   hs = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS").snapshot();
   Assert.ok(hs);
   Assert.ok("baidu.in-content:sap-follow-on:monline_dg" in hs, "The histogram must contain the correct key");
 
   // Baidu organic.
-  SearchTelemetry.recordSearchURLTelemetry("https://www.baidu.com/s?ie=utf-8&f=8&rsv_bp=1&rsv_idx=1&ch=&tn=baidu&bar=&wd=test&rn=&oq=&rsv_pq=RSV_PQ_VALUE&rsv_t=RSV_T_VALUE&rqlang=cn");
+  SearchTelemetry.updateTrackingStatus({}, "https://www.baidu.com/s?ie=utf-8&f=8&rsv_bp=1&rsv_idx=1&ch=&tn=baidu&bar=&wd=test&rn=&oq=&rsv_pq=RSV_PQ_VALUE&rsv_t=RSV_T_VALUE&rqlang=cn");
   hs = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS").snapshot();
   Assert.ok(hs);
   Assert.ok("baidu.in-content:organic:baidu" in hs, "The histogram must contain the correct key");

browser/modules/BrowserUsageTelemetry.jsm

@@ -154,6 +154,9 @@ let URICountListener = {
   },
 
   onLocationChange(browser, webProgress, request, uri, flags) {
+    // By default, assume we no longer need to track this tab.
+    SearchTelemetry.stopTrackingBrowser(browser);
+
     // Don't count this URI if it's an error page.
     if (flags & Ci.nsIWebProgressListener.LOCATION_CHANGE_ERROR_PAGE) {
       return;
@@ -219,7 +222,7 @@ let URICountListener = {
 
     if (shouldRecordSearchCount(browser.getTabBrowser()) &&
         !(flags & Ci.nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT)) {
-      SearchTelemetry.recordSearchURLTelemetry(uriSpec);
+      SearchTelemetry.updateTrackingStatus(browser, uriSpec);
     }
 
     if (!shouldCountURI) {

build/clang-plugin/LoadLibraryUsageChecker.cpp

@@ -26,7 +26,7 @@ void LoadLibraryUsageChecker::check(const MatchFinder::MatchResult &Result) {
   const CallExpr *FuncCall = Result.Nodes.getNodeAs<CallExpr>("funcCall");
 
   if (FuncCall) {
-    diag(FuncCall->getLocStart(),
+    diag(FuncCall->getBeginLoc(),
          "Usage of ASCII file functions (such as %0) is forbidden.",
          DiagnosticIDs::Error)
         << FuncCall->getDirectCallee()->getName();

devtools/client/aboutdebugging-new/aboutdebugging.js

@@ -28,8 +28,6 @@ const {
 } = require("./src/modules/network-locations");
 const {
   addUSBRuntimesObserver,
-  disableUSBRuntimes,
-  enableUSBRuntimes,
   getUSBRuntimes,
   removeUSBRuntimesObserver,
 } = require("./src/modules/usb-runtimes");
@@ -76,8 +74,10 @@ const AboutDebugging = {
     this.actions.updateNetworkLocations(getNetworkLocations());
 
     addNetworkLocationsObserver(this.onNetworkLocationsUpdated);
+
+    // Listen to USB runtime updates and retrieve the initial list of runtimes.
     addUSBRuntimesObserver(this.onUSBRuntimesUpdated);
-    await enableUSBRuntimes();
+    getUSBRuntimes();
 
     adbAddon.on("update", this.onAdbAddonUpdated);
     this.onAdbAddonUpdated();
@@ -113,7 +113,6 @@ const AboutDebugging = {
 
     removeNetworkLocationsObserver(this.onNetworkLocationsUpdated);
     removeUSBRuntimesObserver(this.onUSBRuntimesUpdated);
-    disableUSBRuntimes();
     adbAddon.off("update", this.onAdbAddonUpdated);
     setDebugTargetCollapsibilities(state.ui.debugTargetCollapsibilities);
     unmountComponentAtNode(this.mount);

devtools/client/aboutdebugging-new/src/modules/runtime-client-factory.js

@@ -4,7 +4,7 @@
 
 "use strict";
 
-const { ADB } = require("devtools/shared/adb/adb");
+const { prepareTCPConnection } = require("devtools/shared/adb/commands/index");
 const { DebuggerClient } = require("devtools/shared/client/debugger-client");
 const { DebuggerServer } = require("devtools/server/main");
 const { ClientWrapper } = require("./client-wrapper");
@@ -29,7 +29,7 @@ async function createNetworkClient(host, port) {
 }
 
 async function createUSBClient(socketPath) {
-  const port = await ADB.prepareTCPConnection(socketPath);
+  const port = await prepareTCPConnection(socketPath);
   return createNetworkClient("localhost", port);
 }
 

devtools/client/aboutdebugging-new/src/modules/usb-runtimes.js

@@ -4,42 +4,29 @@
 
 "use strict";
 
-loader.lazyGetter(this, "adbScanner", () => {
-  const { AddonAwareADBScanner } = require("devtools/shared/adb/addon-aware-adb-scanner");
-  return new AddonAwareADBScanner();
-});
+loader.lazyRequireGetter(this, "adb", "devtools/shared/adb/adb", true);
 
 /**
  * This module provides a collection of helper methods to detect USB runtimes whom Firefox
  * is running on.
  */
 function addUSBRuntimesObserver(listener) {
-  adbScanner.on("runtime-list-updated", listener);
+  adb.registerListener(listener);
 }
 exports.addUSBRuntimesObserver = addUSBRuntimesObserver;
 
-function disableUSBRuntimes() {
-  adbScanner.disable();
-}
-exports.disableUSBRuntimes = disableUSBRuntimes;
-
-async function enableUSBRuntimes() {
-  adbScanner.enable();
-}
-exports.enableUSBRuntimes = enableUSBRuntimes;
-
 function getUSBRuntimes() {
-  return adbScanner.listRuntimes();
+  return adb.getRuntimes();
 }
 exports.getUSBRuntimes = getUSBRuntimes;
 
 function removeUSBRuntimesObserver(listener) {
-  adbScanner.off("runtime-list-updated", listener);
+  adb.unregisterListener(listener);
 }
 exports.removeUSBRuntimesObserver = removeUSBRuntimesObserver;
 
 function refreshUSBRuntimes() {
-  return adbScanner.scan();
+  return adb.updateRuntimes();
 }
 exports.refreshUSBRuntimes = refreshUSBRuntimes;
 

devtools/client/aboutdebugging-new/test/browser/browser.ini

@@ -39,6 +39,8 @@ skip-if = (os == 'linux' && bits == 32) # ADB start() fails on linux 32, see Bug
 [browser_aboutdebugging_sidebar_usb_runtime_refresh.js]
 [browser_aboutdebugging_sidebar_usb_status.js]
 skip-if = (os == 'linux' && bits == 32) # ADB start() fails on linux 32, see Bug 1499638
+[browser_aboutdebugging_stop_adb.js]
+skip-if = (os == 'linux' && bits == 32) # ADB start() fails on linux 32, see Bug 1499638
 [browser_aboutdebugging_system_addons.js]
 [browser_aboutdebugging_tab_favicons.js]
 [browser_aboutdebugging_thisfirefox.js]

devtools/client/aboutdebugging-new/test/browser/browser_aboutdebugging_connect_toggle_usb_devices.js

@@ -4,7 +4,7 @@
 "use strict";
 
 const { AddonManager } = require("resource://gre/modules/AddonManager.jsm");
-const { ADB } = require("devtools/shared/adb/adb");
+const { adbProcess } = require("devtools/shared/adb/adb-process");
 
 /**
  * Check that USB Devices scanning can be enabled and disabled from the connect page.
@@ -48,7 +48,7 @@ add_task(async function() {
   // fail and we will have an unhandled promise rejection.
   // See Bug 1498469.
   info("Wait until ADB has started.");
-  await waitUntil(() => ADB.ready);
+  await waitUntil(() => adbProcess.ready);
 
   info("Click on the toggle button");
   usbToggleButton.click();

devtools/client/aboutdebugging-new/test/browser/browser_aboutdebugging_sidebar_usb_status.js

@@ -4,7 +4,7 @@
 "use strict";
 
 const { adbAddon } = require("devtools/shared/adb/adb-addon");
-const { ADB } = require("devtools/shared/adb/adb");
+const { adbProcess } = require("devtools/shared/adb/adb-process");
 
 /**
  * This test asserts that the sidebar shows a message describing the status of the USB
@@ -31,7 +31,7 @@ add_task(async function() {
   // fail and we will have an unhandled promise rejection.
   // See Bug 1498469.
   info("Wait until ADB has started.");
-  await waitUntil(() => ADB.ready);
+  await waitUntil(() => adbProcess.ready);
 
   info("Uninstall the adb extension and wait for the message to udpate");
   adbAddon.uninstall();

devtools/client/aboutdebugging-new/test/browser/browser_aboutdebugging_stop_adb.js

@@ -0,0 +1,79 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const { adbAddon } = require("devtools/shared/adb/adb-addon");
+const { adbProcess } = require("devtools/shared/adb/adb-process");
+const { check } = require("devtools/shared/adb/adb-running-checker");
+
+/**
+ * Check that ADB is stopped:
+ * - when the adb extension is uninstalled
+ * - when no consumer is registered
+ */
+add_task(async function() {
+  await pushPref("devtools.remote.adb.extensionURL",
+                 CHROME_URL_ROOT + "resources/test-adb-extension/adb-extension-#OS#.xpi");
+
+  info("Check if ADB is already running before the test starts");
+  const isAdbAlreadyRunning = await check();
+  if (isAdbAlreadyRunning) {
+    ok(false, "The ADB process is already running on this machine, it should be " +
+      "stopped before running this test");
+    return;
+  }
+
+  const { tab } = await openAboutDebugging();
+
+  info("Install the adb extension and wait for ADB to start");
+  // Use "internal" as the install source to avoid triggering telemetry.
+  adbAddon.install("internal");
+  await waitForAdbStart();
+
+  info("Open a second about:debugging");
+  const { tab: secondTab } = await openAboutDebugging();
+
+  info("Close the second about:debugging and check that ADB is still running");
+  await removeTab(secondTab);
+  ok(await check(), "ADB is still running");
+
+  await removeTab(tab);
+
+  info("Check that the adb process stops after closing about:debugging");
+  await waitForAdbStop();
+
+  info("Open a third about:debugging, wait for the ADB to start again");
+  const { tab: thirdTab } = await openAboutDebugging();
+  await waitForAdbStart();
+
+  info("Uninstall the addon, this should stop ADB as well");
+  adbAddon.uninstall();
+  await waitForAdbStop();
+
+  info("Reinstall the addon, this should start ADB again");
+  adbAddon.install("internal");
+  await waitForAdbStart();
+
+  info("Close the last tab, this should stop ADB");
+  await removeTab(thirdTab);
+  await waitForAdbStop();
+});
+
+async function waitForAdbStart() {
+  info("Wait for ADB to start");
+  return asyncWaitUntil(async () => {
+    const isProcessReady = adbProcess.ready;
+    const isRunning = await check();
+    return isProcessReady && isRunning;
+  });
+}
+
+async function waitForAdbStop() {
+  info("Wait for ADB to stop");
+  return asyncWaitUntil(async () => {
+    const isProcessReady = adbProcess.ready;
+    const isRunning = await check();
+    return !isProcessReady && !isRunning;
+  });
+}

devtools/client/aboutdebugging-new/test/browser/head.js

@@ -30,8 +30,8 @@ registerCleanupFunction(async function() {
   } catch (e) {
     // Will throw if the addon is already uninstalled, ignore exceptions here.
   }
-  const { ADB } = require("devtools/shared/adb/adb");
-  await ADB.kill();
+  const { adbProcess } = require("devtools/shared/adb/adb-process");
+  await adbProcess.kill();
 
   const { remoteClientManager } =
     require("devtools/client/shared/remote-debugging/remote-client-manager");

devtools/client/webide/modules/runtimes.js

@@ -11,10 +11,7 @@ const EventEmitter = require("devtools/shared/event-emitter");
 const {RuntimeTypes} = require("devtools/client/webide/modules/runtime-types");
 const promise = require("promise");
 
-loader.lazyGetter(this, "adbScanner", () => {
-  const { AddonAwareADBScanner } = require("devtools/shared/adb/addon-aware-adb-scanner");
-  return new AddonAwareADBScanner();
-});
+loader.lazyRequireGetter(this, "adb", "devtools/shared/adb/adb", true);
 
 loader.lazyRequireGetter(this, "AuthenticationResult",
   "devtools/shared/security/auth", true);
@@ -198,9 +195,29 @@ exports.RuntimeScanners = RuntimeScanners;
 
 /* SCANNERS */
 
-// The adb-scanner will automatically start and stop when the ADB extension is installed
-// and uninstalled, so the scanner itself can always be used.
-RuntimeScanners.add(adbScanner);
+var UsbScanner = {
+  init() {
+    this._emitUpdated = this._emitUpdated.bind(this);
+  },
+  enable() {
+    adb.registerListener(this._emitUpdated);
+  },
+  disable() {
+    adb.unregisterListener(this._emitUpdated);
+  },
+  scan() {
+    return adb.updateRuntimes();
+  },
+  listRuntimes() {
+    return adb.getRuntimes();
+  },
+  _emitUpdated() {
+    this.emit("runtime-list-updated");
+  },
+};
+EventEmitter.decorate(UsbScanner);
+UsbScanner.init();
+RuntimeScanners.add(UsbScanner);
 
 var WiFiScanner = {
 

devtools/shared/adb/adb-device.js

@@ -4,7 +4,7 @@
 
 "use strict";
 
-const { ADB } = require("devtools/shared/adb/adb");
+const { shell } = require("devtools/shared/adb/commands/index");
 
 /**
  * A Device instance is created and registered with the Devices module whenever
@@ -19,7 +19,7 @@ class AdbDevice {
     if (this._model) {
       return this._model;
     }
-    const model = await ADB.shell("getprop ro.product.model");
+    const model = await shell("getprop ro.product.model");
     this._model = model.trim();
     return this._model;
   }
@@ -33,7 +33,7 @@ class AdbDevice {
     // 00000000: 00000002 00000000 00010000 0001 01 6551588
     //  /data/data/org.mozilla.fennec/firefox-debugger-socket
     const query = "cat /proc/net/unix";
-    const rawSocketInfo = await ADB.shell(query);
+    const rawSocketInfo = await shell(query);
 
     // Filter to lines with "firefox-debugger-socket"
     let socketInfos = rawSocketInfo.split(/\r?\n/);

devtools/shared/adb/adb-devices-registry.js

@@ -1,80 +0,0 @@
-/* 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/. */
-
-"use strict";
-
-const EventEmitter = require("devtools/shared/event-emitter");
-const { adbAddon, ADB_ADDON_STATES } = require("devtools/shared/adb/adb-addon");
-
-/**
- * Shared registry that will hold all the detected devices from ADB.
- * Extends EventEmitter and emits the following events:
- * - "register": a new device has been registered
- * - "unregister": a device has been unregistered
- */
-class AdbDevicesRegistry extends EventEmitter {
-  constructor() {
-    super();
-
-    // Internal object to store the discovered adb devices.
-    this._devices = {};
-
-    // When the addon is uninstalled, the repository should be emptied.
-    // TODO: This should also be done when ADB is stopped.
-    this._onAdbAddonUpdate = this._onAdbAddonUpdate.bind(this);
-    adbAddon.on("update", this._onAdbAddonUpdate);
-  }
-
-  /**
-   * Register a device (Device class defined in from adb-device.js) for the provided name.
-   *
-   * @param {String} name
-   *        Name of the device.
-   * @param {AdbDevice} device
-   *        The device to register.
-   */
-  register(name, device) {
-    this._devices[name] = device;
-    this.emit("register");
-  }
-
-  /**
-   * Unregister a device previously registered under the provided name.
-   *
-   * @param {String} name
-   *        Name of the device.
-   */
-  unregister(name) {
-    delete this._devices[name];
-    this.emit("unregister");
-  }
-
-  /**
-   * Returns an iterable containing the name of all the available devices, sorted by name.
-   */
-  available() {
-    return Object.keys(this._devices).sort();
-  }
-
-  /**
-   * Returns a device previously registered under the provided name.
-   *
-   * @param {String} name
-   *        Name of the device.
-   */
-  getByName(name) {
-    return this._devices[name];
-  }
-
-  _onAdbAddonUpdate() {
-    const installed = adbAddon.status === ADB_ADDON_STATES.INSTALLED;
-    if (!installed) {
-      for (const name in this._devices) {
-        this.unregister(name);
-      }
-    }
-  }
-}
-
-exports.adbDevicesRegistry = new AdbDevicesRegistry();

devtools/shared/adb/adb-process.js

@@ -0,0 +1,137 @@
+/* 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/. */
+
+"use strict";
+
+const { Cc, Ci } = require("chrome");
+const { dumpn } = require("devtools/shared/DevToolsUtils");
+const { getFileForBinary } = require("./adb-binary");
+const { setTimeout } = require("resource://gre/modules/Timer.jsm");
+const { Services } = require("resource://gre/modules/Services.jsm");
+
+loader.lazyRequireGetter(this, "runCommand", "devtools/shared/adb/commands/index", true);
+loader.lazyRequireGetter(this, "check", "devtools/shared/adb/adb-running-checker", true);
+
+// Waits until a predicate returns true or re-tries the predicate calls
+// |retry| times, we wait for 100ms between each calls.
+async function waitUntil(predicate, retry = 20) {
+  let count = 0;
+  while (count++ < retry) {
+    if (await predicate()) {
+      return true;
+    }
+    // Wait for 100 milliseconds.
+    await new Promise(resolve => setTimeout(resolve, 100));
+  }
+  // Timed out after trying too many times.
+  return false;
+}
+
+// Class representing the ADB process.
+class AdbProcess {
+  constructor() {
+    this._ready = false;
+    this._didRunInitially = false;
+  }
+
+  get ready() {
+    return this._ready;
+  }
+
+  _getAdbFile() {
+    if (this._adbFilePromise) {
+      return this._adbFilePromise;
+    }
+    this._adbFilePromise = getFileForBinary();
+    return this._adbFilePromise;
+  }
+
+  async _runProcess(process, params) {
+    return new Promise((resolve, reject) => {
+      process.runAsync(params, params.length, {
+        observe(subject, topic, data) {
+          switch (topic) {
+            case "process-finished":
+              resolve();
+              break;
+            case "process-failed":
+              reject();
+              break;
+          }
+        },
+      }, false);
+    });
+  }
+
+  // We startup by launching adb in server mode, and setting
+  // the tcp socket preference to |true|
+  async start() {
+    return new Promise(async (resolve, reject) => {
+      const onSuccessfulStart = () => {
+        Services.obs.notifyObservers(null, "adb-ready");
+        this._ready = true;
+        resolve();
+      };
+
+      const isAdbRunning = await check();
+      if (isAdbRunning) {
+        dumpn("Found ADB process running, not restarting");
+        onSuccessfulStart();
+        return;
+      }
+      dumpn("Didn't find ADB process running, restarting");
+
+      this._didRunInitially = true;
+      const process = Cc["@mozilla.org/process/util;1"].createInstance(Ci.nsIProcess);
+
+      // FIXME: Bug 1481691 - We should avoid extracting files every time.
+      const adbFile = await this._getAdbFile();
+      process.init(adbFile);
+      // Hide command prompt window on Windows
+      process.startHidden = true;
+      process.noShell = true;
+      const params = ["start-server"];
+      let isStarted = false;
+      try {
+        await this._runProcess(process, params);
+        isStarted = await waitUntil(check);
+      } catch (e) {
+      }
+
+      if (isStarted) {
+        onSuccessfulStart();
+      } else {
+        this._ready = false;
+        reject();
+      }
+    });
+  }
+
+  /**
+   * Stop the ADB server, but only if we started it.  If it was started before
+   * us, we return immediately.
+   */
+  async stop() {
+    if (!this._didRunInitially) {
+      return; // We didn't start the server, nothing to do
+    }
+    await this.kill();
+  }
+
+  /**
+   * Kill the ADB server.
+   */
+  async kill() {
+    try {
+      await runCommand("host:kill");
+    } catch (e) {
+      dumpn("Failed to send host:kill command");
+    }
+    dumpn("adb server was terminated by host:kill");
+    this._ready = false;
+    this._didRunInitially = false;
+  }
+}
+
+exports.adbProcess = new AdbProcess();

devtools/shared/adb/adb-runtime.js

@@ -5,7 +5,7 @@
 "use strict";
 
 const { RuntimeTypes } = require("devtools/client/webide/modules/runtime-types");
-const { ADB } = require("devtools/shared/adb/adb");
+const { prepareTCPConnection } = require("devtools/shared/adb/commands/index");
 
 class AdbRuntime {
   constructor(adbDevice, model, socketPath) {
@@ -33,7 +33,7 @@ class AdbRuntime {
   }
 
   connect(connection) {
-    return ADB.prepareTCPConnection(this._socketPath).then(port => {
+    return prepareTCPConnection(this._socketPath).then(port => {
       connection.host = "localhost";
       connection.port = port;
       connection.connect();

devtools/shared/adb/adb-scanner.js

@@ -1,98 +0,0 @@
-/* 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/. */
-
-"use strict";
-
-const EventEmitter = require("devtools/shared/event-emitter");
-const { dumpn } = require("devtools/shared/DevToolsUtils");
-const { ADB } = require("devtools/shared/adb/adb");
-const { adbDevicesRegistry } = require("devtools/shared/adb/adb-devices-registry");
-const { AdbRuntime } = require("devtools/shared/adb/adb-runtime");
-
-loader.lazyRequireGetter(this, "AdbDevice", "devtools/shared/adb/adb-device");
-
-class ADBScanner extends EventEmitter {
-  constructor() {
-    super();
-    this._runtimes = [];
-
-    this._onDeviceConnected = this._onDeviceConnected.bind(this);
-    this._onDeviceDisconnected = this._onDeviceDisconnected.bind(this);
-    this._updateRuntimes = this._updateRuntimes.bind(this);
-  }
-
-  enable() {
-    EventEmitter.on(ADB, "device-connected", this._onDeviceConnected);
-    EventEmitter.on(ADB, "device-disconnected", this._onDeviceDisconnected);
-
-    adbDevicesRegistry.on("register", this._updateRuntimes);
-    adbDevicesRegistry.on("unregister", this._updateRuntimes);
-
-    ADB.start().then(() => {
-      ADB.trackDevices();
-    });
-    this._updateRuntimes();
-  }
-
-  disable() {
-    EventEmitter.off(ADB, "device-connected", this._onDeviceConnected);
-    EventEmitter.off(ADB, "device-disconnected", this._onDeviceDisconnected);
-    adbDevicesRegistry.off("register", this._updateRuntimes);
-    adbDevicesRegistry.off("unregister", this._updateRuntimes);
-    this._updateRuntimes();
-  }
-
-  _emitUpdated() {
-    this.emit("runtime-list-updated");
-  }
-
-  _onDeviceConnected(deviceId) {
-    const device = new AdbDevice(deviceId);
-    adbDevicesRegistry.register(deviceId, device);
-  }
-
-  _onDeviceDisconnected(deviceId) {
-    adbDevicesRegistry.unregister(deviceId);
-  }
-
-  _updateRuntimes() {
-    if (this._updatingPromise) {
-      return this._updatingPromise;
-    }
-    this._runtimes = [];
-    const promises = [];
-    for (const id of adbDevicesRegistry.available()) {
-      const device = adbDevicesRegistry.getByName(id);
-      promises.push(this._detectRuntimes(device));
-    }
-    this._updatingPromise = Promise.all(promises);
-    this._updatingPromise.then(() => {
-      this._emitUpdated();
-      this._updatingPromise = null;
-    }, () => {
-      this._updatingPromise = null;
-    });
-    return this._updatingPromise;
-  }
-
-  async _detectRuntimes(adbDevice) {
-    const model = await adbDevice.getModel();
-    const socketPaths = await adbDevice.getRuntimeSocketPaths();
-    for (const socketPath of socketPaths) {
-      const runtime = new AdbRuntime(adbDevice, model, socketPath);
-      dumpn("Found " + runtime.name);
-      this._runtimes.push(runtime);
-    }
-  }
-
-  scan() {
-    return this._updateRuntimes();
-  }
-
-  listRuntimes() {
-    return this._runtimes;
-  }
-}
-
-exports.ADBScanner = ADBScanner;

devtools/shared/adb/adb.js

@@ -2,423 +2,110 @@
  * 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/. */
 
-// Wrapper around the ADB utility.
-
 "use strict";
 
-const { Cc, Ci } = require("chrome");
 const EventEmitter = require("devtools/shared/event-emitter");
-const client = require("./adb-client");
-const { dumpn } = require("devtools/shared/DevToolsUtils");
-const { getFileForBinary } = require("./adb-binary");
-const { setTimeout } = require("resource://gre/modules/Timer.jsm");
-const { Services } = require("resource://gre/modules/Services.jsm");
-const { ConnectionManager } = require("devtools/shared/client/connection-manager");
-loader.lazyRequireGetter(this, "check",
-                         "devtools/shared/adb/adb-running-checker", true);
+const { adbProcess } = require("devtools/shared/adb/adb-process");
+const { adbAddon } = require("devtools/shared/adb/adb-addon");
+const AdbDevice = require("devtools/shared/adb/adb-device");
+const { AdbRuntime } = require("devtools/shared/adb/adb-runtime");
+const { TrackDevicesCommand } = require("devtools/shared/adb/commands/track-devices");
 
-let ready = false;
-let didRunInitially = false;
+class Adb extends EventEmitter {
+  constructor() {
+    super();
 
-const OKAY = 0x59414b4f;
+    this._trackDevicesCommand = new TrackDevicesCommand();
 
-const ADB = {
-  get didRunInitially() {
-    return didRunInitially;
-  },
-  set didRunInitially(newVal) {
-    didRunInitially = newVal;
-  },
+    this._isTrackingDevices = false;
+    this._isUpdatingRuntimes = false;
 
-  get ready() {
-    return ready;
-  },
-  set ready(newVal) {
-    ready = newVal;
-  },
+    this._listeners = new Set();
+    this._devices = new Map();
+    this._runtimes = [];
 
-  get adbFilePromise() {
-    if (this._adbFilePromise) {
-      return this._adbFilePromise;
-    }
-    this._adbFilePromise = getFileForBinary();
-    return this._adbFilePromise;
-  },
+    this._updateAdbProcess = this._updateAdbProcess.bind(this);
+    this._onDeviceConnected = this._onDeviceConnected.bind(this);
+    this._onDeviceDisconnected = this._onDeviceDisconnected.bind(this);
 
-  async _runProcess(process, params) {
-    return new Promise((resolve, reject) => {
-      process.runAsync(params, params.length, {
-        observe(subject, topic, data) {
-          switch (topic) {
-            case "process-finished":
-              resolve();
-              break;
-            case "process-failed":
-              reject();
-              break;
-          }
-        },
-      }, false);
-    });
-  },
+    this._trackDevicesCommand.on("device-connected", this._onDeviceConnected);
+    this._trackDevicesCommand.on("device-disconnected", this._onDeviceDisconnected);
+    adbAddon.on("update", this._updateAdbProcess);
+  }
 
-  // Waits until a predicate returns true or re-tries the predicate calls
-  // |retry| times, we wait for 100ms between each calls.
-  async _waitUntil(predicate, retry = 20) {
-    let count = 0;
-    while (count++ < retry) {
-      if (await predicate()) {
-        return true;
-      }
-      // Wait for 100 milliseconds.
-      await new Promise(resolve => setTimeout(resolve, 100));
-    }
-    // Timed out after trying too many times.
-    return false;
-  },
+  registerListener(listener) {
+    this._listeners.add(listener);
+    this.on("runtime-list-updated", listener);
+    this._updateAdbProcess();
+  }
 
-  // We startup by launching adb in server mode, and setting
-  // the tcp socket preference to |true|
-  async start() {
-    return new Promise(async (resolve, reject) => {
-      const onSuccessfulStart = () => {
-        Services.obs.notifyObservers(null, "adb-ready");
-        this.ready = true;
-        resolve();
-      };
+  unregisterListener(listener) {
+    this._listeners.delete(listener);
+    this.off("runtime-list-updated", listener);
+    this._updateAdbProcess();
+  }
 
-      const isAdbRunning = await check();
-      if (isAdbRunning) {
-        dumpn("Found ADB process running, not restarting");
-        onSuccessfulStart();
-        return;
-      }
-      dumpn("Didn't find ADB process running, restarting");
-
-      this.didRunInitially = true;
-      const process = Cc["@mozilla.org/process/util;1"]
-                      .createInstance(Ci.nsIProcess);
-      // FIXME: Bug 1481691 - We should avoid extracting files every time.
-      const adbFile = await this.adbFilePromise;
-      process.init(adbFile);
-      // Hide command prompt window on Windows
-      process.startHidden = true;
-      process.noShell = true;
-      const params = ["start-server"];
-      let isStarted = false;
-      try {
-        await this._runProcess(process, params);
-        isStarted = await this._waitUntil(check);
-      } catch (e) {
-      }
-
-      if (isStarted) {
-        onSuccessfulStart();
-      } else {
-        this.ready = false;
-        reject();
-      }
-    });
-  },
-
-  /**
-   * Stop the ADB server, but only if we started it.  If it was started before
-   * us, we return immediately.
-   */
-  async stop() {
-    if (!this.didRunInitially) {
-      return; // We didn't start the server, nothing to do
-    }
-    await this.kill();
-  },
-
-  /**
-   * Kill the ADB server.
-   */
-  async kill() {
+  async updateRuntimes() {
     try {
-      await this.runCommand("host:kill");
+      const devices = [...this._devices.values()];
+      const promises = devices.map(d => this._getDeviceRuntimes(d));
+      const allRuntimes = await Promise.all(promises);
+
+      this._runtimes = allRuntimes.flat();
+      this.emit("runtime-list-updated");
     } catch (e) {
-      dumpn("Failed to send host:kill command");
+      console.error(e);
     }
-    dumpn("adb server was terminated by host:kill");
-    this.ready = false;
-    this.didRunInitially = false;
-  },
+  }
 
-  // Start tracking devices connecting and disconnecting from the host.
-  // We can't reuse runCommand here because we keep the socket alive.
-  // @return The socket used.
-  trackDevices() {
-    dumpn("trackDevices");
-    const socket = client.connect();
-    let waitForFirst = true;
-    const devices = {};
+  getRuntimes() {
+    return this._runtimes;
+  }
 
-    socket.s.onopen = function() {
-      dumpn("trackDevices onopen");
-      Services.obs.notifyObservers(null, "adb-track-devices-start");
-      const req = client.createRequest("host:track-devices");
-      socket.send(req);
-    };
+  async _startAdb() {
+    this._isTrackingDevices = true;
+    await adbProcess.start();
 
-    socket.s.onerror = function(event) {
-      dumpn("trackDevices onerror: " + event);
-      Services.obs.notifyObservers(null, "adb-track-devices-stop");
-    };
+    this._trackDevicesCommand.run();
+  }
 
-    socket.s.onclose = function() {
-      dumpn("trackDevices onclose");
+  async _stopAdb() {
+    this._isTrackingDevices = false;
+    this._trackDevicesCommand.stop();
+    await adbProcess.stop();
 
-      // Report all devices as disconnected
-      for (const dev in devices) {
-        devices[dev] = false;
-        EventEmitter.emit(ADB, "device-disconnected", dev);
-      }
+    this._devices = new Map();
+    this._runtimes = [];
+    this.updateRuntimes();
+  }
 
-      Services.obs.notifyObservers(null, "adb-track-devices-stop");
+  _shouldTrack() {
+    return adbAddon.status === "installed" && this._listeners.size > 0;
+  }
 
-      // When we lose connection to the server,
-      // and the adb is still on, we most likely got our server killed
-      // by local adb. So we do try to reconnect to it.
-      setTimeout(function() { // Give some time to the new adb to start
-        if (ADB.ready) { // Only try to reconnect/restart if the add-on is still enabled
-          ADB.start().then(function() { // try to connect to the new local adb server
-                                         // or, spawn a new one
-            ADB.trackDevices(); // Re-track devices
-          });
-        }
-      }, 2000);
-    };
+  _updateAdbProcess() {
+    if (!this._isTrackingDevices && this._shouldTrack()) {
+      this._startAdb();
+    } else if (this._isTrackingDevices && !this._shouldTrack()) {
+      this._stopAdb();
+    }
+  }
 
-    socket.s.ondata = function(event) {
-      dumpn("trackDevices ondata");
-      const data = event.data;
-      dumpn("length=" + data.byteLength);
-      const dec = new TextDecoder();
-      dumpn(dec.decode(new Uint8Array(data)).trim());
+  _onDeviceConnected(deviceId) {
+    this._devices.set(deviceId, new AdbDevice(deviceId));
+    this.updateRuntimes();
+  }
 
-      // check the OKAY or FAIL on first packet.
-      if (waitForFirst) {
-        if (!client.checkResponse(data, OKAY)) {
-          socket.close();
-          return;
-        }
-      }
+  _onDeviceDisconnected(deviceId) {
+    this._devices.delete(deviceId);
+    this.updateRuntimes();
+  }
 
-      const packet = client.unpackPacket(data, !waitForFirst);
-      waitForFirst = false;
+  async _getDeviceRuntimes(device) {
+    const model = await device.getModel();
+    const socketPaths = await device.getRuntimeSocketPaths();
+    return [...socketPaths].map(socketPath => new AdbRuntime(device, model, socketPath));
+  }
+}
 
-      if (packet.data == "") {
-        // All devices got disconnected.
-        for (const dev in devices) {
-          devices[dev] = false;
-          EventEmitter.emit(ADB, "device-disconnected", dev);
-        }
-      } else {
-        // One line per device, each line being $DEVICE\t(offline|device)
-        const lines = packet.data.split("\n");
-        const newDev = {};
-        lines.forEach(function(line) {
-          if (line.length == 0) {
-            return;
-          }
-
-          const [dev, status] = line.split("\t");
-          newDev[dev] = status !== "offline";
-        });
-        // Check which device changed state.
-        for (const dev in newDev) {
-          if (devices[dev] != newDev[dev]) {
-            if (dev in devices || newDev[dev]) {
-              const topic = newDev[dev] ? "device-connected"
-                                        : "device-disconnected";
-              EventEmitter.emit(ADB, topic, dev);
-            }
-            devices[dev] = newDev[dev];
-          }
-        }
-      }
-    };
-  },
-
-  // Sends back an array of device names.
-  listDevices() {
-    dumpn("listDevices");
-
-    return this.runCommand("host:devices").then(
-      function onSuccess(data) {
-        const lines = data.split("\n");
-        const res = [];
-        lines.forEach(function(line) {
-          if (line.length == 0) {
-            return;
-          }
-          const [ device ] = line.split("\t");
-          res.push(device);
-        });
-        return res;
-      }
-    );
-  },
-
-  // sends adb forward localPort devicePort
-  forwardPort(localPort, devicePort) {
-    dumpn("forwardPort " + localPort + " -- " + devicePort);
-    // <host-prefix>:forward:<local>;<remote>
-
-    return this.runCommand("host:forward:" + localPort + ";" + devicePort)
-               .then(function onSuccess(data) {
-                 return data;
-               });
-  },
-
-  // Prepare TCP connection for provided socket path.
-  // The returned value is a port number of localhost for the connection.
-  async prepareTCPConnection(socketPath) {
-    const port = ConnectionManager.getFreeTCPPort();
-    const local = `tcp:${ port }`;
-    const remote = socketPath.startsWith("@")
-                     ? `localabstract:${ socketPath.substring(1) }`
-                     : `localfilesystem:${ socketPath }`;
-    await this.forwardPort(local, remote);
-    return port;
-  },
-
-  // Run a shell command
-  async shell(command) {
-    let state;
-    let stdout = "";
-
-    dumpn("shell " + command);
-
-    return new Promise((resolve, reject) => {
-      const shutdown = function() {
-        dumpn("shell shutdown");
-        socket.close();
-        reject("BAD_RESPONSE");
-      };
-
-      const runFSM = function runFSM(data) {
-        dumpn("runFSM " + state);
-        let req;
-        let ignoreResponseCode = false;
-        switch (state) {
-          case "start":
-            state = "send-transport";
-            runFSM();
-            break;
-          case "send-transport":
-            req = client.createRequest("host:transport-any");
-            socket.send(req);
-            state = "wait-transport";
-            break;
-          case "wait-transport":
-            if (!client.checkResponse(data, OKAY)) {
-              shutdown();
-              return;
-            }
-            state = "send-shell";
-            runFSM();
-            break;
-          case "send-shell":
-            req = client.createRequest("shell:" + command);
-            socket.send(req);
-            state = "rec-shell";
-            break;
-          case "rec-shell":
-            if (!client.checkResponse(data, OKAY)) {
-              shutdown();
-              return;
-            }
-            state = "decode-shell";
-            if (client.getBuffer(data).byteLength == 4) {
-              break;
-            }
-            ignoreResponseCode = true;
-            // eslint-disable-next-lined no-fallthrough
-          case "decode-shell":
-            const decoder = new TextDecoder();
-            const text = new Uint8Array(client.getBuffer(data),
-                                        ignoreResponseCode ? 4 : 0);
-            stdout += decoder.decode(text);
-            break;
-          default:
-            dumpn("shell Unexpected State: " + state);
-            reject("UNEXPECTED_STATE");
-        }
-      };
-
-      const socket = client.connect();
-      socket.s.onerror = function(event) {
-        dumpn("shell onerror");
-        reject("SOCKET_ERROR");
-      };
-
-      socket.s.onopen = function(event) {
-        dumpn("shell onopen");
-        state = "start";
-        runFSM();
-      };
-
-      socket.s.onclose = function(event) {
-        resolve(stdout);
-        dumpn("shell onclose");
-      };
-
-      socket.s.ondata = function(event) {
-        dumpn("shell ondata");
-        runFSM(event.data);
-      };
-    });
-  },
-
-  // Asynchronously runs an adb command.
-  // @param command The command as documented in
-  // http://androidxref.com/4.0.4/xref/system/core/adb/SERVICES.TXT
-  runCommand(command) {
-    dumpn("runCommand " + command);
-    return new Promise((resolve, reject) => {
-      if (!this.ready) {
-        setTimeout(function() {
-          reject("ADB_NOT_READY");
-        });
-        return;
-      }
-
-      const socket = client.connect();
-
-      socket.s.onopen = function() {
-        dumpn("runCommand onopen");
-        const req = client.createRequest(command);
-        socket.send(req);
-      };
-
-      socket.s.onerror = function() {
-        dumpn("runCommand onerror");
-        reject("NETWORK_ERROR");
-      };
-
-      socket.s.onclose = function() {
-        dumpn("runCommand onclose");
-      };
-
-      socket.s.ondata = function(event) {
-        dumpn("runCommand ondata");
-        const data = event.data;
-
-        const packet = client.unpackPacket(data, false);
-        if (!client.checkResponse(data, OKAY)) {
-          socket.close();
-          dumpn("Error: " + packet.data);
-          reject("PROTOCOL_ERROR");
-          return;
-        }
-
-        resolve(packet.data);
-      };
-    });
-  },
-};
-
-exports.ADB = ADB;
+exports.adb = new Adb();

devtools/shared/adb/addon-aware-adb-scanner.js

@@ -1,98 +0,0 @@
-/* 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/. */
-
-"use strict";
-
-const EventEmitter = require("devtools/shared/event-emitter");
-
-loader.lazyRequireGetter(this, "adbAddon", "devtools/shared/adb/adb-addon", true);
-loader.lazyRequireGetter(this, "ADBScanner", "devtools/shared/adb/adb-scanner", true);
-
-/**
- * The AddonAwareADBScanner decorates an instance of ADBScanner. It will wait until the
- * ADB addon is installed to enable() the real scanner, and will automatically disable
- * it if the addon is uninstalled.
- *
- * It implements the following public API of ADBScanner:
- * - enable
- * - disable
- * - scan
- * - listRuntimes
- * - event "runtime-list-updated"
- */
-class AddonAwareADBScanner extends EventEmitter {
-  /**
-   * Parameters are provided only to allow tests to replace actual implementations with
-   * mocks.
-   *
-   * @param {ADBScanner} scanner
-   *        Only provided in tests for mocks
-   * @param {ADBAddon} addon
-   *        Only provided in tests for mocks
-   */
-  constructor(scanner = new ADBScanner(), addon = adbAddon) {
-    super();
-
-    this._onScannerListUpdated = this._onScannerListUpdated.bind(this);
-    this._onAddonUpdate = this._onAddonUpdate.bind(this);
-
-    this._scanner = scanner;
-    this._scanner.on("runtime-list-updated", this._onScannerListUpdated);
-
-    this._addon = addon;
-  }
-
-  /**
-   * Only forward the enable() call if the addon is installed, because ADBScanner::enable
-   * only works if the addon is installed.
-   */
-  enable() {
-    if (this._addon.status === "installed") {
-      this._scanner.enable();
-    }
-
-    // Remove any previous listener, to make sure we only add one listener if enable() is
-    // called several times.
-    this._addon.off("update", this._onAddonUpdate);
-
-    this._addon.on("update", this._onAddonUpdate);
-  }
-
-  disable() {
-    this._scanner.disable();
-
-    this._addon.off("update", this._onAddonUpdate);
-  }
-
-  /**
-   * Scan for USB devices.
-   *
-   * @return {Promise} Promise that will resolve when the scan is completed.
-   */
-  scan() {
-    return this._scanner.scan();
-  }
-
-  /**
-   * Get the list of currently detected runtimes.
-   *
-   * @return {Array} Array of currently detected runtimes.
-   */
-  listRuntimes() {
-    return this._scanner.listRuntimes();
-  }
-
-  _onAddonUpdate() {
-    if (this._addon.status === "installed") {
-      this._scanner.enable();
-    } else {
-      this._scanner.disable();
-    }
-  }
-
-  _onScannerListUpdated() {
-    this.emit("runtime-list-updated");
-  }
-}
-exports.AddonAwareADBScanner = AddonAwareADBScanner;

devtools/shared/adb/commands/index.js

@@ -0,0 +1,19 @@
+/* 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/. */
+
+"use strict";
+
+const { listDevices } = require("./list-devices");
+const { prepareTCPConnection } = require("./prepare-tcp-connection");
+const { runCommand } = require("./run-command");
+const { shell } = require("./shell");
+const { TrackDevicesCommand } = require("./track-devices");
+
+module.exports = {
+  listDevices,
+  prepareTCPConnection,
+  runCommand,
+  shell,
+  TrackDevicesCommand,
+};

devtools/shared/adb/commands/list-devices.js

@@ -0,0 +1,31 @@
+/* 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/. */
+
+"use strict";
+
+const { dumpn } = require("devtools/shared/DevToolsUtils");
+/**
+ * The listDevices command is currently unused in DevTools. We are keeping it while
+ * working on RemoteDebugging NG, in case it becomes needed later. We will remove it from
+ * the codebase if unused at the end of the project. See Bug 1511779.
+ */
+const listDevices = function() {
+  dumpn("listDevices");
+
+  return this.runCommand("host:devices").then(
+    function onSuccess(data) {
+      const lines = data.split("\n");
+      const res = [];
+      lines.forEach(function(line) {
+        if (line.length == 0) {
+          return;
+        }
+        const [ device ] = line.split("\t");
+        res.push(device);
+      });
+      return res;
+    }
+  );
+};
+exports.listDevices = listDevices;

devtools/shared/adb/commands/moz.build

@@ -0,0 +1,12 @@
+# 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/.
+
+DevToolsModules(
+    'index.js',
+    'list-devices.js',
+    'prepare-tcp-connection.js',
+    'run-command.js',
+    'shell.js',
+    'track-devices.js',
+)

devtools/shared/adb/commands/prepare-tcp-connection.js

@@ -0,0 +1,33 @@
+/* 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/. */
+
+"use strict";
+
+const { dumpn } = require("devtools/shared/DevToolsUtils");
+const { ConnectionManager } = require("devtools/shared/client/connection-manager");
+const { runCommand } = require("./run-command");
+
+// sends adb forward localPort devicePort
+const forwardPort = function(localPort, devicePort) {
+  dumpn("forwardPort " + localPort + " -- " + devicePort);
+  // <host-prefix>:forward:<local>;<remote>
+
+  return runCommand("host:forward:" + localPort + ";" + devicePort)
+             .then(function onSuccess(data) {
+               return data;
+             });
+};
+
+// Prepare TCP connection for provided socket path.
+// The returned value is a port number of localhost for the connection.
+const prepareTCPConnection = async function(socketPath) {
+  const port = ConnectionManager.getFreeTCPPort();
+  const local = `tcp:${ port }`;
+  const remote = socketPath.startsWith("@")
+                   ? `localabstract:${ socketPath.substring(1) }`
+                   : `localfilesystem:${ socketPath }`;
+  await forwardPort(local, remote);
+  return port;
+};
+exports.prepareTCPConnection = prepareTCPConnection;

devtools/shared/adb/commands/run-command.js

@@ -0,0 +1,62 @@
+/* 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/. */
+
+// Wrapper around the ADB utility.
+
+"use strict";
+
+const { dumpn } = require("devtools/shared/DevToolsUtils");
+const { setTimeout } = require("resource://gre/modules/Timer.jsm");
+const { adbProcess } = require("../adb-process");
+const client = require("../adb-client");
+
+const OKAY = 0x59414b4f;
+
+// Asynchronously runs an adb command.
+// @param command The command as documented in
+// http://androidxref.com/4.0.4/xref/system/core/adb/SERVICES.TXT
+const runCommand = function(command) {
+  dumpn("runCommand " + command);
+  return new Promise((resolve, reject) => {
+    if (!adbProcess.ready) {
+      setTimeout(function() {
+        reject("ADB_NOT_READY");
+      });
+      return;
+    }
+
+    const socket = client.connect();
+
+    socket.s.onopen = function() {
+      dumpn("runCommand onopen");
+      const req = client.createRequest(command);
+      socket.send(req);
+    };
+
+    socket.s.onerror = function() {
+      dumpn("runCommand onerror");
+      reject("NETWORK_ERROR");
+    };
+
+    socket.s.onclose = function() {
+      dumpn("runCommand onclose");
+    };
+
+    socket.s.ondata = function(event) {
+      dumpn("runCommand ondata");
+      const data = event.data;
+
+      const packet = client.unpackPacket(data, false);
+      if (!client.checkResponse(data, OKAY)) {
+        socket.close();
+        dumpn("Error: " + packet.data);
+        reject("PROTOCOL_ERROR");
+        return;
+      }
+
+      resolve(packet.data);
+    };
+  });
+};
+exports.runCommand = runCommand;

devtools/shared/adb/commands/shell.js

@@ -0,0 +1,101 @@
+/* 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/. */
+
+// Wrapper around the ADB utility.
+
+"use strict";
+
+const { dumpn } = require("devtools/shared/DevToolsUtils");
+const client = require("../adb-client");
+
+const OKAY = 0x59414b4f;
+
+const shell = async function(command) {
+  let state;
+  let stdout = "";
+
+  dumpn("shell " + command);
+
+  return new Promise((resolve, reject) => {
+    const shutdown = function() {
+      dumpn("shell shutdown");
+      socket.close();
+      reject("BAD_RESPONSE");
+    };
+
+    const runFSM = function runFSM(data) {
+      dumpn("runFSM " + state);
+      let req;
+      let ignoreResponseCode = false;
+      switch (state) {
+        case "start":
+          state = "send-transport";
+          runFSM();
+          break;
+        case "send-transport":
+          req = client.createRequest("host:transport-any");
+          socket.send(req);
+          state = "wait-transport";
+          break;
+        case "wait-transport":
+          if (!client.checkResponse(data, OKAY)) {
+            shutdown();
+            return;
+          }
+          state = "send-shell";
+          runFSM();
+          break;
+        case "send-shell":
+          req = client.createRequest("shell:" + command);
+          socket.send(req);
+          state = "rec-shell";
+          break;
+        case "rec-shell":
+          if (!client.checkResponse(data, OKAY)) {
+            shutdown();
+            return;
+          }
+          state = "decode-shell";
+          if (client.getBuffer(data).byteLength == 4) {
+            break;
+          }
+          ignoreResponseCode = true;
+          // eslint-disable-next-lined no-fallthrough
+        case "decode-shell":
+          const decoder = new TextDecoder();
+          const text = new Uint8Array(client.getBuffer(data),
+                                      ignoreResponseCode ? 4 : 0);
+          stdout += decoder.decode(text);
+          break;
+        default:
+          dumpn("shell Unexpected State: " + state);
+          reject("UNEXPECTED_STATE");
+      }
+    };
+
+    const socket = client.connect();
+    socket.s.onerror = function(event) {
+      dumpn("shell onerror");
+      reject("SOCKET_ERROR");
+    };
+
+    socket.s.onopen = function(event) {
+      dumpn("shell onopen");
+      state = "start";
+      runFSM();
+    };
+
+    socket.s.onclose = function(event) {
+      resolve(stdout);
+      dumpn("shell onclose");
+    };
+
+    socket.s.ondata = function(event) {
+      dumpn("shell ondata");
+      runFSM(event.data);
+    };
+  });
+};
+
+exports.shell = shell;

devtools/shared/adb/commands/track-devices.js

@@ -0,0 +1,129 @@
+/* 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/. */
+
+// Wrapper around the ADB utility.
+
+"use strict";
+
+const EventEmitter = require("devtools/shared/event-emitter");
+const { dumpn } = require("devtools/shared/DevToolsUtils");
+const { setTimeout } = require("resource://gre/modules/Timer.jsm");
+
+const { adbProcess } = require("../adb-process");
+const client = require("../adb-client");
+
+const OKAY = 0x59414b4f;
+
+// Start tracking devices connecting and disconnecting from the host.
+// We can't reuse runCommand here because we keep the socket alive.
+class TrackDevicesCommand extends EventEmitter {
+  run() {
+    this._waitForFirst = true;
+    this._devices = {};
+    this._socket = client.connect();
+
+    this._socket.s.onopen = this._onOpen.bind(this);
+    this._socket.s.onerror = this._onError.bind(this);
+    this._socket.s.onclose = this._onClose.bind(this);
+    this._socket.s.ondata = this._onData.bind(this);
+  }
+
+  stop() {
+    if (this._socket) {
+      this._socket.close();
+
+      this._socket.s.onopen = null;
+      this._socket.s.onerror = null;
+      this._socket.s.onclose = null;
+      this._socket.s.ondata = null;
+    }
+  }
+
+  _onOpen() {
+    dumpn("trackDevices onopen");
+    const req = client.createRequest("host:track-devices");
+    this._socket.send(req);
+  }
+
+  _onError(event) {
+    dumpn("trackDevices onerror: " + event);
+  }
+
+  _onClose() {
+    dumpn("trackDevices onclose");
+
+    // Report all devices as disconnected
+    for (const dev in this._devices) {
+      this._devices[dev] = false;
+      this.emit("device-disconnected", dev);
+    }
+
+    // When we lose connection to the server,
+    // and the adb is still on, we most likely got our server killed
+    // by local adb. So we do try to reconnect to it.
+
+    // Give some time to the new adb to start
+    setTimeout(() => {
+      // Only try to reconnect/restart if the add-on is still enabled
+      if (adbProcess.ready) {
+        // try to connect to the new local adb server or spawn a new one
+        adbProcess.start().then(() => {
+          // Re-track devices
+          this.run();
+        });
+      }
+    }, 2000);
+  }
+
+  _onData(event) {
+    dumpn("trackDevices ondata");
+    const data = event.data;
+    dumpn("length=" + data.byteLength);
+    const dec = new TextDecoder();
+    dumpn(dec.decode(new Uint8Array(data)).trim());
+
+    // check the OKAY or FAIL on first packet.
+    if (this._waitForFirst) {
+      if (!client.checkResponse(data, OKAY)) {
+        this._socket.close();
+        return;
+      }
+    }
+
+    const packet = client.unpackPacket(data, !this._waitForFirst);
+    this._waitForFirst = false;
+
+    if (packet.data == "") {
+      // All devices got disconnected.
+      for (const dev in this._devices) {
+        this._devices[dev] = false;
+        this.emit("device-disconnected", dev);
+      }
+    } else {
+      // One line per device, each line being $DEVICE\t(offline|device)
+      const lines = packet.data.split("\n");
+      const newDev = {};
+      lines.forEach(function(line) {
+        if (line.length == 0) {
+          return;
+        }
+
+        const [dev, status] = line.split("\t");
+        newDev[dev] = status !== "offline";
+      });
+      // Check which device changed state.
+      for (const dev in newDev) {
+        if (this._devices[dev] != newDev[dev]) {
+          if (dev in this._devices || newDev[dev]) {
+            const topic = newDev[dev] ? "device-connected"
+                                      : "device-disconnected";
+            this.emit(topic, dev);
+          }
+          this._devices[dev] = newDev[dev];
+        }
+      }
+    }
+  }
+}
+exports.TrackDevicesCommand = TrackDevicesCommand;

devtools/shared/adb/moz.build

@@ -2,18 +2,20 @@
 # 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/.
 
+DIRS += [
+    'commands',
+]
+
 DevToolsModules(
     'adb-addon.js',
     'adb-binary.js',
     'adb-client.js',
     'adb-device.js',
-    'adb-devices-registry.js',
+    'adb-process.js',
     'adb-running-checker.js',
     'adb-runtime.js',
-    'adb-scanner.js',
     'adb-socket.js',
     'adb.js',
-    'addon-aware-adb-scanner.js',
 )
 
 with Files('**'):

devtools/shared/adb/test/test_adb.js

@@ -3,12 +3,12 @@
 
 "use strict";
 
-const EventEmitter = require("devtools/shared/event-emitter");
 const { ExtensionTestUtils } = ChromeUtils.import("resource://testing-common/ExtensionXPCShellUtils.jsm", {});
 const { NetUtil } = require("resource://gre/modules/NetUtil.jsm");
 const { getFileForBinary } = require("devtools/shared/adb/adb-binary");
 const { check } = require("devtools/shared/adb/adb-running-checker");
-const { ADB } = require("devtools/shared/adb/adb");
+const { adbProcess } = require("devtools/shared/adb/adb-process");
+const { TrackDevicesCommand } = require("devtools/shared/adb/commands/index");
 
 const ADB_JSON = {
   "Linux": {
@@ -176,22 +176,22 @@ add_task({
   await extension.startup();
 
   // Call start() once and call stop() afterwards.
-  await ADB.start();
-  ok(ADB.ready);
+  await adbProcess.start();
+  ok(adbProcess.ready);
   ok(await check(), "adb is now running");
 
-  await ADB.stop();
-  ok(!ADB.ready);
+  await adbProcess.stop();
+  ok(!adbProcess.ready);
   ok(!(await check()), "adb is no longer running");
 
   // Call start() twice and call stop() afterwards.
-  await ADB.start();
-  await ADB.start();
-  ok(ADB.ready);
+  await adbProcess.start();
+  await adbProcess.start();
+  ok(adbProcess.ready);
   ok(await check(), "adb is now running");
 
-  await ADB.stop();
-  ok(!ADB.ready);
+  await adbProcess.stop();
+  ok(!adbProcess.ready);
   ok(!(await check()), "adb is no longer running");
 
   await extension.unload();
@@ -220,22 +220,23 @@ add_task({
 
   await extension.startup();
 
-  await ADB.start();
-  ok(ADB.ready);
+  await adbProcess.start();
+  ok(adbProcess.ready);
 
   ok(await check(), "adb is now running");
 
   const receivedDeviceId = await new Promise(resolve => {
-    EventEmitter.on(ADB, "device-connected", deviceId => {
+    const trackDevicesCommand = new TrackDevicesCommand();
+    trackDevicesCommand.on("device-connected", deviceId => {
       resolve(deviceId);
     });
-    ADB.trackDevices();
+    trackDevicesCommand.run();
   });
 
   equal(receivedDeviceId, "1234567890");
 
-  await ADB.stop();
-  ok(!ADB.ready);
+  await adbProcess.stop();
+  ok(!adbProcess.ready);
 
   await extension.unload();
 });

devtools/shared/adb/test/test_addon-aware-adb-scanner.js

@@ -1,221 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-const EventEmitter = require("devtools/shared/event-emitter");
-const { AddonAwareADBScanner } = require("devtools/shared/adb/addon-aware-adb-scanner");
-
-/**
- * For the scanner mock, we create an object with spies for each of the public methods
- * used by the AddonAwareADBScanner, and the ability to emit events.
- */
-function prepareMockScanner() {
-  const mockScanner = {
-    enable: sinon.spy(),
-    disable: sinon.spy(),
-    scan: sinon.spy(),
-    listRuntimes: sinon.spy(),
-  };
-  EventEmitter.decorate(mockScanner);
-  return mockScanner;
-}
-
-/**
- * For the addon mock, we simply need an object that is able to emit events and has a
- * status.
- */
-function prepareMockAddon() {
-  const mockAddon = {
-    status: "unknown",
-  };
-  EventEmitter.decorate(mockAddon);
-  return mockAddon;
-}
-
-/**
- * Prepare all mocks needed for the scanner tests.
- */
-function prepareMocks() {
-  const mockScanner = prepareMockScanner();
-  const mockAddon = prepareMockAddon();
-  const addonAwareAdbScanner = new AddonAwareADBScanner(mockScanner, mockAddon);
-  return { addonAwareAdbScanner, mockAddon, mockScanner };
-}
-
-/**
- * This test covers basic usage of enable() on the AddonAwareADBScanner, and checks the
- * different behaviors based on the addon status.
- */
-add_task(async function testCallingEnable() {
-  const { mockScanner, mockAddon, addonAwareAdbScanner } = prepareMocks();
-
-  // Check that enable() is not called if the addon is uninstalled
-  mockAddon.status = "uninstalled";
-  addonAwareAdbScanner.enable();
-  ok(mockScanner.enable.notCalled, "enable() was not called");
-  mockScanner.enable.reset();
-
-  // Check that enable() is called if the addon is installed
-  mockAddon.status = "installed";
-  addonAwareAdbScanner.enable();
-  ok(mockScanner.enable.called, "enable() was called");
-  mockScanner.enable.reset();
-});
-
-/**
- * This test checks that enable()/disable() methods from the internal ADBScanner are
- * called when the addon is installed or uninstalled.
- */
-add_task(async function testUpdatingAddonEnablesDisablesScanner() {
-  const { mockScanner, mockAddon, addonAwareAdbScanner } = prepareMocks();
-
-  // Enable the addon aware scanner
-  addonAwareAdbScanner.enable();
-  ok(mockScanner.enable.notCalled, "enable() was not called initially");
-
-  // Check that enable() is called automatically when the addon is installed
-  mockAddon.status = "installed";
-  mockAddon.emit("update");
-  ok(mockScanner.enable.called, "enable() was called when installing the addon");
-  ok(mockScanner.disable.notCalled, "disable() was not called when installing the addon");
-  mockScanner.enable.reset();
-  mockScanner.disable.reset();
-
-  // Check that disabled() is called automatically when the addon is uninstalled
-  mockAddon.status = "uninstalled";
-  mockAddon.emit("update");
-  ok(mockScanner.enable.notCalled, "enable() was not called when uninstalling the addon");
-  ok(mockScanner.disable.called, "disable() was called when uninstalling the addon");
-  mockScanner.enable.reset();
-  mockScanner.disable.reset();
-
-  // Check that enable() is called again when the addon is reinstalled
-  mockAddon.status = "installed";
-  mockAddon.emit("update");
-  ok(mockScanner.enable.called, "enable() was called when installing the addon");
-  ok(mockScanner.disable.notCalled, "disable() was not called when installing the addon");
-  mockScanner.enable.reset();
-  mockScanner.disable.reset();
-});
-
-/**
- * This test checks that disable() is forwarded from the AddonAwareADBScanner to the real
- * scanner even if the addon is uninstalled. We might miss the addon uninstall
- * notification, so it is safer to always proceed with disabling.
- */
-add_task(async function testScannerIsDisabledWhenMissingAddonUpdate() {
-  const { mockScanner, mockAddon, addonAwareAdbScanner } = prepareMocks();
-
-  // Enable the addon aware scanner
-  mockAddon.status = "installed";
-  addonAwareAdbScanner.enable();
-  ok(mockScanner.enable.called, "enable() was called initially");
-  mockScanner.enable.reset();
-
-  // Uninstall the addon without firing any event
-  mockAddon.status = "uninstalled";
-
-  // Programmatically call disable, check that the scanner's disable is called even though
-  // the addon was uninstalled.
-  addonAwareAdbScanner.disable();
-  ok(mockScanner.disable.called, "disable() was called when uninstalling the addon");
-  mockScanner.disable.reset();
-});
-
-/**
- * This test checks that when the AddonAwareADBScanner is disabled, then enable/disable
- * are not called on the inner scanner when the addon status changes.
- */
-add_task(async function testInnerEnableIsNotCalledIfNotStarted() {
-  const { mockScanner, mockAddon, addonAwareAdbScanner } = prepareMocks();
-
-  // Check that enable() is not called on the inner scanner when the addon is installed
-  // if the AddonAwareADBScanner was not enabled
-  mockAddon.status = "installed";
-  mockAddon.emit("update");
-  ok(mockScanner.enable.notCalled, "enable() was not called");
-
-  // Same for disable() and "uninstall"
-  mockAddon.status = "uninstalled";
-  mockAddon.emit("update");
-  ok(mockScanner.disable.notCalled, "disable() was not called");
-
-  // Enable the addon aware scanner
-  addonAwareAdbScanner.enable();
-  ok(mockScanner.enable.notCalled, "enable() was not called");
-  ok(mockScanner.disable.notCalled, "disable() was not called");
-});
-
-/**
- * This test checks that when the AddonAwareADBScanner is disabled, installing the addon
- * no longer enables the internal ADBScanner.
- */
-add_task(async function testEnableIsNoLongerCalledAfterDisabling() {
-  const { mockScanner, mockAddon, addonAwareAdbScanner } = prepareMocks();
-
-  // Start with the addon installed
-  mockAddon.status = "installed";
-  addonAwareAdbScanner.enable();
-  ok(mockScanner.enable.called, "enable() was called since addon was already installed");
-  mockScanner.enable.reset();
-
-  // Here we call enable again to check that we will not add too many events.
-  // A single call to disable() should stop all listeners, even if we called enable()
-  // several times.
-  addonAwareAdbScanner.enable();
-  ok(mockScanner.enable.called, "enable() was called again");
-  mockScanner.enable.reset();
-
-  // Disable the scanner
-  addonAwareAdbScanner.disable();
-  ok(mockScanner.disable.called, "disable() was called");
-  mockScanner.disable.reset();
-
-  // Emit an addon update event
-  mockAddon.emit("update");
-  ok(mockScanner.enable.notCalled,
-    "enable() is not called since the main scanner is disabled");
-});
-
-/**
- * Basic check that the "runtime-list-updated" event is forwarded.
- */
-add_task(async function testListUpdatedEventForwarding() {
-  const { mockScanner, addonAwareAdbScanner } = prepareMocks();
-
-  const spy = sinon.spy();
-  addonAwareAdbScanner.on("runtime-list-updated", spy);
-  mockScanner.emit("runtime-list-updated");
-  ok(spy.called, "The runtime-list-updated event was forwarded from ADBScanner");
-  addonAwareAdbScanner.off("runtime-list-updated", spy);
-});
-
-/**
- * Basic check that calls to scan() are forwarded.
- */
-add_task(async function testScanCallForwarding() {
-  const { mockScanner, addonAwareAdbScanner } = prepareMocks();
-
-  ok(mockScanner.scan.notCalled, "ADBScanner scan() is not called initially");
-
-  addonAwareAdbScanner.scan();
-  mockScanner.emit("runtime-list-updated");
-  ok(mockScanner.scan.called, "ADBScanner scan() was called");
-  mockScanner.scan.reset();
-});
-
-/**
- * Basic check that calls to scan() are forwarded.
- */
-add_task(async function testListRuntimesCallForwarding() {
-  const { mockScanner, addonAwareAdbScanner } = prepareMocks();
-
-  ok(mockScanner.listRuntimes.notCalled,
-    "ADBScanner listRuntimes() is not called initially");
-
-  addonAwareAdbScanner.listRuntimes();
-  mockScanner.emit("runtime-list-updated");
-  ok(mockScanner.listRuntimes.called, "ADBScanner listRuntimes() was called");
-  mockScanner.scan.reset();
-});

devtools/shared/adb/test/xpcshell.ini

@@ -8,4 +8,3 @@ support-files =
 
 [test_adb.js]
 run-sequentially = An extension having the same id is installed/uninstalled in different tests
-[test_addon-aware-adb-scanner.js]

gfx/webrender_bindings/revision.txt

@@ -1 +1 @@
-8a6a53883bc6fe9522730e09b916f4023ee10d51
+6db2d6c9005cf5888c6b35ca6908449ec591527a

gfx/wr/.taskcluster.yml

@@ -18,19 +18,8 @@ metadata:
 # instances configured and running.
 tasks:
   # For the docker-worker tasks, the Docker image used
-  # (staktrace/webrender-test:freetype28) was created using this Dockerfile:
-  # ---
-  #   FROM ubuntu:16.04
-  #   RUN apt-get -y update && apt-get install -y curl git python python-pip cmake pkg-config libx11-dev libgl1-mesa-dev libfontconfig1-dev software-properties-common
-  #   RUN add-apt-repository -y -u ppa:glasen/freetype2
-  #   RUN apt-get -y install freetype2-demos
-  #   RUN pip install mako voluptuous PyYAML servo-tidy
-  #   RUN useradd -d /home/worker -s /bin/bash -m worker
-  #   USER worker
-  #   WORKDIR /home/worker
-  #   ENV PATH $PATH:/home/worker/.cargo/bin
-  #   CMD /bin/bash
-  # ---
+  # (staktrace/webrender-test:debian) was created using the Dockerfile in
+  # ci-scripts/docker-image.
   #
   # The docker image may need to be updated over time if the set of required
   # packages increases. Note in particular that rust/cargo are not part of the
@@ -55,7 +44,7 @@ tasks:
           - master
     payload:
       maxRunTime: 7200
-      image: 'staktrace/webrender-test:freetype28'
+      image: 'staktrace/webrender-test:debian'
       env:
         RUST_BACKTRACE: 'full'
         RUSTFLAGS: '--deny warnings'
@@ -65,6 +54,7 @@ tasks:
         - '-c'
         - >-
           curl https://sh.rustup.rs -sSf | sh -s -- -y &&
+          source $HOME/.cargo/env &&
           git clone {{event.head.repo.url}} webrender && cd webrender &&
           git checkout {{event.head.sha}} &&
           servo-tidy &&
@@ -88,7 +78,7 @@ tasks:
           - master
     payload:
       maxRunTime: 7200
-      image: 'staktrace/webrender-test:freetype28'
+      image: 'staktrace/webrender-test:debian'
       env:
         RUST_BACKTRACE: 'full'
         RUSTFLAGS: '--deny warnings'
@@ -98,6 +88,7 @@ tasks:
         - '-c'
         - >-
           curl https://sh.rustup.rs -sSf | sh -s -- -y &&
+          source $HOME/.cargo/env &&
           git clone {{event.head.repo.url}} webrender && cd webrender &&
           git checkout {{event.head.sha}} &&
           servo-tidy &&

gfx/wr/ci-scripts/docker-image/Dockerfile

@@ -0,0 +1,9 @@
+FROM debian:stretch-20181112
+
+COPY setup.sh /root
+RUN cd /root && ./setup.sh
+
+RUN useradd -d /home/worker -s /bin/bash -m worker
+USER worker
+WORKDIR /home/worker
+CMD /bin/bash

gfx/wr/ci-scripts/docker-image/setup.sh

@@ -0,0 +1,48 @@
+#!/usr/bin/env bash
+
+# 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/. */
+
+set -o errexit
+set -o nounset
+set -o pipefail
+set -o xtrace
+
+test "$(whoami)" == 'root'
+
+# Install stuff we need
+apt-get -y update
+apt-get install -y \
+    cmake \
+    curl \
+    git \
+    libfontconfig1-dev \
+    libgl1-mesa-dev \
+    libx11-dev \
+    pkg-config \
+    python \
+    python-pip \
+    software-properties-common
+
+# Build freetype with subpixel rendering enabled
+curl -sSfL -o ft.tar.bz2 \
+    https://download.savannah.gnu.org/releases/freetype/freetype-2.8.1.tar.bz2
+tar xjf ft.tar.bz2
+cd freetype-2.8.1
+# Need to respect 80-char line limit for servo-tidy, or this would be neater
+SUBPIXEL_OPTION="FT_CONFIG_OPTION_SUBPIXEL_RENDERING"
+sed --in-place="" \
+    -e "s/.*${SUBPIXEL_OPTION}.*/#define ${SUBPIXEL_OPTION}/" \
+    include/freetype/config/ftoption.h
+./configure
+make
+make install
+
+# Replace the system libfreetype with the one we just built
+cd /usr/lib/x86_64-linux-gnu/
+rm -f libfreetype.so.6
+ln -s /usr/local/lib/libfreetype.so.6
+
+# Other stuff we need
+pip install mako voluptuous PyYAML servo-tidy

gfx/wr/webrender/src/batch.rs

@@ -549,15 +549,9 @@ impl AlphaBatchBuilder {
             render_tasks,
         ).unwrap_or(OPAQUE_TASK_ADDRESS);
 
-        let prim_data = &ctx
-            .resources
-            .prim_data_store[prim_instance.prim_data_handle];
-
-        match (&prim_instance.kind, &prim_data.kind) {
-            (
-                PrimitiveInstanceKind::Clear,
-                PrimitiveTemplateKind::Clear,
-            ) => {
+        match prim_instance.kind {
+            PrimitiveInstanceKind::Clear { data_handle } => {
+                let prim_data = &ctx.resources.prim_data_store[data_handle];
                 let prim_cache_address = gpu_cache.get_address(&prim_data.gpu_cache_handle);
 
                 // TODO(gw): We can abstract some of the common code below into
@@ -601,10 +595,8 @@ impl AlphaBatchBuilder {
                     PrimitiveInstanceData::from(instance),
                 );
             }
-            (
-                PrimitiveInstanceKind::NormalBorder { cache_handles, .. },
-                PrimitiveTemplateKind::NormalBorder { template, .. },
-            ) => {
+            PrimitiveInstanceKind::NormalBorder { data_handle, ref cache_handles, .. } => {
+                let prim_data = &ctx.resources.prim_data_store[data_handle];
                 let prim_cache_address = gpu_cache.get_address(&prim_data.gpu_cache_handle);
                 let cache_handles = &ctx.scratch.border_cache_handles[*cache_handles];
                 let specified_blend_mode = BlendMode::PremultipliedAlpha;
@@ -660,6 +652,10 @@ impl AlphaBatchBuilder {
                     batch_params.prim_user_data,
                 );
 
+                let template = match prim_data.kind {
+                    PrimitiveTemplateKind::NormalBorder { ref template, .. } => template,
+                    _ => unreachable!()
+                };
                 self.add_segmented_prim_to_batch(
                     Some(template.brush_segments.as_slice()),
                     prim_data.opacity,
@@ -676,16 +672,13 @@ impl AlphaBatchBuilder {
                     ctx,
                 );
             }
-            (
-                PrimitiveInstanceKind::TextRun { run_index, .. },
-                PrimitiveTemplateKind::TextRun { .. },
-            ) => {
-                let run = &ctx.prim_store.text_runs[*run_index];
+            PrimitiveInstanceKind::TextRun { data_handle, run_index, .. } => {
+                let run = &ctx.prim_store.text_runs[run_index];
                 let subpx_dir = run.used_font.get_subpx_dir();
 
                 // The GPU cache data is stored in the template and reused across
                 // frames and display lists.
-
+                let prim_data = &ctx.resources.text_run_data_store[data_handle];
                 let glyph_fetch_buffer = &mut self.glyph_fetch_buffer;
                 let alpha_batch_list = &mut self.batch_list.alpha_batch_list;
                 let prim_cache_address = gpu_cache.get_address(&prim_data.gpu_cache_handle);
@@ -789,13 +782,11 @@ impl AlphaBatchBuilder {
                     },
                 );
             }
-            (
-                PrimitiveInstanceKind::LineDecoration { ref cache_handle, .. },
-                PrimitiveTemplateKind::LineDecoration { .. },
-            ) => {
+            PrimitiveInstanceKind::LineDecoration { data_handle, ref cache_handle, .. } => {
                 // The GPU cache data is stored in the template and reused across
                 // frames and display lists.
 
+                let prim_data = &ctx.resources.prim_data_store[data_handle];
                 let prim_cache_address = gpu_cache.get_address(&prim_data.gpu_cache_handle);
 
                 let (batch_kind, textures, prim_user_data, segment_user_data) = match cache_handle {
@@ -877,10 +868,7 @@ impl AlphaBatchBuilder {
                     PrimitiveInstanceData::from(instance),
                 );
             }
-            (
-                PrimitiveInstanceKind::Picture { pic_index },
-                PrimitiveTemplateKind::Unused,
-            ) => {
+            PrimitiveInstanceKind::Picture { pic_index, .. } => {
                 let picture = &ctx.prim_store.pictures[pic_index.0];
                 let non_segmented_blend_mode = BlendMode::PremultipliedAlpha;
                 let prim_cache_address = gpu_cache.get_address(&picture.gpu_location);
@@ -900,7 +888,7 @@ impl AlphaBatchBuilder {
                         for child in list {
                             let prim_instance = &picture.prim_list.prim_instances[child.anchor];
                             let pic_index = match prim_instance.kind {
-                                PrimitiveInstanceKind::Picture { pic_index } => pic_index,
+                                PrimitiveInstanceKind::Picture { pic_index, .. } => pic_index,
                                 PrimitiveInstanceKind::LineDecoration { .. } |
                                 PrimitiveInstanceKind::TextRun { .. } |
                                 PrimitiveInstanceKind::NormalBorder { .. } |
@@ -910,7 +898,7 @@ impl AlphaBatchBuilder {
                                 PrimitiveInstanceKind::Image { .. } |
                                 PrimitiveInstanceKind::LinearGradient { .. } |
                                 PrimitiveInstanceKind::RadialGradient { .. } |
-                                PrimitiveInstanceKind::Clear => {
+                                PrimitiveInstanceKind::Clear { .. } => {
                                     unreachable!();
                                 }
                             };
@@ -1401,10 +1389,15 @@ impl AlphaBatchBuilder {
                     }
                 }
             }
-            (
-                PrimitiveInstanceKind::ImageBorder { .. },
-                PrimitiveTemplateKind::ImageBorder { request, brush_segments, .. }
-            ) => {
+            PrimitiveInstanceKind::ImageBorder { data_handle, .. } => {
+                let prim_data = &ctx.resources.prim_data_store[data_handle];
+                let (request, brush_segments) = match &prim_data.kind {
+                    PrimitiveTemplateKind::ImageBorder { request, brush_segments, .. } => {
+                        (request, brush_segments)
+                    }
+                    _ => unreachable!()
+                };
+
                 let cache_item = resolve_image(
                     *request,
                     ctx.resource_cache,
@@ -1469,12 +1462,10 @@ impl AlphaBatchBuilder {
                     ctx,
                 );
             }
-            (
-                PrimitiveInstanceKind::Rectangle { segment_instance_index, opacity_binding_index, .. },
-                PrimitiveTemplateKind::Rectangle { .. }
-            ) => {
+            PrimitiveInstanceKind::Rectangle { data_handle, segment_instance_index, opacity_binding_index, .. } => {
+                let prim_data = &ctx.resources.prim_data_store[data_handle];
                 let specified_blend_mode = BlendMode::PremultipliedAlpha;
-                let opacity_binding = ctx.prim_store.get_opacity_binding(*opacity_binding_index);
+                let opacity_binding = ctx.prim_store.get_opacity_binding(opacity_binding_index);
 
                 let opacity = PrimitiveOpacity::from_alpha(opacity_binding);
                 let opacity = opacity.combine(prim_data.opacity);
@@ -1495,10 +1486,10 @@ impl AlphaBatchBuilder {
                     0,
                 );
 
-                let (prim_cache_address, segments) = if *segment_instance_index == SegmentInstanceIndex::UNUSED {
+                let (prim_cache_address, segments) = if segment_instance_index == SegmentInstanceIndex::UNUSED {
                     (gpu_cache.get_address(&prim_data.gpu_cache_handle), None)
                 } else {
-                    let segment_instance = &ctx.scratch.segment_instances[*segment_instance_index];
+                    let segment_instance = &ctx.scratch.segment_instances[segment_instance_index];
                     let segments = Some(&ctx.scratch.segments[segment_instance.segments_range]);
                     (gpu_cache.get_address(&segment_instance.gpu_cache_handle), segments)
                 };
@@ -1534,10 +1525,14 @@ impl AlphaBatchBuilder {
                     ctx,
                 );
             }
-            (
-                PrimitiveInstanceKind::YuvImage { segment_instance_index, .. },
-                PrimitiveTemplateKind::YuvImage { format, yuv_key, image_rendering, color_depth, color_space, .. }
-            ) => {
+            PrimitiveInstanceKind::YuvImage { data_handle, segment_instance_index, .. } => {
+                let prim_data = &ctx.resources.prim_data_store[data_handle];
+                let (format, yuv_key, image_rendering, color_depth, color_space) = match prim_data.kind {
+                    PrimitiveTemplateKind::YuvImage { ref format, yuv_key, ref image_rendering, ref color_depth, ref color_space, .. } => {
+                        (format, yuv_key, image_rendering, color_depth, color_space)
+                    }
+                    _ => unreachable!()
+                };
                 let mut textures = BatchTextures::no_texture();
                 let mut uv_rect_addresses = [0; 3];
 
@@ -1604,11 +1599,11 @@ impl AlphaBatchBuilder {
                     BlendMode::None
                 };
 
-                debug_assert!(*segment_instance_index != SegmentInstanceIndex::INVALID);
-                let (prim_cache_address, segments) = if *segment_instance_index == SegmentInstanceIndex::UNUSED {
+                debug_assert!(segment_instance_index != SegmentInstanceIndex::INVALID);
+                let (prim_cache_address, segments) = if segment_instance_index == SegmentInstanceIndex::UNUSED {
                     (gpu_cache.get_address(&prim_data.gpu_cache_handle), None)
                 } else {
-                    let segment_instance = &ctx.scratch.segment_instances[*segment_instance_index];
+                    let segment_instance = &ctx.scratch.segment_instances[segment_instance_index];
                     let segments = Some(&ctx.scratch.segments[segment_instance.segments_range]);
                     (gpu_cache.get_address(&segment_instance.gpu_cache_handle), segments)
                 };
@@ -1644,19 +1639,23 @@ impl AlphaBatchBuilder {
                     ctx,
                 );
             }
-            (
-                PrimitiveInstanceKind::Image { image_instance_index, .. },
-                PrimitiveTemplateKind::Image { source, alpha_type, key, image_rendering, .. }
-            ) => {
-                let image_instance = &ctx.prim_store.images[*image_instance_index];
+            PrimitiveInstanceKind::Image { data_handle, image_instance_index, .. } => {
+                let prim_data = &ctx.resources.prim_data_store[data_handle];
+                let (source, alpha_type, key, image_rendering) = match prim_data.kind {
+                    PrimitiveTemplateKind::Image { ref source, alpha_type, key, image_rendering, .. } => {
+                        (source, alpha_type, key, image_rendering)
+                    }
+                    _ => unreachable!()
+                };
+                let image_instance = &ctx.prim_store.images[image_instance_index];
                 let opacity_binding = ctx.prim_store.get_opacity_binding(image_instance.opacity_binding_index);
                 let specified_blend_mode = match alpha_type {
                     AlphaType::PremultipliedAlpha => BlendMode::PremultipliedAlpha,
                     AlphaType::Alpha => BlendMode::Alpha,
                 };
                 let request = ImageRequest {
-                    key: *key,
-                    rendering: *image_rendering,
+                    key: key,
+                    rendering: image_rendering,
                     tile: None,
                 };
 
@@ -1702,7 +1701,7 @@ impl AlphaBatchBuilder {
                         BrushBatchKind::Image(get_buffer_kind(cache_item.texture_id)),
                         textures,
                         [
-                            ShaderColorMode::Image as i32 | ((*alpha_type as i32) << 16),
+                            ShaderColorMode::Image as i32 | ((alpha_type as i32) << 16),
                             RasterizationSpace::Local as i32,
                             get_shader_opacity(opacity_binding),
                         ],
@@ -1755,7 +1754,7 @@ impl AlphaBatchBuilder {
                             gpu_cache,
                             deferred_resolves,
                             request.with_tile(tile.tile_offset),
-                            *alpha_type,
+                            alpha_type,
                             get_shader_opacity(opacity_binding),
                         ) {
                             let prim_cache_address = gpu_cache.get_address(&tile.handle);
@@ -1784,10 +1783,14 @@ impl AlphaBatchBuilder {
                     }
                 }
             }
-            (
-                PrimitiveInstanceKind::LinearGradient { visible_tiles_range, .. },
-                PrimitiveTemplateKind::LinearGradient { stops_handle, ref brush_segments, .. }
-            ) => {
+            PrimitiveInstanceKind::LinearGradient { data_handle, ref visible_tiles_range, .. } => {
+                let prim_data = &ctx.resources.prim_data_store[data_handle];
+                let (ref stops_handle, brush_segments) = match prim_data.kind {
+                    PrimitiveTemplateKind::LinearGradient { stops_handle, ref brush_segments, .. } => {
+                        (stops_handle, brush_segments)
+                    }
+                    _ => unreachable!()
+                };
                 let specified_blend_mode = BlendMode::PremultipliedAlpha;
 
                 let mut prim_header = PrimitiveHeader {
@@ -1867,10 +1870,14 @@ impl AlphaBatchBuilder {
                     );
                 }
             }
-            (
-                PrimitiveInstanceKind::RadialGradient { visible_tiles_range, .. },
-                PrimitiveTemplateKind::RadialGradient { stops_handle, ref brush_segments, .. }
-            ) => {
+            PrimitiveInstanceKind::RadialGradient { data_handle, ref visible_tiles_range, .. } => {
+                let prim_data = &ctx.resources.prim_data_store[data_handle];
+                let (stops_handle, brush_segments) = match prim_data.kind {
+                    PrimitiveTemplateKind::RadialGradient { ref stops_handle, ref brush_segments, .. } => {
+                        (stops_handle, brush_segments)
+                    }
+                    _ => unreachable!()
+                };
                 let specified_blend_mode = BlendMode::PremultipliedAlpha;
 
                 let mut prim_header = PrimitiveHeader {
@@ -1950,9 +1957,6 @@ impl AlphaBatchBuilder {
                     );
                 }
             }
-            _ => {
-                unreachable!();
-            }
         }
     }
 
@@ -2288,9 +2292,9 @@ impl PrimitiveInstance {
         resource_cache: &ResourceCache,
     ) -> bool {
         let image_key = match self.kind {
-            PrimitiveInstanceKind::Image { .. } |
-            PrimitiveInstanceKind::YuvImage { .. } => {
-                let prim_data = &prim_data_store[self.prim_data_handle];
+            PrimitiveInstanceKind::Image { data_handle, .. } |
+            PrimitiveInstanceKind::YuvImage { data_handle, .. } => {
+                let prim_data = &prim_data_store[data_handle];
                 match prim_data.kind {
                     PrimitiveTemplateKind::YuvImage { ref yuv_key, .. } => {
                         yuv_key[0]
@@ -2309,7 +2313,7 @@ impl PrimitiveInstance {
             PrimitiveInstanceKind::Rectangle { .. } |
             PrimitiveInstanceKind::LinearGradient { .. } |
             PrimitiveInstanceKind::RadialGradient { .. } |
-            PrimitiveInstanceKind::Clear => {
+            PrimitiveInstanceKind::Clear { .. } => {
                 return true;
             }
         };

gfx/wr/webrender/src/clip.rs

@@ -104,7 +104,6 @@ pub type ClipDataStore = intern::DataStore<ClipItemKey, ClipNode, ClipDataMarker
 pub type ClipDataHandle = intern::Handle<ClipDataMarker>;
 pub type ClipDataUpdateList = intern::UpdateList<ClipItemKey>;
 pub type ClipDataInterner = intern::Interner<ClipItemKey, ClipItemSceneData, ClipDataMarker>;
-pub type ClipUid = intern::ItemUid<ClipDataMarker>;
 
 // Result of comparing a clip node instance against a local rect.
 #[derive(Debug)]

gfx/wr/webrender/src/display_list_flattener.rs

@@ -1,4 +1,3 @@
-
 /* 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/. */
@@ -20,16 +19,18 @@ use frame_builder::{ChasePrimitive, FrameBuilder, FrameBuilderConfig};
 use glyph_rasterizer::FontInstance;
 use hit_test::{HitTestingItem, HitTestingRun};
 use image::simplify_repeated_primitive;
+use intern::{Handle, Internable};
 use internal_types::{FastHashMap, FastHashSet};
 use picture::{Picture3DContext, PictureCompositeMode, PicturePrimitive, PrimitiveList};
-use prim_store::{PrimitiveInstance, PrimitiveDataInterner, PrimitiveKeyKind, RadialGradientParams};
+use prim_store::{PrimitiveInstance, PrimitiveKeyKind, RadialGradientParams};
 use prim_store::{PrimitiveKey, PrimitiveSceneData, PrimitiveInstanceKind, GradientStopKey, NinePatchDescriptor};
 use prim_store::{PrimitiveDataHandle, PrimitiveStore, PrimitiveStoreStats, LineDecorationCacheKey};
 use prim_store::{ScrollNodeAndClipChain, PictureIndex, register_prim_chase_id, get_line_decoration_sizes};
+use prim_store::text_run::TextRun;
 use render_backend::{DocumentView};
 use resource_cache::{FontInstanceMap, ImageRequest};
 use scene::{Scene, ScenePipeline, StackingContextHelpers};
-use scene_builder::DocumentResources;
+use scene_builder::{DocumentResources, InternerMut};
 use spatial_node::{StickyFrameInfo, ScrollFrameKind, SpatialNodeType};
 use std::{f32, mem, usize};
 use std::collections::vec_deque::VecDeque;
@@ -308,7 +309,7 @@ impl<'a> DisplayListFlattener<'a> {
 
             let prim_list = PrimitiveList::new(
                 remaining_prims,
-                &self.resources.prim_interner,
+                &self.resources,
             );
 
             // Now, create a picture with tile caching enabled that will hold all
@@ -344,8 +345,10 @@ impl<'a> DisplayListFlattener<'a> {
             ));
 
             let instance = PrimitiveInstance::new(
-                PrimitiveInstanceKind::Picture { pic_index: PictureIndex(pic_index) },
-                primitive_data_handle,
+                PrimitiveInstanceKind::Picture {
+                    data_handle: primitive_data_handle,
+                    pic_index: PictureIndex(pic_index)
+                },
                 ClipChainId::NONE,
                 picture_cache_scroll_root,
             );
@@ -359,7 +362,7 @@ impl<'a> DisplayListFlattener<'a> {
             // Finally, store the sliced primitive list in the root picture.
             self.prim_store.pictures[self.root_pic_index.0].prim_list = PrimitiveList::new(
                 new_prim_list,
-                &self.resources.prim_interner,
+                &self.resources,
             );
         }
     }
@@ -1026,20 +1029,22 @@ impl<'a> DisplayListFlattener<'a> {
     /// Create a primitive and add it to the prim store. This method doesn't
     /// add the primitive to the draw list, so can be used for creating
     /// sub-primitives.
-    pub fn create_primitive(
+    ///
+    /// TODO(djg): Can this inline into `add_interned_prim_to_draw_list`
+    fn create_primitive<P>(
         &mut self,
         info: &LayoutPrimitiveInfo,
         clip_chain_id: ClipChainId,
         spatial_node_index: SpatialNodeIndex,
-        prim_key_kind: PrimitiveKeyKind,
-    ) -> PrimitiveInstance {
+        prim: P,
+    ) -> PrimitiveInstance
+    where
+        P: Internable<InternData=PrimitiveSceneData>,
+        P::Source: AsInstanceKind<Handle<P::Marker>>,
+        DocumentResources: InternerMut<P>,
+    {
         // Build a primitive key.
-        let prim_key = PrimitiveKey::new(
-            info.is_backface_visible,
-            info.rect,
-            info.clip_rect,
-            prim_key_kind,
-        );
+        let prim_key = prim.build_key(info);
 
         // Get a tight bounding / culling rect for this primitive
         // from its local rect intersection with minimal local
@@ -1048,8 +1053,9 @@ impl<'a> DisplayListFlattener<'a> {
             .intersection(&info.rect)
             .unwrap_or(LayoutRect::zero());
 
-        let prim_data_handle = self.resources
-            .prim_interner
+        let interner = self.resources.interner_mut();
+        let prim_data_handle =
+            interner
             .intern(&prim_key, || {
                 PrimitiveSceneData {
                     culling_rect,
@@ -1057,11 +1063,11 @@ impl<'a> DisplayListFlattener<'a> {
                 }
             });
 
-        let instance_kind = prim_key.to_instance_kind(&mut self.prim_store);
+        let instance_kind = prim_key.as_instance_kind(prim_data_handle,
+                                                      &mut self.prim_store);
 
         PrimitiveInstance::new(
             instance_kind,
-            prim_data_handle,
             clip_chain_id,
             spatial_node_index,
         )
@@ -1105,48 +1111,74 @@ impl<'a> DisplayListFlattener<'a> {
 
     /// Convenience interface that creates a primitive entry and adds it
     /// to the draw list.
-    pub fn add_primitive(
+    pub fn add_primitive<P>(
         &mut self,
         clip_and_scroll: ScrollNodeAndClipChain,
         info: &LayoutPrimitiveInfo,
         clip_items: Vec<(LayoutPoint, ClipItemKey)>,
-        key_kind: PrimitiveKeyKind,
-    ) {
+        prim: P,
+    )
+    where
+        P: Internable<InternData = PrimitiveSceneData> + IsVisible,
+        P::Source: AsInstanceKind<Handle<P::Marker>>,
+        DocumentResources: InternerMut<P>,
+        ShadowItem: From<PendingPrimitive<P>>
+    {
         // If a shadow context is not active, then add the primitive
         // directly to the parent picture.
         if self.pending_shadow_items.is_empty() {
-            if key_kind.is_visible() {
+            if prim.is_visible() {
                 let clip_chain_id = self.build_clip_chain(
                     clip_items,
                     clip_and_scroll.spatial_node_index,
                     clip_and_scroll.clip_chain_id,
                 );
-                let prim_instance = self.create_primitive(
+                self.add_prim_to_draw_list(
                     info,
                     clip_chain_id,
-                    clip_and_scroll.spatial_node_index,
-                    key_kind,
+                    clip_and_scroll,
+                    prim
                 );
-                self.register_chase_primitive_by_rect(
-                    &info.rect,
-                    &prim_instance,
-                );
-                self.add_primitive_to_hit_testing_list(info, clip_and_scroll);
-                self.add_primitive_to_draw_list(prim_instance);
             }
         } else {
             debug_assert!(clip_items.is_empty(), "No per-prim clips expected for shadowed primitives");
 
             // There is an active shadow context. Store as a pending primitive
             // for processing during pop_all_shadows.
-            self.pending_shadow_items.push_back(ShadowItem::Primitive(PendingPrimitive {
+            self.pending_shadow_items.push_back(PendingPrimitive {
                 clip_and_scroll,
                 info: *info,
-                key_kind,
-            }));
+                prim: prim.into(),
+            }.into());
         }
     }
 
+    fn add_prim_to_draw_list<P>(
+        &mut self,
+        info: &LayoutPrimitiveInfo,
+        clip_chain_id: ClipChainId,
+        clip_and_scroll: ScrollNodeAndClipChain,
+        prim: P,
+    )
+    where
+        P: Internable<InternData = PrimitiveSceneData>,
+        P::Source: AsInstanceKind<Handle<P::Marker>>,
+        DocumentResources: InternerMut<P>,
+    {
+        let prim_instance = self.create_primitive(
+            info,
+            clip_chain_id,
+            clip_and_scroll.spatial_node_index,
+            prim,
+        );
+        self.register_chase_primitive_by_rect(
+            &info.rect,
+            &prim_instance,
+        );
+        self.add_primitive_to_hit_testing_list(info, clip_and_scroll);
+        self.add_primitive_to_draw_list(prim_instance);
+    }
+
     pub fn push_stacking_context(
         &mut self,
         pipeline_id: PipelineId,
@@ -1181,7 +1213,7 @@ impl<'a> DisplayListFlattener<'a> {
                 // so that the relative order between them and our current SC is preserved.
                 let extra_instance = sc.cut_flat_item_sequence(
                     &mut self.prim_store,
-                    &self.resources.prim_interner,
+                    &self.resources,
                     &self.clip_store,
                 );
                 (sc.is_3d(), extra_instance)
@@ -1332,7 +1364,7 @@ impl<'a> DisplayListFlattener<'a> {
                 stacking_context.requested_raster_space,
                 PrimitiveList::new(
                     stacking_context.primitives,
-                    &self.resources.prim_interner,
+                    &self.resources,
                 ),
                 stacking_context.spatial_node_index,
                 max_clip,
@@ -1344,9 +1376,12 @@ impl<'a> DisplayListFlattener<'a> {
         // mix-blend-mode and/or 3d rendering context containers.
 
         let mut current_pic_index = leaf_pic_index;
+        let data_handle = stacking_context.primitive_data_handle;
         let mut cur_instance = PrimitiveInstance::new(
-            PrimitiveInstanceKind::Picture { pic_index: leaf_pic_index },
-            stacking_context.primitive_data_handle,
+            PrimitiveInstanceKind::Picture {
+                data_handle,
+                pic_index: leaf_pic_index
+            },
             stacking_context.clip_chain_id,
             stacking_context.spatial_node_index,
         );
@@ -1376,7 +1411,7 @@ impl<'a> DisplayListFlattener<'a> {
                     stacking_context.requested_raster_space,
                     PrimitiveList::new(
                         prims,
-                        &self.resources.prim_interner,
+                        &self.resources,
                     ),
                     stacking_context.spatial_node_index,
                     max_clip,
@@ -1384,7 +1419,10 @@ impl<'a> DisplayListFlattener<'a> {
                 ))
             );
 
-            cur_instance.kind = PrimitiveInstanceKind::Picture { pic_index: current_pic_index };
+            cur_instance.kind = PrimitiveInstanceKind::Picture {
+                data_handle,
+                pic_index: current_pic_index
+            };
         }
 
         // For each filter, create a new image with that composite mode.
@@ -1402,7 +1440,7 @@ impl<'a> DisplayListFlattener<'a> {
                     stacking_context.requested_raster_space,
                     PrimitiveList::new(
                         vec![cur_instance.clone()],
-                        &self.resources.prim_interner,
+                        &self.resources,
                     ),
                     stacking_context.spatial_node_index,
                     max_clip,
@@ -1411,7 +1449,10 @@ impl<'a> DisplayListFlattener<'a> {
             );
 
             current_pic_index = filter_pic_index;
-            cur_instance.kind = PrimitiveInstanceKind::Picture { pic_index: current_pic_index };
+            cur_instance.kind = PrimitiveInstanceKind::Picture {
+                data_handle,
+                pic_index: current_pic_index
+            };
 
             if cur_instance.is_chased() {
                 println!("\tis a composite picture for a stacking context with {:?}", filter);
@@ -1435,7 +1476,7 @@ impl<'a> DisplayListFlattener<'a> {
                     stacking_context.requested_raster_space,
                     PrimitiveList::new(
                         vec![cur_instance.clone()],
-                        &self.resources.prim_interner,
+                        &self.resources,
                     ),
                     stacking_context.spatial_node_index,
                     max_clip,
@@ -1444,7 +1485,10 @@ impl<'a> DisplayListFlattener<'a> {
             );
 
             current_pic_index = blend_pic_index;
-            cur_instance.kind = PrimitiveInstanceKind::Picture { pic_index: blend_pic_index };
+            cur_instance.kind = PrimitiveInstanceKind::Picture {
+                data_handle,
+                pic_index: blend_pic_index
+            };
 
             if cur_instance.is_chased() {
                 println!("\tis a mix-blend picture for a stacking context with {:?}", mix_blend_mode);
@@ -1743,24 +1787,15 @@ impl<'a> DisplayListFlattener<'a> {
                     let mut prims = Vec::new();
 
                     for item in &items {
-                        if let ShadowItem::Primitive(ref pending_primitive) = item {
-                            // Offset the local rect and clip rect by the shadow offset.
-                            let mut info = pending_primitive.info.clone();
-                            info.rect = info.rect.translate(&pending_shadow.shadow.offset);
-                            info.clip_rect = info.clip_rect.translate(&pending_shadow.shadow.offset);
-
-                            // Construct and add a primitive for the given shadow.
-                            let shadow_prim_instance = self.create_primitive(
-                                &info,
-                                pending_primitive.clip_and_scroll.clip_chain_id,
-                                pending_primitive.clip_and_scroll.spatial_node_index,
-                                pending_primitive.key_kind.create_shadow(
-                                    &pending_shadow.shadow,
-                                ),
-                            );
-
-                            // Add the new primitive to the shadow picture.
-                            prims.push(shadow_prim_instance);
+                        match item {
+                            // TODO(djg): ugh. de-duplicate this code.
+                            ShadowItem::Primitive(ref pending_primitive) => {
+                                self.add_shadow_prim(&pending_shadow, pending_primitive, &mut prims)
+                            }
+                            ShadowItem::TextRun(ref pending_text_run) => {
+                                self.add_shadow_prim(&pending_shadow, pending_text_run, &mut prims)
+                            }
+                            _ => {}
                         }
                     }
 
@@ -1785,7 +1820,7 @@ impl<'a> DisplayListFlattener<'a> {
                                 raster_space,
                                 PrimitiveList::new(
                                     prims,
-                                    &self.resources.prim_interner,
+                                    &self.resources,
                                 ),
                                 pending_shadow.clip_and_scroll.spatial_node_index,
                                 max_clip,
@@ -1811,8 +1846,10 @@ impl<'a> DisplayListFlattener<'a> {
                         );
 
                         let shadow_prim_instance = PrimitiveInstance::new(
-                            PrimitiveInstanceKind::Picture { pic_index: shadow_pic_index },
-                            shadow_prim_data_handle,
+                            PrimitiveInstanceKind::Picture {
+                                data_handle: shadow_prim_data_handle,
+                                pic_index: shadow_pic_index
+                            },
                             pending_shadow.clip_and_scroll.clip_chain_id,
                             pending_shadow.clip_and_scroll.spatial_node_index,
                         );
@@ -1823,23 +1860,11 @@ impl<'a> DisplayListFlattener<'a> {
                     }
                 }
                 ShadowItem::Primitive(pending_primitive) => {
-                    // For a normal primitive, if it has alpha > 0, then we add this
-                    // as a normal primitive to the parent picture.
-                    if pending_primitive.key_kind.is_visible() {
-                        let prim_instance = self.create_primitive(
-                            &pending_primitive.info,
-                            pending_primitive.clip_and_scroll.clip_chain_id,
-                            pending_primitive.clip_and_scroll.spatial_node_index,
-                            pending_primitive.key_kind,
-                        );
-                        self.register_chase_primitive_by_rect(
-                            &pending_primitive.info.rect,
-                            &prim_instance,
-                        );
-                        self.add_primitive_to_hit_testing_list(&pending_primitive.info, pending_primitive.clip_and_scroll);
-                        self.add_primitive_to_draw_list(prim_instance);
-                    }
-                }
+                    self.add_shadow_prim_to_draw_list(pending_primitive)
+                },
+                ShadowItem::TextRun(pending_text_run) => {
+                    self.add_shadow_prim_to_draw_list(pending_text_run)
+                },
             }
         }
 
@@ -1847,6 +1872,54 @@ impl<'a> DisplayListFlattener<'a> {
         self.pending_shadow_items = items;
     }
 
+    fn add_shadow_prim<P>(
+        &mut self,
+        pending_shadow: &PendingShadow,
+        pending_primitive: &PendingPrimitive<P>,
+        prims: &mut Vec<PrimitiveInstance>,
+    )
+    where
+        P: Internable<InternData=PrimitiveSceneData> + CreateShadow,
+        P::Source: AsInstanceKind<Handle<P::Marker>>,
+        DocumentResources: InternerMut<P>,
+    {
+        // Offset the local rect and clip rect by the shadow offset.
+        let mut info = pending_primitive.info.clone();
+        info.rect = info.rect.translate(&pending_shadow.shadow.offset);
+        info.clip_rect = info.clip_rect.translate(&pending_shadow.shadow.offset);
+
+        // Construct and add a primitive for the given shadow.
+        let shadow_prim_instance = self.create_primitive(
+            &info,
+            pending_primitive.clip_and_scroll.clip_chain_id,
+            pending_primitive.clip_and_scroll.spatial_node_index,
+            pending_primitive.prim.create_shadow(
+                &pending_shadow.shadow,
+            ),
+        );
+
+        // Add the new primitive to the shadow picture.
+        prims.push(shadow_prim_instance);
+    }
+
+    fn add_shadow_prim_to_draw_list<P>(&mut self, pending_primitive: PendingPrimitive<P>)
+    where
+        P: Internable<InternData = PrimitiveSceneData> + IsVisible,
+        P::Source: AsInstanceKind<Handle<P::Marker>>,
+        DocumentResources: InternerMut<P>,
+    {
+        // For a normal primitive, if it has alpha > 0, then we add this
+        // as a normal primitive to the parent picture.
+        if pending_primitive.prim.is_visible() {
+            self.add_prim_to_draw_list(
+                &pending_primitive.info,
+                pending_primitive.clip_and_scroll.clip_chain_id,
+                pending_primitive.clip_and_scroll,
+                pending_primitive.prim,
+            );
+        }
+    }
+
     #[cfg(debug_assertions)]
     fn register_chase_primitive_by_rect(
         &mut self,
@@ -2180,7 +2253,7 @@ impl<'a> DisplayListFlattener<'a> {
         glyph_options: Option<GlyphOptions>,
         pipeline_id: PipelineId,
     ) {
-        let container = {
+        let text_run = {
             let instance_map = self.font_instances.read().unwrap();
             let font_instance = match instance_map.get(font_instance_key) {
                 Some(instance) => instance,
@@ -2229,7 +2302,7 @@ impl<'a> DisplayListFlattener<'a> {
             //           primitive template.
             let glyphs = display_list.get(glyph_range).collect();
 
-            PrimitiveKeyKind::TextRun {
+            TextRun {
                 glyphs,
                 font,
                 offset: offset.to_au(),
@@ -2241,7 +2314,7 @@ impl<'a> DisplayListFlattener<'a> {
             clip_and_scroll,
             prim_info,
             Vec::new(),
-            container,
+            text_run,
         );
     }
 
@@ -2349,6 +2422,22 @@ impl<'a> DisplayListFlattener<'a> {
     }
 }
 
+pub trait AsInstanceKind<H> {
+    fn as_instance_kind(
+        &self,
+        data_handle: H,
+        prim_store: &mut PrimitiveStore,
+    ) -> PrimitiveInstanceKind;
+}
+
+pub trait CreateShadow {
+    fn create_shadow(&self, shadow: &Shadow) -> Self;
+}
+
+pub trait IsVisible {
+    fn is_visible(&self) -> bool;
+}
+
 /// Properties of a stacking context that are maintained
 /// during creation of the scene. These structures are
 /// not persisted after the initial scene build.
@@ -2448,7 +2537,7 @@ impl FlattenedStackingContext {
     pub fn cut_flat_item_sequence(
         &mut self,
         prim_store: &mut PrimitiveStore,
-        prim_interner: &PrimitiveDataInterner,
+        resources: &DocumentResources,
         clip_store: &ClipStore,
     ) -> Option<PrimitiveInstance> {
         if !self.is_3d() || self.primitives.is_empty() {
@@ -2473,7 +2562,7 @@ impl FlattenedStackingContext {
                 self.requested_raster_space,
                 PrimitiveList::new(
                     mem::replace(&mut self.primitives, Vec::new()),
-                    prim_interner,
+                    resources,
                 ),
                 self.spatial_node_index,
                 LayoutRect::max_rect(),
@@ -2482,8 +2571,10 @@ impl FlattenedStackingContext {
         );
 
         Some(PrimitiveInstance::new(
-            PrimitiveInstanceKind::Picture { pic_index },
-            self.primitive_data_handle,
+            PrimitiveInstanceKind::Picture {
+                data_handle: self.primitive_data_handle,
+                pic_index
+            },
             self.clip_chain_id,
             self.spatial_node_index,
         ))
@@ -2493,20 +2584,33 @@ impl FlattenedStackingContext {
 /// A primitive that is added while a shadow context is
 /// active is stored as a pending primitive and only
 /// added to pictures during pop_all_shadows.
-struct PendingPrimitive {
+pub struct PendingPrimitive<T> {
     clip_and_scroll: ScrollNodeAndClipChain,
     info: LayoutPrimitiveInfo,
-    key_kind: PrimitiveKeyKind,
+    prim: T,
 }
 
 /// As shadows are pushed, they are stored as pending
 /// shadows, and handled at once during pop_all_shadows.
-struct PendingShadow {
+pub struct PendingShadow {
     shadow: Shadow,
     clip_and_scroll: ScrollNodeAndClipChain,
 }
 
-enum ShadowItem {
+pub enum ShadowItem {
     Shadow(PendingShadow),
-    Primitive(PendingPrimitive),
+    Primitive(PendingPrimitive<PrimitiveKeyKind>),
+    TextRun(PendingPrimitive<TextRun>),
+}
+
+impl From<PendingPrimitive<PrimitiveKeyKind>> for ShadowItem {
+    fn from(container: PendingPrimitive<PrimitiveKeyKind>) -> Self {
+        ShadowItem::Primitive(container)
+    }
+}
+
+impl From<PendingPrimitive<TextRun>> for ShadowItem {
+    fn from(text_run: PendingPrimitive<TextRun>) -> Self {
+        ShadowItem::TextRun(text_run)
+    }
 }

gfx/wr/webrender/src/frame_builder.rs

@@ -275,7 +275,7 @@ impl FrameBuilder {
             &frame_context,
             resource_cache,
             gpu_cache,
-            &resources.prim_data_store,
+            resources,
             &self.clip_store,
             &mut retained_tiles,
         );

gfx/wr/webrender/src/intern.rs

@@ -2,11 +2,13 @@
  * 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/. */
 
+use api::LayoutPrimitiveInfo;
 use internal_types::FastHashMap;
 use std::fmt::Debug;
 use std::hash::Hash;
 use std::marker::PhantomData;
 use std::{mem, ops, u64};
+use std::sync::atomic::{AtomicUsize, Ordering};
 use util::VecHelper;
 
 /*
@@ -64,26 +66,37 @@ pub struct UpdateList<S> {
     data: Vec<S>,
 }
 
+lazy_static! {
+    static ref NEXT_UID: AtomicUsize = AtomicUsize::new(0);
+}
+
+/// A globally, unique identifier
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
 #[derive(Debug, Copy, Clone, Eq, Hash, PartialEq)]
-pub struct ItemUid<T> {
+pub struct ItemUid {
     uid: usize,
-    _marker: PhantomData<T>,
+}
+
+impl ItemUid {
+    pub fn next_uid() -> ItemUid {
+        let uid = NEXT_UID.fetch_add(1, Ordering::Relaxed);
+        ItemUid { uid }
+    }
 }
 
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
 #[derive(Debug, Copy, Clone)]
-pub struct Handle<T> {
+pub struct Handle<M: Copy> {
     index: u32,
     epoch: Epoch,
-    uid: ItemUid<T>,
-    _marker: PhantomData<T>,
+    uid: ItemUid,
+    _marker: PhantomData<M>,
 }
 
-impl <T> Handle<T> where T: Copy {
-    pub fn uid(&self) -> ItemUid<T> {
+impl <M> Handle<M> where M: Copy {
+    pub fn uid(&self) -> ItemUid {
         self.uid
     }
 }
@@ -122,16 +135,19 @@ pub struct DataStore<S, T, M> {
     _marker: PhantomData<M>,
 }
 
-impl<S, T, M> DataStore<S, T, M> where S: Debug, T: From<S>, M: Debug {
-    /// Construct a new data store
-    pub fn new() -> Self {
+impl<S, T, M> ::std::default::Default for DataStore<S, T, M> where S: Debug, T: From<S>, M: Debug
+{
+    fn default() -> Self {
         DataStore {
             items: Vec::new(),
             _source: PhantomData,
             _marker: PhantomData,
         }
     }
+}
 
+impl<S, T, M> DataStore<S, T, M> where S: Debug, T: From<S>, M: Debug
+{
     /// Apply any updates from the scene builder thread to
     /// this data store.
     pub fn apply_updates(
@@ -160,7 +176,9 @@ impl<S, T, M> DataStore<S, T, M> where S: Debug, T: From<S>, M: Debug {
 }
 
 /// Retrieve an item from the store via handle
-impl<S, T, M> ops::Index<Handle<M>> for DataStore<S, T, M> {
+impl<S, T, M> ops::Index<Handle<M>> for DataStore<S, T, M>
+where M: Copy
+{
     type Output = T;
     fn index(&self, handle: Handle<M>) -> &T {
         let item = &self.items[handle.index as usize];
@@ -171,7 +189,10 @@ impl<S, T, M> ops::Index<Handle<M>> for DataStore<S, T, M> {
 
 /// Retrieve a mutable item from the store via handle
 /// Retrieve an item from the store via handle
-impl<S, T, M> ops::IndexMut<Handle<M>> for DataStore<S, T, M> {
+impl<S, T, M> ops::IndexMut<Handle<M>> for DataStore<S, T, M>
+where
+    M: Copy
+{
     fn index_mut(&mut self, handle: Handle<M>) -> &mut T {
         let item = &mut self.items[handle.index as usize];
         assert_eq!(item.epoch, handle.epoch);
@@ -186,7 +207,11 @@ impl<S, T, M> ops::IndexMut<Handle<M>> for DataStore<S, T, M> {
 /// an update list of additions / removals.
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
-pub struct Interner<S : Eq + Hash + Clone + Debug, D, M> {
+pub struct Interner<S, D, M>
+where
+    S: Eq + Hash + Clone + Debug,
+    M: Copy
+{
     /// Uniquely map an interning key to a handle
     map: FastHashMap<S, Handle<M>>,
     /// List of free slots in the data store for re-use.
@@ -197,27 +222,33 @@ pub struct Interner<S : Eq + Hash + Clone + Debug, D, M> {
     update_data: Vec<S>,
     /// The current epoch for the interner.
     current_epoch: Epoch,
-    /// Incrementing counter for identifying stable values.
-    next_uid: usize,
     /// The information associated with each interned
     /// item that can be accessed by the interner.
     local_data: Vec<Item<D>>,
 }
 
-impl<S, D, M> Interner<S, D, M> where S: Eq + Hash + Clone + Debug, M: Copy + Debug {
-    /// Construct a new interner
-    pub fn new() -> Self {
+impl<S, D, M> ::std::default::Default for Interner<S, D, M>
+where
+    S: Eq + Hash + Clone + Debug,
+    M: Copy + Debug
+{
+    fn default() -> Self {
         Interner {
             map: FastHashMap::default(),
             free_list: Vec::new(),
             updates: Vec::new(),
             update_data: Vec::new(),
             current_epoch: Epoch(1),
-            next_uid: 0,
             local_data: Vec::new(),
         }
     }
+}
 
+impl<S, D, M> Interner<S, D, M>
+where
+    S: Eq + Hash + Clone + Debug,
+    M: Copy + Debug
+{
     /// Intern a data structure, and return a handle to
     /// that data. The handle can then be stored in the
     /// frame builder, and safely accessed via the data
@@ -268,17 +299,13 @@ impl<S, D, M> Interner<S, D, M> where S: Eq + Hash + Clone + Debug, M: Copy + De
         let handle = Handle {
             index: index as u32,
             epoch: self.current_epoch,
-            uid: ItemUid {
-                uid: self.next_uid,
-                _marker: PhantomData,
-            },
+            uid: ItemUid::next_uid(),
             _marker: PhantomData,
         };
 
         // Store this handle so the next time it is
         // interned, it gets re-used.
         self.map.insert(data.clone(), handle);
-        self.next_uid += 1;
 
         // Create the local data for this item that is
         // being interned.
@@ -338,7 +365,11 @@ impl<S, D, M> Interner<S, D, M> where S: Eq + Hash + Clone + Debug, M: Copy + De
 }
 
 /// Retrieve the local data for an item from the interner via handle
-impl<S, D, M> ops::Index<Handle<M>> for Interner<S, D, M> where S: Eq + Clone + Hash + Debug, M: Copy + Debug {
+impl<S, D, M> ops::Index<Handle<M>> for Interner<S, D, M>
+where
+    S: Eq + Clone + Hash + Debug,
+    M: Copy + Debug
+{
     type Output = D;
     fn index(&self, handle: Handle<M>) -> &D {
         let item = &self.local_data[handle.index as usize];
@@ -346,3 +377,16 @@ impl<S, D, M> ops::Index<Handle<M>> for Interner<S, D, M> where S: Eq + Clone +
         &item.data
     }
 }
+
+/// Implement `Internable` for a type that wants participate in interning.
+///
+/// see DisplayListFlattener::add_interned_primitive<P>
+pub trait Internable {
+    type Marker: Copy + Debug;
+    type Source: Eq + Hash + Clone + Debug;
+    type StoreData: From<Self::Source>;
+    type InternData;
+
+    /// Build a new key from self with `info`.
+    fn build_key(self, info: &LayoutPrimitiveInfo) -> Self::Source;
+}

gfx/wr/webrender/src/lib.rs

@@ -64,9 +64,6 @@ extern crate serde;
 #[macro_use]
 extern crate thread_profiler;
 
-#[macro_use]
-mod storage;
-
 mod batch;
 mod border;
 mod box_shadow;
@@ -112,6 +109,7 @@ mod scene_builder;
 mod segment;
 mod shade;
 mod spatial_node;
+mod storage;
 mod surface;
 mod texture_allocator;
 mod texture_cache;

gfx/wr/webrender/src/picture.rs

@@ -7,24 +7,27 @@ use api::{DeviceIntRect, DevicePoint, LayoutRect, PictureToRasterTransform, Layo
 use api::{DevicePixelScale, RasterRect, RasterSpace, PictureSize, DeviceIntPoint, ColorF, ImageKey, DirtyRect};
 use api::{PicturePixel, RasterPixel, WorldPixel, WorldRect, ImageFormat, ImageDescriptor};
 use box_shadow::{BLUR_SAMPLE_SCALE};
-use clip::{ClipNodeCollector, ClipStore, ClipChainId, ClipChainNode, ClipUid};
+use clip::{ClipNodeCollector, ClipStore, ClipChainId, ClipChainNode};
 use clip_scroll_tree::{ROOT_SPATIAL_NODE_INDEX, ClipScrollTree, SpatialNodeIndex};
 use device::TextureFilter;
 use euclid::{TypedScale, vec3, TypedRect, TypedPoint2D, TypedSize2D};
 use euclid::approxeq::ApproxEq;
+use intern::ItemUid;
 use internal_types::{FastHashMap, PlaneSplitter};
 use frame_builder::{FrameBuildingContext, FrameBuildingState, PictureState, PictureContext};
 use gpu_cache::{GpuCache, GpuCacheAddress, GpuCacheHandle};
 use gpu_types::{TransformPalette, TransformPaletteId, UvRectKind};
 use internal_types::FastHashSet;
 use plane_split::{Clipper, Polygon, Splitter};
-use prim_store::{PictureIndex, PrimitiveInstance, SpaceMapper, VisibleFace, PrimitiveInstanceKind, PrimitiveUid};
-use prim_store::{get_raster_rects, PrimitiveDataInterner, PrimitiveDataStore, CoordinateSpaceMapping};
+use prim_store::{PictureIndex, PrimitiveInstance, SpaceMapper, VisibleFace, PrimitiveInstanceKind};
+use prim_store::{get_raster_rects, CoordinateSpaceMapping};
 use prim_store::{OpacityBindingStorage, PrimitiveTemplateKind, ImageInstanceStorage, OpacityBindingIndex, SizeKey};
+use render_backend::FrameResources;
 use render_task::{ClearMode, RenderTask, RenderTaskCacheEntryHandle, TileBlit};
 use render_task::{RenderTaskCacheKey, RenderTaskCacheKeyKind, RenderTaskId, RenderTaskLocation};
 use resource_cache::ResourceCache;
 use scene::{FilterOpHelpers, SceneProperties};
+use scene_builder::DocumentResources;
 use smallvec::SmallVec;
 use surface::{SurfaceDescriptor, TransformKey};
 use std::{mem, ops};
@@ -236,11 +239,11 @@ pub struct TileTransformIndex(u32);
 pub struct TileDescriptor {
     /// List of primitive unique identifiers. The uid is guaranteed
     /// to uniquely describe the content of the primitive.
-    pub prim_uids: Vec<PrimitiveUid>,
+    pub prim_uids: Vec<ItemUid>,
 
     /// List of clip node unique identifiers. The uid is guaranteed
     /// to uniquely describe the content of the clip node.
-    pub clip_uids: Vec<ClipUid>,
+    pub clip_uids: Vec<ItemUid>,
 
     /// List of local tile transform ids that are used to position
     /// the primitive and clip items above.
@@ -600,7 +603,7 @@ impl TileCache {
         prim_instance: &PrimitiveInstance,
         surface_spatial_node_index: SpatialNodeIndex,
         clip_scroll_tree: &ClipScrollTree,
-        prim_data_store: &PrimitiveDataStore,
+        resources: &FrameResources,
         clip_chain_nodes: &[ClipChainNode],
         pictures: &[PicturePrimitive],
         resource_cache: &ResourceCache,
@@ -612,7 +615,7 @@ impl TileCache {
             clip_scroll_tree,
         );
 
-        let prim_data = &prim_data_store[prim_instance.prim_data_handle];
+        let prim_data = &resources.as_common_data(&prim_instance);
 
         // Map the primitive local rect into the picture space.
         // TODO(gw): We should maybe store this in the primitive template
@@ -648,18 +651,18 @@ impl TileCache {
         // Build the list of resources that this primitive has dependencies on.
         let mut opacity_bindings: SmallVec<[PropertyBindingId; 4]> = SmallVec::new();
         let mut clip_chain_spatial_nodes: SmallVec<[SpatialNodeIndex; 8]> = SmallVec::new();
-        let mut clip_chain_uids: SmallVec<[ClipUid; 8]> = SmallVec::new();
+        let mut clip_chain_uids: SmallVec<[ItemUid; 8]> = SmallVec::new();
         let mut image_keys: SmallVec<[ImageKey; 8]> = SmallVec::new();
         let mut current_clip_chain_id = prim_instance.clip_chain_id;
 
         // Some primitives can not be cached (e.g. external video images)
         let is_cacheable = prim_instance.is_cacheable(
-            prim_data_store,
+            &resources.prim_data_store,
             resource_cache,
         );
 
         match prim_instance.kind {
-            PrimitiveInstanceKind::Picture { pic_index } => {
+            PrimitiveInstanceKind::Picture { pic_index,.. } => {
                 // Pictures can depend on animated opacity bindings.
                 let pic = &pictures[pic_index.0];
                 if let Some(PictureCompositeMode::Filter(FilterOp::Opacity(binding, _))) = pic.requested_composite_mode {
@@ -678,7 +681,8 @@ impl TileCache {
                     }
                 }
             }
-            PrimitiveInstanceKind::Image { image_instance_index, .. } => {
+            PrimitiveInstanceKind::Image { data_handle, image_instance_index, .. } => {
+                let prim_data = &resources.prim_data_store[data_handle];
                 let image_instance = &image_instances[image_instance_index];
                 let opacity_binding_index = image_instance.opacity_binding_index;
 
@@ -700,7 +704,8 @@ impl TileCache {
                     }
                 }
             }
-            PrimitiveInstanceKind::YuvImage { .. } => {
+            PrimitiveInstanceKind::YuvImage { data_handle, .. } => {
+                let prim_data = &resources.prim_data_store[data_handle];
                 match prim_data.kind {
                     PrimitiveTemplateKind::YuvImage { ref yuv_key, .. } => {
                         image_keys.extend_from_slice(yuv_key);
@@ -712,7 +717,7 @@ impl TileCache {
             }
             PrimitiveInstanceKind::TextRun { .. } |
             PrimitiveInstanceKind::LineDecoration { .. } |
-            PrimitiveInstanceKind::Clear |
+            PrimitiveInstanceKind::Clear { .. } |
             PrimitiveInstanceKind::NormalBorder { .. } |
             PrimitiveInstanceKind::LinearGradient { .. } |
             PrimitiveInstanceKind::RadialGradient { .. } |
@@ -777,7 +782,7 @@ impl TileCache {
                 }
 
                 // Update the tile descriptor, used for tile comparison during scene swaps.
-                tile.descriptor.prim_uids.push(prim_instance.prim_data_handle.uid());
+                tile.descriptor.prim_uids.push(prim_instance.uid());
                 tile.descriptor.clip_uids.extend_from_slice(&clip_chain_uids);
             }
         }
@@ -1258,7 +1263,7 @@ impl PrimitiveList {
     /// significantly faster.
     pub fn new(
         mut prim_instances: Vec<PrimitiveInstance>,
-        prim_interner: &PrimitiveDataInterner,
+        resources: &DocumentResources
     ) -> Self {
         let mut pictures = SmallVec::new();
         let mut clusters_map = FastHashMap::default();
@@ -1271,7 +1276,7 @@ impl PrimitiveList {
             // Check if this primitive is a picture. In future we should
             // remove this match and embed this info directly in the primitive instance.
             let is_pic = match prim_instance.kind {
-                PrimitiveInstanceKind::Picture { pic_index } => {
+                PrimitiveInstanceKind::Picture { pic_index, .. } => {
                     pictures.push(pic_index);
                     true
                 }
@@ -1280,9 +1285,26 @@ impl PrimitiveList {
                 }
             };
 
+            let prim_data = match prim_instance.kind {
+                PrimitiveInstanceKind::Picture { data_handle, .. } |
+                PrimitiveInstanceKind::LineDecoration { data_handle, .. } |
+                PrimitiveInstanceKind::NormalBorder { data_handle, .. } |
+                PrimitiveInstanceKind::ImageBorder { data_handle, .. } |
+                PrimitiveInstanceKind::Rectangle { data_handle, .. } |
+                PrimitiveInstanceKind::YuvImage { data_handle, .. } |
+                PrimitiveInstanceKind::Image { data_handle, .. } |
+                PrimitiveInstanceKind::LinearGradient { data_handle, .. } |
+                PrimitiveInstanceKind::RadialGradient { data_handle, ..} |
+                PrimitiveInstanceKind::Clear {  data_handle, .. } => {
+                    &resources.prim_interner[data_handle]
+                }
+                PrimitiveInstanceKind::TextRun { data_handle, .. } => {
+                    &resources.text_run_interner[data_handle]
+                }
+            };
+
             // Get the key for the cluster that this primitive should
             // belong to.
-            let prim_data = &prim_interner[prim_instance.prim_data_handle];
             let key = PrimitiveClusterKey {
                 spatial_node_index: prim_instance.spatial_node_index,
                 is_backface_visible: prim_data.is_backface_visible,
@@ -1908,7 +1930,7 @@ impl PicturePrimitive {
         state: &mut PictureUpdateState,
         frame_context: &FrameBuildingContext,
         resource_cache: &mut ResourceCache,
-        prim_data_store: &PrimitiveDataStore,
+        resources: &FrameResources,
         pictures: &[PicturePrimitive],
         clip_store: &ClipStore,
         opacity_binding_store: &OpacityBindingStorage,
@@ -1926,7 +1948,7 @@ impl PicturePrimitive {
                     prim_instance,
                     surface_spatial_node_index,
                     &frame_context.clip_scroll_tree,
-                    prim_data_store,
+                    resources,
                     &clip_store.clip_chain_nodes,
                     pictures,
                     resource_cache,

gfx/wr/webrender/src/prim_store/text_run.rs

@@ -0,0 +1,329 @@
+/* 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/. */
+
+use api::{AuHelpers, ColorF, DevicePixelScale, GlyphInstance, LayoutPrimitiveInfo};
+use api::{LayoutToWorldTransform, LayoutVector2DAu, RasterSpace};
+use api::Shadow;
+use display_list_flattener::{AsInstanceKind, CreateShadow, IsVisible};
+use frame_builder::{FrameBuildingState, PictureContext};
+use glyph_rasterizer::{FontInstance, FontTransform, GlyphKey, FONT_SIZE_LIMIT};
+use gpu_cache::GpuCache;
+use intern;
+use prim_store::{PrimitiveOpacity, PrimitiveSceneData,  PrimitiveScratchBuffer};
+use prim_store::{PrimitiveStore, PrimKeyCommonData, PrimTemplateCommonData};
+use render_task::{RenderTaskTree};
+use renderer::{MAX_VERTEX_TEXTURE_WIDTH};
+use resource_cache::{ResourceCache};
+use tiling::SpecialRenderPasses;
+use util::{MatrixHelpers};
+use prim_store::PrimitiveInstanceKind;
+use std::ops;
+use storage;
+
+/// A run of glyphs, with associated font information.
+#[cfg_attr(feature = "capture", derive(Serialize))]
+#[cfg_attr(feature = "replay", derive(Deserialize))]
+#[derive(Debug, Clone, Eq, PartialEq, Hash)]
+pub struct TextRunKey {
+    pub common: PrimKeyCommonData,
+    pub font: FontInstance,
+    pub offset: LayoutVector2DAu,
+    pub glyphs: Vec<GlyphInstance>,
+    pub shadow: bool,
+}
+
+impl TextRunKey {
+    pub fn new(info: &LayoutPrimitiveInfo, text_run: TextRun) -> Self {
+        TextRunKey {
+            common: PrimKeyCommonData::with_info(info),
+            font: text_run.font,
+            offset: text_run.offset.into(),
+            glyphs: text_run.glyphs,
+            shadow: text_run.shadow,
+        }
+    }
+}
+
+impl AsInstanceKind<TextRunDataHandle> for TextRunKey {
+    /// Construct a primitive instance that matches the type
+    /// of primitive key.
+    fn as_instance_kind(
+        &self,
+        data_handle: TextRunDataHandle,
+        prim_store: &mut PrimitiveStore,
+    ) -> PrimitiveInstanceKind {
+        let run_index = prim_store.text_runs.push(TextRunPrimitive {
+            used_font: self.font.clone(),
+            glyph_keys_range: storage::Range::empty(),
+            shadow: self.shadow,
+        });
+
+        PrimitiveInstanceKind::TextRun{ data_handle, run_index }
+    }
+}
+
+#[cfg_attr(feature = "capture", derive(Serialize))]
+#[cfg_attr(feature = "replay", derive(Deserialize))]
+pub struct TextRunTemplate {
+    pub common: PrimTemplateCommonData,
+    pub font: FontInstance,
+    pub offset: LayoutVector2DAu,
+    pub glyphs: Vec<GlyphInstance>,
+}
+
+impl ops::Deref for TextRunTemplate {
+    type Target = PrimTemplateCommonData;
+    fn deref(&self) -> &Self::Target {
+        &self.common
+    }
+}
+
+impl ops::DerefMut for TextRunTemplate {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.common
+    }
+}
+
+impl From<TextRunKey> for TextRunTemplate {
+    fn from(item: TextRunKey) -> Self {
+        let common = PrimTemplateCommonData::with_key_common(item.common);
+        TextRunTemplate {
+            common,
+            font: item.font,
+            offset: item.offset,
+            glyphs: item.glyphs,
+        }
+    }
+}
+
+impl TextRunTemplate {
+    /// Update the GPU cache for a given primitive template. This may be called multiple
+    /// times per frame, by each primitive reference that refers to this interned
+    /// template. The initial request call to the GPU cache ensures that work is only
+    /// done if the cache entry is invalid (due to first use or eviction).
+    pub fn update(
+        &mut self,
+        frame_state: &mut FrameBuildingState,
+    ) {
+        self.write_prim_gpu_blocks(frame_state);
+        self.opacity = PrimitiveOpacity::translucent();
+    }
+
+    fn write_prim_gpu_blocks(
+        &mut self,
+        frame_state: &mut FrameBuildingState,
+    ) {
+        if let Some(mut request) = frame_state.gpu_cache.request(&mut self.common.gpu_cache_handle) {
+            request.push(ColorF::from(self.font.color).premultiplied());
+            // this is the only case where we need to provide plain color to GPU
+            let bg_color = ColorF::from(self.font.bg_color);
+            request.push([bg_color.r, bg_color.g, bg_color.b, 1.0]);
+            request.push([
+                self.offset.x.to_f32_px(),
+                self.offset.y.to_f32_px(),
+                0.0,
+                0.0,
+            ]);
+
+            let mut gpu_block = [0.0; 4];
+            for (i, src) in self.glyphs.iter().enumerate() {
+                // Two glyphs are packed per GPU block.
+
+                if (i & 1) == 0 {
+                    gpu_block[0] = src.point.x;
+                    gpu_block[1] = src.point.y;
+                } else {
+                    gpu_block[2] = src.point.x;
+                    gpu_block[3] = src.point.y;
+                    request.push(gpu_block);
+                }
+            }
+
+            // Ensure the last block is added in the case
+            // of an odd number of glyphs.
+            if (self.glyphs.len() & 1) != 0 {
+                request.push(gpu_block);
+            }
+
+            assert!(request.current_used_block_num() <= MAX_VERTEX_TEXTURE_WIDTH);
+        }
+    }
+}
+
+#[cfg_attr(feature = "capture", derive(Serialize))]
+#[cfg_attr(feature = "replay", derive(Deserialize))]
+#[derive(Clone, Copy, Debug, Hash, Eq, PartialEq)]
+pub struct TextRunDataMarker;
+
+pub type TextRunDataStore = intern::DataStore<TextRunKey, TextRunTemplate, TextRunDataMarker>;
+pub type TextRunDataHandle = intern::Handle<TextRunDataMarker>;
+pub type TextRunDataUpdateList = intern::UpdateList<TextRunKey>;
+pub type TextRunDataInterner = intern::Interner<TextRunKey, PrimitiveSceneData, TextRunDataMarker>;
+
+pub struct TextRun {
+    pub font: FontInstance,
+    pub offset: LayoutVector2DAu,
+    pub glyphs: Vec<GlyphInstance>,
+    pub shadow: bool,
+}
+
+impl intern::Internable for TextRun {
+    type Marker = TextRunDataMarker;
+    type Source = TextRunKey;
+    type StoreData = TextRunTemplate;
+    type InternData = PrimitiveSceneData;
+
+    /// Build a new key from self with `info`.
+    fn build_key(self, info: &LayoutPrimitiveInfo) -> TextRunKey {
+        TextRunKey::new(info, self)
+    }
+}
+
+impl CreateShadow for TextRun {
+    fn create_shadow(&self, shadow: &Shadow) -> Self {
+        let mut font = FontInstance {
+            color: shadow.color.into(),
+            ..self.font.clone()
+        };
+        if shadow.blur_radius > 0.0 {
+            font.disable_subpixel_aa();
+        }
+
+        TextRun {
+            font,
+            glyphs: self.glyphs.clone(),
+            offset: self.offset + shadow.offset.to_au(),
+            shadow: true
+        }
+    }
+}
+
+impl IsVisible for TextRun {
+    fn is_visible(&self) -> bool {
+        self.font.color.a > 0
+    }
+}
+
+#[derive(Debug)]
+pub struct TextRunPrimitive {
+    pub used_font: FontInstance,
+    pub glyph_keys_range: storage::Range<GlyphKey>,
+    pub shadow: bool,
+}
+
+impl TextRunPrimitive {
+    pub fn update_font_instance(
+        &mut self,
+        specified_font: &FontInstance,
+        device_pixel_scale: DevicePixelScale,
+        transform: &LayoutToWorldTransform,
+        allow_subpixel_aa: bool,
+        raster_space: RasterSpace,
+    ) -> bool {
+        // Get the current font size in device pixels
+        let device_font_size = specified_font.size.scale_by(device_pixel_scale.0);
+
+        // Determine if rasterizing glyphs in local or screen space.
+        // Only support transforms that can be coerced to simple 2D transforms.
+        let transform_glyphs = if transform.has_perspective_component() ||
+           !transform.has_2d_inverse() ||
+           // Font sizes larger than the limit need to be scaled, thus can't use subpixels.
+           transform.exceeds_2d_scale(FONT_SIZE_LIMIT / device_font_size.to_f64_px()) ||
+           // Otherwise, ensure the font is rasterized in screen-space.
+           raster_space != RasterSpace::Screen {
+            false
+        } else {
+            true
+        };
+
+        // Get the font transform matrix (skew / scale) from the complete transform.
+        let font_transform = if transform_glyphs {
+            // Quantize the transform to minimize thrashing of the glyph cache.
+            FontTransform::from(transform).quantize()
+        } else {
+            FontTransform::identity()
+        };
+
+        // If the transform or device size is different, then the caller of
+        // this method needs to know to rebuild the glyphs.
+        let cache_dirty =
+            self.used_font.transform != font_transform ||
+            self.used_font.size != device_font_size;
+
+        // Construct used font instance from the specified font instance
+        self.used_font = FontInstance {
+            transform: font_transform,
+            size: device_font_size,
+            ..specified_font.clone()
+        };
+
+        // If subpixel AA is disabled due to the backing surface the glyphs
+        // are being drawn onto, disable it (unless we are using the
+        // specifial subpixel mode that estimates background color).
+        if (!allow_subpixel_aa && self.used_font.bg_color.a == 0) ||
+            // If using local space glyphs, we don't want subpixel AA.
+            !transform_glyphs {
+            self.used_font.disable_subpixel_aa();
+        }
+
+        cache_dirty
+    }
+
+    pub fn prepare_for_render(
+        &mut self,
+        specified_font: &FontInstance,
+        glyphs: &[GlyphInstance],
+        device_pixel_scale: DevicePixelScale,
+        transform: &LayoutToWorldTransform,
+        pic_context: &PictureContext,
+        resource_cache: &mut ResourceCache,
+        gpu_cache: &mut GpuCache,
+        render_tasks: &mut RenderTaskTree,
+        special_render_passes: &mut SpecialRenderPasses,
+        scratch: &mut PrimitiveScratchBuffer,
+    ) {
+        let cache_dirty = self.update_font_instance(
+            specified_font,
+            device_pixel_scale,
+            transform,
+            pic_context.allow_subpixel_aa,
+            pic_context.raster_space,
+        );
+
+        if self.glyph_keys_range.is_empty() || cache_dirty {
+            let subpx_dir = self.used_font.get_subpx_dir();
+
+            self.glyph_keys_range = scratch.glyph_keys.extend(
+                glyphs.iter().map(|src| {
+                    let world_offset = self.used_font.transform.transform(&src.point);
+                    let device_offset = device_pixel_scale.transform_point(&world_offset);
+                    GlyphKey::new(src.index, device_offset, subpx_dir)
+                }));
+        }
+
+        resource_cache.request_glyphs(
+            self.used_font.clone(),
+            &scratch.glyph_keys[self.glyph_keys_range],
+            gpu_cache,
+            render_tasks,
+            special_render_passes,
+        );
+    }
+}
+
+#[test]
+#[cfg(target_os = "linux")]
+fn test_struct_sizes() {
+    use std::mem;
+    // The sizes of these structures are critical for performance on a number of
+    // talos stress tests. If you get a failure here on CI, there's two possibilities:
+    // (a) You made a structure smaller than it currently is. Great work! Update the
+    //     test expectations and move on.
+    // (b) You made a structure larger. This is not necessarily a problem, but should only
+    //     be done with care, and after checking if talos performance regresses badly.
+    assert_eq!(mem::size_of::<TextRun>(), 112, "TextRun size changed");
+    assert_eq!(mem::size_of::<TextRunTemplate>(), 168, "TextRunTemplate size changed");
+    assert_eq!(mem::size_of::<TextRunKey>(), 144, "TextRunKey size changed");
+    assert_eq!(mem::size_of::<TextRunPrimitive>(), 88, "TextRunPrimitive size changed");
+}

gfx/wr/webrender/src/render_backend.rs

@@ -30,7 +30,9 @@ use frame_builder::{FrameBuilder, FrameBuilderConfig};
 use gpu_cache::GpuCache;
 use hit_test::{HitTest, HitTester};
 use internal_types::{DebugOutput, FastHashMap, FastHashSet, RenderedDocument, ResultMsg};
-use prim_store::{PrimitiveDataStore, PrimitiveScratchBuffer};
+use prim_store::{PrimitiveDataStore, PrimitiveScratchBuffer, PrimitiveInstance};
+use prim_store::{PrimitiveInstanceKind, PrimTemplateCommonData};
+use prim_store::text_run::TextRunDataStore;
 use profiler::{BackendProfileCounters, IpcProfileCounters, ResourceProfileCounters};
 use record::ApiRecordingReceiver;
 use renderer::{AsyncPropertySampler, PipelineInfo};
@@ -192,6 +194,7 @@ impl FrameStamp {
 // between display lists.
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
+#[derive(Default)]
 pub struct FrameResources {
     /// The store of currently active / available clip nodes. This is kept
     /// in sync with the clip interner in the scene builder for each document.
@@ -200,13 +203,32 @@ pub struct FrameResources {
     /// Currently active / available primitives. Kept in sync with the
     /// primitive interner in the scene builder, per document.
     pub prim_data_store: PrimitiveDataStore,
+    pub text_run_data_store: TextRunDataStore,
 }
 
 impl FrameResources {
-    fn new() -> Self {
-        FrameResources {
-            clip_data_store: ClipDataStore::new(),
-            prim_data_store: PrimitiveDataStore::new(),
+    pub fn as_common_data(
+        &self,
+        prim_inst: &PrimitiveInstance
+    ) -> &PrimTemplateCommonData {
+        match prim_inst.kind {
+            PrimitiveInstanceKind::Picture { data_handle, .. } |
+            PrimitiveInstanceKind::LineDecoration { data_handle, .. } |
+            PrimitiveInstanceKind::NormalBorder { data_handle, .. } |
+            PrimitiveInstanceKind::ImageBorder { data_handle, .. } |
+            PrimitiveInstanceKind::Rectangle { data_handle, .. } |
+            PrimitiveInstanceKind::YuvImage { data_handle, .. } |
+            PrimitiveInstanceKind::Image { data_handle, .. } |
+            PrimitiveInstanceKind::LinearGradient { data_handle, .. } |
+            PrimitiveInstanceKind::RadialGradient { data_handle, .. } |
+            PrimitiveInstanceKind::Clear { data_handle, .. } => {
+                let prim_data = &self.prim_data_store[data_handle];
+                &prim_data.common
+            }
+            PrimitiveInstanceKind::TextRun { data_handle, .. }  => {
+                let prim_data = &self.text_run_data_store[data_handle];
+                &prim_data.common
+            }
         }
     }
 }
@@ -289,7 +311,7 @@ impl Document {
             hit_tester_is_valid: false,
             rendered_frame_is_valid: false,
             has_built_scene: false,
-            resources: FrameResources::new(),
+            resources: FrameResources::default(),
             scratch: PrimitiveScratchBuffer::new(),
         }
     }
@@ -1183,6 +1205,7 @@ impl RenderBackend {
         if let Some(updates) = doc_resource_updates {
             doc.resources.clip_data_store.apply_updates(updates.clip_updates);
             doc.resources.prim_data_store.apply_updates(updates.prim_updates);
+            doc.resources.text_run_data_store.apply_updates(updates.text_run_updates);
         }
 
         // TODO: this scroll variable doesn't necessarily mean we scrolled. It is only used

gfx/wr/webrender/src/scene_builder.rs

@@ -12,8 +12,11 @@ use frame_builder::{FrameBuilderConfig, FrameBuilder};
 use clip::{ClipDataInterner, ClipDataUpdateList};
 use clip_scroll_tree::ClipScrollTree;
 use display_list_flattener::DisplayListFlattener;
+use intern::{Internable, Interner};
 use internal_types::{FastHashMap, FastHashSet};
-use prim_store::{PrimitiveDataInterner, PrimitiveDataUpdateList, PrimitiveStoreStats};
+use prim_store::{PrimitiveDataInterner, PrimitiveDataUpdateList, PrimitiveKeyKind};
+use prim_store::PrimitiveStoreStats;
+use prim_store::text_run::{TextRunDataInterner, TextRun, TextRunDataUpdateList};
 use resource_cache::FontInstanceMap;
 use render_backend::DocumentView;
 use renderer::{PipelineInfo, SceneBuilderHooks};
@@ -28,6 +31,7 @@ use std::time::Duration;
 pub struct DocumentResourceUpdates {
     pub clip_updates: ClipDataUpdateList,
     pub prim_updates: PrimitiveDataUpdateList,
+    pub text_run_updates: TextRunDataUpdateList,
 }
 
 /// Represents the work associated to a transaction before scene building.
@@ -159,20 +163,32 @@ pub enum SceneSwapResult {
 //   display lists is (a) fast (b) done during scene building.
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
+#[derive(Default)]
 pub struct DocumentResources {
     pub clip_interner: ClipDataInterner,
     pub prim_interner: PrimitiveDataInterner,
+    pub text_run_interner: TextRunDataInterner,
 }
 
-impl DocumentResources {
-    fn new() -> Self {
-        DocumentResources {
-            clip_interner: ClipDataInterner::new(),
-            prim_interner: PrimitiveDataInterner::new(),
-        }
+// Access to `DocumentResources` interners by `Internable`
+pub trait InternerMut<I: Internable>
+{
+    fn interner_mut(&mut self) -> &mut Interner<I::Source, I::InternData, I::Marker>;
+}
+
+impl InternerMut<PrimitiveKeyKind> for DocumentResources {
+    fn interner_mut(&mut self) -> &mut PrimitiveDataInterner {
+        &mut self.prim_interner
     }
 }
 
+impl InternerMut<TextRun> for DocumentResources {
+    fn interner_mut(&mut self) -> &mut TextRunDataInterner {
+        &mut self.text_run_interner
+    }
+}
+
+
 // A document in the scene builder contains the current scene,
 // as well as a persistent clip interner. This allows clips
 // to be de-duplicated, and persisted in the GPU cache between
@@ -187,7 +203,7 @@ impl Document {
     fn new(scene: Scene) -> Self {
         Document {
             scene,
-            resources: DocumentResources::new(),
+            resources: DocumentResources::default(),
             prim_store_stats: PrimitiveStoreStats::empty(),
         }
     }
@@ -341,10 +357,16 @@ impl SceneBuilder {
                     .prim_interner
                     .end_frame_and_get_pending_updates();
 
+                let text_run_updates = item
+                    .doc_resources
+                    .text_run_interner
+                    .end_frame_and_get_pending_updates();
+
                 doc_resource_updates = Some(
                     DocumentResourceUpdates {
                         clip_updates,
                         prim_updates,
+                        text_run_updates,
                     }
                 );
 
@@ -453,10 +475,16 @@ impl SceneBuilder {
                     .prim_interner
                     .end_frame_and_get_pending_updates();
 
+                let text_run_updates = doc
+                    .resources
+                    .text_run_interner
+                    .end_frame_and_get_pending_updates();
+
                 doc_resource_updates = Some(
                     DocumentResourceUpdates {
                         clip_updates,
                         prim_updates,
+                        text_run_updates,
                     }
                 );
 

gfx/wr/webrender/src/surface.rs

@@ -3,11 +3,12 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use api::{LayoutPixel, PicturePixel, RasterSpace};
-use clip::{ClipChainId, ClipStore, ClipUid};
+use clip::{ClipChainId, ClipStore};
 use clip_scroll_tree::{ClipScrollTree, SpatialNodeIndex};
 use euclid::TypedTransform3D;
+use intern::ItemUid;
 use internal_types::FastHashSet;
-use prim_store::{CoordinateSpaceMapping, PrimitiveUid, PrimitiveInstance, PrimitiveInstanceKind};
+use prim_store::{CoordinateSpaceMapping, PrimitiveInstance, PrimitiveInstanceKind};
 use std::hash;
 use util::ScaleOffset;
 
@@ -165,10 +166,10 @@ impl<F, T> From<CoordinateSpaceMapping<F, T>> for TransformKey {
 pub struct SurfaceCacheKey {
     /// The list of primitives that are part of this surface.
     /// The uid uniquely identifies the content of the primitive.
-    pub primitive_ids: Vec<PrimitiveUid>,
+    pub primitive_ids: Vec<ItemUid>,
     /// The list of clips that affect the primitives on this surface.
     /// The uid uniquely identifies the content of the clip.
-    pub clip_ids: Vec<ClipUid>,
+    pub clip_ids: Vec<ItemUid>,
     /// A list of transforms that can affect the contents of primitives
     /// and/or clips on this picture surface.
     pub transforms: Vec<TransformKey>,
@@ -235,25 +236,13 @@ impl SurfaceDescriptor {
             // For now, we only handle interned primitives. If we encounter
             // a legacy primitive or picture, then fail to create a cache
             // descriptor.
-            match prim_instance.kind {
-                PrimitiveInstanceKind::Picture { .. } => {
-                    return None;
-                }
-                PrimitiveInstanceKind::Image { .. } |
-                PrimitiveInstanceKind::YuvImage { .. } |
-                PrimitiveInstanceKind::LineDecoration { .. } |
-                PrimitiveInstanceKind::LinearGradient { .. } |
-                PrimitiveInstanceKind::RadialGradient { .. } |
-                PrimitiveInstanceKind::TextRun { .. } |
-                PrimitiveInstanceKind::NormalBorder { .. } |
-                PrimitiveInstanceKind::Rectangle { .. } |
-                PrimitiveInstanceKind::ImageBorder { .. } |
-                PrimitiveInstanceKind::Clear => {}
+            if let PrimitiveInstanceKind::Picture { .. } = prim_instance.kind {
+                return None;
             }
 
             // Record the unique identifier for the content represented
             // by this primitive.
-            primitive_ids.push(prim_instance.prim_data_handle.uid());
+            primitive_ids.push(prim_instance.uid());
         }
 
         // Get a list of spatial nodes that are relevant for the contents

js/src/old-configure.in

@@ -572,7 +572,7 @@ dnl ========================================================
 
 case "$target" in
 *-darwin*)
-    MOZ_OPTIMIZE_FLAGS="-O3 -fno-stack-protector"
+    MOZ_OPTIMIZE_FLAGS="-O3"
     CFLAGS="$CFLAGS -fno-common"
     CXXFLAGS="$CXXFLAGS -fno-common -stdlib=libc++"
     DSO_LDOPTS=''

python/mozbuild/mozbuild/mach_commands.py

@@ -12,6 +12,7 @@ import logging
 import operator
 import os
 import re
+import shutil
 import subprocess
 import sys
 import tempfile
@@ -564,9 +565,8 @@ class Warnings(MachCommandBase):
         joined_path = mozpath.join(dir1, dir2)
         if os.path.isdir(joined_path):
             return joined_path
-        else:
-            print('Specified directory not found.')
-            return None
+        print('Specified directory not found.')
+        return None
 
 @CommandProvider
 class GTestCommands(MachCommandBase):
@@ -1311,7 +1311,6 @@ class PackageFrontend(MachCommandBase):
         from requests.adapters import HTTPAdapter
         import redo
         import requests
-        import shutil
 
         from taskgraph.util.taskcluster import (
             get_artifact_url,
@@ -1931,13 +1930,13 @@ class StaticAnalysis(MachCommandBase):
                 infer_args + [gradle] + args,
                 env=dict(os.environ, **extra_env),
                 cwd=cwd, stdout=devnull, stderr=subprocess.STDOUT, close_fds=True)
-        else:
-            return self.run_process(
-                infer_args + [gradle] + args,
-                append_env=extra_env,
-                pass_thru=True,  # Allow user to run gradle interactively.
-                ensure_exit_code=False,  # Don't throw on non-zero exit code.
-                cwd=cwd)
+
+        return self.run_process(
+            infer_args + [gradle] + args,
+            append_env=extra_env,
+            pass_thru=True,  # Allow user to run gradle interactively.
+            ensure_exit_code=False,  # Don't throw on non-zero exit code.
+            cwd=cwd)
 
     @StaticAnalysisSubCommand('static-analysis', 'autotest',
                               'Run the auto-test suite in order to determine that'
@@ -2012,7 +2011,6 @@ class StaticAnalysis(MachCommandBase):
 
         import concurrent.futures
         import multiprocessing
-        import shutil
 
         max_workers = multiprocessing.cpu_count()
 
@@ -2221,7 +2219,6 @@ class StaticAnalysis(MachCommandBase):
             return self.TOOLS_GRADLE_FAILED
         issues = json.load(open(mozpath.join(out_folder, 'report.json')))
         # remove folder that infer creates because the issues are loaded into memory
-        import shutil
         shutil.rmtree(out_folder)
         # Verify to see if we got any issues, if not raise exception
         if not issues:
@@ -2367,9 +2364,9 @@ class StaticAnalysis(MachCommandBase):
         if path is None:
             return self._run_clang_format_diff(self._clang_format_diff,
                                                self._clang_format_path, show)
-        else:
-            if assume_filename:
-                return self._run_clang_format_in_console(self._clang_format_path, path, assume_filename)
+
+        if assume_filename:
+            return self._run_clang_format_in_console(self._clang_format_path, path, assume_filename)
 
         return self._run_clang_format_path(self._clang_format_path, show, path)
 
@@ -2524,26 +2521,26 @@ class StaticAnalysis(MachCommandBase):
         self._compile_db = mozpath.join(self.topobjdir, 'compile_commands.json')
         if os.path.exists(self._compile_db):
             return 0
-        else:
-            rc, config, ran_configure = self._get_config_environment()
+
+        rc, config, ran_configure = self._get_config_environment()
+        if rc != 0:
+            return rc
+
+        if ran_configure:
+            # Configure may have created the compilation database if the
+            # mozconfig enables building the CompileDB backend by default,
+            # So we recurse to see if the file exists once again.
+            return self._build_compile_db(verbose=verbose)
+
+        if config:
+            print('Looks like a clang compilation database has not been '
+                  'created yet, creating it now...')
+            builder = Build(self._mach_context)
+            rc = builder.build_backend(['CompileDB'], verbose=verbose)
             if rc != 0:
                 return rc
-
-            if ran_configure:
-                # Configure may have created the compilation database if the
-                # mozconfig enables building the CompileDB backend by default,
-                # So we recurse to see if the file exists once again.
-                return self._build_compile_db(verbose=verbose)
-
-            if config:
-                print('Looks like a clang compilation database has not been '
-                      'created yet, creating it now...')
-                builder = Build(self._mach_context)
-                rc = builder.build_backend(['CompileDB'], verbose=verbose)
-                if rc != 0:
-                    return rc
-                assert os.path.exists(self._compile_db)
-                return 0
+            assert os.path.exists(self._compile_db)
+            return 0
 
     def _build_export(self, jobs, verbose=False):
         def on_line(line):
@@ -2611,7 +2608,6 @@ class StaticAnalysis(MachCommandBase):
         if os.path.isdir(self._clang_tools_path) and download_if_needed:
             # The directory exists, perhaps it's corrupted?  Delete it
             # and start from scratch.
-            import shutil
             shutil.rmtree(self._clang_tools_path)
             return self._get_clang_tools(force=force, skip_cache=skip_cache,
                                             source=source, verbose=verbose,
@@ -2634,8 +2630,8 @@ class StaticAnalysis(MachCommandBase):
             raise Exception('The current platform isn\'t supported. '
                             'Currently only the following platforms are '
                             'supported: win32/win64, linux64 and macosx64.')
-        else:
-            job += '-clang-tidy'
+
+        job += '-clang-tidy'
 
         # We want to unpack data in the clang-tidy mozbuild folder
         currentWorkingDir = os.getcwd()
@@ -2714,35 +2710,34 @@ class StaticAnalysis(MachCommandBase):
             return not os.path.exists(self._infer_path)
         if os.path.exists(self._infer_path) and not force:
             return 0
+
+        if os.path.isdir(infer_path) and download_if_needed:
+            # The directory exists, perhaps it's corrupted?  Delete it
+            # and start from scratch.
+            shutil.rmtree(infer_path)
+            return self._get_infer(force=force, skip_cache=skip_cache,
+                                   verbose=verbose,
+                                   download_if_needed=download_if_needed)
+        os.mkdir(infer_path)
+        self._artifact_manager = PackageFrontend(self._mach_context)
+        if not download_if_needed:
+            return 0
+        job, _ = self.platform
+        if job != 'linux64':
+            return -1
         else:
-            if os.path.isdir(infer_path) and download_if_needed:
-                # The directory exists, perhaps it's corrupted?  Delete it
-                # and start from scratch.
-                import shutil
-                shutil.rmtree(infer_path)
-                return self._get_infer(force=force, skip_cache=skip_cache,
-                                       verbose=verbose,
-                                       download_if_needed=download_if_needed)
-            os.mkdir(infer_path)
-            self._artifact_manager = PackageFrontend(self._mach_context)
-            if not download_if_needed:
-                return 0
-            job, _ = self.platform
-            if job != 'linux64':
-                return -1
-            else:
-                job += '-infer'
-            # We want to unpack data in the infer mozbuild folder
-            currentWorkingDir = os.getcwd()
-            os.chdir(infer_path)
-            rc = self._artifact_manager.artifact_toolchain(verbose=verbose,
-                                                           skip_cache=skip_cache,
-                                                           from_build=[job],
-                                                           no_unpack=False,
-                                                           retry=0)
-            # Change back the cwd
-            os.chdir(currentWorkingDir)
-            return rc
+            job += '-infer'
+        # We want to unpack data in the infer mozbuild folder
+        currentWorkingDir = os.getcwd()
+        os.chdir(infer_path)
+        rc = self._artifact_manager.artifact_toolchain(verbose=verbose,
+                                                       skip_cache=skip_cache,
+                                                       from_build=[job],
+                                                       no_unpack=False,
+                                                       retry=0)
+        # Change back the cwd
+        os.chdir(currentWorkingDir)
+        return rc
 
     def _run_clang_format_diff(self, clang_format_diff, clang_format, show):
         # Run clang-format on the diff
@@ -2831,7 +2826,6 @@ class StaticAnalysis(MachCommandBase):
             return 0
 
     def _run_clang_format_path(self, clang_format, show, paths):
-        import shutil
 
         # Run clang-format on files or directories directly
         from subprocess import check_output, CalledProcessError

security/manager/ssl/StaticHPKPins.h

@@ -1171,4 +1171,4 @@ static const TransportSecurityPreload kPublicKeyPinningPreloadList[] = {
 
 static const int32_t kUnknownId = -1;
 
-static const PRTime kPreloadPKPinsExpirationTime = INT64_C(1552306193127000);
+static const PRTime kPreloadPKPinsExpirationTime = INT64_C(1552565349122000);

services/settings/dumps/main/sites-classification.json

@@ -1 +1 @@
-{"data":[{"type":"known-hijacker-v2","weight":5,"criteria":[{"url":"https://search.avast.com/AV752/"},{"url":"https://www.bing.com/?pc=COSP&ptag=N752A9E7F85937E&form=CONMHP&conlogo=CT3210127"},{"url":"https://www.yahoo.com/?fr=hp-avast&type=752"},{"url":"https://www.google.com/?bcutc=sp-004-752"},{"hostname":"mystart.incredibar.com"},{"hostname":"search.babylon.com"},{"hostname":"astromenda.com"},{"hostname":"ask.com"},{"hostname":"binkiland.com"},{"hostname":"esurf.biz"},{"hostname":"search.conduit.com"},{"hostname":"en.4yendex.com"},{"hostname":"istartsurf.com"},{"hostname":"sear4m.xyz"},{"hostname":"search-daily.com"},{"hostname":"searchhult.com"},{"hostname":"searchgol.com"},{"hostname":"searchnu.com"},{"hostname":"shorte.st"},{"hostname":"snap.do"},{"hostname":"taplika.com/"},{"hostname":"vosteran.com"},{"hostname":"trovi.com"}],"id":"dc211d38-924a-4aec-8375-75f7d9abff56","last_modified":1541012656964},{"type":"search-engine-mozilla-tag","weight":4,"criteria":[{"sld":"google","params":[{"key":"client","prefix":"firefox"}]},{"params":[{"key":"t","prefix":"ff"}],"hostname":"duckduckgo.com"},{"params":[{"key":"tn","prefix":"monline_dg"}],"hostname":"baidu.com"},{"params":[{"key":"pc","prefix":"MOZ"}],"hostname":"bing.com"},{"params":[{"key":"pc","prefix":"MZ"}],"hostname":"bing.com"}],"id":"351864eb-eca8-4c84-b036-b847d046c864","last_modified":1539907365852},{"type":"search-engine-other-tag","weight":3,"criteria":[{"sld":"google","params":[{"key":"client"}]},{"params":[{"key":"t"}],"hostname":"duckduckgo.com"},{"params":[{"key":"tn"}],"hostname":"baidu.com"},{"params":[{"key":"pc"}],"hostname":"bing.com"},{"params":[{"key":"fr"}],"hostname":"search.yahoo.com"},{"params":[{"key":"hsimp"}],"hostname":"search.yahoo.com"}],"id":"9a3ed0f2-4207-4d03-8bed-6c38242dbdbd","last_modified":1539907352029},{"type":"search-engine","weight":2,"criteria":[{"sld":"google"},{"hostname":"duckduckgo.com"},{"hostname":"baidu.com"},{"hostname":"bing.com"},{"hostname":"search.yahoo.com"},{"sld":"yandex"}],"id":"c32a3278-e9ba-4090-8269-49d262fdea04","last_modified":1539907334747},{"type":"news-portal","weight":1,"criteria":[{"sld":"yahoo"},{"hostname":"msn.com"},{"hostname":"digg.com"},{"hostname":"aol.com"},{"hostname":"cnn.com"},{"hostname":"bbc.com"},{"hostname":"bbc.co.uk"},{"hostname":"nytimes.com"},{"hostname":"qq.com"},{"hostname":"sohu.com"}],"id":"12743b39-7f79-4620-bc37-a87b8ab2ba46","last_modified":1539907318995},{"type":"social-media","weight":1,"criteria":[{"hostname":"facebook.com"},{"hostname":"twitter.com"},{"hostname":"instagram.com"},{"hostname":"reddit.com"},{"hostname":"myspace.com"},{"hostname":"linkedin.com"},{"hostname":"pinterest.com"},{"hostname":"quora.com"},{"hostname":"tuenti.com"},{"hostname":"tumblr.com"},{"hostname":"untappd.com"},{"hostname":"yammer.com"},{"hostname":"slack.com"},{"hostname":"youtube.com"},{"hostname":"vk.com"},{"hostname":"twitch.tv"}],"id":"bdde6edd-83a5-4c34-b302-4810e9d10d04","last_modified":1539907302037},{"type":"ecommerce","weight":1,"criteria":[{"sld":"amazon"},{"sld":"ebay"},{"hostname":"alibaba.com"},{"hostname":"walmart.com"},{"hostname":"taobao.com"},{"hostname":"tmall.com"},{"hostname":"flipkart.com"},{"hostname":"snapdeal.com"},{"hostname":"bestbuy.com"},{"hostname":"jabong.com"}],"id":"48ec9db9-0de1-49aa-9b82-1a2372dce31e","last_modified":1539907283705}]}
+{"data":[{"type":"known-hijacker-v3","weight":5,"criteria":[{"url":"https://search.avast.com/AV752/"},{"url":"https://www.bing.com/?pc=COSP&ptag=N752A9E7F85937E&form=CONMHP&conlogo=CT3210127"},{"url":"https://www.yahoo.com/?fr=hp-avast&type=752"},{"url":"https://www.google.com/?bcutc=sp-004-752"},{"hostname":"mystart.incredibar.com"},{"hostname":"search.babylon.com"},{"hostname":"astromenda.com"},{"hostname":"ask.com"},{"hostname":"binkiland.com"},{"hostname":"esurf.biz"},{"hostname":"search.conduit.com"},{"hostname":"en.4yendex.com"},{"hostname":"istartsurf.com"},{"hostname":"sear4m.xyz"},{"hostname":"search-daily.com"},{"hostname":"searchhult.com"},{"hostname":"searchgol.com"},{"hostname":"searchnu.com"},{"hostname":"shorte.st"},{"hostname":"snap.do"},{"hostname":"taplika.com/"},{"hostname":"vosteran.com"},{"hostname":"trovi.com"},{"hostname":"myway.com"},{"hostname":"ap.myway.com"},{"hostname":"av.myway.com"},{"hostname":"bc.myway.com"},{"hostname":"canada.myway.com"},{"hostname":"db.myway.com"},{"hostname":"dc.myway.com"},{"hostname":"dell.myway.com"},{"hostname":"di.myway.com"},{"hostname":"ed.myway.com"},{"hostname":"es.myway.com"},{"hostname":"games.myway.com"},{"hostname":"gr.myway.com"},{"hostname":"finance.myway.com"},{"hostname":"fr.myway.com"},{"hostname":"health.myway.com"},{"hostname":"hp.myway.com"},{"hostname":"home.myway.com"},{"hostname":"info.myway.com"},{"hostname":"iq.myway.com"},{"hostname":"me.myway.com"},{"hostname":"my.myway.com"},{"hostname":"news.myway.com"},{"hostname":"nl.myway.com"},{"hostname":"nr.myway.com"},{"hostname":"ns.myway.com"},{"hostname":"nt.myway.com"},{"hostname":"ro.myway.com"},{"hostname":"search.myway.com"},{"hostname":"shipping.myway.com"},{"hostname":"sw.myway.com"},{"hostname":"ta.myway.com"},{"hostname":"td.myway.com"},{"hostname":"tr.myway.com"},{"hostname":"ts.myway.com"},{"hostname":"video.myway.com"},{"hostname":"www1.myway.com"},{"hostname":"www2.myway.com"}],"id":"dc211d38-924a-4aec-8375-75f7d9abff56","last_modified":1544035467383},{"type":"search-engine-mozilla-tag","weight":4,"criteria":[{"sld":"google","params":[{"key":"client","prefix":"firefox"}]},{"params":[{"key":"t","prefix":"ff"}],"hostname":"duckduckgo.com"},{"params":[{"key":"tn","prefix":"monline_dg"}],"hostname":"baidu.com"},{"params":[{"key":"pc","prefix":"MOZ"}],"hostname":"bing.com"},{"params":[{"key":"pc","prefix":"MZ"}],"hostname":"bing.com"}],"id":"351864eb-eca8-4c84-b036-b847d046c864","last_modified":1539907365852},{"type":"search-engine-other-tag","weight":3,"criteria":[{"sld":"google","params":[{"key":"client"}]},{"params":[{"key":"t"}],"hostname":"duckduckgo.com"},{"params":[{"key":"tn"}],"hostname":"baidu.com"},{"params":[{"key":"pc"}],"hostname":"bing.com"},{"params":[{"key":"fr"}],"hostname":"search.yahoo.com"},{"params":[{"key":"hsimp"}],"hostname":"search.yahoo.com"}],"id":"9a3ed0f2-4207-4d03-8bed-6c38242dbdbd","last_modified":1539907352029},{"type":"search-engine","weight":2,"criteria":[{"sld":"google"},{"hostname":"duckduckgo.com"},{"hostname":"baidu.com"},{"hostname":"bing.com"},{"hostname":"search.yahoo.com"},{"sld":"yandex"}],"id":"c32a3278-e9ba-4090-8269-49d262fdea04","last_modified":1539907334747},{"type":"news-portal","weight":1,"criteria":[{"sld":"yahoo"},{"hostname":"msn.com"},{"hostname":"digg.com"},{"hostname":"aol.com"},{"hostname":"cnn.com"},{"hostname":"bbc.com"},{"hostname":"bbc.co.uk"},{"hostname":"nytimes.com"},{"hostname":"qq.com"},{"hostname":"sohu.com"}],"id":"12743b39-7f79-4620-bc37-a87b8ab2ba46","last_modified":1539907318995},{"type":"social-media","weight":1,"criteria":[{"hostname":"facebook.com"},{"hostname":"twitter.com"},{"hostname":"instagram.com"},{"hostname":"reddit.com"},{"hostname":"myspace.com"},{"hostname":"linkedin.com"},{"hostname":"pinterest.com"},{"hostname":"quora.com"},{"hostname":"tuenti.com"},{"hostname":"tumblr.com"},{"hostname":"untappd.com"},{"hostname":"yammer.com"},{"hostname":"slack.com"},{"hostname":"youtube.com"},{"hostname":"vk.com"},{"hostname":"twitch.tv"}],"id":"bdde6edd-83a5-4c34-b302-4810e9d10d04","last_modified":1539907302037},{"type":"ecommerce","weight":1,"criteria":[{"sld":"amazon"},{"sld":"ebay"},{"hostname":"alibaba.com"},{"hostname":"walmart.com"},{"hostname":"taobao.com"},{"hostname":"tmall.com"},{"hostname":"flipkart.com"},{"hostname":"snapdeal.com"},{"hostname":"bestbuy.com"},{"hostname":"jabong.com"}],"id":"48ec9db9-0de1-49aa-9b82-1a2372dce31e","last_modified":1539907283705}]}

testing/mozharness/external_tools/robustcheckout.py

@@ -301,7 +301,7 @@ def robustcheckout(ui, url, dest, upstream=None, revision=None, branch=None,
 
         # We break out overall operations primarily by their network interaction
         # We have variants within for working directory operations.
-        if 'clone' in behaviors:
+        if 'clone' in behaviors and 'create-store' in behaviors:
             record_op('overall_clone')
 
             if 'sparse-update' in behaviors:
@@ -309,7 +309,7 @@ def robustcheckout(ui, url, dest, upstream=None, revision=None, branch=None,
             else:
                 record_op('overall_clone_fullcheckout')
 
-        elif 'pull' in behaviors:
+        elif 'pull' in behaviors or 'clone' in behaviors:
             record_op('overall_pull')
 
             if 'sparse-update' in behaviors:
@@ -609,6 +609,9 @@ def _docheckout(ui, url, dest, upstream, revision, branch, purge, sharebase,
         if upstream:
             ui.write('(cloning from upstream repo %s)\n' % upstream)
 
+        if not storevfs.exists():
+            behaviors.add('create-store')
+
         try:
             with timeit('clone', 'clone'):
                 shareopts = {'pool': sharebase, 'mode': 'identity'}
@@ -786,29 +789,6 @@ def _docheckout(ui, url, dest, upstream, revision, branch, purge, sharebase,
 
     ui.write('updated to %s\n' % checkoutrevision)
 
-    # HACK workaround https://bz.mercurial-scm.org/show_bug.cgi?id=5905
-    # and https://bugzilla.mozilla.org/show_bug.cgi?id=1462323.
-    #
-    # hg.mozilla.org's load balancer may route requests to different origin
-    # servers, thus exposing inconsistent state of repositories to the peer.
-    # This confuses Mercurial into promoting draft changesets to public. And
-    # this confuses various processes that rely on changeset phases being
-    # accurate.
-    #
-    # Our hack is to verify that changesets on non-publishing repositories are
-    # draft and to nuke the repo if they are wrong. We only apply this hack to
-    # the Try repo because it is the only repo where we can safely assume
-    # that the requested revision must be draft. This is because CI is triggered
-    # from special changesets that when pushed are always heads. And since the
-    # repo is non-publishing, these changesets should never be public.
-    if url in ('https://hg.mozilla.org/try',
-               'https://hg.mozilla.org/try-comm-central'):
-        if repo[checkoutrevision].phase() == phases.public:
-            ui.write(_('error: phase of revision is public; this is likely '
-                       'a manifestation of bug 1462323; the task will be '
-                       'retried\n'))
-            return EXIT_PURGE_CACHE
-
     return None
 
 

testing/web-platform/meta/__dir__.ini

@@ -1,5 +1,5 @@
 lsan-allowed: [js_pod_malloc, js_pod_calloc, js_pod_realloc, js_arena_calloc,js_pod_arena_calloc, maybe_pod_calloc, pod_calloc, make_zeroed_pod_array, js_arena_malloc]
 leak-threshold:
   if webrender: [tab:10000, geckomediaplugin:20000, default:16000]
-  [tab:10000, geckomediaplugin:20000]
+  [tab:10000, geckomediaplugin:20000, rdd:400]
 

testing/web-platform/mozilla/meta/__dir__.ini

@@ -1 +1,3 @@
 lsan-allowed: [Init, nsHostResolver::ResolveHost]
+leak-threshold:
+  [rdd:400]

toolkit/components/telemetry/Scalars.yaml

@@ -2288,6 +2288,40 @@ update:
     record_in_processes:
       - main
 
+# The following section contains search counters.
+browser.search:
+  with_ads:
+    bug_numbers:
+      - 1495548
+      - 1505411
+    description: >
+      Records counts of SERP pages with adverts displayed. The key format is ‘<engine-name>’.
+    expires: never
+    keyed: true
+    kind: uint
+    notification_emails:
+      - fx-search@mozilla.com
+      - adw@mozilla.com
+    release_channel_collection: opt-out
+    record_in_processes:
+      - main
+
+  ad_clicks:
+    bug_numbers:
+      - 1495548
+      - 1505411
+    description: >
+      Records clicks of adverts on SERP pages. The key format is ‘<engine-name>’.
+    expires: never
+    keyed: true
+    kind: uint
+    notification_emails:
+      - fx-search@mozilla.com
+      - adw@mozilla.com
+    release_channel_collection: opt-out
+    record_in_processes:
+      - main
+
 # The following section is for probes testing the Telemetry system. They will not be
 # submitted in pings and are only used for testing.
 telemetry.test:

toolkit/themes/shared/in-content/common.inc.css

@@ -422,11 +422,11 @@ html|textarea {
   padding: 5px 8px;
 }
 
-html|input[type="email"]:enabled:hover,
-html|input[type="tel"]:enabled:hover,
-html|input[type="text"]:enabled:hover,
-html|textarea:enabled:hover,
-xul|textbox:not([disabled="true"]):hover {
+html|input[type="email"]:enabled:not(:focus):hover,
+html|input[type="tel"]:enabled:not(:focus):hover,
+html|input[type="text"]:enabled:not(:focus):hover,
+html|textarea:enabled:not(:focus):hover,
+xul|textbox:not([disabled="true"]):not([focused]):hover {
   border-color: var(--in-content-border-hover);
 }
 

widget/gtk/mozcontainer.cpp

@@ -177,7 +177,7 @@ static void moz_container_map_wayland(MozContainer *container) {
   container->surface_needs_clear = true;
   container->ready_to_draw = false;
 
-  static wl_surface *gtk_container_surface =
+  wl_surface *gtk_container_surface =
       moz_container_get_gtk_container_surface(container);
   container->frame_callback_handler = wl_surface_frame(gtk_container_surface);
   wl_callback_add_listener(container->frame_callback_handler, &frame_listener,