merge fx-team to mozilla-central a=merge

devtools/client/memory/actions/snapshot.js

@@ -6,9 +6,23 @@
 const { PROMISE } = require("devtools/client/shared/redux/middleware/promise");
 const { actions } = require("../constants");
 
+/**
+ * @param {MemoryFront}
+ */
 const takeSnapshot = exports.takeSnapshot = function takeSnapshot (front) {
   return {
     type: actions.TAKE_SNAPSHOT,
     [PROMISE]: front.saveHeapSnapshot()
   };
 };
+
+/**
+ * @param {Snapshot}
+ * @see {Snapshot} model defined in devtools/client/memory/app.js
+ */
+const selectSnapshot = exports.selectSnapshot = function takeSnapshot (snapshot) {
+  return {
+    type: actions.SELECT_SNAPSHOT,
+    snapshot
+  };
+};

devtools/client/memory/app.js

@@ -0,0 +1,77 @@
+const { DOM: dom, createClass, createFactory, PropTypes } = require("devtools/client/shared/vendor/react");
+const { connect } = require("devtools/client/shared/vendor/react-redux");
+const { selectSnapshot, takeSnapshot } = require("./actions/snapshot");
+const Toolbar = createFactory(require("./components/toolbar"));
+const List = createFactory(require("./components/list"));
+const SnapshotListItem = createFactory(require("./components/snapshot-list-item"));
+
+const stateModel = {
+  /**
+   * {MemoryFront}
+   * Used to communicate with the platform.
+   */
+  front: PropTypes.any,
+
+  /**
+   * {Array<Snapshot>}
+   * List of references to all snapshots taken
+   */
+  snapshots: PropTypes.arrayOf(PropTypes.shape({
+    id: PropTypes.number.isRequired,
+    snapshotId: PropTypes.string,
+    selected: PropTypes.bool.isRequired,
+    status: PropTypes.oneOf([
+      "start",
+      "done",
+      "error",
+    ]).isRequired,
+  }))
+};
+
+
+const App = createClass({
+  displayName: "memory-tool",
+
+  propTypes: stateModel,
+
+  childContextTypes: {
+    front: PropTypes.any,
+  },
+
+  getChildContext() {
+    return {
+      front: this.props.front,
+    }
+  },
+
+  render() {
+    let { dispatch, snapshots, front } = this.props;
+    return (
+      dom.div({ id: "memory-tool" }, [
+
+        Toolbar({
+          buttons: [{
+            className: "take-snapshot",
+            onClick: () => dispatch(takeSnapshot(front))
+          }]
+        }),
+
+        List({
+          itemComponent: SnapshotListItem,
+          items: snapshots,
+          onClick: snapshot => dispatch(selectSnapshot(snapshot))
+        })
+      ])
+    );
+  },
+});
+
+/**
+ * Passed into react-redux's `connect` method that is called on store change
+ * and passed to components.
+ */
+function mapStateToProps (state) {
+  return { snapshots: state.snapshots };
+}
+
+module.exports = connect(mapStateToProps)(App);

devtools/client/memory/components/list.js

@@ -0,0 +1,28 @@
+const { DOM: dom, createClass, PropTypes } = require("devtools/client/shared/vendor/react");
+
+/**
+ * Generic list component that takes another react component to represent
+ * the children nodes as `itemComponent`, and a list of items to render
+ * as that component with a click handler.
+ */
+const List = module.exports = createClass({
+  displayName: "list",
+
+  propTypes: {
+    itemComponent: PropTypes.any.isRequired,
+    onClick: PropTypes.func,
+    items: PropTypes.array.isRequired,
+  },
+
+  render() {
+    let { items, onClick, itemComponent: Item } = this.props;
+
+    return (
+      dom.ul({ className: "list" }, items.map((item, index) => {
+        return Item({
+          item, index, onClick: () => onClick(item),
+        });
+      }))
+    );
+  }
+});

devtools/client/memory/components/moz.build

@@ -0,0 +1,10 @@
+# vim: set filetype=python:
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+DevToolsModules(
+    'list.js',
+    'snapshot-list-item.js',
+    'toolbar.js',
+)

devtools/client/memory/components/snapshot-list-item.js

@@ -0,0 +1,22 @@
+const { DOM: dom, createClass, PropTypes } = require("devtools/client/shared/vendor/react");
+
+const SnapshotListItem = module.exports = createClass({
+  displayName: "snapshot-list-item",
+
+  propTypes: {
+    onClick: PropTypes.func,
+    item: PropTypes.any.isRequired,
+    index: PropTypes.number.isRequired,
+  },
+
+  render() {
+    let { index, item, onClick } = this.props;
+    let className = `snapshot-list-item ${item.selected ? " selected" : ""}`;
+    return (
+      dom.li({ className, onClick },
+        dom.span({ className: "snapshot-title" }, `Snapshot #${index}`)
+      )
+    );
+  }
+});
+

devtools/client/memory/components/toolbar.js

@@ -0,0 +1,16 @@
+const { DOM, createClass } = require("devtools/client/shared/vendor/react");
+
+const Toolbar = module.exports = createClass({
+  displayName: "toolbar",
+
+  render() {
+    let buttons = this.props.buttons;
+    return (
+      DOM.div({ className: "devtools-toolbar" }, ...buttons.map(spec => {
+        return DOM.button(Object.assign({}, spec, {
+          className: `${spec.className || "" } devtools-button`
+        }));
+      }))
+    );
+  }
+});

devtools/client/memory/constants.js

@@ -7,3 +7,6 @@ const actions = exports.actions = {};
 
 // Fired by UI to request a snapshot from the actor.
 actions.TAKE_SNAPSHOT = "take-snapshot";
+
+// Fired by UI to select a snapshot to view.
+actions.SELECT_SNAPSHOT = "select-snapshot";

devtools/client/memory/controller.js

@@ -1,28 +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 { Task } = require("resource://gre/modules/Task.jsm");
-const Store = require("./store");
-
-/**
- * The current target, toolbox and MemoryFront, set by this tool's host.
- */
-var gToolbox, gTarget, gFront;
-
-const REDUX_METHODS_TO_PIPE = ["dispatch", "subscribe", "getState"];
-
-const MemoryController = exports.MemoryController = function ({ toolbox, target, front }) {
-  this.store = Store();
-  this.toolbox = toolbox;
-  this.target = target;
-  this.front = front;
-};
-
-REDUX_METHODS_TO_PIPE.map(m =>
-  MemoryController.prototype[m] = function (...args) { return this.store[m](...args); });
-
-MemoryController.prototype.destroy = function () {
-  this.store = this.toolbox = this.target = this.front = null;
-};

devtools/client/memory/initializer.js

@@ -3,10 +3,15 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
-var { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components;
-const { require } = Cu.import("resource://gre/modules/devtools/Loader.jsm", {});
+const { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components;
+const BrowserLoaderModule = {};
+Cu.import("resource:///modules/devtools/client/shared/browser-loader.js", BrowserLoaderModule);
+const { require } = BrowserLoaderModule.BrowserLoader("resource:///modules/devtools/client/memory/", this);
 const { Task } = require("resource://gre/modules/Task.jsm");
-const { MemoryController } = require("devtools/client/memory/controller");
+const { createFactory, createElement, render } = require("devtools/client/shared/vendor/react");
+const { Provider } = require("devtools/client/shared/vendor/react-redux");
+const App = createFactory(require("devtools/client/memory/app"));
+const Store = require("devtools/client/memory/store");
 
 /**
  * The current target, toolbox and MemoryFront, set by this tool's host.
@@ -14,17 +19,20 @@ const { MemoryController } = require("devtools/client/memory/controller");
 var gToolbox, gTarget, gFront;
 
 /**
- * Initializes the profiler controller and views.
+ * The current target, toolbox and MemoryFront, set by this tool's host.
  */
-var controller = null;
+var gToolbox, gTarget, gFront;
+
 function initialize () {
-  return Task.spawn(function *() {
-    controller = new MemoryController({ toolbox: gToolbox, target: gTarget, front: gFront });
+  return Task.spawn(function*() {
+    let root = document.querySelector("#app");
+    let store = Store();
+    let app = createElement(App, { front: gFront });
+    let provider = createElement(Provider, { store }, app);
+    render(provider, root);
   });
 }
 
 function destroy () {
-  return Task.spawn(function *() {
-    controller.destroy();
-  });
+  return Task.spawn(function*(){});
 }

devtools/client/memory/memory.xhtml

@@ -11,8 +11,6 @@
    - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
 <html xmlns="http://www.w3.org/1999/xhtml">
   <head>
-    <link rel="stylesheet" href="chrome://browser/skin/" type="text/css"/>
-    <link rel="stylesheet" href="chrome://browser/content/devtools/widgets.css" type="text/css"/>
     <link rel="stylesheet" href="chrome://devtools/skin/themes/common.css" type="text/css"/>
     <link rel="stylesheet" href="chrome://devtools/skin/themes/widgets.css" type="text/css"/>
     <link rel="stylesheet" href="chrome://devtools/skin/themes/memory.css" type="text/css"/>
@@ -23,18 +21,7 @@
             src="initializer.js"></script>
   </head>
   <body class="theme-body">
-    <div class="devtools-toolbar">
-      <div id="snapshot-button" class="devtools-toolbarbutton" />
-    </div>
-    <div class="devtools-horizontal-splitter"></div>
-    <div id="memory-content"
-         class="devtools-responsive-container"
-         flex="1">
-      <toolbar class="devtools-toolbar">
-        <spacer flex="1"></spacer>
-      </toolbar>
-      <hbox flex="1">
-      </hbox>
+    <div id="app">
     </div>
   </body>
 </html>

devtools/client/memory/moz.build

@@ -5,18 +5,20 @@
 
 DIRS += [
     'actions',
+    'components',
     'modules',
     'reducers',
 ]
 
 DevToolsModules(
+    'app.js',
     'constants.js',
-    'controller.js',
     'initializer.js',
     'panel.js',
     'reducers.js',
     'store.js',
 )
 
+BROWSER_CHROME_MANIFESTS += ['test/browser/browser.ini']
 MOCHITEST_CHROME_MANIFESTS += ['test/mochitest/chrome.ini']
 XPCSHELL_TESTS_MANIFESTS += ['test/unit/xpcshell.ini']

devtools/client/memory/reducers/snapshot.js

@@ -8,7 +8,9 @@ function handleTakeSnapshot (state, action) {
     case "start":
       return [...state, {
         id: action.seqId,
-        status: action.status
+        status: action.status,
+        // auto selected if this is the first snapshot
+        selected: state.length === 0
       }];
 
     case "done":
@@ -27,10 +29,25 @@ function handleTakeSnapshot (state, action) {
   return [...state];
 }
 
+function handleSelectSnapshot (state, action) {
+  let selected = state.find(s => s.id === action.snapshot.id);
+
+  if (!selected) {
+    DevToolsUtils.reportException(`Cannot select non-existant snapshot ${snapshot.id}`);
+  }
+
+  return state.map(s => {
+    s.selected = s === selected;
+    return s;
+  });
+}
+
 module.exports = function (state=[], action) {
   switch (action.type) {
     case actions.TAKE_SNAPSHOT:
       return handleTakeSnapshot(state, action);
+    case actions.SELECT_SNAPSHOT:
+      return handleSelectSnapshot(state, action);
   }
 
   return state;

devtools/client/memory/store.js

@@ -4,5 +4,6 @@ const reducers = require("./reducers");
 const DevToolsUtils = require("devtools/shared/DevToolsUtils");
 
 module.exports = function () {
-  return createStore({ log: DevToolsUtils.testing })(combineReducers(reducers), {});
+  let shouldLog = DevToolsUtils.testing;
+  return createStore({ log: shouldLog })(combineReducers(reducers), {});
 };

devtools/client/memory/test/unit/head.js

@@ -12,12 +12,11 @@ var { TargetFactory } = require("devtools/client/framework/target");
 var DevToolsUtils = require("devtools/shared/DevToolsUtils");
 var promise = require("promise");
 var { Task } = Cu.import("resource://gre/modules/Task.jsm", {});
-var { MemoryController } = require("devtools/client/memory/controller");
 var { expectState } = require("devtools/server/actors/common");
 var HeapSnapshotFileUtils = require("devtools/shared/heapsnapshot/HeapSnapshotFileUtils");
 var { addDebuggerToGlobal } = require("resource://gre/modules/jsdebugger.jsm");
+var Store = require("devtools/client/memory/store");
 var SYSTEM_PRINCIPAL = Cc["@mozilla.org/systemprincipal;1"].createInstance(Ci.nsIPrincipal);
-var { setTimeout } = require("sdk/timers");
 
 DevToolsUtils.testing = true;
 
@@ -46,11 +45,17 @@ StubbedMemoryFront.prototype.saveHeapSnapshot = expectState("attached", Task.asy
 
 function waitUntilState (store, predicate) {
   let deferred = promise.defer();
-  let unsubscribe = store.subscribe(() => {
+  let unsubscribe = store.subscribe(check);
+
+  function check () {
     if (predicate(store.getState())) {
       unsubscribe();
       deferred.resolve()
     }
-  });
+  }
+
+  // Fire the check immediately incase the action has already occurred
+  check();
+
   return deferred.promise;
 }

devtools/client/memory/test/unit/test_action-select-snapshot.js

@@ -0,0 +1,38 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Tests the reducer responding to the action `selectSnapshot(snapshot)`
+ */
+
+let actions = require("devtools/client/memory/actions/snapshot");
+
+function run_test() {
+  run_next_test();
+}
+
+add_task(function *() {
+  let front = new StubbedMemoryFront();
+  yield front.attach();
+  let store = Store();
+
+  for (let i = 0; i < 5; i++) {
+    store.dispatch(actions.takeSnapshot(front));
+  }
+
+  yield waitUntilState(store, ({ snapshots }) => snapshots.length === 5 && snapshots.every(isDone));
+
+  ok(store.getState().snapshots[0].selected, "snapshot[0] selected by default");
+
+  for (let i = 1; i < 5; i++) {
+    do_print(`Selecting snapshot[${i}]`);
+    store.dispatch(actions.selectSnapshot(store.getState().snapshots[i]));
+    yield waitUntilState(store, ({ snapshots }) => snapshots[i].selected);
+
+    let { snapshots } = store.getState();
+    ok(snapshots[i].selected, `snapshot[${i}] selected`);
+    equal(snapshots.filter(s => !s.selected).length, 4, "All other snapshots are unselected");
+  }
+});
+
+function isDone (s) { return s.status === "done"; }

devtools/client/memory/test/unit/test_action-take-snapshot.js

@@ -2,7 +2,7 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 /**
- * Tests the async action creator `takeSnapshot(front)`
+ * Tests the async reducer responding to the action `takeSnapshot(front)`
  */
 
 var actions = require("devtools/client/memory/actions/snapshot");
@@ -14,32 +14,50 @@ function run_test() {
 add_task(function *() {
   let front = new StubbedMemoryFront();
   yield front.attach();
-  let controller = new MemoryController({ toolbox: {}, target: {}, front });
+  let store = Store();
 
-  let unsubscribe = controller.subscribe(checkState);
+  let unsubscribe = store.subscribe(checkState);
 
   let foundPendingState = false;
   let foundDoneState = false;
+  let foundAllSnapshots = false;
 
   function checkState () {
-    let state = controller.getState();
-    if (state.snapshots.length === 1 && state.snapshots[0].status === "start") {
+    let { snapshots } = store.getState();
+
+    if (snapshots.length === 1 && snapshots[0].status === "start") {
       foundPendingState = true;
       ok(foundPendingState, "Got state change for pending heap snapshot request");
-      ok(!(state.snapshots[0].snapshotId), "Snapshot does not yet have a snapshotId");
+      ok(snapshots[0].selected, "First snapshot is auto-selected");
+      ok(!(snapshots[0].snapshotId), "Snapshot does not yet have a snapshotId");
     }
-    if (state.snapshots.length === 1 && state.snapshots[0].status === "done") {
+    if (snapshots.length === 1 && snapshots[0].status === "done") {
       foundDoneState = true;
       ok(foundDoneState, "Got state change for completed heap snapshot request");
-      ok(state.snapshots[0].snapshotId, "Snapshot fetched with a snapshotId");
+      ok(snapshots[0].snapshotId, "Snapshot fetched with a snapshotId");
     }
-    if (state.snapshots.lenght === 1 && state.snapshots[0].status === "error") {
+    if (snapshots.length === 1 && snapshots[0].status === "error") {
       ok(false, "takeSnapshot's promise returned with an error");
     }
+
+    if (snapshots.length === 5 && snapshots.every(s => s.status === "done")) {
+      foundAllSnapshots = true;
+      ok(snapshots.every(s => s.status === "done"), "All snapshots have a snapshotId");
+      equal(snapshots.length, 5, "Found 5 snapshots");
+      ok(snapshots.every(s => s.snapshotId), "All snapshots have a snapshotId");
+      ok(snapshots[0].selected, "First snapshot still selected");
+      equal(snapshots.filter(s => !s.selected).length, 4, "All other snapshots are unselected");
+    }
   }
 
-  controller.dispatch(actions.takeSnapshot(front));
-  yield waitUntilState(controller, () => foundPendingState && foundDoneState);
+  store.dispatch(actions.takeSnapshot(front));
 
+  yield waitUntilState(store, () => foundPendingState && foundDoneState);
+
+  for (let i = 0; i < 4; i++) {
+    store.dispatch(actions.takeSnapshot(front));
+  }
+
+  yield waitUntilState(store, () => foundAllSnapshots);
   unsubscribe();
 });

devtools/client/memory/test/unit/xpcshell.ini

@@ -5,4 +5,5 @@ tail =
 firefox-appdir = browser
 skip-if = toolkit == 'android' || toolkit == 'gonk'
 
+[test_action-select-snapshot.js]
 [test_action-take-snapshot.js]

devtools/client/shared/browser-loader.js

@@ -23,7 +23,7 @@ catch(e) {
   };
 }
 
-const VENDOR_CONTENT_URL = "resource:///modules/devtools/shared/vendor";
+const VENDOR_CONTENT_URL = "resource:///modules/devtools/client/shared/vendor";
 
 /*
  * Create a loader to be used in a browser environment. This evaluates

devtools/client/sourceeditor/codemirror/mode/htmlmixed.js

@@ -89,8 +89,8 @@
       tags.script.unshift(["type", configScript[i].matches, configScript[i].mode])
 
     function html(stream, state) {
-      var tagName = state.htmlState.tagName;
-      var tagInfo = tagName && tags[tagName.toLowerCase()];
+      var tagName = state.htmlState.tagName && state.htmlState.tagName.toLowerCase();
+      var tagInfo = tagName && tags.hasOwnProperty(tagName) && tags[tagName];
 
       var style = htmlMode.token(stream, state.htmlState), modeSpec;
 

devtools/client/themes/common.css

@@ -15,6 +15,10 @@
 %endif
 }
 
+.theme-body {
+  margin: 0;
+}
+
 .devtools-monospace {
   font-family: var(--monospace-font-family);
 %if defined(MOZ_WIDGET_GTK) || defined(MOZ_WIDGET_QT)

devtools/client/themes/memory.css

@@ -20,6 +20,94 @@
   --row-hover-background-color: rgba(76,158,217,0.2);
 }
 
+/**
+ * TODO bug 1213100
+ * should generalize toolbar buttons with images in them
+ * toolbars.inc.css contains definitions for .devtools-button,
+ * I wager that many of the below styles can be rolled into that
+ */
+.devtools-button.take-snapshot {
+  margin: 2px 1px;
+  padding: 1px;
+  border-width: 0px;
+  /* [standalone] buttons override min-height from 18px to 24px -- why? */
+  min-height: 18px;
+  /* not sure why this is needed for positioning */
+  display: -moz-box;
+}
+
+.devtools-button.take-snapshot::before {
+  background-image: url(images/command-screenshot.png);
+  -moz-appearance: none;
+  width: 16px;
+  height: 16px;
+  background-size: 64px 16px;
+  background-position: 0 center;
+  background-repeat: no-repeat;
+}
+@media (min-resolution: 1.1dppx) {
+  .devtools-button.take-snapshot::before {
+    background-image: url(images/command-screenshot@2x.png);
+  }
+}
+
+/**
+ * TODO bug 1213100
+ * Should this be codified in .devtools-toolbar itself?
+ */
+#memory-tool .devtools-toolbar {
+  display: flex;
+  flex-direction: row;
+  align-items: center;
+  height: 20px;
+}
+
+/**
+ * TODO bug 1213100
+ * Once we figure out how to store invertable buttons (pseudo element like in this case?)
+ * we should add a .invertable class to handle this generally, rather than the definitions
+ * in toolbars.inc.css.
+ *
+ * @see bug 1173397 for another inverted related bug
+ */
+.theme-light .devtools-toolbarbutton.take-snapshot::before {
+  filter: url(images/filters.svg#invert);
+}
+
+/**
+ * TODO bug 1213100
+ * The .list style is for a generalized React list component. It's children (.list > li)
+ * are generally styled here, as the component can take any type of child component.
+ * Memory tool specific styling are handling in (li.snapshot-list-item).
+ */
+
+.list {
+  margin: 0;
+  padding: 0;
+  width: 186px;
+  list-style-type: none;
+  font-size: 12px;
+}
+
+.list > li {
+  height: 40px;
+  color: var(--theme-body-color);
+  border-bottom: 1px solid transparent;
+  border-top: 1px solid rgba(128,128,128,0.15);
+  padding: 8px;
+  cursor: pointer;
+}
+
+.list > li.selected {
+  background-color: var(--theme-selection-background);
+  color: var(--theme-selection-color);
+}
+
+
+/**
+ * Heap View
+ */
+
 .heap-view {
   position: relative;
 }

mobile/android/base/home/SearchEngineRow.java

@@ -23,6 +23,10 @@ import org.mozilla.gecko.widget.FlowLayout;
 import android.content.Context;
 import android.content.SharedPreferences;
 import android.graphics.drawable.Drawable;
+import android.graphics.Typeface;
+import android.text.style.StyleSpan;
+import android.text.Spannable;
+import android.text.SpannableStringBuilder;
 import android.util.AttributeSet;
 import android.view.KeyEvent;
 import android.view.LayoutInflater;
@@ -74,6 +78,11 @@ class SearchEngineRow extends AnimatedHeightLayout {
     private int mMaxSavedSuggestions;
     private int mMaxSearchSuggestions;
 
+    // Styles for text in a suggestion 'button' that is not part of the first instance of mUserSearchTerm
+    // Even though they're the same style, SpannableStringBuilder will interpret there as being only one Span present if we re-use a StyleSpan
+    StyleSpan mPriorToSearchTerm;
+    StyleSpan mAfterSearchTerm;
+
     public SearchEngineRow(Context context) {
         this(context, null);
     }
@@ -139,6 +148,9 @@ class SearchEngineRow extends AnimatedHeightLayout {
         // Suggestion limits
         mMaxSavedSuggestions = getResources().getInteger(R.integer.max_saved_suggestions);
         mMaxSearchSuggestions = getResources().getInteger(R.integer.max_search_suggestions);
+
+        mPriorToSearchTerm = new StyleSpan(Typeface.BOLD);
+        mAfterSearchTerm = new StyleSpan(Typeface.BOLD);
     }
 
     private void setDescriptionOnSuggestion(View v, String suggestion) {
@@ -161,7 +173,20 @@ class SearchEngineRow extends AnimatedHeightLayout {
         }
 
         final TextView suggestionText = (TextView) v.findViewById(R.id.suggestion_text);
-        suggestionText.setText(suggestion);
+        final String searchTerm = getSuggestionTextFromView(mUserEnteredView);
+        // If there is more than one copy of mUserSearchTerm, only the first is not bolded
+        final int startOfSearchTerm = suggestion.indexOf(searchTerm);
+        // Sometimes the suggestion does not contain mUserSearmTerm at all, in which case, bold nothing
+        if (startOfSearchTerm >= 0) {
+            final int endOfSearchTerm = startOfSearchTerm + searchTerm.length();
+            final SpannableStringBuilder sb = new SpannableStringBuilder(suggestion);
+            sb.setSpan(mPriorToSearchTerm, 0, startOfSearchTerm, Spannable.SPAN_INCLUSIVE_INCLUSIVE);
+            sb.setSpan(mAfterSearchTerm, endOfSearchTerm, suggestion.length(), Spannable.SPAN_INCLUSIVE_INCLUSIVE);
+            suggestionText.setText(sb);
+        } else {
+            suggestionText.setText(suggestion);
+        }
+
         setDescriptionOnSuggestion(suggestionText, suggestion);
     }
 

mobile/android/chrome/content/browser.js

@@ -1339,15 +1339,16 @@ var BrowserApp = {
    * Otherwise, a new tab is opened with the given URL.
    *
    * @param aURL URL to open
-   * @param  aFlags Options for the search. Currently supports:
+   * @param aParam Options used if a tab is created
+   * @param aFlags Options for the search. Currently supports:
    **  @option startsWith a Boolean indicating whether to search for a tab who's url starts with the
    *           requested url. Useful if you want to ignore hash codes on the end of a url. For instance
    *           to have about:downloads match about:downloads#123.
    */
-  selectOrOpenTab: function selectOrOpenTab(aURL, aFlags) {
+  selectOrAddTab: function selectOrAddTab(aURL, aParams, aFlags) {
     let tab = this.getTabWithURL(aURL, aFlags);
     if (tab == null) {
-      tab = this.addTab(aURL);
+      tab = this.addTab(aURL, aParams);
     } else {
       this.selectTab(tab);
     }
@@ -5952,7 +5953,7 @@ var XPInstallObserver = {
     }).show((data) => {
       if (data.button === 0) {
         // TODO: Open about:addons to show only unsigned add-ons?
-        BrowserApp.addTab("about:addons", { parentId: BrowserApp.selectedTab.id });
+        BrowserApp.selectOrAddTab("about:addons", { parentId: BrowserApp.selectedTab.id });
       }
     });
   },
@@ -5980,7 +5981,10 @@ var XPInstallObserver = {
           button: {
             icon: "drawable://alert_addon",
             label: Strings.browser.GetStringFromName("alertAddonsInstalledNoRestart.action2"),
-            callback: () => { BrowserApp.addTab("about:addons#" + aAddon.id, { parentId: BrowserApp.selectedTab.id }); },
+            callback: () => {
+              UITelemetry.addEvent("show.1", "toast", null, "addons");
+              BrowserApp.selectOrAddTab("about:addons", { parentId: BrowserApp.selectedTab.id });
+            },
           }
         });
       }

mobile/android/modules/DownloadNotifications.jsm

@@ -19,7 +19,7 @@ XPCOMUtils.defineLazyModuleGetter(this, "Services", "resource://gre/modules/Serv
 XPCOMUtils.defineLazyServiceGetter(this, "ParentalControls",
   "@mozilla.org/parental-controls-service;1", "nsIParentalControlsService");
 
-var Log = Cu.import("resource://gre/modules/AndroidLog.jsm", {}).AndroidLog.i.bind(null, "DownloadNotifications"); 
+var Log = Cu.import("resource://gre/modules/AndroidLog.jsm", {}).AndroidLog.i.bind(null, "DownloadNotifications");
 
 XPCOMUtils.defineLazyGetter(this, "strings",
                             () => Services.strings.createBundle("chrome://browser/locale/browser.properties"));
@@ -121,8 +121,8 @@ var DownloadNotifications = {
   showInAboutDownloads: function (download) {
     let hash = "#" + window.encodeURIComponent(download.target.path);
 
-    // we can't use selectOrOpenTab, since it uses string equality to find a tab
-    window.BrowserApp.selectOrOpenTab("about:downloads" + hash, { startsWith: true });
+    // Force using string equality to find a tab
+    window.BrowserApp.selectOrAddTab("about:downloads" + hash, null, { startsWith: true });
   },
 
   onClick: function(cookie) {

services/common/tests/unit/test_tokenserverclient.js

@@ -100,6 +100,8 @@ add_test(function test_conditions_required_response_handling() {
   function onResponse(error, token) {
     do_check_true(error instanceof TokenServerClientServerError);
     do_check_eq(error.cause, "conditions-required");
+    // Check a JSON.stringify works on our errors as our logging will try and use it.
+    do_check_true(JSON.stringify(error), "JSON.stringify worked");
     do_check_null(token);
 
     do_check_eq(error.urls.tos, tosURL);

services/common/tokenserverclient.js

@@ -44,6 +44,11 @@ TokenServerClientError.prototype._toStringFields = function() {
 TokenServerClientError.prototype.toString = function() {
   return this.name + "(" + JSON.stringify(this._toStringFields()) + ")";
 }
+TokenServerClientError.prototype.toJSON = function() {
+  let result = this._toStringFields();
+  result["name"] = this.name;
+  return result;
+}
 
 /**
  * Represents a TokenServerClient error that occurred in the network layer.