Merge fx-team to m-c a=merge CLOSED TREE

browser/app/profile/firefox.js

@@ -1698,6 +1698,7 @@ pref("loop.debug.loglevel", "Error");
 pref("loop.debug.dispatcher", false);
 pref("loop.debug.websocket", false);
 pref("loop.debug.sdk", false);
+pref("loop.debug.twoWayMediaTelemetry", false);
 #ifdef DEBUG
 pref("loop.CSP", "default-src 'self' about: file: chrome: http://localhost:*; img-src 'self' data: https://www.gravatar.com/ about: file: chrome:; font-src 'none'; connect-src wss://*.tokbox.com https://*.opentok.com https://*.tokbox.com wss://*.mozilla.com https://*.mozilla.org wss://*.mozaws.net http://localhost:* ws://localhost:*; media-src blob:");
 #else
@@ -1878,5 +1879,7 @@ pref("dom.ipc.reportProcessHangs", true);
 pref("reader.parse-on-load.enabled", false);
 #endif
 
-// Disable ReadingList by default.
+// Disable ReadingList browser UI by default.
 pref("browser.readinglist.enabled", false);
+// Enable the readinglist engine by default.
+pref("readinglist.scheduler.enabled", true);

browser/base/content/browser-context.inc

@@ -312,7 +312,7 @@
       <menuitem id="context-shareselect"
                 label="&shareSelect.label;"
                 accesskey="&shareSelect.accesskey;"
-                oncommand="gContextMenu.shareSelect(getBrowserSelection());"/>
+                oncommand="gContextMenu.shareSelect();"/>
       <menuseparator id="frame-sep"/>
       <menu id="frame" label="&thisFrameMenu.label;" accesskey="&thisFrameMenu.accesskey;">
         <menupopup>

browser/base/content/browser-readinglist.js

@@ -232,12 +232,22 @@ let ReadingListUI = {
       // nothing to do if we have no button.
       return;
     }
-    if (!this.enabled || state == "invalid") {
+
+    let uri;
+    if (this.enabled && state == "valid") {
+      uri = gBrowser.currentURI;
+      if (uri.schemeIs("about"))
+        uri = ReaderParent.parseReaderUrl(uri.spec);
+      else if (!uri.schemeIs("http") && !uri.schemeIs("https"))
+        uri = null;
+    }
+
+    if (!uri) {
       this.toolbarButton.setAttribute("hidden", true);
       return;
     }
 
-    let isInList = yield ReadingList.containsURL(gBrowser.currentURI);
+    let isInList = yield ReadingList.containsURL(uri);
     this.setToolbarButtonState(isInList);
   }),
 
@@ -268,11 +278,17 @@ let ReadingListUI = {
    * @returns {Promise} Promise resolved when operation has completed.
    */
   togglePageByBrowser: Task.async(function* (browser) {
-    let item = yield ReadingList.getItemForURL(browser.currentURI);
+    let uri = browser.currentURI;
+    if (uri.spec.startsWith("about:reader?"))
+      uri = ReaderParent.parseReaderUrl(uri.spec);
+    if (!uri)
+      return;
+
+    let item = yield ReadingList.getItemForURL(uri);
     if (item) {
       yield item.delete();
     } else {
-      yield ReadingList.addItemFromBrowser(browser);
+      yield ReadingList.addItemFromBrowser(browser, uri);
     }
   }),
 
@@ -284,6 +300,9 @@ let ReadingListUI = {
    */
   isItemForCurrentBrowser(item) {
     let currentURL = gBrowser.currentURI.spec;
+    if (currentURL.startsWith("about:reader?"))
+      currentURL = ReaderParent.parseReaderUrl(currentURL);
+
     if (item.url == currentURL || item.resolvedURL == currentURL) {
       return true;
     }

browser/base/content/browser-social.js

@@ -658,7 +658,7 @@ SocialShare = {
         pageData.microdata = msg.data;
         this.sharePage(providerOrigin, pageData, target);
       });
-      gBrowser.selectedBrowser.messageManager.sendAsyncMessage("PageMetadata:GetMicrodata", null, target);
+      gBrowser.selectedBrowser.messageManager.sendAsyncMessage("PageMetadata:GetMicrodata", null, { target });
       return;
     }
     this.currentShare = pageData;

browser/base/content/content.js

@@ -486,6 +486,7 @@ let AboutReaderListener = {
   init: function() {
     addEventListener("AboutReaderContentLoaded", this, false, true);
     addEventListener("DOMContentLoaded", this, false);
+    addEventListener("pageshow", this, false);
     addEventListener("pagehide", this, false);
     addMessageListener("Reader:ParseDocument", this);
   },
@@ -525,6 +526,13 @@ let AboutReaderListener = {
         sendAsyncMessage("Reader:UpdateReaderButton", { isArticle: false });
         break;
 
+      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.
+        if (!aEvent.persisted) {
+          break;
+        }
+        // Fall through.
       case "DOMContentLoaded":
         if (!ReaderMode.isEnabledForParseOnLoad || this.isAboutReader) {
           return;
@@ -1016,7 +1024,7 @@ let PageMetadataMessenger = {
       }
 
       case "PageMetadata:GetMicrodata": {
-        let target = message.objects;
+        let target = message.objects.target;
         let result = PageMetadata.getMicrodata(content.document, target);
         sendAsyncMessage("PageMetadata:MicrodataResult", result);
         break;

browser/base/content/nsContextMenu.js

@@ -151,6 +151,7 @@ nsContextMenu.prototype = {
       if (uri && uri.host) {
         this.linkURI = uri;
         this.linkURL = this.linkURI.spec;
+        this.linkText = linkText;
         this.onPlainTextLink = true;
       }
     }
@@ -576,6 +577,7 @@ nsContextMenu.prototype = {
     this.link              = null;
     this.linkURL           = "";
     this.linkURI           = null;
+    this.linkText          = "";
     this.linkProtocol      = "";
     this.linkHasNoReferrer = false;
     this.onMathML          = false;
@@ -737,6 +739,7 @@ nsContextMenu.prototype = {
           this.link = elem;
           this.linkURL = this.getLinkURL();
           this.linkURI = this.getLinkURI();
+          this.linkText = this.getLinkText();
           this.linkProtocol = this.getLinkProtocol();
           this.onMailtoLink = (this.linkProtocol == "mailto");
           this.onSaveableLink = this.isLinkSaveable( this.link );
@@ -1302,15 +1305,8 @@ nsContextMenu.prototype = {
   // Save URL of clicked-on link.
   saveLink: function() {
     var doc =  this.target.ownerDocument;
-    var linkText;
-    // If selected text is found to match valid URL pattern.
-    if (this.onPlainTextLink)
-      linkText = this.focusedWindow.getSelection().toString().trim();
-    else
-      linkText = this.linkText();
     urlSecurityCheck(this.linkURL, this.principal);
-
+    this.saveHelper(this.linkURL, this.linkText, null, true, doc);
-    this.saveHelper(this.linkURL, linkText, null, true, doc);
   },
 
   // Backwards-compatibility wrapper
@@ -1503,7 +1499,7 @@ nsContextMenu.prototype = {
   },
 
   // Get text of link.
-  linkText: function() {
+  getLinkText: function() {
     var text = gatherTextUnder(this.link);
     if (!text || !text.match(/\S/)) {
       text = this.link.getAttribute("title");
@@ -1598,14 +1594,8 @@ nsContextMenu.prototype = {
   },
 
   bookmarkLink: function CM_bookmarkLink() {
-    var linkText;
+    window.top.PlacesCommandHook.bookmarkLink(PlacesUtils.bookmarksMenuFolderId,
-    // If selected text is found to match valid URL pattern.
+                                              this.linkURL, this.linkText);
-    if (this.onPlainTextLink)
-      linkText = this.focusedWindow.getSelection().toString().trim();
-    else
-      linkText = this.linkText();
-    window.top.PlacesCommandHook.bookmarkLink(PlacesUtils.bookmarksMenuFolderId, this.linkURL,
-                                              linkText);
   },
 
   addBookmarkForFrame: function CM_addBookmarkForFrame() {
@@ -1650,8 +1640,8 @@ nsContextMenu.prototype = {
     SocialShare.sharePage(null, { url: this.mediaURL, source: this.mediaURL }, this.target);
   },
 
-  shareSelect: function CM_shareSelect(selection) {
+  shareSelect: function CM_shareSelect() {
-    SocialShare.sharePage(null, { url: this.browser.currentURI.spec, text: selection }, this.target);
+    SocialShare.sharePage(null, { url: this.browser.currentURI.spec, text: this.textSelected }, this.target);
   },
 
   savePageAs: function CM_savePageAs() {
@@ -1699,7 +1689,7 @@ nsContextMenu.prototype = {
   // Formats the 'Search <engine> for "<selection or link text>"' context menu.
   formatSearchContextItem: function() {
     var menuItem = document.getElementById("context-searchselect");
-    var selectedText = this.isTextSelected ? this.textSelected : this.linkText();
+    let selectedText = this.isTextSelected ? this.textSelected : this.linkText;
 
     // Store searchTerms in context menu item so we know what to search onclick
     menuItem.searchTerms = selectedText;

browser/base/content/popup-notifications.inc

@@ -60,6 +60,14 @@
       </popupnotificationcontent>
     </popupnotification>
 
+    <popupnotification id="password-notification" hidden="true">
+      <popupnotificationcontent orient="vertical">
+        <textbox id="password-notification-username" disabled="true"/>
+        <textbox id="password-notification-password" type="password"
+                 disabled="true"/>
+      </popupnotificationcontent>
+    </popupnotification>
+
 #ifdef E10S_TESTING_ONLY
     <popupnotification id="enable-e10s-notification" hidden="true">
       <popupnotificationcontent orient="vertical"/>

browser/base/content/socialmarks.xml

@@ -163,7 +163,7 @@
             pageData.microdata = msg.data;
             this.loadPanel(pageData, target);
           });
-          gBrowser.selectedBrowser.messageManager.sendAsyncMessage("PageMetadata:GetMicrodata", null, target);
+          gBrowser.selectedBrowser.messageManager.sendAsyncMessage("PageMetadata:GetMicrodata", null, { target });
           return;
         }
         this.pageData = pageData;

browser/base/content/sync/customize.js

@@ -4,8 +4,31 @@
 
 "use strict";
 
+Components.utils.import("resource://gre/modules/Services.jsm");
+
+addEventListener("load", function () {
+  // unhide the reading-list engine if readinglist is enabled (note this
+  // dialog is only used with FxA sync, so no special action is needed
+  // for legacy sync.)
+  if (Services.prefs.getBoolPref("browser.readinglist.enabled")) {
+    document.getElementById("readinglist-engine").removeAttribute("hidden");
+  }
+});
+
 addEventListener("dialogaccept", function () {
   let pane = document.getElementById("sync-customize-pane");
+  // First determine what the preference for the "global" sync enabled pref
+  // should be based on the engines selected.
+  let prefElts = pane.querySelectorAll("preferences > preference");
+  let syncEnabled = false;
+  for (let elt of prefElts) {
+    if (elt.name.startsWith("services.sync.") && elt.value) {
+      syncEnabled = true;
+      break;
+    }
+  }
+  Services.prefs.setBoolPref("services.sync.enabled", syncEnabled);
+  // and write the individual prefs.
   pane.writePreferences(true);
   window.arguments[0].accepted = true;
 });

browser/base/content/sync/customize.xul

@@ -27,6 +27,8 @@
       <preference id="engine.passwords" name="services.sync.engine.passwords" type="bool"/>
       <preference id="engine.addons"    name="services.sync.engine.addons"    type="bool"/>
       <preference id="engine.prefs"     name="services.sync.engine.prefs"     type="bool"/>
+      <!-- non Sync-Engine engines -->
+      <preference id="engine.readinglist" name="readinglist.scheduler.enabled" type="bool"/>
     </preferences>
 
     <label id="sync-customize-title" value="&syncCustomize.title;"/>
@@ -51,6 +53,11 @@
       <checkbox label="&engine.history.label;"
                 accesskey="&engine.history.accesskey;"
                 preference="engine.history"/>
+      <checkbox id="readinglist-engine"
+                label="&engine.readinglist.label;"
+                accesskey="&engine.readinglist.accesskey;"
+                preference="engine.readinglist"
+                hidden="true"/>
       <checkbox label="&engine.addons.label;"
                 accesskey="&engine.addons.accesskey;"
                 preference="engine.addons"/>

browser/base/content/test/general/browser_getshortcutoruri.js

@@ -91,57 +91,54 @@ var testData = [
    new keywordResult(null, null, true)]
 ];
 
-function test() {
+add_task(function* test_getshortcutoruri() {
-  waitForExplicitFinish();
+  yield setupKeywords();
 
-  setupKeywords();
+  for (let item of testData) {
+    let [data, result] = item;
 
-  Task.spawn(function() {
+    let query = data.keyword;
-    for each (var item in testData) {
+    if (data.searchWord)
-      let [data, result] = item;
+      query += " " + data.searchWord;
+    let returnedData = yield new Promise(
+      resolve => getShortcutOrURIAndPostData(query, resolve));
+    // null result.url means we should expect the same query we sent in
+    let expected = result.url || query;
+    is(returnedData.url, expected, "got correct URL for " + data.keyword);
+    is(getPostDataString(returnedData.postData), result.postData, "got correct postData for " + data.keyword);
+    is(returnedData.mayInheritPrincipal, !result.isUnsafe, "got correct mayInheritPrincipal for " + data.keyword);
+  }
 
-      let query = data.keyword;
+  yield cleanupKeywords();
-      if (data.searchWord)
+});
-        query += " " + data.searchWord;
-      let returnedData = yield new Promise(
-        resolve => getShortcutOrURIAndPostData(query, resolve));
-      // null result.url means we should expect the same query we sent in
-      let expected = result.url || query;
-      is(returnedData.url, expected, "got correct URL for " + data.keyword);
-      is(getPostDataString(returnedData.postData), result.postData, "got correct postData for " + data.keyword);
-      is(returnedData.mayInheritPrincipal, !result.isUnsafe, "got correct mayInheritPrincipal for " + data.keyword);
-    }
-    cleanupKeywords();
-  }).then(finish);
-}
 
-var gBMFolder = null;
+let folder = null;
-var gAddedEngines = [];
+let gAddedEngines = [];
-function setupKeywords() {
+
-  gBMFolder = Application.bookmarks.menu.addFolder("keyword-test");
+function* setupKeywords() {
-  for each (var item in testData) {
+  folder = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
-    var data = item[0];
+                                                type: PlacesUtils.bookmarks.TYPE_FOLDER,
+                                                title: "keyword-test" });
+  for (let item of testData) {
+    let data = item[0];
     if (data instanceof bmKeywordData) {
-      var bm = gBMFolder.addBookmark(data.keyword, data.uri);
+      yield PlacesUtils.bookmarks.insert({ url: data.uri, parentGuid: folder.guid });
-      bm.keyword = data.keyword;
+      yield PlacesUtils.keywords.insert({ keyword: data.keyword, url: data.uri.spec, postData: data.postData });
-      if (data.postData)
-        bm.annotations.set("bookmarkProperties/POSTData", data.postData, Ci.nsIAnnotationService.EXPIRE_SESSION);
     }
 
     if (data instanceof searchKeywordData) {
       Services.search.addEngineWithDetails(data.keyword, "", data.keyword, "", data.method, data.uri.spec);
-      var addedEngine = Services.search.getEngineByName(data.keyword);
+      let addedEngine = Services.search.getEngineByName(data.keyword);
       if (data.postData) {
-        var [paramName, paramValue] = data.postData.split("=");
+        let [paramName, paramValue] = data.postData.split("=");
         addedEngine.addParam(paramName, paramValue, null);
       }
-
       gAddedEngines.push(addedEngine);
     }
   }
 }
 
-function cleanupKeywords() {
+function* cleanupKeywords() {
-  gBMFolder.remove();
+  PlacesUtils.bookmarks.remove(folder);
   gAddedEngines.map(Services.search.removeEngine);
 }

browser/base/content/test/general/browser_keywordBookmarklets.js

@@ -1,38 +1,34 @@
-/* Any copyright is dedicated to the Public Domain.
+"use strict"
-   http://creativecommons.org/publicdomain/zero/1.0/ */
 
-function test() {
+add_task(function* test_keyword_bookmarklet() {
-  waitForExplicitFinish();
+  let bm = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
-
+                                                title: "bookmarklet",
-  let bmFolder = Application.bookmarks.menu.addFolder("keyword-test");
+                                                url: "javascript:1;" });
   let tab = gBrowser.selectedTab = gBrowser.addTab();
-
+  registerCleanupFunction (function* () {
-  registerCleanupFunction (function () {
-    bmFolder.remove();
     gBrowser.removeTab(tab);
+    yield PlacesUtils.bookmarks.remove(bm);
   });
+  yield promisePageShow();
+  let originalPrincipal = gBrowser.contentPrincipal;
 
-  let bm = bmFolder.addBookmark("bookmarklet", makeURI("javascript:1;"));
+  yield PlacesUtils.keywords.insert({ keyword: "bm", url: "javascript:1;" })
-  bm.keyword = "bm";
 
-  addPageShowListener(function () {
+  // Enter bookmarklet keyword in the URL bar
-    let originalPrincipal = gBrowser.contentPrincipal;
+  gURLBar.value = "bm";
+  gURLBar.focus();
+  EventUtils.synthesizeKey("VK_RETURN", {});
 
-    // Enter bookmarklet keyword in the URL bar
+  yield promisePageShow();
-    gURLBar.value = "bm";
-    gURLBar.focus();
-    EventUtils.synthesizeKey("VK_RETURN", {});
 
-    addPageShowListener(function () {
+  ok(gBrowser.contentPrincipal.equals(originalPrincipal), "javascript bookmarklet should inherit principal");
-      ok(gBrowser.contentPrincipal.equals(originalPrincipal), "javascript bookmarklet should inherit principal");
+});
-      finish();
+
+function* promisePageShow() {
+  return new Promise(resolve => {
+    gBrowser.selectedBrowser.addEventListener("pageshow", function listen() {
+      gBrowser.selectedBrowser.removeEventListener("pageshow", listen);
+      resolve();
     });
   });
 }
-
-function addPageShowListener(func) {
-  gBrowser.selectedBrowser.addEventListener("pageshow", function loadListener() {
-    gBrowser.selectedBrowser.removeEventListener("pageshow", loadListener, false);
-    func();
-  });
-}

browser/base/content/test/general/browser_tab_detach_restore.js

@@ -21,8 +21,8 @@ add_task(function*() {
   is(SessionStore.getClosedWindowCount(), 1, "Should have restore data for the closed window");
 
   win = SessionStore.undoCloseWindow(0);
-  yield BrowserTestUtils.waitForEvent(win, "load", 10000);
+  yield BrowserTestUtils.waitForEvent(win, "load");
-  yield BrowserTestUtils.waitForEvent(win.gBrowser.tabs[0], "SSTabRestored", 10000);
+  yield BrowserTestUtils.waitForEvent(win.gBrowser.tabs[0], "SSTabRestored");
 
   is(win.gBrowser.tabs.length, 1, "Should have restored one tab");
   is(win.gBrowser.selectedBrowser.currentURI.spec, uri, "Should have restored the right page");

browser/base/content/test/general/readerModeArticle.html

@@ -11,6 +11,9 @@
 <h2 class="author">by Jane Doe</h2>
 <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec a diam lectus. Sed sit amet ipsum mauris. Maecenas congue ligula ac quam viverra nec consectetur ante hendrerit. Donec et mollis dolor. Praesent et diam eget libero egestas mattis sit amet vitae augue. Nam tincidunt congue enim, ut porta lorem lacinia consectetur. Donec ut libero sed arcu vehicula ultricies a non tortor. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean ut gravida lorem. Ut turpis felis, pulvinar a semper sed, adipiscing id dolor. Pellentesque auctor nisi id magna consequat sagittis. Curabitur dapibus enim sit amet elit pharetra tincidunt feugiat nisl imperdiet. Ut convallis libero in urna ultrices accumsan. Donec sed odio eros. Donec viverra mi quis quam pulvinar at malesuada arcu rhoncus. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. In rutrum accumsan ultricies. Mauris vitae nisi at sem facilisis semper ac in est.</p>
 <p>Vivamus fermentum semper porta. Nunc diam velit, adipiscing ut tristique vitae, sagittis vel odio. Maecenas convallis ullamcorper ultricies. Curabitur ornare, ligula semper consectetur sagittis, nisi diam iaculis velit, id fringilla sem nunc vel mi. Nam dictum, odio nec pretium volutpat, arcu ante placerat erat, non tristique elit urna et turpis. Quisque mi metus, ornare sit amet fermentum et, tincidunt et orci. Fusce eget orci a orci congue vestibulum. Ut dolor diam, elementum et vestibulum eu, porttitor vel elit. Curabitur venenatis pulvinar tellus gravida ornare. Sed et erat faucibus nunc euismod ultricies ut id justo. Nullam cursus suscipit nisi, et ultrices justo sodales nec. Fusce venenatis facilisis lectus ac semper. Aliquam at massa ipsum. Quisque bibendum purus convallis nulla ultrices ultricies. Nullam aliquam, mi eu aliquam tincidunt, purus velit laoreet tortor, viverra pretium nisi quam vitae mi. Fusce vel volutpat elit. Nam sagittis nisi dui.</p>
+<p>Vivamus fermentum semper porta. Nunc diam velit, adipiscing ut tristique vitae, sagittis vel odio. Maecenas convallis ullamcorper ultricies. Curabitur ornare, ligula semper consectetur sagittis, nisi diam iaculis velit, id fringilla sem nunc vel mi. Nam dictum, odio nec pretium volutpat, arcu ante placerat erat, non tristique elit urna et turpis. Quisque mi metus, ornare sit amet fermentum et, tincidunt et orci. Fusce eget orci a orci congue vestibulum. Ut dolor diam, elementum et vestibulum eu, porttitor vel elit. Curabitur venenatis pulvinar tellus gravida ornare. Sed et erat faucibus nunc euismod ultricies ut id justo. Nullam cursus suscipit nisi, et ultrices justo sodales nec. Fusce venenatis facilisis lectus ac semper. Aliquam at massa ipsum. Quisque bibendum purus convallis nulla ultrices ultricies. Nullam aliquam, mi eu aliquam tincidunt, purus velit laoreet tortor, viverra pretium nisi quam vitae mi. Fusce vel volutpat elit. Nam sagittis nisi dui.</p>
+<p>Vivamus fermentum semper porta. Nunc diam velit, adipiscing ut tristique vitae, sagittis vel odio. Maecenas convallis ullamcorper ultricies. Curabitur ornare, ligula semper consectetur sagittis, nisi diam iaculis velit, id fringilla sem nunc vel mi. Nam dictum, odio nec pretium volutpat, arcu ante placerat erat, non tristique elit urna et turpis. Quisque mi metus, ornare sit amet fermentum et, tincidunt et orci. Fusce eget orci a orci congue vestibulum. Ut dolor diam, elementum et vestibulum eu, porttitor vel elit. Curabitur venenatis pulvinar tellus gravida ornare. Sed et erat faucibus nunc euismod ultricies ut id justo. Nullam cursus suscipit nisi, et ultrices justo sodales nec. Fusce venenatis facilisis lectus ac semper. Aliquam at massa ipsum. Quisque bibendum purus convallis nulla ultrices ultricies. Nullam aliquam, mi eu aliquam tincidunt, purus velit laoreet tortor, viverra pretium nisi quam vitae mi. Fusce vel volutpat elit. Nam sagittis nisi dui.</p>
+<p>Vivamus fermentum semper porta. Nunc diam velit, adipiscing ut tristique vitae, sagittis vel odio. Maecenas convallis ullamcorper ultricies. Curabitur ornare, ligula semper consectetur sagittis, nisi diam iaculis velit, id fringilla sem nunc vel mi. Nam dictum, odio nec pretium volutpat, arcu ante placerat erat, non tristique elit urna et turpis. Quisque mi metus, ornare sit amet fermentum et, tincidunt et orci. Fusce eget orci a orci congue vestibulum. Ut dolor diam, elementum et vestibulum eu, porttitor vel elit. Curabitur venenatis pulvinar tellus gravida ornare. Sed et erat faucibus nunc euismod ultricies ut id justo. Nullam cursus suscipit nisi, et ultrices justo sodales nec. Fusce venenatis facilisis lectus ac semper. Aliquam at massa ipsum. Quisque bibendum purus convallis nulla ultrices ultricies. Nullam aliquam, mi eu aliquam tincidunt, purus velit laoreet tortor, viverra pretium nisi quam vitae mi. Fusce vel volutpat elit. Nam sagittis nisi dui.</p>
 </div>
 </body>
 </html>

browser/base/content/test/referrer/head.js

@@ -99,26 +99,45 @@ function clickTheLink(aWindow, aLinkId, aOptions) {
                     function(data) {
     let element = content.document.getElementById(data.id);
     let options = data.options;
-    element.focus();
 
     // EventUtils.synthesizeMouseAtCenter(element, options, content);
     // Alas, EventUtils doesn't work in the content task environment.
-    var domWindowUtils =
+    function doClick() {
-        content.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
+      var domWindowUtils =
-        .getInterface(Components.interfaces.nsIDOMWindowUtils);
+          content.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
-    var rect = element.getBoundingClientRect();
+          .getInterface(Components.interfaces.nsIDOMWindowUtils);
-    var left = rect.left + rect.width / 2;
+      var rect = element.getBoundingClientRect();
-    var top = rect.top + rect.height / 2;
+      var left = rect.left + rect.width / 2;
-    var button = options.button || 0;
+      var top = rect.top + rect.height / 2;
-    function sendMouseEvent(type) {
+      var button = options.button || 0;
-      domWindowUtils.sendMouseEvent(type, left, top, button,
+      function sendMouseEvent(type) {
-                                    1, 0, false, 0, 0, true);
+        domWindowUtils.sendMouseEvent(type, left, top, button,
+                                      1, 0, false, 0, 0, true);
+      }
+      if ("type" in options) {
+        sendMouseEvent(options.type);  // e.g., "contextmenu"
+      } else {
+        sendMouseEvent("mousedown");
+        sendMouseEvent("mouseup");
+      }
     }
-    if ("type" in options) {
+
-      sendMouseEvent(options.type);  // e.g., "contextmenu"
+    // waitForFocus(doClick, content);
+    let focusManager = Components.classes["@mozilla.org/focus-manager;1"].
+                       getService(Components.interfaces.nsIFocusManager);
+    let desiredWindow = {};
+    focusManager.getFocusedElementForWindow(content, true, desiredWindow);
+    desiredWindow = desiredWindow.value;
+    if (desiredWindow == focusManager.focusedWindow) {
+      // The window is already focused - click away.
+      doClick();
     } else {
-      sendMouseEvent("mousedown");
+      // Focus the window first, then click.
-      sendMouseEvent("mouseup");
+      desiredWindow.addEventListener("focus", function onFocus() {
+        desiredWindow.removeEventListener("focus", onFocus, true);
+        setTimeout(doClick, 0);
+      }, true);
+      desiredWindow.focus();
     }
   });
 }

browser/branding/aurora/content/jar.mn

@@ -16,6 +16,7 @@ browser.jar:
   content/branding/icon128.png                   (../mozicon128.png)
   content/branding/identity-icons-brand.png      (identity-icons-brand.png)
   content/branding/identity-icons-brand@2x.png   (identity-icons-brand@2x.png)
+  content/branding/silhouette-40.svg             (silhouette-40.svg)
   content/branding/aboutDialog.css               (aboutDialog.css)
 #ifdef MOZ_METRO
   content/branding/metro-about.css               (metro-about.css)

browser/branding/aurora/content/silhouette-40.svg

@@ -0,0 +1,25 @@
+<?xml version="1.0" encoding="utf-8"?>
+<svg version="1.1" xmlns="http://www.w3.org/2000/svg"
+  x="0px" y="0px" viewBox="-45 31 40 40"
+  enable-background="new -45 31 40 40">
+  <path fill="#CCCCCC" d="M-14.1,54.7c0.7-1.4,1.7-4.4,0.8-6.9c0,0,0,0,0,0.1l0,0c0,0-0.2,0.5-0.4,1.3c0-0.1,0-0.2,0-0.3
+    c0.1-0.9,0-1.9-0.1-2.9c-0.3-1.5-1.4-2.8-2-3.2c0,0,0.1,0,0.1,0.1c-0.1-0.1-0.1-0.1-0.1-0.1s0,0.1,0.1,0.4c-0.7-1.1-1.6-1.5-1.6-1.5
+    s0,0.2,0.1,0.5c-2-1.9-4.7-3-7.6-3c-3,0-5.7,1.2-7.8,3.1c0.1,0.1,0.2,0.3,0.4,0.5c0,0,0.8-0.1,1.7-0.1c1.7-1.2,3.6-1.8,5.7-1.8
+    c2.6,0,5.1,1.1,7,3c-0.2-0.1-0.1,0,0,0.1c-0.6-0.4-1.2-0.8-1.7-0.8c1,0.8,2.6,2.7,2.4,6.2c-0.3-0.6-0.6-1-0.9-1.3
+    c0.4,3.5,0,4.2-0.2,5.1c0-0.4-0.2-0.7-0.3-0.9c0,0,0,1.1-0.7,2.6c-0.5,1.2-1.1,1.5-1.3,1.5c-0.2,0-0.1-0.2-0.1-0.4
+    c0,0-0.4,0.2-0.7,0.6c-0.3,0.4-0.6,0.8-0.8,0.6c0.1-0.1,0.2-0.3,0.3-0.4c-0.1,0.1-0.5,0.4-1.2,0.5c-0.3,0-1.6,0.3-3.3-0.6
+    c0.3,0,0.6-0.1,0.9,0.1c-0.3-0.3-1-0.3-1.5-0.4c-0.5-0.4-1.1-1-1.4-1.4c1.3,0.3,2.8,0.1,3.6-0.5s1.3-1,1.8-0.9
+    c0.4,0.1,0.7-0.4,0.4-0.8c-0.3-0.4-1.2-1-2.3-0.7c-0.8,0.2-1.8,1.1-3.3,0.2c-1.3-0.8-1.3-1.4-1.3-1.8c0-0.3,0.2-0.7,0.5-0.8
+    c0.2,0.1,0.3,0.1,0.3,0.1s-0.1-0.1-0.1-0.2l0,0c0.1,0,0.4,0.2,0.6,0.2c0.2,0.1,0.3,0.2,0.3,0.2s0,0,0-0.1c0,0-0.1-0.2-0.3-0.3l0,0
+    c0.1,0,0.2,0.1,0.4,0.2c0-0.2,0.1-0.4,0.1-0.7c0-0.2,0-0.3-0.1-0.4c-0.1-0.1,0-0.1,0.1,0c0-0.1,0-0.1-0.1-0.2l0,0c0,0,0,0,0-0.1
+    c0.2-0.3,1.8-1.2,1.9-1.3c0.2-0.1,0.3-0.3,0.4-0.5c0.2-0.1,0.3-0.5,0.3-0.8c0-0.1-0.2-0.3-0.4-0.3c-0.1,0-0.4-0.1-0.6,0l0,0
+    c-0.3,0-0.7,0-1.2,0s-0.8-0.3-1-0.6c0-0.1-0.1-0.1-0.1-0.2c0-0.1-0.1-0.2-0.1-0.2c0.2-0.8,0.7-1.5,1.4-2.1c0,0-0.2,0-0.1,0
+    c0,0,0.3-0.2,0.4-0.2c0.1,0-0.3-0.1-0.6-0.1c-0.5,0.2-0.6,0.2-0.8,0.3c0.1-0.1,0.3-0.2,0.2-0.2c-0.3,0.1-0.7,0.4-1.1,0.6v-0.1
+    c-0.2,0.1-0.6,0.4-0.7,0.7c0-0.1,0-0.1,0-0.1c-0.1,0-0.2,0.2-0.3,0.3l0,0c-1.1-0.3-2-0.2-2.8,0c-0.2-0.1-0.6-0.5-0.9-1
+    c0,0,0,0.1-0.1,0.1c-0.1-0.4-0.3-0.9-0.3-1.3v-0.1c0,0-0.1,0.1-0.3,0.3c-0.1,0.2-0.2,0.3-0.2,0.5c0,0.1-0.1,0.2-0.1,0.2v-0.2
+    c0,0.1-0.1,0.2-0.2,0.3c0,0.2,0,0.3-0.1,0.4l0,0c0,0,0-0.2,0-0.1c-0.1,0.2-0.2,0.5-0.2,0.8c-0.1,0.3-0.1,0.5-0.1,0.8s0,0.7,0,1.2
+    c0,0.1,0,0.1,0,0.2c-0.3,0.4-0.5,0.7-0.6,0.9c-0.4,0.7-0.7,1.8-1,3.5c0,0,0.2-0.6,0.6-1.3l0,0c-0.3,0.9-0.5,2.3-0.4,4.4
+    c0-0.1,0.1-0.6,0.2-1.3c0.1,1.4,0.5,3.1,1.5,5c0.8,1.4,1.7,2.4,2.7,3.2c0.2,0.2,0.4,0.3,0.6,0.5c1.3,1,3.3,2.1,5,2.4
+    c-0.6-0.2-1-0.5-1-0.5s2,0.7,3.5,0.6c-0.5-0.1-0.6-0.3-0.6-0.3s4.2,0.2,6.4-1.5c0.5-0.4,0.8-0.8,0.9-1.2c0.6-0.4,1.3-0.8,2-1.6
+    c1.2-1.2,1.3-2.1,1.4-3v0.1C-14,55.2-14,54.9-14.1,54.7z"/>
+  </svg>

browser/branding/nightly/content/jar.mn

@@ -16,6 +16,7 @@ browser.jar:
   content/branding/icon128.png                   (../mozicon128.png)
   content/branding/identity-icons-brand.png      (identity-icons-brand.png)
   content/branding/identity-icons-brand@2x.png   (identity-icons-brand@2x.png)
+  content/branding/silhouette-40.svg             (silhouette-40.svg)
   content/branding/aboutDialog.css               (aboutDialog.css)
 #ifdef MOZ_METRO
   content/branding/metro-about.css               (metro-about.css)

browser/branding/official/content/jar.mn

@@ -15,6 +15,7 @@ browser.jar:
   content/branding/icon128.png                   (../mozicon128.png)
   content/branding/identity-icons-brand.png      (identity-icons-brand.png)
   content/branding/identity-icons-brand@2x.png   (identity-icons-brand@2x.png)
+  content/branding/silhouette-40.svg             (silhouette-40.svg)
   content/branding/aboutDialog.css               (aboutDialog.css)
 #ifdef MOZ_METRO
   content/branding/metro-about.css               (metro-about.css)

browser/branding/official/content/silhouette-40.svg

@@ -0,0 +1,25 @@
+<?xml version="1.0" encoding="utf-8"?>
+<svg version="1.1"
+ xmlns="http://www.w3.org/2000/svg" x="0px" y="0px" viewBox="-45 31 40 40"
+ enable-background="new -45 31 40 40">
+ <path fill="#CCCCCC" d="M-14.1,54.7c0.7-1.4,1.7-4.4,0.8-6.9c0,0,0,0,0,0.1l0,0c0,0-0.2,0.5-0.4,1.3c0-0.1,0-0.2,0-0.3
+  c0.1-0.9,0-1.9-0.1-2.9c-0.3-1.5-1.4-2.8-2-3.2c0,0,0.1,0,0.1,0.1c-0.1-0.1-0.1-0.1-0.1-0.1s0,0.1,0.1,0.4c-0.7-1.1-1.6-1.5-1.6-1.5
+  s0,0.2,0.1,0.5c-2-1.9-4.7-3-7.6-3c-3,0-5.7,1.2-7.8,3.1c0.1,0.1,0.2,0.3,0.4,0.5c0,0,0.8-0.1,1.7-0.1c1.7-1.2,3.6-1.8,5.7-1.8
+  c2.6,0,5.1,1.1,7,3c-0.2-0.1-0.1,0,0,0.1c-0.6-0.4-1.2-0.8-1.7-0.8c1,0.8,2.6,2.7,2.4,6.2c-0.3-0.6-0.6-1-0.9-1.3
+  c0.4,3.5,0,4.2-0.2,5.1c0-0.4-0.2-0.7-0.3-0.9c0,0,0,1.1-0.7,2.6c-0.5,1.2-1.1,1.5-1.3,1.5c-0.2,0-0.1-0.2-0.1-0.4
+  c0,0-0.4,0.2-0.7,0.6c-0.3,0.4-0.6,0.8-0.8,0.6c0.1-0.1,0.2-0.3,0.3-0.4c-0.1,0.1-0.5,0.4-1.2,0.5c-0.3,0-1.6,0.3-3.3-0.6
+  c0.3,0,0.6-0.1,0.9,0.1c-0.3-0.3-1-0.3-1.5-0.4c-0.5-0.4-1.1-1-1.4-1.4c1.3,0.3,2.8,0.1,3.6-0.5s1.3-1,1.8-0.9
+  c0.4,0.1,0.7-0.4,0.4-0.8c-0.3-0.4-1.2-1-2.3-0.7c-0.8,0.2-1.8,1.1-3.3,0.2c-1.3-0.8-1.3-1.4-1.3-1.8c0-0.3,0.2-0.7,0.5-0.8
+  c0.2,0.1,0.3,0.1,0.3,0.1s-0.1-0.1-0.1-0.2l0,0c0.1,0,0.4,0.2,0.6,0.2c0.2,0.1,0.3,0.2,0.3,0.2s0,0,0-0.1c0,0-0.1-0.2-0.3-0.3l0,0
+  c0.1,0,0.2,0.1,0.4,0.2c0-0.2,0.1-0.4,0.1-0.7c0-0.2,0-0.3-0.1-0.4c-0.1-0.1,0-0.1,0.1,0c0-0.1,0-0.1-0.1-0.2l0,0c0,0,0,0,0-0.1
+  c0.2-0.3,1.8-1.2,1.9-1.3c0.2-0.1,0.3-0.3,0.4-0.5c0.2-0.1,0.3-0.5,0.3-0.8c0-0.1-0.2-0.3-0.4-0.3c-0.1,0-0.4-0.1-0.6,0l0,0
+  c-0.3,0-0.7,0-1.2,0s-0.8-0.3-1-0.6c0-0.1-0.1-0.1-0.1-0.2c0-0.1-0.1-0.2-0.1-0.2c0.2-0.8,0.7-1.5,1.4-2.1c0,0-0.2,0-0.1,0
+  c0,0,0.3-0.2,0.4-0.2c0.1,0-0.3-0.1-0.6-0.1c-0.5,0.2-0.6,0.2-0.8,0.3c0.1-0.1,0.3-0.2,0.2-0.2c-0.3,0.1-0.7,0.4-1.1,0.6v-0.1
+  c-0.2,0.1-0.6,0.4-0.7,0.7c0-0.1,0-0.1,0-0.1c-0.1,0-0.2,0.2-0.3,0.3l0,0c-1.1-0.3-2-0.2-2.8,0c-0.2-0.1-0.6-0.5-0.9-1
+  c0,0,0,0.1-0.1,0.1c-0.1-0.4-0.3-0.9-0.3-1.3v-0.1c0,0-0.1,0.1-0.3,0.3c-0.1,0.2-0.2,0.3-0.2,0.5c0,0.1-0.1,0.2-0.1,0.2v-0.2
+  c0,0.1-0.1,0.2-0.2,0.3c0,0.2,0,0.3-0.1,0.4l0,0c0,0,0-0.2,0-0.1c-0.1,0.2-0.2,0.5-0.2,0.8c-0.1,0.3-0.1,0.5-0.1,0.8s0,0.7,0,1.2
+  c0,0.1,0,0.1,0,0.2c-0.3,0.4-0.5,0.7-0.6,0.9c-0.4,0.7-0.7,1.8-1,3.5c0,0,0.2-0.6,0.6-1.3l0,0c-0.3,0.9-0.5,2.3-0.4,4.4
+  c0-0.1,0.1-0.6,0.2-1.3c0.1,1.4,0.5,3.1,1.5,5c0.8,1.4,1.7,2.4,2.7,3.2c0.2,0.2,0.4,0.3,0.6,0.5c1.3,1,3.3,2.1,5,2.4
+  c-0.6-0.2-1-0.5-1-0.5s2,0.7,3.5,0.6c-0.5-0.1-0.6-0.3-0.6-0.3s4.2,0.2,6.4-1.5c0.5-0.4,0.8-0.8,0.9-1.2c0.6-0.4,1.3-0.8,2-1.6
+  c1.2-1.2,1.3-2.1,1.4-3v0.1C-14,55.2-14,54.9-14.1,54.7z"/>
+</svg>

browser/branding/unofficial/content/jar.mn

@@ -16,6 +16,7 @@ browser.jar:
   content/branding/icon128.png                   (../mozicon128.png)
   content/branding/identity-icons-brand.png      (identity-icons-brand.png)
   content/branding/identity-icons-brand@2x.png   (identity-icons-brand@2x.png)
+  content/branding/silhouette-40.svg             (silhouette-40.svg)
   content/branding/aboutDialog.css               (aboutDialog.css)
 #ifdef MOZ_METRO
   content/branding/metro-about.css               (metro-about.css)

browser/components/loop/content/shared/js/crypto.js

@@ -0,0 +1,238 @@
+/* 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/. */
+
+/* global loop:true */
+
+var loop = loop || {};
+
+loop.crypto = (function() {
+  "use strict";
+
+  var ALGORITHM = "AES-GCM";
+  var KEY_LENGTH = 128;
+  // We use JSON web key formats for the generated keys.
+  // https://tools.ietf.org/html/draft-ietf-jose-json-web-key-41
+  var KEY_FORMAT = "jwk";
+  // This is the JSON web key type from the generateKey algorithm.
+  var KEY_TYPE = "oct";
+  var ENCRYPT_TAG_LENGTH = 128;
+  var INITIALIZATION_VECTOR_LENGTH = 12;
+
+  var sharedUtils = loop.shared.utils;
+
+  /**
+   * Root object, by default set to window.
+   * @type {DOMWindow|Object}
+   */
+  var rootObject = window;
+
+  /**
+   * Sets a new root object.  This is useful for testing crypto not supported as
+   * it allows us to fake crypto not being present.
+   * In beforeEach(), loop.crypto.setRootObject is used to
+   * substitute a fake window, and in afterEach(), the real window object is
+   * replaced.
+   *
+   * @param {Object}
+   */
+  function setRootObject(obj) {
+    console.log("loop.crpyto.mixins: rootObject set to " + obj);
+    rootObject = obj;
+  }
+
+  /**
+   * Determines if Web Crypto is supported by this browser.
+   *
+   * @return {Boolean} True if Web Crypto is supported
+   */
+  function isSupported() {
+    return "crypto" in rootObject;
+  }
+
+  /**
+   * Generates a random key using the Web Crypto libraries.
+   *
+   * @return {Promise} A promise which is rejected on failure, or resolved
+   *                   with a string that is in the JSON web key format.
+   */
+  function generateKey() {
+    if (!isSupported()) {
+      throw new Error("Web Crypto is not supported");
+    }
+
+    return new Promise(function(resolve, reject) {
+      // First get a crypto key.
+      rootObject.crypto.subtle.generateKey({name: ALGORITHM, length: KEY_LENGTH },
+        // `true` means that the key can be extracted from the CryptoKey object.
+        true,
+        // Usages for the key.
+        ["encrypt", "decrypt"]
+      ).then(function(cryptoKey) {
+        // Now extract the key in the JSON web key format.
+        return rootObject.crypto.subtle.exportKey(KEY_FORMAT, cryptoKey);
+      }).then(function(exportedKey) {
+        // Lastly resolve the promise with the new key.
+        resolve(exportedKey.k);
+      }).catch(function(error) {
+        reject(error);
+      });
+    });
+  }
+
+  /**
+   * Encrypts an object using the specified key.
+   *
+   * @param {String} key      The key to use for encryption. This should have
+   *                          been generated by generateKey.
+   * @param {String} data     The string to be encrypted.
+   *
+   * @return {Promise} A promise which is rejected on failure, or resolved
+   *                   with a string that is the encrypted context.
+   */
+  function encryptBytes(key, data) {
+    if (!isSupported()) {
+      throw new Error("Web Crypto is not supported");
+    }
+
+    var iv = new Uint8Array(INITIALIZATION_VECTOR_LENGTH);
+
+    return new Promise(function(resolve, reject) {
+      // First import the key to a format we can use.
+      rootObject.crypto.subtle.importKey(KEY_FORMAT,
+        {k: key, kty: KEY_TYPE},
+        ALGORITHM,
+        // If the key is extractable.
+        true,
+        // What we're using it for.
+        ["encrypt"]
+      ).then(function(cryptoKey) {
+        // Now we've got the cryptoKey, we can do the actual encryption.
+
+        // First get the data into the format we need.
+        var dataBuffer = sharedUtils.strToUint8Array(data);
+
+        // It is critically important to change the IV any time the
+        // encrypted information is updated.
+        rootObject.crypto.getRandomValues(iv);
+
+        return rootObject.crypto.subtle.encrypt({
+            name: ALGORITHM,
+            iv: iv,
+            tagLength: ENCRYPT_TAG_LENGTH
+          }, cryptoKey,
+          dataBuffer);
+      }).then(function(cipherText) {
+        // Join the initialization vector and context for returning.
+        var joinedData = _mergeIVandCipherText(iv, new DataView(cipherText));
+
+        // Now convert to a string and base-64 encode.
+        var encryptedData = loop.shared.utils.btoa(joinedData);
+
+        resolve(encryptedData);
+      }).catch(function(error) {
+        reject(error);
+      });
+    });
+  }
+
+  /**
+   * Decrypts an object using the specified key.
+   *
+   * @param {String} key           The key to use for encryption. This should have
+   *                               been generated by generateKey.
+   * @param {String} encryptedData The encrypted context.
+   * @return {Promise} A promise which is rejected on failure, or resolved
+   *                   with a string that is the decrypted context.
+   */
+  function decryptBytes(key, encryptedData) {
+    if (!isSupported()) {
+      throw new Error("Web Crypto is not supported");
+    }
+
+    return new Promise(function(resolve, reject) {
+      // First import the key to a format we can use.
+      rootObject.crypto.subtle.importKey(KEY_FORMAT,
+        {k: key, kty: KEY_TYPE},
+        ALGORITHM,
+        // If the key is extractable.
+        true,
+        // What we're using it for.
+        ["decrypt"]
+      ).then(function(cryptoKey) {
+        // Now we've got the key, start the decryption.
+        var splitData = _splitIVandCipherText(encryptedData);
+
+        return rootObject.crypto.subtle.decrypt({
+          name: ALGORITHM,
+          iv: splitData.iv,
+          tagLength: ENCRYPT_TAG_LENGTH
+        }, cryptoKey, splitData.cipherText);
+      }).then(function(plainText) {
+        // Now we just turn it back into a string and then an object.
+        resolve(sharedUtils.Uint8ArrayToStr(new Uint8Array(plainText)));
+      }).catch(function(error) {
+        reject(error);
+      });
+    });
+  }
+
+  /**
+   * Appends the cipher text to the end of the initialization vector and
+   * returns the result.
+   *
+   * @param {Uint8Array} ivArray The array of initialization vector values.
+   * @param {DataView} cipherTextDataView The cipherText in data view format.
+   * @return {Uint8Array} An array of the IV and cipherText.
+   */
+  function _mergeIVandCipherText(ivArray, cipherTextDataView) {
+    // First we translate the data view to an array so we can get
+    // the length.
+    var cipherText = new Uint8Array(cipherTextDataView.buffer);
+    var cipherTextLength = cipherText.length;
+
+    var joinedContext = new Uint8Array(INITIALIZATION_VECTOR_LENGTH + cipherTextLength);
+
+    var i;
+    for (i = 0; i < INITIALIZATION_VECTOR_LENGTH; i++) {
+      joinedContext[i] = ivArray[i];
+    }
+
+    for (i = 0; i < cipherTextLength; i++) {
+      joinedContext[i + INITIALIZATION_VECTOR_LENGTH] = cipherText[i];
+    }
+
+    return joinedContext;
+  }
+
+  /**
+   * Takes the IV from the start of the passed in array and separates
+   * out the cipher text.
+   *
+   * @param {String} encryptedData Encrypted data in base64 format.
+   * @return {Object} An object consisting of two items: iv and cipherText,
+   *                  both are Uint8Arrays.
+   */
+  function _splitIVandCipherText(encryptedData) {
+    // Convert into byte arrays.
+    var encryptedDataArray = loop.shared.utils.atob(encryptedData);
+
+    // Now split out the initialization vector and the cipherText.
+    var iv = encryptedDataArray.slice(0, INITIALIZATION_VECTOR_LENGTH);
+    var cipherText = encryptedDataArray.slice(INITIALIZATION_VECTOR_LENGTH,
+                                              encryptedDataArray.length);
+
+    return {
+      iv: iv,
+      cipherText: cipherText
+    };
+  }
+
+  return {
+    decryptBytes: decryptBytes,
+    encryptBytes: encryptBytes,
+    generateKey: generateKey,
+    isSupported: isSupported,
+    setRootObject: setRootObject
+  };
+})();

browser/components/loop/content/shared/js/otSdkDriver.js

@@ -27,25 +27,37 @@ loop.OTSdkDriver = (function() {
       this.dispatcher = options.dispatcher;
       this.sdk = options.sdk;
 
-      // Note that this will only be defined and usable in a desktop-local
+      this._isDesktop = !!options.isDesktop;
-      // context, not in the standalone web client.
+
-      this.mozLoop = options.mozLoop;
+      if (this._isDesktop) {
+        if (!options.mozLoop) {
+          throw new Error("Missing option mozLoop");
+        }
+        this.mozLoop = options.mozLoop;
+      }
 
       this.connections = {};
-      this.connectionStartTime = this.CONNECTION_START_TIME_UNINITIALIZED;
+      this._setTwoWayMediaStartTime(this.CONNECTION_START_TIME_UNINITIALIZED);
 
       this.dispatcher.register(this, [
         "setupStreamElements",
         "setMute"
       ]);
 
+      // Set loop.debug.twoWayMediaTelemetry to true in the browser
+      // by changing the hidden pref loop.debug.twoWayMediaTelemetry using
+      // about:config, or use
+      //
+      // localStorage.setItem("debug.twoWayMediaTelemetry", true);
+      this._debugTwoWayMediaTelemetry =
+        loop.shared.utils.getBoolPreference("debug.twoWayMediaTelemetry");
+
     /**
      * XXX This is a workaround for desktop machines that do not have a
      * camera installed. As we don't yet have device enumeration, when
      * we do, this can be removed (bug 1138851), and the sdk should handle it.
      */
-    if ("isDesktop" in options && options.isDesktop &&
+    if (this._isDesktop && !window.MediaStreamTrack.getSources) {
-        !window.MediaStreamTrack.getSources) {
       // If there's no getSources function, the sdk defines its own and caches
       // the result. So here we define the "normal" one which doesn't get cached, so
       // we can change it later.
@@ -56,9 +68,6 @@ loop.OTSdkDriver = (function() {
   };
 
   OTSdkDriver.prototype = {
-    CONNECTION_START_TIME_UNINITIALIZED: -1,
-    CONNECTION_START_TIME_ALREADY_NOTED: -2,
-
     /**
      * Clones the publisher config into a new object, as the sdk modifies the
      * properties object.
@@ -236,7 +245,7 @@ loop.OTSdkDriver = (function() {
         delete this.publisher;
       }
 
-      this._noteConnectionLengthIfNeeded(this.connectionStartTime, performance.now());
+      this._noteConnectionLengthIfNeeded(this._getTwoWayMediaStartTime(), performance.now());
 
       // Also, tidy these variables ready for next time.
       delete this._sessionConnected;
@@ -244,7 +253,7 @@ loop.OTSdkDriver = (function() {
       delete this._publishedLocalStream;
       delete this._subscribedRemoteStream;
       this.connections = {};
-      this.connectionStartTime = this.CONNECTION_START_TIME_UNINITIALIZED;
+      this._setTwoWayMediaStartTime(this.CONNECTION_START_TIME_UNINITIALIZED);
     },
 
     /**
@@ -308,7 +317,7 @@ loop.OTSdkDriver = (function() {
       if (connection && (connection.id in this.connections)) {
         delete this.connections[connection.id];
       }
-      this._noteConnectionLengthIfNeeded(this.connectionStartTime, performance.now());
+      this._noteConnectionLengthIfNeeded(this._getTwoWayMediaStartTime(), performance.now());
       this.dispatcher.dispatch(new sharedActions.RemotePeerDisconnected({
         peerHungup: event.reason === "clientDisconnected"
       }));
@@ -335,7 +344,7 @@ loop.OTSdkDriver = (function() {
           return;
       }
 
-      this._noteConnectionLengthIfNeeded(this.connectionStartTime,
+      this._noteConnectionLengthIfNeeded(this._getTwoWayMediaStartTime(),
         performance.now());
       this.dispatcher.dispatch(new sharedActions.ConnectionFailure({
         reason: reason
@@ -408,7 +417,7 @@ loop.OTSdkDriver = (function() {
 
       this._subscribedRemoteStream = true;
       if (this._checkAllStreamsConnected()) {
-        this.connectionStartTime = performance.now();
+        this._setTwoWayMediaStartTime(performance.now());
         this.dispatcher.dispatch(new sharedActions.MediaConnected());
       }
     },
@@ -429,6 +438,56 @@ loop.OTSdkDriver = (function() {
       }
     },
 
+    /**
+     * Implementation detail, may be set to one of the CONNECTION_START_TIME
+     * constants, or a positive integer in milliseconds.
+     *
+     * @private
+     */
+    __twoWayMediaStartTime: undefined,
+
+    /**
+     * Used as a guard to make sure we don't inadvertently use an
+     * uninitialized value.
+     */
+    CONNECTION_START_TIME_UNINITIALIZED: -1,
+
+    /**
+     * Use as a guard to ensure that we don't note any bidirectional sessions
+     * twice.
+     */
+    CONNECTION_START_TIME_ALREADY_NOTED: -2,
+
+    /**
+     * Set and get the start time of the two-way media connection.  These
+     * are done as wrapper functions so that we can log sets to make manual
+     * verification of various telemetry scenarios possible.  The get API is
+     * analogous in order to follow the principle of least surprise for
+     * people consuming this code.
+     *
+     * If this._isDesktop is not true, returns immediately without making
+     * any changes, since this data is not used, and it makes reading
+     * the logs confusing for manual verification of both ends of the call in
+     * the same browser, which is a case we care about.
+     *
+     * @param start  start time in milliseconds, as returned by
+     *               performance.now()
+     * @private
+     */
+    _setTwoWayMediaStartTime: function(start) {
+      if (!this._isDesktop) {
+        return;
+      }
+
+      this.__twoWayMediaStartTime = start;
+      if (this._debugTwoWayMediaTelemetry) {
+        console.log("Loop Telemetry: noted two-way connection start, " +
+                    "start time in ms:", start);
+      }
+    },
+    _getTwoWayMediaStartTime: function() {
+      return this.__twoWayMediaStartTime;
+    },
 
     /**
      * Handles the event when the remote stream is destroyed.
@@ -528,7 +587,7 @@ loop.OTSdkDriver = (function() {
         // Now record the fact, and check if we've got all media yet.
         this._publishedLocalStream = true;
         if (this._checkAllStreamsConnected()) {
-          this.connectionStartTime = performance.now();
+          this._setTwoWayMediaStartTime(performance.now);
           this.dispatcher.dispatch(new sharedActions.MediaConnected());
         }
       }
@@ -562,6 +621,14 @@ loop.OTSdkDriver = (function() {
       }));
     },
 
+    /*
+     * XXX all of the bi-directional media connection telemetry stuff in this
+     * file, (much, but not all, of it is below) should be hoisted into its
+     * own object for maintainability and clarity, also in part because this
+     * stuff only wants to run one side of the connection, not both (tracked
+     * by bug 1145237).
+     */
+
     /**
      * A hook exposed only for the use of the functional tests so that
      * they can check that the bi-directional media count is being updated
@@ -574,7 +641,7 @@ loop.OTSdkDriver = (function() {
 
     /**
      * Wrapper for adding a keyed value that also updates
-     * connectionLengthNoted calls and sets this.connectionStartTime to
+     * connectionLengthNoted calls and sets the twoWayMediaStartTime to
      * this.CONNECTION_START_TIME_ALREADY_NOTED.
      *
      * @param {number} callLengthSeconds  the call length in seconds
@@ -594,15 +661,19 @@ loop.OTSdkDriver = (function() {
 
       this.mozLoop.telemetryAddKeyedValue("LOOP_TWO_WAY_MEDIA_CONN_LENGTH",
         bucket);
-      this.connectionStartTime = this.CONNECTION_START_TIME_ALREADY_NOTED;
+      this._setTwoWayMediaStartTime(this.CONNECTION_START_TIME_ALREADY_NOTED);
 
       this._connectionLengthNotedCalls++;
+      if (this._debugTwoWayMediaTelemetry) {
+        console.log('Loop Telemetry: noted two-way media connection ' +
+          'in bucket: ', bucket);
+      }
     },
 
     /**
      * Note connection length if it's valid (the startTime has been initialized
      * and is not later than endTime) and not yet already noted.  If
-     * this.mozLoop is not defined, we're assumed to be running in the
+     * this._isDesktop is not true, we're assumed to be running in the
      * standalone client and return immediately.
      *
      * @param {number} startTime  in milliseconds
@@ -610,23 +681,31 @@ loop.OTSdkDriver = (function() {
      * @private
      */
     _noteConnectionLengthIfNeeded: function(startTime, endTime) {
-      if (!this.mozLoop) {
+      if (!this._isDesktop) {
         return;
       }
 
       if (startTime == this.CONNECTION_START_TIME_ALREADY_NOTED ||
           startTime == this.CONNECTION_START_TIME_UNINITIALIZED ||
           startTime > endTime) {
-        console.log("_noteConnectionLengthIfNeeded called with " +
+        if (this._debugTwoWayMediaTelemetry) {
-                    " invalid params, either the calls were never" +
+          console.log("_noteConnectionLengthIfNeeded called with " +
-                    " connected or there is a bug; startTime:", startTime,
+            " invalid params, either the calls were never" +
-                    "endTime:", endTime);
+            " connected or there is a bug; startTime:", startTime,
+            "endTime:", endTime);
+        }
         return;
       }
 
       var callLengthSeconds = (endTime - startTime) / 1000;
       this._noteConnectionLength(callLengthSeconds);
-    }
+    },
+
+    /**
+     * If set to true, make it easy to test/verify 2-way media connection
+     * telemetry code operation by viewing the logs.
+     */
+    _debugTwoWayMediaTelemetry: false
   };
 
   return OTSdkDriver;

browser/components/loop/content/shared/js/utils.js

@@ -168,6 +168,228 @@ loop.shared.utils = (function(mozL10n) {
     );
   }
 
+  /**
+   * Binary-compatible Base64 decoding.
+   *
+   * Taken from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Base64_encoding_and_decoding
+   *
+   * @param {String} base64str The string to decode.
+   * @return {Uint8Array} The decoded result in array format.
+   */
+  function atob(base64str) {
+    var strippedEncoding = base64str.replace(/[^A-Za-z0-9\+\/]/g, "");
+    var inLength = strippedEncoding.length;
+    var outLength = inLength * 3 + 1 >> 2;
+    var result = new Uint8Array(outLength);
+
+    var mod3;
+    var mod4;
+    var uint24 = 0;
+    var outIndex = 0;
+
+    for (var inIndex = 0; inIndex < inLength; inIndex++) {
+      mod4 = inIndex & 3;
+      uint24 |= _b64ToUint6(strippedEncoding.charCodeAt(inIndex)) << 6 * (3 - mod4);
+
+      if (mod4 === 3 || inLength - inIndex === 1) {
+        for (mod3 = 0; mod3 < 3 && outIndex < outLength; mod3++, outIndex++) {
+          result[outIndex] = uint24 >>> (16 >>> mod3 & 24) & 255;
+        }
+        uint24 = 0;
+      }
+    }
+
+    return result;
+  }
+
+  /**
+   * Binary-compatible Base64 encoding.
+   *
+   * Taken from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Base64_encoding_and_decoding
+   *
+   * @param {Uint8Array} bytes The data to encode.
+   * @return {String} The base64 encoded string.
+   */
+  function btoa(bytes) {
+    var mod3 = 2;
+    var result = "";
+    var length = bytes.length;
+    var uint24 = 0;
+
+    for (var index = 0; index < length; index++) {
+      mod3 = index % 3;
+      if (index > 0 && (index * 4 / 3) % 76 === 0) {
+        result += "\r\n";
+      }
+      uint24 |= bytes[index] << (16 >>> mod3 & 24);
+      if (mod3 === 2 || length - index === 1) {
+        result += String.fromCharCode(_uint6ToB64(uint24 >>> 18 & 63),
+          _uint6ToB64(uint24 >>> 12 & 63),
+          _uint6ToB64(uint24 >>> 6 & 63),
+          _uint6ToB64(uint24 & 63));
+        uint24 = 0;
+      }
+    }
+
+    return result.substr(0, result.length - 2 + mod3) +
+      (mod3 === 2 ? "" : mod3 === 1 ? "=" : "==");
+  }
+
+  /**
+   * Utility function to decode a base64 character into an integer.
+   *
+   * Taken from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Base64_encoding_and_decoding
+   *
+   * @param {Number} chr The character code to decode.
+   * @return {Number} The decoded value.
+   */
+  function _b64ToUint6 (chr) {
+    return chr > 64 && chr < 91  ? chr - 65 :
+           chr > 96 && chr < 123 ? chr - 71 :
+           chr > 47 && chr < 58  ? chr + 4  :
+           chr === 43            ? 62       :
+           chr === 47            ? 63       : 0;
+  }
+
+  /**
+   * Utility function to encode an integer into a base64 character code.
+   *
+   * Taken from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Base64_encoding_and_decoding
+   *
+   * @param {Number} uint6 The number to encode.
+   * @return {Number} The encoded value.
+   */
+  function _uint6ToB64 (uint6) {
+    return uint6 < 26   ? uint6 + 65 :
+           uint6 < 52   ? uint6 + 71 :
+           uint6 < 62   ? uint6 - 4  :
+           uint6 === 62 ? 43         :
+           uint6 === 63 ? 47         : 65;
+  }
+
+  /**
+   * Utility function to convert a string into a uint8 array.
+   *
+   * Taken from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Base64_encoding_and_decoding
+   *
+   * @param {String} inString The string to convert.
+   * @return {Uint8Array} The converted string in array format.
+   */
+  function strToUint8Array(inString) {
+    var inLength = inString.length;
+    var arrayLength = 0;
+    var chr;
+
+    // Mapping.
+    for (var mapIndex = 0; mapIndex < inLength; mapIndex++) {
+      chr = inString.charCodeAt(mapIndex);
+      arrayLength += chr < 0x80      ? 1 :
+                     chr < 0x800     ? 2 :
+                     chr < 0x10000   ? 3 :
+                     chr < 0x200000  ? 4 :
+                     chr < 0x4000000 ? 5 : 6;
+    }
+
+    var result = new Uint8Array(arrayLength);
+    var index = 0;
+
+    // Transcription.
+    for (var chrIndex = 0; index < arrayLength; chrIndex++) {
+      chr = inString.charCodeAt(chrIndex);
+      if (chr < 128) {
+        // One byte.
+        result[index++] = chr;
+      } else if (chr < 0x800) {
+        // Two bytes.
+        result[index++] = 192 + (chr >>> 6);
+        result[index++] = 128 + (chr & 63);
+      } else if (chr < 0x10000) {
+        // Three bytes.
+        result[index++] = 224 + (chr >>> 12);
+        result[index++] = 128 + (chr >>> 6 & 63);
+        result[index++] = 128 + (chr & 63);
+      } else if (chr < 0x200000) {
+        // Four bytes.
+        result[index++] = 240 + (chr >>> 18);
+        result[index++] = 128 + (chr >>> 12 & 63);
+        result[index++] = 128 + (chr >>> 6 & 63);
+        result[index++] = 128 + (chr & 63);
+      } else if (chr < 0x4000000) {
+        // Five bytes.
+        result[index++] = 248 + (chr >>> 24);
+        result[index++] = 128 + (chr >>> 18 & 63);
+        result[index++] = 128 + (chr >>> 12 & 63);
+        result[index++] = 128 + (chr >>> 6 & 63);
+        result[index++] = 128 + (chr & 63);
+      } else { // if (chr <= 0x7fffffff)
+        // Six bytes.
+        result[index++] = 252 + (chr >>> 30);
+        result[index++] = 128 + (chr >>> 24 & 63);
+        result[index++] = 128 + (chr >>> 18 & 63);
+        result[index++] = 128 + (chr >>> 12 & 63);
+        result[index++] = 128 + (chr >>> 6 & 63);
+        result[index++] = 128 + (chr & 63);
+      }
+    }
+
+    return result;
+  }
+
+  /**
+   * Utility function to change a uint8 based integer array to a string.
+   *
+   * Taken from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Base64_encoding_and_decoding
+   *
+   * @param {Uint8Array} arrayBytes Array to convert.
+   * @param {String} The array as a string.
+   */
+  function Uint8ArrayToStr(arrayBytes) {
+    var result = "";
+    var length = arrayBytes.length;
+    var part;
+
+    for (var index = 0; index < length; index++) {
+      part = arrayBytes[index];
+      result += String.fromCharCode(
+        part > 251 && part < 254 && index + 5 < length ?
+          // Six bytes.
+          // (part - 252 << 30) may be not so safe in ECMAScript! So...:
+          (part - 252) * 1073741824 +
+          (arrayBytes[++index] - 128 << 24) +
+          (arrayBytes[++index] - 128 << 18) +
+          (arrayBytes[++index] - 128 << 12) +
+          (arrayBytes[++index] - 128 << 6) +
+           arrayBytes[++index] - 128 :
+        part > 247 && part < 252 && index + 4 < length ?
+          // Five bytes.
+          (part - 248 << 24) +
+          (arrayBytes[++index] - 128 << 18) +
+          (arrayBytes[++index] - 128 << 12) +
+          (arrayBytes[++index] - 128 << 6) +
+           arrayBytes[++index] - 128 :
+        part > 239 && part < 248 && index + 3 < length ?
+          // Four bytes.
+          (part - 240 << 18) +
+          (arrayBytes[++index] - 128 << 12) +
+          (arrayBytes[++index] - 128 << 6) +
+           arrayBytes[++index] - 128 :
+        part > 223 && part < 240 && index + 2 < length ?
+          // Three bytes.
+          (part - 224 << 12) +
+          (arrayBytes[++index] - 128 << 6) +
+           arrayBytes[++index] - 128 :
+        part > 191 && part < 224 && index + 1 < length ?
+          // Two bytes.
+          (part - 192 << 6) +
+           arrayBytes[++index] - 128 :
+          // One byte.
+          part
+      );
+    }
+
+    return result;
+  }
+
   return {
     CALL_TYPES: CALL_TYPES,
     FAILURE_DETAILS: FAILURE_DETAILS,
@@ -183,6 +405,10 @@ loop.shared.utils = (function(mozL10n) {
     isFirefoxOS: isFirefoxOS,
     isOpera: isOpera,
     getUnsupportedPlatform: getUnsupportedPlatform,
-    locationData: locationData
+    locationData: locationData,
+    atob: atob,
+    btoa: btoa,
+    strToUint8Array: strToUint8Array,
+    Uint8ArrayToStr: Uint8ArrayToStr
   };
 })(document.mozL10n || navigator.mozL10n);

browser/components/loop/test/functional/test_1_browser_call.py

@@ -193,7 +193,7 @@ class Test1BrowserCall(MarionetteTestCase):
 
     def local_get_media_start_time(self):
         return self.local_get_chatbox_window_expr(
-            "loop.conversation._sdkDriver.connectionStartTime")
+            "loop.conversation._sdkDriver._getTwoWayMediaStartTime()")
 
     # XXX could be memoized
     def local_get_media_start_time_uninitialized(self):
@@ -221,7 +221,7 @@ class Test1BrowserCall(MarionetteTestCase):
 
         self.assertGreater(noted_calls, 0,
                            "OTSdkDriver._connectionLengthNotedCalls should be "
-                           "> 0")
+                           "> 0, noted_calls = " + str(noted_calls))
 
     def test_1_browser_call(self):
         self.switch_to_panel()
@@ -252,8 +252,8 @@ class Test1BrowserCall(MarionetteTestCase):
         # self.local_enable_screenshare()
         # self.standalone_check_remote_screenshare()
 
-        # We hangup on the remote side, because this also leaves the
+        # We hangup on the remote (standalone) side, because this also leaves
-        # local chatbox with the local publishing media still connected,
+        # the local chatbox with the local publishing media still connected,
         # which means that the local_check_connection_length below
         # verifies that the connection is noted at the time the remote media
         # drops, rather than waiting until the window closes.

browser/components/loop/test/shared/crypto_test.js

@@ -0,0 +1,113 @@
+/* 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/. */
+
+/* global loop, sinon */
+
+var expect = chai.expect;
+
+describe("loop.crypto", function() {
+  "use strict";
+
+  var sandbox, oldCrypto;
+
+  beforeEach(function() {
+    sandbox = sinon.sandbox.create();
+  });
+
+  afterEach(function() {
+    sandbox.restore();
+    loop.crypto.setRootObject(window);
+  });
+
+  describe("#isSupported", function() {
+    it("should return true by default", function() {
+      expect(loop.crypto.isSupported()).eql(true);
+    });
+
+    it("should return false if crypto isn't supported", function() {
+      loop.crypto.setRootObject({});
+
+      expect(loop.crypto.isSupported()).eql(false);
+    });
+  });
+
+  describe("#generateKey", function() {
+    it("should throw if web crypto is not available", function() {
+      loop.crypto.setRootObject({});
+
+      expect(function() {
+        loop.crypto.generateKey();
+      }).to.Throw(/not supported/);
+    });
+
+    it("should generate a key", function() {
+      // The key is a random string, so we can't really test much else.
+      return expect(loop.crypto.generateKey()).to.eventually.be.a("string");
+    });
+  });
+
+  describe("#encryptBytes", function() {
+    it("should throw if web crypto is not available", function() {
+      loop.crypto.setRootObject({});
+
+      expect(function() {
+        loop.crypto.encryptBytes();
+      }).to.Throw(/not supported/);
+    });
+
+    it("should encrypt an object with a specific key", function() {
+      return expect(loop.crypto.encryptBytes("Wt2-bZKeHO2wnaq00ZM6Nw",
+        JSON.stringify({test: true}))).to.eventually.be.a("string");
+    });
+  });
+
+  describe("#decryptBytes", function() {
+    it("should throw if web crypto is not available", function() {
+      loop.crypto.setRootObject({});
+
+      expect(function() {
+        loop.crypto.decryptBytes();
+      }).to.Throw(/not supported/);
+    });
+
+    it("should decypt an object via a specific key", function() {
+      var key = "Wt2-bZKeHO2wnaq00ZM6Nw";
+      var encryptedContext = "XvN9FDEm/GtE/5Bx5ezpn7JVDeZrtwOJy2CBjTGgJ4L33HhHOqEW+5k=";
+
+      return expect(loop.crypto.decryptBytes(key, encryptedContext)).to.eventually.eql(JSON.stringify({test: true}));
+    });
+
+    it("should fail if the key didn't work", function() {
+      var bad = "Bad-bZKeHO2wnaq00ZM6Nw";
+      var encryptedContext = "TGZaAE3mqsBFK0GfheZXXDCaRKXJmIKJ8WzF0KBEl4Aldzf3iYlAsLQdA8XSXXvtJR2UYz+f";
+
+      return expect(loop.crypto.decryptBytes(bad, encryptedContext)).to.be.rejected;
+    });
+  });
+
+  describe("Full cycle", function() {
+    it("should be able to encrypt and decypt in a full cycle", function(done) {
+      var context = JSON.stringify({
+        contextObject: true,
+        UTF8String: "对话"
+      });
+
+      return loop.crypto.generateKey().then(function (key) {
+        loop.crypto.encryptBytes(key, context).then(function(encryptedContext) {
+          loop.crypto.decryptBytes(key, encryptedContext).then(function(decryptedContext) {
+            expect(decryptedContext).eql(context);
+            done();
+          }).catch(function(error) {
+            done(error);
+          });
+        }).catch(function(error) {
+          done(error);
+        });
+      }).catch(function(error) {
+        done(error);
+      });
+    });
+  });
+
+});

browser/components/loop/test/shared/index.html

@@ -31,6 +31,7 @@
   <!-- test dependencies -->
   <script src="vendor/mocha-2.2.1.js"></script>
   <script src="vendor/chai-2.1.0.js"></script>
+  <script src="vendor/chai-as-promised-4.3.0.js"></script>
   <script src="vendor/sinon-1.13.0.js"></script>
   <script>
     /*global chai, mocha */
@@ -42,6 +43,7 @@
   <script src="../../content/shared/js/utils.js"></script>
   <script src="../../content/shared/js/models.js"></script>
   <script src="../../content/shared/js/mixins.js"></script>
+  <script src="../../content/shared/js/crypto.js"></script>
   <script src="../../content/shared/js/websocket.js"></script>
   <script src="../../content/shared/js/feedbackApiClient.js"></script>
   <script src="../../content/shared/js/validate.js"></script>
@@ -62,6 +64,7 @@
   <script src="models_test.js"></script>
   <script src="mixins_test.js"></script>
   <script src="utils_test.js"></script>
+  <script src="crypto_test.js"></script>
   <script src="views_test.js"></script>
   <script src="websocket_test.js"></script>
   <script src="feedbackApiClient_test.js"></script>

browser/components/loop/test/shared/otSdkDriver_test.js

@@ -77,7 +77,8 @@ describe("loop.OTSdkDriver", function () {
     driver = new loop.OTSdkDriver({
       dispatcher: dispatcher,
       sdk: sdk,
-      mozLoop: mozLoop
+      mozLoop: mozLoop,
+      isDesktop: true
     });
   });
 
@@ -98,10 +99,12 @@ describe("loop.OTSdkDriver", function () {
       }).to.Throw(/sdk/);
     });
 
-    it("should initialize the connectionStartTime to 'uninitialized'", function() {
+    it("should set the two-way media start time to 'uninitialized'", function() {
-      var driver = new loop.OTSdkDriver({sdk: sdk, dispatcher: dispatcher, mozLoop: mozLoop});
+      var driver = new loop.OTSdkDriver(
+        {sdk: sdk, dispatcher: dispatcher, mozLoop: mozLoop, isDesktop: true});
 
-      expect(driver.connectionStartTime).to.eql(driver.CONNECTION_START_TIME_UNINITIALIZED);
+      expect(driver._getTwoWayMediaStartTime()).to.
+        eql(driver.CONNECTION_START_TIME_UNINITIALIZED);
     });
   });
 
@@ -331,7 +334,7 @@ describe("loop.OTSdkDriver", function () {
       driver.session = session;
       var startTime = 1;
       var endTime = 3;
-      driver.connectionStartTime = startTime;
+      driver._setTwoWayMediaStartTime(startTime);
       sandbox.stub(performance, "now").returns(endTime);
       sandbox.stub(driver, "_noteConnectionLengthIfNeeded");
 
@@ -341,16 +344,17 @@ describe("loop.OTSdkDriver", function () {
                               endTime);
     });
 
-    it("should reset the connectionStartTime", function() {
+    it("should reset the two-way media connection start time", function() {
       driver.session = session;
       var startTime = 1;
-      driver.connectionStartTime = startTime;
+      driver._setTwoWayMediaStartTime(startTime);
       sandbox.stub(performance, "now");
       sandbox.stub(driver, "_noteConnectionLengthIfNeeded");
 
       driver.disconnectSession();
 
-      expect(driver.connectionStartTime).to.eql(driver.CONNECTION_START_TIME_UNINITIALIZED);
+      expect(driver._getTwoWayMediaStartTime()).to.
+        eql(driver.CONNECTION_START_TIME_UNINITIALIZED);
     });
   });
 
@@ -358,15 +362,15 @@ describe("loop.OTSdkDriver", function () {
     var startTimeMS;
     beforeEach(function() {
       startTimeMS = 1;
-      driver.connectionStartTime = startTimeMS;
+      driver._setTwoWayMediaStartTime(startTimeMS);
     });
 
-
+    it("should set two-way media start time to CONNECTION_START_TIME_ALREADY_NOTED", function() {
-    it("should set connectionStartTime to CONNECTION_START_TIME_ALREADY_NOTED", function() {
       var endTimeMS = 3;
       driver._noteConnectionLengthIfNeeded(startTimeMS, endTimeMS);
 
-      expect(driver.connectionStartTime).to.eql(driver.CONNECTION_START_TIME_ALREADY_NOTED);
+      expect(driver._getTwoWayMediaStartTime()).to.
+        eql(driver.CONNECTION_START_TIME_ALREADY_NOTED);
     });
 
     it("should call mozLoop.noteConnectionLength with SHORTER_THAN_10S for calls less than 10s", function() {
@@ -414,6 +418,17 @@ describe("loop.OTSdkDriver", function () {
         "LOOP_TWO_WAY_MEDIA_CONN_LENGTH",
         mozLoop.TWO_WAY_MEDIA_CONN_LENGTH.MORE_THAN_5M);
     });
+
+    it("should not call mozLoop.noteConnectionLength if driver._isDesktop " +
+       "is false",
+      function() {
+        var endTimeMS = 10 * 60 * 1000;
+        driver._isDesktop = false;
+
+        driver._noteConnectionLengthIfNeeded(startTimeMS, endTimeMS);
+
+        sinon.assert.notCalled(mozLoop.telemetryAddKeyedValue);
+      });
   });
 
   describe("#forceDisconnectAll", function() {
@@ -499,7 +514,7 @@ describe("loop.OTSdkDriver", function () {
         driver.session = session;
         var startTime = 1;
         var endTime = 3;
-        driver.connectionStartTime = startTime;
+        driver._setTwoWayMediaStartTime(startTime);
         sandbox.stub(performance, "now").returns(endTime);
         sandbox.stub(driver, "_noteConnectionLengthIfNeeded");
 
@@ -543,7 +558,7 @@ describe("loop.OTSdkDriver", function () {
         driver.session = session;
         var startTime = 1;
         var endTime = 3;
-        driver.connectionStartTime = startTime;
+        driver._setTwoWayMediaStartTime(startTime);
         sandbox.stub(performance, "now").returns(endTime);
         sandbox.stub(driver, "_noteConnectionLengthIfNeeded");
 
@@ -629,14 +644,27 @@ describe("loop.OTSdkDriver", function () {
           sinon.match.hasOwn("name", "mediaConnected"));
       });
 
-      it("should store the start time when both streams are up", function() {
+      it("should store the start time when both streams are up and" +
+      " driver._isDesktop is true", function() {
         driver._publishedLocalStream = true;
         var startTime = 1;
         sandbox.stub(performance, "now").returns(startTime);
 
         session.trigger("streamCreated", {stream: fakeStream});
 
-        expect(driver.connectionStartTime).to.eql(startTime);
+        expect(driver._getTwoWayMediaStartTime()).to.eql(startTime);
+      });
+
+      it("should not store the start time when both streams are up and" +
+         " driver._isDesktop is false", function() {
+        driver._isDesktop = false ;
+        driver._publishedLocalStream = true;
+        var startTime = 73;
+        sandbox.stub(performance, "now").returns(startTime);
+
+        session.trigger("streamCreated", {stream: fakeStream});
+
+        expect(driver._getTwoWayMediaStartTime()).to.not.eql(startTime);
       });
 
 

browser/components/loop/test/shared/utils_test.js

@@ -171,4 +171,51 @@ describe("loop.shared.utils", function() {
                               "subject", "body", "fake@invalid.tld");
     });
   });
+
+  describe("#btoa", function() {
+    it("should encode a basic base64 string", function() {
+      var result = sharedUtils.btoa(sharedUtils.strToUint8Array("crypto is great"));
+
+      expect(result).eql("Y3J5cHRvIGlzIGdyZWF0");
+    });
+
+    it("should pad encoded base64 strings", function() {
+      var result = sharedUtils.btoa(sharedUtils.strToUint8Array("crypto is grea"));
+
+      expect(result).eql("Y3J5cHRvIGlzIGdyZWE=");
+
+      result = sharedUtils.btoa(sharedUtils.strToUint8Array("crypto is gre"));
+
+      expect(result).eql("Y3J5cHRvIGlzIGdyZQ==");
+    });
+
+    it("should encode a non-unicode base64 string", function() {
+      var result = sharedUtils.btoa(sharedUtils.strToUint8Array("\uFDFD"));
+      expect(result).eql("77e9");
+    });
+  });
+
+  describe("#atob", function() {
+    it("should decode a basic base64 string", function() {
+      var result = sharedUtils.Uint8ArrayToStr(sharedUtils.atob("Y3J5cHRvIGlzIGdyZWF0"));
+
+      expect(result).eql("crypto is great");
+    });
+
+    it("should decode a padded base64 string", function() {
+      var result = sharedUtils.Uint8ArrayToStr(sharedUtils.atob("Y3J5cHRvIGlzIGdyZWE="));
+
+      expect(result).eql("crypto is grea");
+
+      result = sharedUtils.Uint8ArrayToStr(sharedUtils.atob("Y3J5cHRvIGlzIGdyZQ=="));
+
+      expect(result).eql("crypto is gre");
+    });
+
+    it("should decode a base64 string that has unicode characters", function() {
+      var result = sharedUtils.Uint8ArrayToStr(sharedUtils.atob("77e9"));
+
+      expect(result).eql("\uFDFD");
+    });
+  });
 });

browser/components/loop/test/shared/vendor/chai-as-promised-4.3.0.js

@@ -0,0 +1,377 @@
+(function () {
+    "use strict";
+
+    // Module systems magic dance.
+
+    /* istanbul ignore else */
+    if (typeof require === "function" && typeof exports === "object" && typeof module === "object") {
+        // NodeJS
+        module.exports = chaiAsPromised;
+    } else if (typeof define === "function" && define.amd) {
+        // AMD
+        define(function () {
+            return chaiAsPromised;
+        });
+    } else {
+        /*global self: false */
+
+        // Other environment (usually <script> tag): plug in to global chai instance directly.
+        chai.use(chaiAsPromised);
+
+        // Expose as a property of the global object so that consumers can configure the `transferPromiseness` property.
+        self.chaiAsPromised = chaiAsPromised;
+    }
+
+    chaiAsPromised.transferPromiseness = function (assertion, promise) {
+        assertion.then = promise.then.bind(promise);
+    };
+
+    chaiAsPromised.transformAsserterArgs = function (values) {
+        return values;
+    };
+
+    function chaiAsPromised(chai, utils) {
+        var Assertion = chai.Assertion;
+        var assert = chai.assert;
+
+        function isJQueryPromise(thenable) {
+            return typeof thenable.always === "function" &&
+                   typeof thenable.done === "function" &&
+                   typeof thenable.fail === "function" &&
+                   typeof thenable.pipe === "function" &&
+                   typeof thenable.progress === "function" &&
+                   typeof thenable.state === "function";
+        }
+
+        function assertIsAboutPromise(assertion) {
+            if (typeof assertion._obj.then !== "function") {
+                throw new TypeError(utils.inspect(assertion._obj) + " is not a thenable.");
+            }
+            if (isJQueryPromise(assertion._obj)) {
+                throw new TypeError("Chai as Promised is incompatible with jQuery's thenables, sorry! Please use a " +
+                                    "Promises/A+ compatible library (see http://promisesaplus.com/).");
+            }
+        }
+
+        function method(name, asserter) {
+            utils.addMethod(Assertion.prototype, name, function () {
+                assertIsAboutPromise(this);
+                return asserter.apply(this, arguments);
+            });
+        }
+
+        function property(name, asserter) {
+            utils.addProperty(Assertion.prototype, name, function () {
+                assertIsAboutPromise(this);
+                return asserter.apply(this, arguments);
+            });
+        }
+
+        function doNotify(promise, done) {
+            promise.then(function () { done(); }, done);
+        }
+
+        // These are for clarity and to bypass Chai refusing to allow `undefined` as actual when used with `assert`.
+        function assertIfNegated(assertion, message, extra) {
+            assertion.assert(true, null, message, extra.expected, extra.actual);
+        }
+
+        function assertIfNotNegated(assertion, message, extra) {
+            assertion.assert(false, message, null, extra.expected, extra.actual);
+        }
+
+        function getBasePromise(assertion) {
+            // We need to chain subsequent asserters on top of ones in the chain already (consider
+            // `eventually.have.property("foo").that.equals("bar")`), only running them after the existing ones pass.
+            // So the first base-promise is `assertion._obj`, but after that we use the assertions themselves, i.e.
+            // previously derived promises, to chain off of.
+            return typeof assertion.then === "function" ? assertion : assertion._obj;
+        }
+
+        // Grab these first, before we modify `Assertion.prototype`.
+
+        var propertyNames = Object.getOwnPropertyNames(Assertion.prototype);
+
+        var propertyDescs = {};
+        propertyNames.forEach(function (name) {
+            propertyDescs[name] = Object.getOwnPropertyDescriptor(Assertion.prototype, name);
+        });
+
+        property("fulfilled", function () {
+            var that = this;
+            var derivedPromise = getBasePromise(that).then(
+                function (value) {
+                    that._obj = value;
+                    assertIfNegated(that,
+                                    "expected promise not to be fulfilled but it was fulfilled with #{act}",
+                                    { actual: value });
+                    return value;
+                },
+                function (reason) {
+                    assertIfNotNegated(that,
+                                       "expected promise to be fulfilled but it was rejected with #{act}",
+                                       { actual: reason });
+                }
+            );
+
+            chaiAsPromised.transferPromiseness(that, derivedPromise);
+        });
+
+        property("rejected", function () {
+            var that = this;
+            var derivedPromise = getBasePromise(that).then(
+                function (value) {
+                    that._obj = value;
+                    assertIfNotNegated(that,
+                                       "expected promise to be rejected but it was fulfilled with #{act}",
+                                       { actual: value });
+                    return value;
+                },
+                function (reason) {
+                    assertIfNegated(that,
+                                    "expected promise not to be rejected but it was rejected with #{act}",
+                                    { actual: reason });
+
+                    // Return the reason, transforming this into a fulfillment, to allow further assertions, e.g.
+                    // `promise.should.be.rejected.and.eventually.equal("reason")`.
+                    return reason;
+                }
+            );
+
+            chaiAsPromised.transferPromiseness(that, derivedPromise);
+        });
+
+        method("rejectedWith", function (Constructor, message) {
+            var desiredReason = null;
+            var constructorName = null;
+
+            if (Constructor instanceof RegExp || typeof Constructor === "string") {
+                message = Constructor;
+                Constructor = null;
+            } else if (Constructor && Constructor instanceof Error) {
+                desiredReason = Constructor;
+                Constructor = null;
+                message = null;
+            } else if (typeof Constructor === "function") {
+                constructorName = (new Constructor()).name;
+            } else {
+                Constructor = null;
+            }
+
+            var that = this;
+            var derivedPromise = getBasePromise(that).then(
+                function (value) {
+                    var assertionMessage = null;
+                    var expected = null;
+
+                    if (Constructor) {
+                        assertionMessage = "expected promise to be rejected with #{exp} but it was fulfilled with " +
+                                           "#{act}";
+                        expected = constructorName;
+                    } else if (message) {
+                        var verb = message instanceof RegExp ? "matching" : "including";
+                        assertionMessage = "expected promise to be rejected with an error " + verb + " #{exp} but it " +
+                                           "was fulfilled with #{act}";
+                        expected = message;
+                    } else if (desiredReason) {
+                        assertionMessage = "expected promise to be rejected with #{exp} but it was fulfilled with " +
+                                           "#{act}";
+                        expected = desiredReason;
+                    }
+
+                    that._obj = value;
+
+                    assertIfNotNegated(that, assertionMessage, { expected: expected, actual: value });
+                },
+                function (reason) {
+                    if (Constructor) {
+                        that.assert(reason instanceof Constructor,
+                                    "expected promise to be rejected with #{exp} but it was rejected with #{act}",
+                                    "expected promise not to be rejected with #{exp} but it was rejected with #{act}",
+                                    constructorName,
+                                    reason);
+                    }
+
+                    var reasonMessage = utils.type(reason) === "object" && "message" in reason ?
+                                            reason.message :
+                                            "" + reason;
+                    if (message && reasonMessage !== null && reasonMessage !== undefined) {
+                        if (message instanceof RegExp) {
+                            that.assert(message.test(reasonMessage),
+                                        "expected promise to be rejected with an error matching #{exp} but got #{act}",
+                                        "expected promise not to be rejected with an error matching #{exp}",
+                                        message,
+                                        reasonMessage);
+                        }
+                        if (typeof message === "string") {
+                            that.assert(reasonMessage.indexOf(message) !== -1,
+                                        "expected promise to be rejected with an error including #{exp} but got #{act}",
+                                        "expected promise not to be rejected with an error including #{exp}",
+                                        message,
+                                        reasonMessage);
+                        }
+                    }
+
+                    if (desiredReason) {
+                        that.assert(reason === desiredReason,
+                                    "expected promise to be rejected with #{exp} but it was rejected with #{act}",
+                                    "expected promise not to be rejected with #{exp}",
+                                    desiredReason,
+                                    reason);
+                    }
+                }
+            );
+
+            chaiAsPromised.transferPromiseness(that, derivedPromise);
+        });
+
+        property("eventually", function () {
+            utils.flag(this, "eventually", true);
+        });
+
+        method("notify", function (done) {
+            doNotify(getBasePromise(this), done);
+        });
+
+        method("become", function (value) {
+            return this.eventually.deep.equal(value);
+        });
+
+        ////////
+        // `eventually`
+
+        // We need to be careful not to trigger any getters, thus `Object.getOwnPropertyDescriptor` usage.
+        var methodNames = propertyNames.filter(function (name) {
+            return name !== "assert" && typeof propertyDescs[name].value === "function";
+        });
+
+        methodNames.forEach(function (methodName) {
+            Assertion.overwriteMethod(methodName, function (originalMethod) {
+                return function () {
+                    doAsserterAsyncAndAddThen(originalMethod, this, arguments);
+                };
+            });
+        });
+
+        var getterNames = propertyNames.filter(function (name) {
+            return name !== "_obj" && typeof propertyDescs[name].get === "function";
+        });
+
+        getterNames.forEach(function (getterName) {
+            var propertyDesc = propertyDescs[getterName];
+
+            // Chainable methods are things like `an`, which can work both for `.should.be.an.instanceOf` and as
+            // `should.be.an("object")`. We need to handle those specially.
+            var isChainableMethod = false;
+            try {
+                isChainableMethod = typeof propertyDesc.get.call({}) === "function";
+            } catch (e) { }
+
+            if (isChainableMethod) {
+                Assertion.addChainableMethod(
+                    getterName,
+                    function () {
+                        var assertion = this;
+                        function originalMethod() {
+                            return propertyDesc.get.call(assertion).apply(assertion, arguments);
+                        }
+                        doAsserterAsyncAndAddThen(originalMethod, this, arguments);
+                    },
+                    function () {
+                        var originalGetter = propertyDesc.get;
+                        doAsserterAsyncAndAddThen(originalGetter, this);
+                    }
+                );
+            } else {
+                Assertion.overwriteProperty(getterName, function (originalGetter) {
+                    return function () {
+                        doAsserterAsyncAndAddThen(originalGetter, this);
+                    };
+                });
+            }
+        });
+
+        function doAsserterAsyncAndAddThen(asserter, assertion, args) {
+            // Since we're intercepting all methods/properties, we need to just pass through if they don't want
+            // `eventually`, or if we've already fulfilled the promise (see below).
+            if (!utils.flag(assertion, "eventually")) {
+                return asserter.apply(assertion, args);
+            }
+
+            var derivedPromise = getBasePromise(assertion).then(function (value) {
+                // Set up the environment for the asserter to actually run: `_obj` should be the fulfillment value, and
+                // now that we have the value, we're no longer in "eventually" mode, so we won't run any of this code,
+                // just the base Chai code that we get to via the short-circuit above.
+                assertion._obj = value;
+                utils.flag(assertion, "eventually", false);
+
+                return args ? chaiAsPromised.transformAsserterArgs(args) : args;
+            }).then(function (args) {
+                asserter.apply(assertion, args);
+
+                // Because asserters, for example `property`, can change the value of `_obj` (i.e. change the "object"
+                // flag), we need to communicate this value change to subsequent chained asserters. Since we build a
+                // promise chain paralleling the asserter chain, we can use it to communicate such changes.
+                return assertion._obj;
+            });
+
+            chaiAsPromised.transferPromiseness(assertion, derivedPromise);
+        }
+
+        ///////
+        // Now use the `Assertion` framework to build an `assert` interface.
+        var originalAssertMethods = Object.getOwnPropertyNames(assert).filter(function (propName) {
+            return typeof assert[propName] === "function";
+        });
+
+        assert.isFulfilled = function (promise, message) {
+            return (new Assertion(promise, message)).to.be.fulfilled;
+        };
+
+        assert.isRejected = function (promise, toTestAgainst, message) {
+            if (typeof toTestAgainst === "string") {
+                message = toTestAgainst;
+                toTestAgainst = undefined;
+            }
+
+            var assertion = (new Assertion(promise, message));
+            return toTestAgainst !== undefined ? assertion.to.be.rejectedWith(toTestAgainst) : assertion.to.be.rejected;
+        };
+
+        assert.becomes = function (promise, value, message) {
+            return assert.eventually.deepEqual(promise, value, message);
+        };
+
+        assert.doesNotBecome = function (promise, value, message) {
+            return assert.eventually.notDeepEqual(promise, value, message);
+        };
+
+        assert.eventually = {};
+        originalAssertMethods.forEach(function (assertMethodName) {
+            assert.eventually[assertMethodName] = function (promise) {
+                var otherArgs = Array.prototype.slice.call(arguments, 1);
+
+                var customRejectionHandler;
+                var message = arguments[assert[assertMethodName].length - 1];
+                if (typeof message === "string") {
+                    customRejectionHandler = function (reason) {
+                        throw new chai.AssertionError(message + "\n\nOriginal reason: " + utils.inspect(reason));
+                    };
+                }
+
+                var returnedPromise = promise.then(
+                    function (fulfillmentValue) {
+                        return assert[assertMethodName].apply(assert, [fulfillmentValue].concat(otherArgs));
+                    },
+                    customRejectionHandler
+                );
+
+                returnedPromise.notify = function (done) {
+                    doNotify(returnedPromise, done);
+                };
+
+                return returnedPromise;
+            };
+        });
+    }
+}());

browser/components/nsBrowserGlue.js

@@ -1429,7 +1429,7 @@ BrowserGlue.prototype = {
         () => BookmarkHTMLUtils.exportToFile(BookmarkHTMLUtils.defaultPath));
     }
 
-    Task.spawn(function() {
+    Task.spawn(function* () {
       // Check if Safe Mode or the user has required to restore bookmarks from
       // default profile's bookmarks.html
       let restoreDefaultBookmarks = false;
@@ -1505,23 +1505,21 @@ BrowserGlue.prototype = {
         if (bookmarksUrl) {
           // Import from bookmarks.html file.
           try {
-            BookmarkHTMLUtils.importFromURL(bookmarksUrl, true).then(null,
+            yield BookmarkHTMLUtils.importFromURL(bookmarksUrl, true);
-              function onFailure() {
+          } catch (e) {
-                Cu.reportError("Bookmarks.html file could be corrupt.");
+            Cu.reportError("Bookmarks.html file could be corrupt. " + e);
-              }
-            ).then(
-              function onComplete() {
-                // Now apply distribution customized bookmarks.
-                // This should always run after Places initialization.
-                this._distributionCustomizer.applyBookmarks();
-                // Ensure that smart bookmarks are created once the operation is
-                // complete.
-                this.ensurePlacesDefaultQueriesInitialized();
-              }.bind(this)
-            );
-          } catch (err) {
-            Cu.reportError("Bookmarks.html file could be corrupt. " + err);
           }
+          try {
+            // Now apply distribution customized bookmarks.
+            // This should always run after Places initialization.
+            this._distributionCustomizer.applyBookmarks();
+            // Ensure that smart bookmarks are created once the operation is
+            // complete.
+            this.ensurePlacesDefaultQueriesInitialized();
+          } catch (e) {
+            Cu.reportError(e);
+          }
+
         }
         else {
           Cu.reportError("Unable to find bookmarks.html file.");

browser/components/places/tests/unit/head_bookmarks.js

@@ -78,22 +78,6 @@ function checkItemHasAnnotation(guid, name) {
   });
 }
 
-function waitForImportAndSmartBookmarks() {
-  return Promise.all([
-    promiseTopicObserved("bookmarks-restore-success"),
-    PlacesTestUtils.promiseAsyncUpdates()
-  ]);
-}
-
-function promiseEndUpdateBatch() {
-  return new Promise(resolve => {
-    PlacesUtils.bookmarks.addObserver({
-      __proto__: NavBookmarkObserver.prototype,
-      onEndUpdateBatch: resolve
-    }, false);
-  });
-}
-
 let createCorruptDB = Task.async(function* () {
   let dbPath = OS.Path.join(OS.Constants.Path.profileDir, "places.sqlite");
   yield OS.File.remove(dbPath);

browser/components/places/tests/unit/test_browserGlue_corrupt.js

@@ -41,7 +41,7 @@ add_task(function* test_main() {
 
   // The test will continue once restore has finished and smart bookmarks
   // have been created.
-  yield promiseEndUpdateBatch();
+  yield promiseTopicObserved("places-browser-init-complete");
 
   let bm = yield PlacesUtils.bookmarks.fetch({
     parentGuid: PlacesUtils.bookmarks.toolbarGuid,

browser/components/places/tests/unit/test_browserGlue_corrupt_nobackup.js

@@ -35,7 +35,7 @@ add_task(function* () {
 
   // The test will continue once import has finished and smart bookmarks
   // have been created.
-  yield promiseEndUpdateBatch();
+  yield promiseTopicObserved("places-browser-init-complete");
 
   let bm = yield PlacesUtils.bookmarks.fetch({
     parentGuid: PlacesUtils.bookmarks.toolbarGuid,

browser/components/places/tests/unit/test_browserGlue_corrupt_nobackup_default.js

@@ -33,7 +33,7 @@ add_task(function* () {
 
   // The test will continue once import has finished and smart bookmarks
   // have been created.
-  yield promiseEndUpdateBatch();
+  yield promiseTopicObserved("places-browser-init-complete");
 
   let bm = yield PlacesUtils.bookmarks.fetch({
     parentGuid: PlacesUtils.bookmarks.toolbarGuid,

browser/components/places/tests/unit/test_browserGlue_migrate.js

@@ -40,7 +40,7 @@ add_task(function* test_migrate_bookmarks() {
     title: "migrated"
   });
 
-  let promise = promiseEndUpdateBatch();
+  let promise = promiseTopicObserved("places-browser-init-complete");
   bg.observe(null, "initial-migration-did-import-default-bookmarks", null);
   yield promise;
 

browser/components/places/tests/unit/test_browserGlue_prefs.js

@@ -38,11 +38,9 @@ do_register_cleanup(function () {
 
 function simulatePlacesInit() {
   do_print("Simulate Places init");
-  let promise = waitForImportAndSmartBookmarks();
-
   // Force nsBrowserGlue::_initPlaces().
   bg.observe(null, TOPIC_BROWSERGLUE_TEST, TOPICDATA_FORCE_PLACES_INIT);
-  return promise;
+  return promiseTopicObserved("places-browser-init-complete");
 }
 
 add_task(function* test_checkPreferences() {

browser/components/places/tests/unit/test_browserGlue_restore.js

@@ -44,7 +44,7 @@ add_task(function* test_main() {
 
   // The test will continue once restore has finished and smart bookmarks
   // have been created.
-  yield promiseEndUpdateBatch();
+  yield promiseTopicObserved("places-browser-init-complete");
 
   let bm = yield PlacesUtils.bookmarks.fetch({
     parentGuid: PlacesUtils.bookmarks.toolbarGuid,

browser/components/places/tests/unit/test_browserGlue_smartBookmarks.js

@@ -70,8 +70,6 @@ add_task(function* setup() {
   Assert.ok(!Services.prefs.getBoolPref(PREF_AUTO_EXPORT_HTML));
   Assert.ok(!Services.prefs.getBoolPref(PREF_RESTORE_DEFAULT_BOOKMARKS));
   Assert.throws(() => Services.prefs.getBoolPref(PREF_IMPORT_BOOKMARKS_HTML));
-
-  yield waitForImportAndSmartBookmarks();
 });
 
 add_task(function* test_version_0() {

browser/components/preferences/in-content/sync.js

@@ -237,6 +237,11 @@ let gSyncPane = {
     // service.fxAccountsEnabled is false iff sync is already configured for
     // the legacy provider.
     if (service.fxAccountsEnabled) {
+      // unhide the reading-list engine if readinglist is enabled (note we do
+      // it here as it must remain disabled for legacy sync users)
+      if (Services.prefs.getBoolPref("browser.readinglist.enabled")) {
+        document.getElementById("readinglist-engine").removeAttribute("hidden");
+      }
       // determine the fxa status...
       this.page = PAGE_PLEASE_WAIT;
       fxAccounts.getSignedInUser().then(data => {
@@ -372,6 +377,19 @@ let gSyncPane = {
     document.getElementById("sync-migration-deck").selectedIndex = selIndex;
   },
 
+  // Called whenever one of the sync engine preferences is changed.
+  onPreferenceChanged: function() {
+    let prefElts = document.querySelectorAll("#syncEnginePrefs > preference");
+    let syncEnabled = false;
+    for (let elt of prefElts) {
+      if (elt.name.startsWith("services.sync.") && elt.value) {
+        syncEnabled = true;
+        break;
+      }
+    }
+    Services.prefs.setBoolPref("services.sync.enabled", syncEnabled);
+  },
+
   startOver: function (showDialog) {
     if (showDialog) {
       let flags = Services.prompt.BUTTON_POS_0 * Services.prompt.BUTTON_TITLE_IS_STRING +

browser/components/preferences/in-content/sync.xul

@@ -4,7 +4,8 @@
 
 <!-- Sync panel -->
 
-<preferences hidden="true" data-category="paneSync">
+<preferences id="syncEnginePrefs" hidden="true" data-category="paneSync"
+             onchange="gSyncPane.onPreferenceChanged();">
   <preference id="engine.addons"
               name="services.sync.engine.addons"
               type="bool"/>
@@ -23,6 +24,10 @@
   <preference id="engine.passwords"
               name="services.sync.engine.passwords"
               type="bool"/>
+  <!-- non Sync-Engine engines -->
+  <preference id="engine.readinglist"
+              name="readinglist.scheduler.enabled"
+              type="bool"/>
 </preferences>
 
 <script type="application/javascript"
@@ -290,6 +295,11 @@
           <checkbox label="&engine.history.label;"
                     accesskey="&engine.history.accesskey;"
                     preference="engine.history"/>
+          <checkbox id="readinglist-engine"
+                    label="&engine.readinglist.label;"
+                    accesskey="&engine.readinglist.accesskey;"
+                    preference="engine.readinglist"
+                    hidden="true"/>
           <checkbox label="&engine.addons.label;"
                     accesskey="&engine.addons.accesskey;"
                     preference="engine.addons"/>

browser/components/preferences/sync.js

@@ -54,6 +54,26 @@ let gSyncPane = {
   },
 
   init: function () {
+    // We use a preference observer to notice changes to the Sync engines
+    // enabled state - other techniques are problematic due to the window
+    // being instant-apply on Mac etc but modal on Windows.
+    let prefObserver = () => {
+      // If all our Sync engines are disabled we flip the "master" Sync-enabled pref.
+      let prefElts = document.querySelectorAll("#syncEnginePrefs > preference");
+      let syncEnabled = false;
+      for (let elt of prefElts) {
+        if (elt.name.startsWith("services.sync.") && elt.value) {
+          syncEnabled = true;
+          break;
+        }
+      }
+      Services.prefs.setBoolPref("services.sync.enabled", syncEnabled);
+    }
+    Services.prefs.addObserver("services.sync.engine.", prefObserver, false);
+    window.addEventListener("unload", () => {
+      Services.prefs.removeObserver("services.sync.engine.", prefObserver);
+    }, false);
+
     // If the Service hasn't finished initializing, wait for it.
     let xps = Components.classes["@mozilla.org/weave/service;1"]
                                 .getService(Components.interfaces.nsISupports)
@@ -136,6 +156,11 @@ let gSyncPane = {
     // service.fxAccountsEnabled is false iff sync is already configured for
     // the legacy provider.
     if (service.fxAccountsEnabled) {
+      // unhide the reading-list engine if readinglist is enabled (note we do
+      // it here as it must remain disabled for legacy sync users)
+      if (Services.prefs.getBoolPref("browser.readinglist.enabled")) {
+        document.getElementById("readinglist-engine").removeAttribute("hidden");
+      }
       // determine the fxa status...
       this.page = PAGE_PLEASE_WAIT;
       fxAccounts.getSignedInUser().then(data => {

browser/components/preferences/sync.xul

@@ -21,13 +21,15 @@
             helpTopic="prefs-weave"
             onpaneload="gSyncPane.init()">
 
-    <preferences>
+    <preferences id="syncEnginePrefs">
       <preference id="engine.addons"    name="services.sync.engine.addons"    type="bool"/>
       <preference id="engine.bookmarks" name="services.sync.engine.bookmarks" type="bool"/>
       <preference id="engine.history"   name="services.sync.engine.history"   type="bool"/>
       <preference id="engine.tabs"      name="services.sync.engine.tabs"      type="bool"/>
       <preference id="engine.prefs"     name="services.sync.engine.prefs"     type="bool"/>
       <preference id="engine.passwords" name="services.sync.engine.passwords" type="bool"/>
+      <!-- non Sync-Engine engines -->
+      <preference id="engine.readinglist" name="readinglist.scheduler.enabled" type="bool"/>
     </preferences>
 
 
@@ -285,21 +287,33 @@
               <vbox>
                 <checkbox label="&engine.tabs.label;"
                           accesskey="&engine.tabs.accesskey;"
+                          onsynctopreference="gSyncPane.onPreferenceChanged();"
                           preference="engine.tabs"/>
                 <checkbox label="&engine.bookmarks.label;"
                           accesskey="&engine.bookmarks.accesskey;"
+                          onsynctopreference="gSyncPane.onPreferenceChanged();"
                           preference="engine.bookmarks"/>
                 <checkbox label="&engine.passwords.label;"
                           accesskey="&engine.passwords.accesskey;"
+                          onsynctopreference="gSyncPane.onPreferenceChanged();"
                           preference="engine.passwords"/>
                 <checkbox label="&engine.history.label;"
                           accesskey="&engine.history.accesskey;"
+                          onsynctopreference="gSyncPane.onPreferenceChanged(this);"
                           preference="engine.history"/>
+                <!-- onpreferencechanged not needed for the readinglist engine -->
+                <checkbox id="readinglist-engine"
+                          label="&engine.readinglist.label;"
+                          accesskey="&engine.readinglist.accesskey;"
+                          preference="engine.readinglist"
+                          hidden="true"/>
                 <checkbox label="&engine.addons.label;"
                           accesskey="&engine.addons.accesskey;"
+                          onsynctopreference="gSyncPane.onPreferenceChanged();"
                           preference="engine.addons"/>
                 <checkbox label="&engine.prefs.label;"
                           accesskey="&engine.prefs.accesskey;"
+                          onsynctopreference="gSyncPane.onPreferenceChanged();"
                           preference="engine.prefs"/>
               </vbox>
               <spacer/>

browser/components/readinglist/ReadingList.jsm

@@ -42,6 +42,7 @@ const ITEM_BASIC_PROPERTY_NAMES = `
   resolvedURL
   resolvedTitle
   excerpt
+  preview
   status
   favorite
   isArticle
@@ -289,24 +290,22 @@ ReadingListImpl.prototype = {
    /**
    * Add to the ReadingList the page that is loaded in a given browser.
    *
-   * @param {<xul:browser>} browser - Browser element for the document.
+   * @param {<xul:browser>} browser - Browser element for the document,
+   * used to get metadata about the article.
+   * @param {nsIURI/string} url - url to add to the reading list.
    * @return {Promise} Promise that is fullfilled with the added item.
    */
-  addItemFromBrowser: Task.async(function* (browser) {
+  addItemFromBrowser: Task.async(function* (browser, url) {
     let metadata = yield getMetadataFromBrowser(browser);
     let itemData = {
-      url: browser.currentURI,
+      url: url,
       title: metadata.title,
       resolvedURL: metadata.url,
       excerpt: metadata.description,
     };
 
-    if (metadata.description) {
-      itemData.exerpt = metadata.description;
-    }
-
     if (metadata.previews.length > 0) {
-      itemData.image = metadata.previews[0];
+      itemData.preview = metadata.previews[0];
     }
 
     let item = yield ReadingList.addItem(itemData);
@@ -713,6 +712,14 @@ ReadingListItem.prototype = {
     this._properties.readPosition = val;
   },
 
+  /**
+   * The URL to a preview image.
+   * @type string
+   */
+   get preview() {
+     return this._properties.preview;
+   },
+
   /**
    * Sets the given properties of the item, optionally calling list.updateItem().
    *
@@ -916,7 +923,7 @@ function getMetadataFromBrowser(browser) {
 Object.defineProperty(this, "ReadingList", {
   get() {
     if (!this._singleton) {
-      let store = new SQLiteStore("reading-list-temp.sqlite");
+      let store = new SQLiteStore("reading-list-temp2.sqlite");
       this._singleton = new ReadingListImpl(store);
     }
     return this._singleton;

browser/components/readinglist/SQLiteStore.jsm

@@ -204,7 +204,8 @@ this.SQLiteStore.prototype = {
         storedOn INTEGER,
         markedReadBy TEXT,
         markedReadOn INTEGER,
-        readPosition INTEGER
+        readPosition INTEGER,
+        preview TEXT
       );
     `);
     yield conn.execute(`

browser/components/readinglist/Scheduler.jsm

@@ -172,6 +172,10 @@ InternalScheduler.prototype = {
 
   // canSync indicates if we can currently sync.
   _canSync(ignoreBlockingErrors = false) {
+    if (!prefs.get("enabled")) {
+      this.log.info("canSync=false - syncing is disabled");
+      return false;
+    }
     if (Services.io.offline) {
       this.log.info("canSync=false - we are offline");
       return false;

browser/components/readinglist/sidebar.js

@@ -139,6 +139,12 @@ let RLSidebar = {
 
     itemNode.querySelector(".item-title").textContent = item.title;
     itemNode.querySelector(".item-domain").textContent = item.domain;
+    let thumb = itemNode.querySelector(".item-thumb-container");
+    if (item.preview) {
+      thumb.style.backgroundImage = "url(" + item.preview + ")";
+    } else {
+      thumb.style.removeProperty("background-image");
+    }
   },
 
   /**
@@ -165,7 +171,7 @@ let RLSidebar = {
   },
 
   /**
-   * The currently active element in the list.
+   * The list item displayed in the current tab.
    * @type {Element}
    */
   get activeItem() {
@@ -204,7 +210,7 @@ let RLSidebar = {
   },
 
   /**
-   * The currently selected item in the list.
+   * The list item selected with the keyboard.
    * @type {Element}
    */
   get selectedItem() {
@@ -366,15 +372,14 @@ let RLSidebar = {
   },
 
   /**
-   * Handle a mousemove event over the list box.
+   * Handle a mousemove event over the list box:
+   * If the hovered item isn't the selected one, clear the selection.
    * @param {Event} event - Triggering event.
    */
   onListMouseMove(event) {
     let itemNode = this.findParentItemNode(event.target);
-    if (!itemNode)
+    if (itemNode != this.selectedItem)
-      return;
+      this.selectedItem = null;
-
-    this.selectedItem = itemNode;
   },
 
   /**

browser/themes/linux/browser.css

@@ -1628,7 +1628,11 @@ richlistitem[type~="action"][actiontype="switchtab"] > .ac-url-box > .ac-action-
   list-style-image: url("chrome://browser/skin/Info.png");
 }
 
-%include ../shared/readinglist.inc.css
+%include ../shared/readinglist/readinglist.inc.css
+
+#readinglist-addremove-button {
+  padding: 0 2px;
+}
 
 /* Reader mode button */
 

browser/themes/osx/browser.css

@@ -2527,7 +2527,13 @@ richlistitem[type~="action"][actiontype="switchtab"][selected="true"] > .ac-url-
   }
 }
 
-%include ../shared/readinglist.inc.css
+%include ../shared/readinglist/readinglist.inc.css
+
+#readinglist-addremove-button {
+  padding: 3px;
+  -moz-padding-start: 2px;
+  -moz-padding-end: 1px;
+}
 
 /* Reader mode button */
 

browser/themes/shared/customizableui/panelUIOverlay.inc.css

@@ -505,7 +505,7 @@ toolbarpaletteitem[place="palette"] > toolbaritem > toolbarbutton {
   min-height: 40px;
   -moz-appearance: none;
   box-shadow: none;
-  border: none;  
+  border: none;
   border-radius: 0;
   transition: background-color;
   -moz-box-orient: horizontal;
@@ -796,7 +796,7 @@ panelview .toolbarbutton-1@buttonStateHover@,
 toolbarbutton.subviewbutton@buttonStateHover@,
 menu.subviewbutton@menuStateHover@,
 menuitem.subviewbutton@menuStateHover@,
-.share-provider-button@buttonStateHover@,
+.share-provider-button@buttonStateHover@:not([checked="true"]),
 .widget-overflow-list .toolbarbutton-1@buttonStateHover@,
 .toolbaritem-combined-buttons@inAnyPanel@ > toolbarbutton@buttonStateHover@ {
   background-color: hsla(210,4%,10%,.08);

browser/themes/shared/readinglist/icons.svg

@@ -13,25 +13,15 @@
         display: none;
       }
 
-      #addpage {
+      #addpage, #alreadyadded {
         fill: #808080;
       }
-      #addpage-hover {
+      #addpage-hover, #alreadyadded-hover {
         fill: #555555;
       }
-      #addpage-active {
+      #addpage-active, #alreadyadded-active {
         fill: #0095DD;
       }
-
-      #alreadyadded {
-        fill: #0095DD;
-      }
-      #alreadyadded-hover {
-        fill: #555555;
-      }
-      #alreadyadded-active {
-        fill: #808080;
-      }
     </style>
 
     <mask id="plus-mask">
@@ -40,18 +30,27 @@
       <rect x="7.5" y="4" width="1" height="8"/>
     </mask>
 
+    <mask id="minus-mask">
+      <rect width="100%" height="100%" fill="white"/>
+      <rect x="4" y="7.5" width="8" height="1"/>
+    </mask>
+
     <g id="addpage-shape">
       <circle cx="8" cy="8" r="7" mask="url(#plus-mask)"/>
     </g>
 
+    <g id="removepage-shape">
+      <circle cx="8" cy="8" r="7" mask="url(#minus-mask)"/>
+    </g>
+
   </defs>
 
   <use id="addpage"                 xlink:href="#addpage-shape"/>
   <use id="addpage-hover"           xlink:href="#addpage-shape"/>
   <use id="addpage-active"          xlink:href="#addpage-shape"/>
 
-  <use id="alreadyadded"            xlink:href="#addpage-shape"/>
+  <use id="alreadyadded"            xlink:href="#removepage-shape"/>
-  <use id="alreadyadded-hover"      xlink:href="#addpage-shape"/>
+  <use id="alreadyadded-hover"      xlink:href="#removepage-shape"/>
-  <use id="alreadyadded-active"     xlink:href="#addpage-shape"/>
+  <use id="alreadyadded-active"     xlink:href="#removepage-shape"/>
 
 </svg>

browser/themes/shared/readinglist/readinglist.inc.css

@@ -1,10 +1,13 @@
 /* Reading List button */
 
+#urlbar:not([focused]):not(:hover) #readinglist-addremove-button {
+  display: none;
+}
+
 #readinglist-addremove-button {
   -moz-appearance: none;
   border: none;
   list-style-image: url("chrome://browser/skin/readinglist/icons.svg#addpage");
-  padding: 3px;
 }
 
 #readinglist-addremove-button:hover {
@@ -12,15 +15,15 @@
 }
 
 #readinglist-addremove-button > .toolbarbutton-icon {
-  width: 16px;
+  width: 14px;
-  height: 16px
+  height: 14px
 }
 
-#readinglist-addremove-button:not([already-added="true"]):hover {
+#readinglist-addremove-button:hover {
   list-style-image: url("chrome://browser/skin/readinglist/icons.svg#addpage-hover");
 }
 
-#readinglist-addremove-button:not([already-added="true"]):active {
+#readinglist-addremove-button:active {
   list-style-image: url("chrome://browser/skin/readinglist/icons.svg#addpage-active");
 }
 
@@ -35,4 +38,3 @@
 #readinglist-addremove-button[already-added="true"]:active {
   list-style-image: url("chrome://browser/skin/readinglist/icons.svg#alreadyadded-active");
 }
-

browser/themes/shared/readinglist/sidebar.inc.css

@@ -47,10 +47,14 @@ body {
   max-width: 64px;
   min-height: 40px;
   max-height: 40px;
-  background: #EBEBEB;
   border: 1px solid white;
   box-shadow: 0px 1px 2px rgba(0,0,0,.35);
   margin: 5px;
+  background-color: #fff;
+  background-size: contain;
+  background-repeat: no-repeat;
+  background-position: center;
+  background-image: url("chrome://branding/content/silhouette-40.svg");
 }
 
 .item-summary-container {
@@ -84,11 +88,12 @@ body {
   color: #008ACB;
 }
 
-.item:not(:hover) .remove-button {
+.item:not(:hover):not(.selected) .remove-button {
   display: none;
 }
 
 .remove-button {
+  padding: 0;
   width: 16px;
   height: 16px;
   background-size: contain;

browser/themes/windows/browser.css

@@ -1576,7 +1576,11 @@ richlistitem[type~="action"][actiontype="switchtab"] > .ac-url-box > .ac-action-
   -moz-image-region: rect(0, 48px, 16px, 32px);
 }
 
-%include ../shared/readinglist.inc.css
+%include ../shared/readinglist/readinglist.inc.css
+
+#readinglist-addremove-button {
+  padding: 0 2px;
+}
 
 /* Reader mode button */
 

dom/locales/en-US/chrome/plugins.properties

@@ -25,7 +25,7 @@ learn_more_label=Learn More
 gmp_license_info=License information
 
 openH264_name=OpenH264 Video Codec provided by Cisco Systems, Inc.
-openH264_description=Play back web video and use video chats.
+openH264_description2=This plugin is automatically installed by Mozilla to comply with the WebRTC specification and to enable WebRTC calls with devices that require the H.264 video codec. Visit http://www.openh264.org/ to view the codec source code and learn more about the implementation.
 
 eme-adobe_name=Primetime Content Decryption Module provided by Adobe Systems, Incorporated
 eme-adobe_description=Play back protected web video.

mobile/android/base/locales/en-US/android_strings.dtd

@@ -186,6 +186,20 @@
 
 <!ENTITY tab_queue_toast_message "Open later">
 <!ENTITY tab_queue_toast_action "Open now">
+<!-- Localization note (tab_queue_notification_text_plural) : The
+     formatD is replaced with the number of tabs queued.  The
+     number of tabs queued is always more than one.  We can't use
+     Android plural forms, sadly. See Bug #753859. -->
+<!ENTITY tab_queue_notification_text_plural "&formatD; tabs queued">
+<!-- Localization note (tab_queue_notification_title_plural) : This is the
+     title of a notification; we expect more than one tab queued. -->
+<!ENTITY tab_queue_notification_title_plural "Tabs Queued">
+<!-- Localization note (tab_queue_notification_title_singular) : This is the
+     title of a notification; we expect only one tab queued. -->
+<!ENTITY tab_queue_notification_title_singular "Tab Queued">
+<!-- Localization note (tab_queue_notification_text_singular) : This is the
+     text of a notification; we expect only one tab queued. -->
+<!ENTITY tab_queue_notification_text_singular "1 tab queued">
 
 <!ENTITY pref_char_encoding "Character encoding">
 <!ENTITY pref_char_encoding_on "Show menu">

mobile/android/base/resources/values/ids.xml

@@ -5,6 +5,7 @@
 
 <resources>
 
+    <item type="id" name="tabQueueNotification"/>
     <item type="id" name="guestNotification"/>
     <item type="id" name="original_height"/>
     <item type="id" name="menu_items"/>

mobile/android/base/strings.xml.in

@@ -241,6 +241,10 @@
 
   <string name="tab_queue_toast_message">&tab_queue_toast_message;</string>
   <string name="tab_queue_toast_action">&tab_queue_toast_action;</string>
+  <string name="tab_queue_notification_text_singular">&tab_queue_notification_text_singular;</string>
+  <string name="tab_queue_notification_text_plural">&tab_queue_notification_text_plural;</string>
+  <string name="tab_queue_notification_title_singular">&tab_queue_notification_title_singular;</string>
+  <string name="tab_queue_notification_title_plural">&tab_queue_notification_title_plural;</string>
 
   <string name="pref_about_firefox">&pref_about_firefox;</string>
   <string name="pref_vendor_faqs">&pref_vendor_faqs;</string>

mobile/android/base/tabqueue/TabQueueHelper.java

@@ -5,20 +5,24 @@
 
 package org.mozilla.gecko.tabqueue;
 
-import org.mozilla.gecko.GeckoProfile;
+import android.app.NotificationManager;
-import org.mozilla.gecko.util.ThreadUtils;
+import android.app.PendingIntent;
-
+import android.content.Context;
-import android.text.TextUtils;
+import android.content.Intent;
-import android.util.Log;
+import android.content.res.Resources;
+import android.support.v4.app.NotificationCompat;
 import org.json.JSONArray;
-import org.json.JSONException;
+import org.mozilla.gecko.BrowserApp;
-
+import org.mozilla.gecko.GeckoProfile;
-import java.io.IOException;
+import org.mozilla.gecko.R;
+import org.mozilla.gecko.util.ThreadUtils;
 
 public class TabQueueHelper {
     private static final String LOGTAG = "Gecko" + TabQueueHelper.class.getSimpleName();
 
     public static final String FILE_NAME = "tab_queue_url_list.json";
+    public static final String LOAD_URLS_ACTION = "TAB_QUEUE_LOAD_URLS_ACTION";
+    public static final int TAB_QUEUE_NOTIFICATION_ID = R.id.tabQueueNotification;
 
     /**
      * Reads file and converts any content to JSON, adds passed in URL to the data and writes back to the file,
@@ -27,8 +31,9 @@ public class TabQueueHelper {
      * @param profile
      * @param url      URL to add
      * @param filename filename to add URL to
+     * @return the number of tabs currently queued
      */
-    public static void queueURL(final GeckoProfile profile, final String url, final String filename) {
+    public static int queueURL(final GeckoProfile profile, final String url, final String filename) {
         ThreadUtils.assertNotOnUiThread();
 
         JSONArray jsonArray = profile.readJSONArrayFromFile(filename);
@@ -36,5 +41,40 @@ public class TabQueueHelper {
         jsonArray.put(url);
 
         profile.writeFile(filename, jsonArray.toString());
+
+        return jsonArray.length();
+    }
+
+    /**
+     * Displays a notification showing the total number of tabs queue.  If there is already a notification displayed, it
+     * will be replaced.
+     *
+     * @param context
+     * @param tabsQueued
+     */
+    static public void showNotification(Context context, int tabsQueued) {
+        Intent resultIntent = new Intent(context, BrowserApp.class);
+        resultIntent.setAction(TabQueueHelper.LOAD_URLS_ACTION);
+
+        PendingIntent pendingIntent = PendingIntent.getActivity(context, 0, resultIntent, PendingIntent.FLAG_CANCEL_CURRENT);
+
+        String title, text;
+        final Resources resources = context.getResources();
+        if(tabsQueued == 1) {
+            title = resources.getString(R.string.tab_queue_notification_title_singular);
+            text = resources.getString(R.string.tab_queue_notification_text_singular);
+        } else {
+            title = resources.getString(R.string.tab_queue_notification_title_plural);
+            text = resources.getString(R.string.tab_queue_notification_text_plural, tabsQueued);
+        }
+
+        NotificationCompat.Builder builder = new NotificationCompat.Builder(context)
+                                                                   .setSmallIcon(R.drawable.ic_status_logo)
+                                                                   .setContentTitle(title)
+                                                                   .setContentText(text)
+                                                                   .setContentIntent(pendingIntent);
+
+        NotificationManager notificationManager = (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE);
+        notificationManager.notify(TabQueueHelper.TAB_QUEUE_NOTIFICATION_ID, builder.build());
     }
 }

mobile/android/base/tabqueue/TabQueueService.java

@@ -6,6 +6,7 @@
 package org.mozilla.gecko.tabqueue;
 
 import android.app.Service;
+import android.content.Context;
 import android.content.Intent;
 import android.content.res.Resources;
 import android.graphics.PixelFormat;
@@ -160,8 +161,10 @@ public class TabQueueService extends Service {
         executorService.submit(new Runnable() {
             @Override
             public void run() {
-                final GeckoProfile profile = GeckoProfile.get(getApplicationContext());
+                Context applicationContext = getApplicationContext();
-                TabQueueHelper.queueURL(profile, intentData, filename);
+                final GeckoProfile profile = GeckoProfile.get(applicationContext);
+                int tabsQueued = TabQueueHelper.queueURL(profile, intentData, filename);
+                TabQueueHelper.showNotification(applicationContext, tabsQueued);
             }
         });
     }

modules/libpref/init/all.js

@@ -4585,9 +4585,6 @@ pref("media.gmp-manager.certs.1.issuerName", "CN=DigiCert Secure Server CA,O=Dig
 pref("media.gmp-manager.certs.1.commonName", "aus4.mozilla.org");
 pref("media.gmp-manager.certs.2.issuerName", "CN=Thawte SSL CA,O=\"Thawte, Inc.\",C=US");
 pref("media.gmp-manager.certs.2.commonName", "aus4.mozilla.org");
-
-// Adobe EME is currently pref'd off by default and hidden in the addon manager.
-pref("media.gmp-eme-adobe.hidden", true);
 #endif
 
 // Whether or not to perform reader mode article parsing on page load.

services/sync/modules/browserid_identity.js

@@ -282,7 +282,7 @@ this.BrowserIDManager.prototype = {
       // The exception is when we've initialized with a user that needs to
       // reauth with the server - in that case we will also get here, but
       // should have the same identity.
-      // initializeWithCurrentIdentity will throw and log if these contraints
+      // initializeWithCurrentIdentity will throw and log if these constraints
       // aren't met, so just go ahead and do the init.
       this.initializeWithCurrentIdentity(true);
       break;

services/sync/modules/service.js

@@ -305,6 +305,21 @@ Sync11Service.prototype = {
     return false;
   },
 
+  // The global "enabled" state comes from prefs, and will be set to false
+  // whenever the UI that exposes what to sync finds all Sync engines disabled.
+  get enabled() {
+    return Svc.Prefs.get("enabled");
+  },
+  set enabled(val) {
+    // There's no real reason to impose this other than to catch someone doing
+    // something we don't expect with bad consequences - all setting of this
+    // pref are in the UI code and external to this module.
+    if (val) {
+      throw new Error("Only disabling via this setter is supported");
+    }
+    Svc.Prefs.set("enabled", val);
+  },
+
   /**
    * Prepare to initialize the rest of Weave after waiting a little bit
    */
@@ -334,8 +349,6 @@ Sync11Service.prototype = {
     this._clusterManager = this.identity.createClusterManager(this);
     this.recordManager = new RecordManager(this);
 
-    this.enabled = true;
-
     this._registerEngines();
 
     let ua = Cc["@mozilla.org/network/protocol;1?name=http"].
@@ -1245,6 +1258,10 @@ Sync11Service.prototype = {
   },
 
   sync: function sync() {
+    if (!this.enabled) {
+      this._log.debug("Not syncing as Sync is disabled.");
+      return;
+    }
     let dateStr = new Date().toLocaleFormat(LOG_DATE_FORMAT);
     this._log.debug("User-Agent: " + SyncStorageRequest.prototype.userAgent);
     this._log.info("Starting sync at " + dateStr);

services/sync/services-sync.js

@@ -24,6 +24,11 @@ pref("services.sync.scheduler.sync11.singleDeviceInterval", 86400); // 1 day
 
 pref("services.sync.errorhandler.networkFailureReportTimeout", 1209600); // 2 weeks
 
+// A "master" pref for Sync being enabled. Will be set to false if the sync
+// customization UI finds all our builtin engines disabled (and addons are
+// free to force this to true if they have their own engine)
+pref("services.sync.enabled", true);
+// Our engines.
 pref("services.sync.engine.addons", true);
 pref("services.sync.engine.bookmarks", true);
 pref("services.sync.engine.history", true);

services/sync/tests/unit/test_service_login.js

@@ -183,7 +183,7 @@ add_test(function test_login_on_sync() {
     // This test exercises these two branches.
 
     _("We're ready to sync if locked.");
-    Service.enabled = true;
+    Svc.Prefs.set("enabled", true);
     Services.io.offline = false;
     Service.scheduler.checkSyncStatus();
     do_check_true(scheduleCalled);

testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm

@@ -19,6 +19,7 @@ const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
+Cu.import("resource://gre/modules/Task.jsm");
 Cu.import("resource://gre/modules/Timer.jsm");
 Cu.import("resource://testing-common/TestUtils.jsm");
 
@@ -27,24 +28,54 @@ Cc["@mozilla.org/globalmessagemanager;1"]
   .loadFrameScript(
     "chrome://mochikit/content/tests/BrowserTestUtils/content-utils.js", true);
 
-
-/**
- * Default wait period in millseconds, when waiting for the expected
- * event to occur.
- * @type {number}
- */
-const DEFAULT_WAIT = 2000;
-
-
 this.BrowserTestUtils = {
   /**
+   * Loads a page in a new tab, executes a Task and closes the tab.
+   *
+   * @param options
+   *        An object with the following properties:
+   *        {
+   *          gBrowser:
+   *            Reference to the "tabbrowser" element where the new tab should
+   *            be opened.
+   *          url:
+   *            String with the URL of the page to load.
+   *        }
+   * @param taskFn
+   *        Generator function representing a Task that will be executed while
+   *        the tab is loaded. The first argument passed to the function is a
+   *        reference to the browser object for the new tab.
+   *
+   * @return {Promise}
+   * @resolves When the tab has been closed.
+   * @rejects Any exception from taskFn is propagated.
+   */
+  withNewTab: Task.async(function* (options, taskFn) {
+    let tab = options.gBrowser.addTab(options.url);
+    yield BrowserTestUtils.browserLoaded(tab.linkedBrowser);
+    options.gBrowser.selectedTab = tab;
+
+    yield taskFn(tab.linkedBrowser);
+
+    options.gBrowser.removeTab(tab);
+  }),
+
+  /**
+   * Waits for an ongoing page load in a browser window to complete.
+   *
+   * This can be used in conjunction with any synchronous method for starting a
+   * load, like the "addTab" method on "tabbrowser", and must be called before
+   * yielding control to the event loop. This is guaranteed to work because the
+   * way we're listening for the load is in the content-utils.js frame script,
+   * and then sending an async message up, so we can't miss the message.
+   *
    * @param {xul:browser} browser
    *        A xul:browser.
    * @param {Boolean} includeSubFrames
    *        A boolean indicating if loads from subframes should be included.
+   *
    * @return {Promise}
-   *         A Promise which resolves when a load event is triggered
+   * @resolves When a load event is triggered for the browser.
-   *         for browser.
    */
   browserLoaded(browser, includeSubFrames=false) {
     return new Promise(resolve => {
@@ -128,51 +159,51 @@ this.BrowserTestUtils = {
     });
   },
 
-
   /**
-   * Waits a specified number of miliseconds for a specified event to be
+   * Waits for an event to be fired on a specified element.
-   * fired on a specified element.
    *
    * Usage:
-   *    let receivedEvent = BrowserTestUtil.waitForEvent(element, "eventName");
+   *    let promiseEvent = BrowserTestUtil.waitForEvent(element, "eventName");
    *    // Do some processing here that will cause the event to be fired
    *    // ...
    *    // Now yield until the Promise is fulfilled
-   *    yield receivedEvent;
+   *    let receivedEvent = yield promiseEvent;
-   *    if (receivedEvent && !(receivedEvent instanceof Error)) {
-   *      receivedEvent.msg == "eventName";
-   *      // ...
-   *    }
    *
-   * @param {Element} subject - The element that should receive the event.
+   * @param {Element} subject
-   * @param {string} eventName - The event to wait for.
+   *        The element that should receive the event.
-   * @param {number} timeoutMs - The number of miliseconds to wait before giving up.
+   * @param {string} eventName
-   * @param {Element} target - Expected target of the event.
+   *        Name of the event to listen to.
-   * @returns {Promise} A Promise that resolves to the received event, or
+   * @param {function} checkFn [optional]
-   *                    rejects with an Error.
+   *        Called with the Event object as argument, should return true if the
+   *        event is the expected one, or false if it should be ignored and
+   *        listening should continue. If not specified, the first event with
+   *        the specified name resolves the returned promise.
+   *
+   * @note Because this function is intended for testing, any error in checkFn
+   *       will cause the returned promise to be rejected instead of waiting for
+   *       the next event, since this is probably a bug in the test.
+   *
+   * @returns {Promise}
+   * @resolves The Event object.
    */
-  waitForEvent(subject, eventName, timeoutMs, target) {
+  waitForEvent(subject, eventName, checkFn) {
     return new Promise((resolve, reject) => {
-      function listener(event) {
+      subject.addEventListener(eventName, function listener(event) {
-        if (target && target !== event.target) {
+        try {
-          return;
+          if (checkFn && !checkFn(event)) {
+            return;
+          }
+          subject.removeEventListener(eventName, listener);
+          resolve(event);
+        } catch (ex) {
+          try {
+            subject.removeEventListener(eventName, listener);
+          } catch (ex2) {
+            // Maybe the provided object does not support removeEventListener.
+          }
+          reject(ex);
         }
-
+      });
-        subject.removeEventListener(eventName, listener);
-        clearTimeout(timerID);
-        resolve(event);
-      }
-
-      timeoutMs = timeoutMs || DEFAULT_WAIT;
-      let stack = new Error().stack;
-
-      let timerID = setTimeout(() => {
-        subject.removeEventListener(eventName, listener);
-        reject(new Error(`${eventName} event timeout at ${stack}`));
-      }, timeoutMs);
-
-
-      subject.addEventListener(eventName, listener);
     });
   },
 };

testing/modules/TestUtils.jsm

@@ -49,15 +49,13 @@ this.TestUtils = {
     return new Promise((resolve, reject) => {
       Services.obs.addObserver(function observer(subject, topic, data) {
         try {
-          try {
+          if (checkFn && !checkFn(subject, data)) {
-            if (checkFn && !checkFn(subject, data)) {
+            return;
-              return;
-            }
-          } finally {
-            Services.obs.removeObserver(observer, topic);
           }
+          Services.obs.removeObserver(observer, topic);
           resolve([subject, data]);
         } catch (ex) {
+          Services.obs.removeObserver(observer, topic);
           reject(ex);
         }
       }, topic, false);

toolkit/components/passwordmgr/moz.build

@@ -9,6 +9,10 @@ MOCHITEST_CHROME_MANIFESTS += ['test/chrome.ini']
 BROWSER_CHROME_MANIFESTS += ['test/browser/browser.ini']
 XPCSHELL_TESTS_MANIFESTS += ['test/unit/xpcshell.ini']
 
+TESTING_JS_MODULES += [
+    'test/LoginTestUtils.jsm',
+]
+
 XPIDL_SOURCES += [
     'nsILoginInfo.idl',
     'nsILoginManager.idl',

toolkit/components/passwordmgr/nsLoginManagerPrompter.js

@@ -806,19 +806,13 @@ LoginManagerPrompter.prototype = {
           this._getLocalizedString("notifyBarRememberPasswordButtonText");
     var rememberButtonAccessKey =
           this._getLocalizedString("notifyBarRememberPasswordButtonAccessKey");
+    var usernamePlaceholder =
+          this._getLocalizedString("noUsernamePlaceholder");
 
     var displayHost = this._getShortDisplayHost(aLogin.hostname);
-    var notificationText;
+    var notificationText = this._getLocalizedString(
-    if (aLogin.username) {
-      var displayUser = this._sanitizeUsername(aLogin.username);
-      notificationText  = this._getLocalizedString(
-                                  "rememberPasswordMsg",
-                                  [displayUser, displayHost]);
-    } else {
-      notificationText  = this._getLocalizedString(
                                   "rememberPasswordMsgNoUsername",
                                   [displayHost]);
-    }
 
     // The callbacks in |buttons| have a closure to access the variables
     // in scope here; set one to |this._pwmgr| so we can get back to pwmgr
@@ -854,12 +848,28 @@ LoginManagerPrompter.prototype = {
 
       var { browser } = this._getNotifyWindow();
 
+      let eventCallback = function (topic) {
+        if (topic != "showing") {
+          return false;
+        }
+
+        let chromeDoc = this.browser.ownerDocument;
+
+        chromeDoc.getElementById("password-notification-username")
+                 .setAttribute("placeholder", usernamePlaceholder);
+        chromeDoc.getElementById("password-notification-username")
+                 .setAttribute("value", aLogin.username);
+        chromeDoc.getElementById("password-notification-password")
+                 .setAttribute("value", aLogin.password);
+      };
+
       aNotifyObj.show(browser, "password", notificationText,
                       "password-notification-icon", mainAction,
                       secondaryActions,
                       { timeout: Date.now() + 10000,
                         persistWhileVisible: true,
-                        passwordNotificationType: "password-save" });
+                        passwordNotificationType: "password-save",
+                        eventCallback });
     } else {
       var notNowButtonText =
             this._getLocalizedString("notifyBarNotNowButtonText");
@@ -1016,21 +1026,18 @@ LoginManagerPrompter.prototype = {
    *        A notification box or a popup notification.
    */
   _showChangeLoginNotification : function (aNotifyObj, aOldLogin, aNewPassword) {
-    var notificationText;
-    if (aOldLogin.username) {
-      var displayUser = this._sanitizeUsername(aOldLogin.username);
-      notificationText  = this._getLocalizedString(
-                                    "updatePasswordMsg",
-                                    [displayUser]);
-    } else {
-      notificationText  = this._getLocalizedString(
-                                    "updatePasswordMsgNoUser");
-    }
-
     var changeButtonText =
           this._getLocalizedString("notifyBarUpdateButtonText");
     var changeButtonAccessKey =
           this._getLocalizedString("notifyBarUpdateButtonAccessKey");
+    var usernamePlaceholder =
+          this._getLocalizedString("noUsernamePlaceholder");
+
+    // We reuse the existing message, even if it expects a username, until we
+    // switch to the final terminology in bug 1144856.
+    var displayHost = this._getShortDisplayHost(aOldLogin.hostname);
+    var notificationText = this._getLocalizedString("updatePasswordMsg",
+                                                    [displayHost]);
 
     // The callbacks in |buttons| have a closure to access the variables
     // in scope here; set one to |this._pwmgr| so we can get back to pwmgr
@@ -1053,12 +1060,28 @@ LoginManagerPrompter.prototype = {
 
       var { browser } = this._getNotifyWindow();
 
+      let eventCallback = function (topic) {
+        if (topic != "showing") {
+          return false;
+        }
+
+        let chromeDoc = this.browser.ownerDocument;
+
+        chromeDoc.getElementById("password-notification-username")
+                 .setAttribute("placeholder", usernamePlaceholder);
+        chromeDoc.getElementById("password-notification-username")
+                 .setAttribute("value", aOldLogin.username);
+        chromeDoc.getElementById("password-notification-password")
+                 .setAttribute("value", aNewPassword);
+      };
+
       Services.telemetry.getHistogramById("PWMGR_PROMPT_UPDATE_ACTION").add(PROMPT_DISPLAYED);
       aNotifyObj.show(browser, "password", notificationText,
                       "password-notification-icon", mainAction,
                       null, { timeout: Date.now() + 10000,
                               persistWhileVisible: true,
-                              passwordNotificationType: "password-change" });
+                              passwordNotificationType: "password-change",
+                              eventCallback });
     } else {
       var dontChangeButtonText =
             this._getLocalizedString("notifyBarDontChangeButtonText");

toolkit/components/passwordmgr/test/LoginTestUtils.jsm

@@ -0,0 +1,239 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/*
+ * Shared functions generally available for testing login components.
+ */
+
+"use strict";
+
+this.EXPORTED_SYMBOLS = [
+  "LoginTestUtils",
+];
+
+const { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components;
+
+Cu.import("resource://gre/modules/Promise.jsm");
+Cu.import("resource://gre/modules/Services.jsm");
+Cu.import("resource://gre/modules/Task.jsm");
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+
+Cu.import("resource://testing-common/TestUtils.jsm");
+
+const LoginInfo =
+      Components.Constructor("@mozilla.org/login-manager/loginInfo;1",
+                             "nsILoginInfo", "init");
+
+// For now, we need consumers to provide a reference to Assert.jsm.
+let Assert = null;
+
+this.LoginTestUtils = {
+  set Assert(assert) {
+    Assert = assert;
+  },
+
+  /**
+   * Forces the storage module to save all data, and the Login Manager service
+   * to replace the storage module with a newly initialized instance.
+   */
+  reloadData() {
+    Services.obs.notifyObservers(null, "passwordmgr-storage-replace", null);
+    yield TestUtils.topicObserved("passwordmgr-storage-replace-complete");
+  },
+
+  /**
+   * Erases all the data stored by the Login Manager service.
+   */
+  clearData() {
+    Services.logins.removeAllLogins();
+    for (let hostname of Services.logins.getAllDisabledHosts()) {
+      Services.logins.setLoginSavingEnabled(hostname, true);
+    }
+  },
+
+  /**
+   * Checks that the currently stored list of nsILoginInfo matches the provided
+   * array.  The comparison uses the "equals" method of nsILoginInfo, that does
+   * not include nsILoginMetaInfo properties in the test.
+   */
+  checkLogins(expectedLogins) {
+    this.assertLoginListsEqual(Services.logins.getAllLogins(), expectedLogins);
+  },
+
+  /**
+   * Checks that the two provided arrays of nsILoginInfo have the same length,
+   * and every login in "expected" is also found in "actual".  The comparison
+   * uses the "equals" method of nsILoginInfo, that does not include
+   * nsILoginMetaInfo properties in the test.
+   */
+  assertLoginListsEqual(actual, expected) {
+    Assert.equal(expected.length, actual.length);
+    Assert.ok(expected.every(e => actual.some(a => a.equals(e))));
+  },
+
+  /**
+   * Checks that the two provided arrays of strings contain the same values,
+   * maybe in a different order, case-sensitively.
+   */
+  assertDisabledHostsEqual(actual, expected) {
+    Assert.deepEqual(actual.sort(), expected.sort());
+  },
+
+  /**
+   * Checks whether the given time, expressed as the number of milliseconds
+   * since January 1, 1970, 00:00:00 UTC, falls within 30 seconds of now.
+   */
+  assertTimeIsAboutNow(timeMs) {
+    Assert.ok(Math.abs(timeMs - Date.now()) < 30000);
+  },
+};
+
+/**
+ * This object contains functions that return new instances of nsILoginInfo for
+ * every call.  The returned instances can be compared using their "equals" or
+ * "matches" methods, or modified for the needs of the specific test being run.
+ *
+ * Any modification to the test data requires updating the tests accordingly, in
+ * particular the search tests.
+ */
+this.LoginTestUtils.testData = {
+  /**
+   * Returns a new nsILoginInfo for use with form submits.
+   *
+   * @param modifications
+   *        Each property of this object replaces the property of the same name
+   *        in the returned nsILoginInfo or nsILoginMetaInfo.
+   */
+  formLogin(modifications) {
+    let loginInfo = new LoginInfo("http://www3.example.com",
+                                  "http://www.example.com", null,
+                                  "the username", "the password",
+                                  "form_field_username", "form_field_password");
+    loginInfo.QueryInterface(Ci.nsILoginMetaInfo);
+    if (modifications) {
+      for (let [name, value] of Iterator(modifications)) {
+        loginInfo[name] = value;
+      }
+    }
+    return loginInfo;
+  },
+
+  /**
+   * Returns a new nsILoginInfo for use with HTTP authentication.
+   *
+   * @param modifications
+   *        Each property of this object replaces the property of the same name
+   *        in the returned nsILoginInfo or nsILoginMetaInfo.
+   */
+  authLogin(modifications) {
+    let loginInfo = new LoginInfo("http://www.example.org", null,
+                                  "The HTTP Realm", "the username",
+                                  "the password", "", "");
+    loginInfo.QueryInterface(Ci.nsILoginMetaInfo);
+    if (modifications) {
+      for (let [name, value] of Iterator(modifications)) {
+        loginInfo[name] = value;
+      }
+    }
+    return loginInfo;
+  },
+
+  /**
+   * Returns an array of typical nsILoginInfo that could be stored in the
+   * database.
+   */
+  loginList() {
+    return [
+      // --- Examples of form logins (subdomains of example.com) ---
+
+      // Simple form login with named fields for username and password.
+      new LoginInfo("http://www.example.com", "http://www.example.com", null,
+                    "the username", "the password for www.example.com",
+                    "form_field_username", "form_field_password"),
+
+      // Different schemes are treated as completely different sites.
+      new LoginInfo("https://www.example.com", "https://www.example.com", null,
+                    "the username", "the password for https",
+                    "form_field_username", "form_field_password"),
+
+      // Subdomains are treated as completely different sites.
+      new LoginInfo("https://example.com", "https://example.com", null,
+                    "the username", "the password for example.com",
+                    "form_field_username", "form_field_password"),
+
+      // Forms found on the same host, but with different hostnames in the
+      // "action" attribute, are handled independently.
+      new LoginInfo("http://www3.example.com", "http://www.example.com", null,
+                    "the username", "the password",
+                    "form_field_username", "form_field_password"),
+      new LoginInfo("http://www3.example.com", "https://www.example.com", null,
+                    "the username", "the password",
+                    "form_field_username", "form_field_password"),
+      new LoginInfo("http://www3.example.com", "http://example.com", null,
+                    "the username", "the password",
+                    "form_field_username", "form_field_password"),
+
+      // It is not possible to store multiple passwords for the same username,
+      // however multiple passwords can be stored when the usernames differ.
+      // An empty username is a valid case and different from the others.
+      new LoginInfo("http://www4.example.com", "http://www4.example.com", null,
+                    "username one", "password one",
+                    "form_field_username", "form_field_password"),
+      new LoginInfo("http://www4.example.com", "http://www4.example.com", null,
+                    "username two", "password two",
+                    "form_field_username", "form_field_password"),
+      new LoginInfo("http://www4.example.com", "http://www4.example.com", null,
+                    "", "password three",
+                    "form_field_username", "form_field_password"),
+
+      // Username and passwords fields in forms may have no "name" attribute.
+      new LoginInfo("http://www5.example.com", "http://www5.example.com", null,
+                    "multi username", "multi password", "", ""),
+
+      // Forms with PIN-type authentication will typically have no username.
+      new LoginInfo("http://www6.example.com", "http://www6.example.com", null,
+                    "", "12345", "", "form_field_password"),
+
+      // --- Examples of authentication logins (subdomains of example.org) ---
+
+      // Simple HTTP authentication login.
+      new LoginInfo("http://www.example.org", null, "The HTTP Realm",
+                    "the username", "the password", "", ""),
+
+      // Simple FTP authentication login.
+      new LoginInfo("ftp://ftp.example.org", null, "ftp://ftp.example.org",
+                    "the username", "the password", "", ""),
+
+      // Multiple HTTP authentication logins can be stored for different realms.
+      new LoginInfo("http://www2.example.org", null, "The HTTP Realm",
+                    "the username", "the password", "", ""),
+      new LoginInfo("http://www2.example.org", null, "The HTTP Realm Other",
+                    "the username other", "the password other", "", ""),
+
+      // --- Both form and authentication logins (example.net) ---
+
+      new LoginInfo("http://example.net", "http://example.net", null,
+                    "the username", "the password",
+                    "form_field_username", "form_field_password"),
+      new LoginInfo("http://example.net", "http://www.example.net", null,
+                    "the username", "the password",
+                    "form_field_username", "form_field_password"),
+      new LoginInfo("http://example.net", "http://www.example.net", null,
+                    "username two", "the password",
+                    "form_field_username", "form_field_password"),
+      new LoginInfo("http://example.net", null, "The HTTP Realm",
+                    "the username", "the password", "", ""),
+      new LoginInfo("http://example.net", null, "The HTTP Realm Other",
+                    "username two", "the password", "", ""),
+      new LoginInfo("ftp://example.net", null, "ftp://example.net",
+                    "the username", "the password", "", ""),
+
+      // --- Examples of logins added by extensions (chrome scheme) ---
+
+      new LoginInfo("chrome://example_extension", null, "Example Login One",
+                    "the username", "the password one", "", ""),
+      new LoginInfo("chrome://example_extension", null, "Example Login Two",
+                    "the username", "the password two", "", ""),
+    ];
+  },
+};

toolkit/components/passwordmgr/test/browser/browser_notifications.js

@@ -1,33 +1,83 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
-add_task(function* test_save() {
+Cu.import("resource://testing-common/LoginTestUtils.jsm", this);
-  let tab = gBrowser.addTab("https://example.com/browser/toolkit/components/" +
-                            "passwordmgr/test/browser/form_basic.html");
-  let browser = tab.linkedBrowser;
-  yield BrowserTestUtils.browserLoaded(browser);
-  gBrowser.selectedTab = tab;
 
-  let promiseShown = BrowserTestUtils.waitForEvent(PopupNotifications.panel,
+/**
-                                                   "Shown");
+ * Test that the doorhanger notification for password saving is populated with
-  yield ContentTask.spawn(browser, null, function* () {
+ * the correct values in various password capture cases.
-    content.document.getElementById("form-basic-username").value = "username";
+ */
-    content.document.getElementById("form-basic-password").value = "password";
+add_task(function* test_save_change() {
-    content.document.getElementById("form-basic").submit();
+  let testCases = [{
-  });
+    username: "username",
-  yield promiseShown;
+    password: "password",
-  let notificationElement = PopupNotifications.panel.childNodes[0];
+  }, {
+    username: "",
+    password: "password",
+  }, {
+    username: "username",
+    oldPassword: "password",
+    password: "newPassword",
+  }, {
+    username: "",
+    oldPassword: "password",
+    password: "newPassword",
+  }];
 
-  let promiseLogin = TestUtils.topicObserved("passwordmgr-storage-changed",
+  for (let { username, oldPassword, password } of testCases) {
-                                             (_, data) => data == "addLogin");
+    // Add a login for the origin of the form if testing a change notification.
-  notificationElement.button.doCommand();
+    if (oldPassword) {
-  let [login] = yield promiseLogin;
+      Services.logins.addLogin(LoginTestUtils.testData.formLogin({
-  login.QueryInterface(Ci.nsILoginInfo);
+        hostname: "https://example.com",
+        formSubmitURL: "https://example.com",
+        username,
+        password: oldPassword,
+      }));
+    }
 
-  Assert.equal(login.username, "username");
+    yield BrowserTestUtils.withNewTab({
-  Assert.equal(login.password, "password");
+      gBrowser,
+      url: "https://example.com/browser/toolkit/components/" +
+           "passwordmgr/test/browser/form_basic.html",
+    }, function* (browser) {
+      // Submit the form in the content page with the credentials from the test
+      // case. This will cause the doorhanger notification to be displayed.
+      let promiseShown = BrowserTestUtils.waitForEvent(PopupNotifications.panel,
+                                                       "Shown");
+      yield ContentTask.spawn(browser, { username, password },
+        function* ({ username, password }) {
+          let doc = content.document;
+          doc.getElementById("form-basic-username").value = username;
+          doc.getElementById("form-basic-password").value = password;
+          doc.getElementById("form-basic").submit();
+        });
+      yield promiseShown;
 
-  // Cleanup.
+      // Check the actual content of the popup notification.
-  Services.logins.removeAllLogins();
+      Assert.equal(document.getElementById("password-notification-username")
-  gBrowser.removeTab(tab);
+                           .getAttribute("value"), username);
+      Assert.equal(document.getElementById("password-notification-password")
+                           .getAttribute("value"), password);
+
+      // Simulate the action on the notification to request the login to be
+      // saved, and wait for the data to be updated or saved based on the type
+      // of operation we expect.
+      let expectedNotification = oldPassword ? "modifyLogin" : "addLogin";
+      let promiseLogin = TestUtils.topicObserved("passwordmgr-storage-changed",
+                         (_, data) => data == expectedNotification);
+      let notificationElement = PopupNotifications.panel.childNodes[0];
+      notificationElement.button.doCommand();
+      let [result] = yield promiseLogin;
+
+      // Check that the values in the database match the expected values.
+      let login = oldPassword ? result.QueryInterface(Ci.nsIArray)
+                                      .queryElementAt(1, Ci.nsILoginInfo)
+                              : result.QueryInterface(Ci.nsILoginInfo);
+      Assert.equal(login.username, username);
+      Assert.equal(login.password, password);
+    });
+
+    // Clean up the database before the next test case is executed.
+    Services.logins.removeAllLogins();
+  }
 });

toolkit/components/passwordmgr/test/notification_common.js

@@ -64,7 +64,7 @@ function clickPopupButton(aPopup, aButtonIndex) {
         ok(true, "Triggering main action");
         notification.button.doCommand();
     } else if (aButtonIndex <= aPopup.secondaryActions.length) {
-        var index = aButtonIndex - 1;
+        var index = aButtonIndex;
         ok(true, "Triggering secondary action " + index);
         notification.childNodes[index].doCommand();
     }

toolkit/components/passwordmgr/test/test_notifications.html

@@ -347,7 +347,7 @@ function checkTest() {
         ok(popup, "got notification popup");
         // Check the text, which comes from the localized saveLoginText string.
         notificationText = popup.message;
-        expectedText = /^Would you like to remember the password for \"notifyu1\" on example.org\?$/;
+        expectedText = /^Would you like to remember the password on example.org\?$/;
         ok(expectedText.test(notificationText), "Checking text: " + notificationText);
         popup.remove();
         break;
@@ -360,7 +360,7 @@ function checkTest() {
         ok(popup, "got notification popup");
         // Check the text, which comes from the localized saveLoginText string.
         notificationText = popup.message;
-        expectedText = /^Would you like to remember the password for \"nowisthetimeforallgoodmentocom[^e]\" on example.org\?$/;
+        expectedText = /^Would you like to remember the password on example.org\?$/;
         ok(expectedText.test(notificationText), "Checking text: " + notificationText);
         popup.remove();
         break;

toolkit/components/passwordmgr/test/unit/head.js

@@ -31,6 +31,13 @@ const LoginInfo =
       Components.Constructor("@mozilla.org/login-manager/loginInfo;1",
                              "nsILoginInfo", "init");
 
+// Import LoginTestUtils.jsm as LoginTest.
+XPCOMUtils.defineLazyModuleGetter(this, "LoginTest",
+                                  "resource://testing-common/LoginTestUtils.jsm",
+                                  "LoginTestUtils");
+LoginTest.Assert = Assert;
+const TestData = LoginTest.testData;
+
 /**
  * All the tests are implemented with add_task, this starts them automatically.
  */
@@ -86,29 +93,6 @@ function getTempFile(aLeafName)
   return file;
 }
 
-/**
- * Allows waiting for an observer notification once.
- *
- * @param aTopic
- *        Notification topic to observe.
- *
- * @return {Promise}
- * @resolves The array [aSubject, aData] from the observed notification.
- * @rejects Never.
- */
-function promiseTopicObserved(aTopic)
-{
-  let deferred = Promise.defer();
-
-  Services.obs.addObserver(
-    function PTO_observe(aSubject, aTopic, aData) {
-      Services.obs.removeObserver(PTO_observe, aTopic);
-      deferred.resolve([aSubject, aData]);
-    }, aTopic, false);
-
-  return deferred.promise;
-}
-
 /**
  * Returns a new XPCOM property bag with the provided properties.
  *
@@ -134,70 +118,6 @@ function newPropertyBag(aProperties)
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-//// Local helpers
-
-const LoginTest = {
-  /**
-   * Forces the storage module to save all data, and the Login Manager service
-   * to replace the storage module with a newly initialized instance.
-   */
-  reloadData: function ()
-  {
-    Services.obs.notifyObservers(null, "passwordmgr-storage-replace", null);
-    yield promiseTopicObserved("passwordmgr-storage-replace-complete");
-  },
-
-  /**
-   * Erases all the data stored by the Login Manager service.
-   */
-  clearData: function ()
-  {
-    Services.logins.removeAllLogins();
-    for (let hostname of Services.logins.getAllDisabledHosts()) {
-      Services.logins.setLoginSavingEnabled(hostname, true);
-    }
-  },
-
-  /**
-   * Checks that the currently stored list of nsILoginInfo matches the provided
-   * array.  The comparison uses the "equals" method of nsILoginInfo, that does
-   * not include nsILoginMetaInfo properties in the test.
-   */
-  checkLogins: function (aExpectedLogins)
-  {
-    this.assertLoginListsEqual(Services.logins.getAllLogins(), aExpectedLogins);
-  },
-
-  /**
-   * Checks that the two provided arrays of nsILoginInfo have the same length,
-   * and every login in aExpectedLogins is also found in aActualLogins.  The
-   * comparison uses the "equals" method of nsILoginInfo, that does not include
-   * nsILoginMetaInfo properties in the test.
-   */
-  assertLoginListsEqual: function (aActual, aExpected)
-  {
-    do_check_eq(aExpected.length, aActual.length);
-    do_check_true(aExpected.every(e => aActual.some(a => a.equals(e))));
-  },
-
-  /**
-   * Checks that the two provided arrays of strings contain the same values,
-   * maybe in a different order, case-sensitively.
-   */
-  assertDisabledHostsEqual: function (aActual, aExpected)
-  {
-    Assert.deepEqual(aActual.sort(), aExpected.sort());
-  },
-
-  /**
-   * Checks whether the given time, expressed as the number of milliseconds
-   * since January 1, 1970, 00:00:00 UTC, falls within 30 seconds of now.
-   */
-  assertTimeIsAboutNow: function (aTimeMs)
-  {
-    do_check_true(Math.abs(aTimeMs - Date.now()) < 30000);
-  }
-};
 
 const RecipeHelpers = {
   initNewParent() {
@@ -238,163 +158,6 @@ const RecipeHelpers = {
   }
 };
 
-////////////////////////////////////////////////////////////////////////////////
-//// Predefined test data
-
-/**
- * This object contains functions that return new instances of nsILoginInfo for
- * every call.  The returned instances can be compared using their "equals" or
- * "matches" methods, or modified for the needs of the specific test being run.
- *
- * Any modification to the test data requires updating the tests accordingly, in
- * particular the search tests.
- */
-const TestData = {
-  /**
-   * Returns a new nsILoginInfo for use with form submits.
-   *
-   * @param aModifications
-   *        Each property of this object replaces the property of the same name
-   *        in the returned nsILoginInfo or nsILoginMetaInfo.
-   */
-  formLogin: function (aModifications)
-  {
-    let loginInfo = new LoginInfo("http://www3.example.com",
-                                  "http://www.example.com", null,
-                                  "the username", "the password",
-                                  "form_field_username", "form_field_password");
-    loginInfo.QueryInterface(Ci.nsILoginMetaInfo);
-    if (aModifications) {
-      for (let [name, value] of Iterator(aModifications)) {
-        loginInfo[name] = value;
-      }
-    }
-    return loginInfo;
-  },
-
-  /**
-   * Returns a new nsILoginInfo for use with HTTP authentication.
-   *
-   * @param aModifications
-   *        Each property of this object replaces the property of the same name
-   *        in the returned nsILoginInfo or nsILoginMetaInfo.
-   */
-  authLogin: function (aModifications)
-  {
-    let loginInfo = new LoginInfo("http://www.example.org", null,
-                                  "The HTTP Realm", "the username",
-                                  "the password", "", "");
-    loginInfo.QueryInterface(Ci.nsILoginMetaInfo);
-    if (aModifications) {
-      for (let [name, value] of Iterator(aModifications)) {
-        loginInfo[name] = value;
-      }
-    }
-    return loginInfo;
-  },
-
-  /**
-   * Returns an array of typical nsILoginInfo that could be stored in the
-   * database.
-   */
-  loginList: function ()
-  {
-    return [
-      // --- Examples of form logins (subdomains of example.com) ---
-
-      // Simple form login with named fields for username and password.
-      new LoginInfo("http://www.example.com", "http://www.example.com", null,
-                    "the username", "the password for www.example.com",
-                    "form_field_username", "form_field_password"),
-
-      // Different schemes are treated as completely different sites.
-      new LoginInfo("https://www.example.com", "https://www.example.com", null,
-                    "the username", "the password for https",
-                    "form_field_username", "form_field_password"),
-
-      // Subdomains are treated as completely different sites.
-      new LoginInfo("https://example.com", "https://example.com", null,
-                    "the username", "the password for example.com",
-                    "form_field_username", "form_field_password"),
-
-      // Forms found on the same host, but with different hostnames in the
-      // "action" attribute, are handled independently.
-      new LoginInfo("http://www3.example.com", "http://www.example.com", null,
-                    "the username", "the password",
-                    "form_field_username", "form_field_password"),
-      new LoginInfo("http://www3.example.com", "https://www.example.com", null,
-                    "the username", "the password",
-                    "form_field_username", "form_field_password"),
-      new LoginInfo("http://www3.example.com", "http://example.com", null,
-                    "the username", "the password",
-                    "form_field_username", "form_field_password"),
-
-      // It is not possible to store multiple passwords for the same username,
-      // however multiple passwords can be stored when the usernames differ.
-      // An empty username is a valid case and different from the others.
-      new LoginInfo("http://www4.example.com", "http://www4.example.com", null,
-                    "username one", "password one",
-                    "form_field_username", "form_field_password"),
-      new LoginInfo("http://www4.example.com", "http://www4.example.com", null,
-                    "username two", "password two",
-                    "form_field_username", "form_field_password"),
-      new LoginInfo("http://www4.example.com", "http://www4.example.com", null,
-                    "", "password three",
-                    "form_field_username", "form_field_password"),
-
-      // Username and passwords fields in forms may have no "name" attribute.
-      new LoginInfo("http://www5.example.com", "http://www5.example.com", null,
-                    "multi username", "multi password", "", ""),
-
-      // Forms with PIN-type authentication will typically have no username.
-      new LoginInfo("http://www6.example.com", "http://www6.example.com", null,
-                    "", "12345", "", "form_field_password"),
-
-      // --- Examples of authentication logins (subdomains of example.org) ---
-
-      // Simple HTTP authentication login.
-      new LoginInfo("http://www.example.org", null, "The HTTP Realm",
-                    "the username", "the password", "", ""),
-
-      // Simple FTP authentication login.
-      new LoginInfo("ftp://ftp.example.org", null, "ftp://ftp.example.org",
-                    "the username", "the password", "", ""),
-
-      // Multiple HTTP authentication logins can be stored for different realms.
-      new LoginInfo("http://www2.example.org", null, "The HTTP Realm",
-                    "the username", "the password", "", ""),
-      new LoginInfo("http://www2.example.org", null, "The HTTP Realm Other",
-                    "the username other", "the password other", "", ""),
-
-      // --- Both form and authentication logins (example.net) ---
-
-      new LoginInfo("http://example.net", "http://example.net", null,
-                    "the username", "the password",
-                    "form_field_username", "form_field_password"),
-      new LoginInfo("http://example.net", "http://www.example.net", null,
-                    "the username", "the password",
-                    "form_field_username", "form_field_password"),
-      new LoginInfo("http://example.net", "http://www.example.net", null,
-                    "username two", "the password",
-                    "form_field_username", "form_field_password"),
-      new LoginInfo("http://example.net", null, "The HTTP Realm",
-                    "the username", "the password", "", ""),
-      new LoginInfo("http://example.net", null, "The HTTP Realm Other",
-                    "username two", "the password", "", ""),
-      new LoginInfo("ftp://example.net", null, "ftp://example.net",
-                    "the username", "the password", "", ""),
-
-      // --- Examples of logins added by extensions (chrome scheme) ---
-
-      new LoginInfo("chrome://example_extension", null, "Example Login One",
-                    "the username", "the password one", "", ""),
-      new LoginInfo("chrome://example_extension", null, "Example Login Two",
-                    "the username", "the password two", "", ""),
-    ];
-  },
-};
-
-////////////////////////////////////////////////////////////////////////////////
 //// Initialization functions common to all tests
 
 add_task(function test_common_initialize()

toolkit/components/places/Bookmarks.jsm

@@ -258,7 +258,7 @@ let Bookmarks = Object.freeze({
                         , validIf: b => b.lastModified >= item.dateAdded }
         });
 
-      let db = yield DBConnPromised;
+      let db = yield PlacesUtils.promiseWrappedConnection();
       let parent;
       if (updateInfo.hasOwnProperty("parentGuid")) {
         if (item.type == this.TYPE_FOLDER) {
@@ -426,7 +426,7 @@ let Bookmarks = Object.freeze({
    * @resolves once the removal is complete.
    */
   eraseEverything: Task.async(function* () {
-    let db = yield DBConnPromised;
+    let db = yield PlacesUtils.promiseWrappedConnection();
     yield db.executeTransaction(function* () {
       const folderGuids = [this.toolbarGuid, this.menuGuid, this.unfiledGuid];
       yield removeFoldersContents(db, folderGuids);
@@ -670,29 +670,11 @@ function notify(observers, notification, args) {
   }
 }
 
-XPCOMUtils.defineLazyGetter(this, "DBConnPromised",
-  () => new Promise((resolve, reject) => {
-    Sqlite.wrapStorageConnection({ connection: PlacesUtils.history.DBConnection } )
-          .then(db => {
-      try {
-        Sqlite.shutdown.addBlocker("Places Bookmarks.jsm wrapper closing",
-                                   db.close.bind(db));
-      }
-      catch (ex) {
-        // It's too late to block shutdown, just close the connection.
-        db.close();
-        reject(ex);
-      }
-      resolve(db);
-    });
-  })
-);
-
 ////////////////////////////////////////////////////////////////////////////////
 // Update implementation.
 
 function* updateBookmark(info, item, newParent) {
-  let db = yield DBConnPromised;
+  let db = yield PlacesUtils.promiseWrappedConnection();
 
   let tuples = new Map();
   if (info.hasOwnProperty("lastModified"))
@@ -779,7 +761,7 @@ function* updateBookmark(info, item, newParent) {
 // Insert implementation.
 
 function* insertBookmark(item, parent) {
-  let db = yield DBConnPromised;
+  let db = yield PlacesUtils.promiseWrappedConnection();
 
   // If a guid was not provided, generate one, so we won't need to fetch the
   // bookmark just after having created it.
@@ -834,7 +816,7 @@ function* insertBookmark(item, parent) {
 // Fetch implementation.
 
 function* fetchBookmark(info) {
-  let db = yield DBConnPromised;
+  let db = yield PlacesUtils.promiseWrappedConnection();
 
   let rows = yield db.executeCached(
     `SELECT b.guid, IFNULL(p.guid, "") AS parentGuid, b.position AS 'index',
@@ -852,7 +834,7 @@ function* fetchBookmark(info) {
 }
 
 function* fetchBookmarkByPosition(info) {
-  let db = yield DBConnPromised;
+  let db = yield PlacesUtils.promiseWrappedConnection();
   let index = info.index == Bookmarks.DEFAULT_INDEX ? null : info.index;
 
   let rows = yield db.executeCached(
@@ -874,7 +856,7 @@ function* fetchBookmarkByPosition(info) {
 }
 
 function* fetchBookmarksByURL(info) {
-  let db = yield DBConnPromised;
+  let db = yield PlacesUtils.promiseWrappedConnection();
 
   let rows = yield db.executeCached(
     `SELECT b.guid, IFNULL(p.guid, "") AS parentGuid, b.position AS 'index',
@@ -895,7 +877,7 @@ function* fetchBookmarksByURL(info) {
 }
 
 function* fetchBookmarksByParent(info) {
-  let db = yield DBConnPromised;
+  let db = yield PlacesUtils.promiseWrappedConnection();
 
   let rows = yield db.executeCached(
     `SELECT b.guid, IFNULL(p.guid, "") AS parentGuid, b.position AS 'index',
@@ -917,7 +899,7 @@ function* fetchBookmarksByParent(info) {
 // Remove implementation.
 
 function* removeBookmark(item) {
-  let db = yield DBConnPromised;
+  let db = yield PlacesUtils.promiseWrappedConnection();
 
   let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId;
 
@@ -960,7 +942,7 @@ function* removeBookmark(item) {
 // Reorder implementation.
 
 function* reorderChildren(parent, orderedChildrenGuids) {
-  let db = yield DBConnPromised;
+  let db = yield PlacesUtils.promiseWrappedConnection();
 
   return db.executeTransaction(function* () {
     // Select all of the direct children for the given parent.

toolkit/components/places/Database.cpp

@@ -735,6 +735,13 @@ Database::InitSchema(bool* aDatabaseMigrated)
 
       // Firefox 37 uses schema version 26.
 
+      if (currentSchemaVersion < 27) {
+        rv = MigrateV27Up();
+        NS_ENSURE_SUCCESS(rv, rv);
+      }
+
+      // Firefox 38 uses schema version 27.
+
       // Schema Upgrades must add migration code here.
 
       rv = UpdateBookmarkRootTitles();
@@ -801,6 +808,8 @@ Database::InitSchema(bool* aDatabaseMigrated)
     // moz_keywords.
     rv = mMainConn->ExecuteSimpleSQL(CREATE_MOZ_KEYWORDS);
     NS_ENSURE_SUCCESS(rv, rv);
+    rv = mMainConn->ExecuteSimpleSQL(CREATE_IDX_MOZ_KEYWORDS_PLACEPOSTDATA);
+    NS_ENSURE_SUCCESS(rv, rv);
 
     // moz_favicons.
     rv = mMainConn->ExecuteSimpleSQL(CREATE_MOZ_FAVICONS);
@@ -951,11 +960,18 @@ Database::InitTempTriggers()
   rv = mMainConn->ExecuteSimpleSQL(CREATE_PLACES_AFTERUPDATE_TYPED_TRIGGER);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = mMainConn->ExecuteSimpleSQL(CREATE_FOREIGNCOUNT_AFTERDELETE_TRIGGER);
+  rv = mMainConn->ExecuteSimpleSQL(CREATE_BOOKMARKS_FOREIGNCOUNT_AFTERDELETE_TRIGGER);
   NS_ENSURE_SUCCESS(rv, rv);
-  rv = mMainConn->ExecuteSimpleSQL(CREATE_FOREIGNCOUNT_AFTERINSERT_TRIGGER);
+  rv = mMainConn->ExecuteSimpleSQL(CREATE_BOOKMARKS_FOREIGNCOUNT_AFTERINSERT_TRIGGER);
   NS_ENSURE_SUCCESS(rv, rv);
-  rv = mMainConn->ExecuteSimpleSQL(CREATE_FOREIGNCOUNT_AFTERUPDATE_TRIGGER);
+  rv = mMainConn->ExecuteSimpleSQL(CREATE_BOOKMARKS_FOREIGNCOUNT_AFTERUPDATE_TRIGGER);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = mMainConn->ExecuteSimpleSQL(CREATE_KEYWORDS_FOREIGNCOUNT_AFTERDELETE_TRIGGER);
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = mMainConn->ExecuteSimpleSQL(CREATE_KEYWORDS_FOREIGNCOUNT_AFTERINSERT_TRIGGER);
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = mMainConn->ExecuteSimpleSQL(CREATE_KEYWORDS_FOREIGNCOUNT_AFTERUPDATE_TRIGGER);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
@@ -1487,6 +1503,66 @@ Database::MigrateV26Up() {
   return NS_OK;
 }
 
+nsresult
+Database::MigrateV27Up() {
+  MOZ_ASSERT(NS_IsMainThread());
+
+  // Change keywords store, moving their relation from bookmarks to urls.
+  nsCOMPtr<mozIStorageStatement> stmt;
+  nsresult rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
+    "SELECT place_id FROM moz_keywords"
+  ), getter_AddRefs(stmt));
+  if (NS_FAILED(rv)) {
+    // Even if these 2 columns have a unique constraint, we allow NULL values
+    // for backwards compatibility. NULL never breaks a unique constraint.
+    rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+      "ALTER TABLE moz_keywords ADD COLUMN place_id INTEGER"));
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+      "ALTER TABLE moz_keywords ADD COLUMN post_data TEXT"));
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = mMainConn->ExecuteSimpleSQL(CREATE_IDX_MOZ_KEYWORDS_PLACEPOSTDATA);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  // Associate keywords with uris.  A keyword could be associated to multiple
+  // bookmarks uris, or multiple keywords could be associated to the same uri.
+  // The new system only allows multiple uris per keyword, provided they have
+  // a different post_data value.
+  rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "INSERT OR REPLACE INTO moz_keywords (id, keyword, place_id, post_data) "
+    "SELECT k.id, k.keyword, h.id, MAX(a.content) "
+    "FROM moz_places h "
+    "JOIN moz_bookmarks b ON b.fk = h.id "
+    "JOIN moz_keywords k ON k.id = b.keyword_id "
+    "LEFT JOIN moz_items_annos a ON a.item_id = b.id "
+    "LEFT JOIN moz_anno_attributes n ON a.anno_attribute_id = n.id "
+                                   "AND n.name = 'bookmarkProperties/POSTData'"
+    "WHERE k.place_id ISNULL "
+    "GROUP BY keyword"));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // Remove any keyword that points to a non-existing place id.
+  rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "DELETE FROM moz_keywords "
+    "WHERE NOT EXISTS (SELECT 1 FROM moz_places WHERE id = moz_keywords.place_id)"));
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "UPDATE moz_bookmarks SET keyword_id = NULL "
+    "WHERE NOT EXISTS (SELECT 1 FROM moz_keywords WHERE id = moz_bookmarks.keyword_id)"));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // Adjust foreign_count for all the rows.
+  rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "UPDATE moz_places SET foreign_count = "
+    "(SELECT count(*) FROM moz_bookmarks WHERE fk = moz_places.id) + "
+    "(SELECT count(*) FROM moz_keywords WHERE place_id = moz_places.id) "
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return NS_OK;
+}
+
 void
 Database::Shutdown()
 {

toolkit/components/places/Database.h

@@ -16,7 +16,7 @@
 
 // This is the schema version. Update it at any schema change and add a
 // corresponding migrateVxx method below.
-#define DATABASE_SCHEMA_VERSION 26
+#define DATABASE_SCHEMA_VERSION 27
 
 // Fired after Places inited.
 #define TOPIC_PLACES_INIT_COMPLETE "places-init-complete"
@@ -274,6 +274,7 @@ protected:
   nsresult MigrateV24Up();
   nsresult MigrateV25Up();
   nsresult MigrateV26Up();
+  nsresult MigrateV27Up();
 
   nsresult UpdateBookmarkRootTitles();
 

toolkit/components/places/PlacesCategoriesStarter.js

@@ -36,22 +36,25 @@ function PlacesCategoriesStarter()
   Services.obs.addObserver(this, PlacesUtils.TOPIC_SHUTDOWN, false);
 
   // nsINavBookmarkObserver implementation.
-  let notify = (function () {
+  let notify = () => {
     if (!this._notifiedBookmarksSvcReady) {
+      // TODO (bug 1145424): for whatever reason, even if we remove this
+      // component from the category (and thus from the category cache we use
+      // to notify), we keep being notified.
+      this._notifiedBookmarksSvcReady = true;
       // For perf reasons unregister from the category, since no further
       // notifications are needed.
       Cc["@mozilla.org/categorymanager;1"]
         .getService(Ci.nsICategoryManager)
-        .deleteCategoryEntry("bookmarks-observer", this, false);
+        .deleteCategoryEntry("bookmark-observers", "PlacesCategoriesStarter", false);
       // Directly notify PlacesUtils, to ensure it catches the notification.
       PlacesUtils.observe(null, "bookmarks-service-ready", null);
     }
-  }).bind(this);
+  };
+
   [ "onItemAdded", "onItemRemoved", "onItemChanged", "onBeginUpdateBatch",
-    "onEndUpdateBatch", "onItemVisited",
+    "onEndUpdateBatch", "onItemVisited", "onItemMoved"
-    "onItemMoved" ].forEach(function(aMethod) {
+  ].forEach(aMethod => this[aMethod] = notify);
-      this[aMethod] = notify;
-    }, this);
 }
 
 PlacesCategoriesStarter.prototype = {

toolkit/components/places/PlacesDBUtils.jsm

@@ -474,23 +474,6 @@ this.PlacesDBUtils = {
     fixOrphanItems.params["tagsGuid"] = PlacesUtils.bookmarks.tagsGuid;
     cleanupStatements.push(fixOrphanItems);
 
-    // D.5 fix wrong keywords
-    let fixInvalidKeywords = DBConn.createAsyncStatement(
-      `UPDATE moz_bookmarks SET keyword_id = NULL WHERE guid NOT IN (
-         :rootGuid, :menuGuid, :toolbarGuid, :unfiledGuid, :tagsGuid  /* skip roots */
-       ) AND id IN (
-         SELECT id FROM moz_bookmarks b
-         WHERE keyword_id NOT NULL
-           AND NOT EXISTS
-             (SELECT id FROM moz_keywords WHERE id = b.keyword_id LIMIT 1)
-       )`);
-    fixInvalidKeywords.params["rootGuid"] = PlacesUtils.bookmarks.rootGuid;
-    fixInvalidKeywords.params["menuGuid"] = PlacesUtils.bookmarks.menuGuid;
-    fixInvalidKeywords.params["toolbarGuid"] = PlacesUtils.bookmarks.toolbarGuid;
-    fixInvalidKeywords.params["unfiledGuid"] = PlacesUtils.bookmarks.unfiledGuid;
-    fixInvalidKeywords.params["tagsGuid"] = PlacesUtils.bookmarks.tagsGuid;
-    cleanupStatements.push(fixInvalidKeywords);
-
     // D.6 fix wrong item types
     //     Folders and separators should not have an fk.
     //     If they have a valid fk convert them to bookmarks. Later in D.9 we
@@ -681,7 +664,7 @@ this.PlacesDBUtils = {
       `DELETE FROM moz_keywords WHERE id IN (
          SELECT id FROM moz_keywords k
          WHERE NOT EXISTS
-           (SELECT id FROM moz_bookmarks WHERE keyword_id = k.id LIMIT 1)
+           (SELECT 1 FROM moz_places h WHERE k.place_id = h.id)
        )`);
     cleanupStatements.push(deleteUnusedKeywords);
 

toolkit/components/places/PlacesUtils.jsm

@@ -27,6 +27,8 @@ this.EXPORTED_SYMBOLS = [
 
 const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components;
 
+Cu.importGlobalProperties(["URL"]);
+
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Services",
                                   "resource://gre/modules/Services.jsm");
@@ -73,6 +75,55 @@ function QI_node(aNode, aIID) {
 function asContainer(aNode) QI_node(aNode, Ci.nsINavHistoryContainerResultNode);
 function asQuery(aNode) QI_node(aNode, Ci.nsINavHistoryQueryResultNode);
 
+/**
+ * Sends a bookmarks notification through the given observers.
+ *
+ * @param observers
+ *        array of nsINavBookmarkObserver objects.
+ * @param notification
+ *        the notification name.
+ * @param args
+ *        array of arguments to pass to the notification.
+ */
+function notify(observers, notification, args) {
+  for (let observer of observers) {
+    try {
+      observer[notification](...args);
+    } catch (ex) {}
+  }
+}
+
+/**
+ * Sends a keyword change notification.
+ *
+ * @param url
+ *        the url to notify about.
+ * @param keyword
+ *        The keyword to notify, or empty string if a keyword was removed.
+ */
+function* notifyKeywordChange(url, keyword) {
+  // Notify bookmarks about the removal.
+  let bookmarks = [];
+  yield PlacesUtils.bookmarks.fetch({ url }, b => bookmarks.push(b));
+  // We don't want to yield in the gIgnoreKeywordNotifications section.
+  for (let bookmark of bookmarks) {
+    bookmark.id = yield PlacesUtils.promiseItemId(bookmark.guid);
+    bookmark.parentId = yield PlacesUtils.promiseItemId(bookmark.parentGuid);
+  }
+  let observers = PlacesUtils.bookmarks.getObservers();
+  gIgnoreKeywordNotifications = true;
+  for (let bookmark of bookmarks) {
+    notify(observers, "onItemChanged", [ bookmark.id, "keyword", false,
+                                         keyword,
+                                         bookmark.lastModified * 1000,
+                                         bookmark.type,
+                                         bookmark.parentId,
+                                         bookmark.guid, bookmark.parentGuid
+                                       ]);
+  }
+  gIgnoreKeywordNotifications = false;
+}
+
 this.PlacesUtils = {
   // Place entries that are containers, e.g. bookmark folders or queries.
   TYPE_X_MOZ_PLACE_CONTAINER: "text/x-moz-place-container",
@@ -244,6 +295,11 @@ this.PlacesUtils = {
           let observerInfo = this._bookmarksServiceObserversQueue.shift();
           this.bookmarks.addObserver(observerInfo.observer, observerInfo.weak);
         }
+
+        // Initialize the keywords cache to start observing bookmarks
+        // notifications.  This is needed as far as we support both the old and
+        // the new bookmarking APIs at the same time.
+        gKeywordsCachePromise.catch(Cu.reportError);
         break;
     }
   },
@@ -810,15 +866,43 @@ this.PlacesUtils = {
    * Set the POST data associated with a bookmark, if any.
    * Used by POST keywords.
    *   @param aBookmarkId
-   *   @returns string of POST data
    */
-  setPostDataForBookmark: function PU_setPostDataForBookmark(aBookmarkId, aPostData) {
+  setPostDataForBookmark(aBookmarkId, aPostData) {
-    const annos = this.annotations;
+    if (!aPostData)
-    if (aPostData)
+      throw new Error("Must provide valid POST data");
-      annos.setItemAnnotation(aBookmarkId, this.POST_DATA_ANNO, aPostData, 
+    // For now we don't have a unified API to create a keyword with postData,
-                              0, Ci.nsIAnnotationService.EXPIRE_NEVER);
+    // thus here we can just try to complete a keyword that should already exist
-    else if (annos.itemHasAnnotation(aBookmarkId, this.POST_DATA_ANNO))
+    // without any post data.
-      annos.removeItemAnnotation(aBookmarkId, this.POST_DATA_ANNO);
+    let stmt = PlacesUtils.history.DBConnection.createStatement(
+      `UPDATE moz_keywords SET post_data = :post_data
+       WHERE id = (SELECT k.id FROM moz_keywords k
+                   JOIN moz_bookmarks b ON b.fk = k.place_id
+                   WHERE b.id = :item_id
+                   AND post_data ISNULL
+                   LIMIT 1)`);
+    stmt.params.item_id = aBookmarkId;
+    stmt.params.post_data = aPostData;
+    try {
+      stmt.execute();
+    }
+    finally {
+      stmt.finalize();
+    }
+
+    // Update the cache.
+    return Task.spawn(function* () {
+      let guid = yield PlacesUtils.promiseItemGuid(aBookmarkId);
+      let bm = yield PlacesUtils.bookmarks.fetch(guid);
+
+      // Fetch keywords for this href.
+      let cache = yield gKeywordsCachePromise;
+      for (let [ keyword, entry ] of cache) {
+        // Set the POST data on keywords not having it.
+        if (entry.url.href == bm.url.href && !entry.postData) {
+          entry.postData = aPostData;
+        }
+      }
+    }).catch(Cu.reportError);
   },
 
   /**
@@ -826,12 +910,22 @@ this.PlacesUtils = {
    * @param aBookmarkId
    * @returns string of POST data if set for aBookmarkId. null otherwise.
    */
-  getPostDataForBookmark: function PU_getPostDataForBookmark(aBookmarkId) {
+  getPostDataForBookmark(aBookmarkId) {
-    const annos = this.annotations;
+    let stmt = PlacesUtils.history.DBConnection.createStatement(
-    if (annos.itemHasAnnotation(aBookmarkId, this.POST_DATA_ANNO))
+      `SELECT k.post_data
-      return annos.getItemAnnotation(aBookmarkId, this.POST_DATA_ANNO);
+       FROM moz_keywords k
-
+       JOIN moz_places h ON h.id = k.place_id
-    return null;
+       JOIN moz_bookmarks b ON b.fk = h.id
+       WHERE b.id = :item_id`);
+    stmt.params.item_id = aBookmarkId;
+    try {
+      if (!stmt.executeStep())
+        return null;
+      return stmt.row.post_data;
+    }
+    finally {
+      stmt.finalize();
+    }
   },
 
   /**
@@ -839,24 +933,21 @@ this.PlacesUtils = {
    * @param aKeyword string keyword
    * @returns an array containing a string URL and a string of POST data
    */
-  getURLAndPostDataForKeyword: function PU_getURLAndPostDataForKeyword(aKeyword) {
+  getURLAndPostDataForKeyword(aKeyword) {
-    var url = null, postdata = null;
+    let stmt = PlacesUtils.history.DBConnection.createStatement(
+      `SELECT h.url, k.post_data
+       FROM moz_keywords k
+       JOIN moz_places h ON h.id = k.place_id
+       WHERE k.keyword = :keyword`);
+    stmt.params.keyword = aKeyword;
     try {
-      var uri = this.bookmarks.getURIForKeyword(aKeyword);
+      if (!stmt.executeStep())
-      if (uri) {
+        return [ null, null ];
-        url = uri.spec;
+      return [ stmt.row.url, stmt.row.post_data ];
-        var bookmarks = this.bookmarks.getBookmarkIdsForURI(uri);
+    }
-        for (let i = 0; i < bookmarks.length; i++) {
+    finally {
-          var bookmark = bookmarks[i];
+      stmt.finalize();
-          var kw = this.bookmarks.getKeywordForBookmark(bookmark);
+    }
-          if (kw == aKeyword) {
-            postdata = this.getPostDataForBookmark(bookmark);
-            break;
-          }
-        }
-      }
-    } catch(ex) {}
-    return [url, postdata];
   },
 
   /**
@@ -1236,6 +1327,14 @@ this.PlacesUtils = {
    */
   promiseDBConnection: () => gAsyncDBConnPromised,
 
+  /**
+   * Gets a Sqlite.jsm wrapped connection to the Places database.
+   * This is intended to be used mostly internally, and by other Places modules.
+   * Keep in mind the Places DB schema is by no means frozen or even stable.
+   * Your custom queries can - and will - break overtime.
+   */
+  promiseWrappedConnection: () => gAsyncDBWrapperPromised,
+
   /**
    * Given a uri returns list of itemIds associated to it.
    *
@@ -1822,6 +1921,8 @@ XPCOMUtils.defineLazyServiceGetter(PlacesUtils, "livemarks",
                                    "@mozilla.org/browser/livemark-service;2",
                                    "mozIAsyncLivemarks");
 
+XPCOMUtils.defineLazyGetter(PlacesUtils, "keywords", () => Keywords);
+
 XPCOMUtils.defineLazyGetter(PlacesUtils, "transactionManager", function() {
   let tm = Cc["@mozilla.org/transactionmanager;1"].
            createInstance(Ci.nsITransactionManager);
@@ -1861,24 +1962,266 @@ XPCOMUtils.defineLazyGetter(this, "bundle", function() {
          createBundle(PLACES_STRING_BUNDLE_URI);
 });
 
-XPCOMUtils.defineLazyGetter(this, "gAsyncDBConnPromised", () => {
+XPCOMUtils.defineLazyGetter(this, "gAsyncDBConnPromised",
-  let connPromised = Sqlite.cloneStorageConnection({
+  () => new Promise((resolve) => {
-    connection: PlacesUtils.history.DBConnection,
+    Sqlite.cloneStorageConnection({
-    readOnly:   true });
+      connection: PlacesUtils.history.DBConnection,
-  connPromised.then(conn => {
+      readOnly:   true
-    try {
+    }).then(conn => {
-      Sqlite.shutdown.addBlocker("Places DB readonly connection closing",
+      try {
-                                 conn.close.bind(conn));
+        Sqlite.shutdown.addBlocker(
+          "PlacesUtils read-only connection closing",
+          conn.close.bind(conn));
+      } catch(ex) {
+        // It's too late to block shutdown, just close the connection.
+        conn.close();
+        throw ex;
+      }
+      resolve(conn);
+    });
+  })
+);
+
+XPCOMUtils.defineLazyGetter(this, "gAsyncDBWrapperPromised",
+  () => new Promise((resolve) => {
+    Sqlite.wrapStorageConnection({
+      connection: PlacesUtils.history.DBConnection,
+    }).then(conn => {
+      try {
+        Sqlite.shutdown.addBlocker(
+          "PlacesUtils wrapped connection closing",
+          conn.close.bind(conn));
+      } catch(ex) {
+        // It's too late to block shutdown, just close the connection.
+        conn.close();
+        throw ex;
+      }
+      resolve(conn);
+    });
+  })
+);
+
+/**
+ * Keywords management API.
+ * Sooner or later these keywords will merge with search keywords, this is an
+ * interim API that should then be replaced by a unified one.
+ * Keywords are associated with URLs and can have POST data.
+ * A single URL can have multiple keywords, provided they differ by POST data.
+ */
+let Keywords = {
+  /**
+   * Fetches URL and postData for a given keyword.
+   *
+   * @param keyword
+   *        The keyword to fetch.
+   * @return {Promise}
+   * @resolves to an object in the form: { keyword, url, postData },
+   *           or null if a keyword was not found.
+   */
+  fetch(keyword) {
+    if (!keyword || typeof(keyword) != "string")
+      throw new Error("Invalid keyword");
+    keyword = keyword.trim().toLowerCase();
+    return gKeywordsCachePromise.then(cache => cache.get(keyword) || null);
+  },
+
+  /**
+   * Adds a new keyword and postData for the given URL.
+   *
+   * @param keywordEntry
+   *        An object describing the keyword to insert, in the form:
+   *        {
+   *          keyword: non-empty string,
+   *          URL: URL or href to associate to the keyword,
+   *          postData: optional POST data to associate to the keyword
+   *        }
+   * @note Do not define a postData property if there isn't any POST data.
+   * @resolves when the addition is complete.
+   */
+  insert(keywordEntry) {
+    if (!keywordEntry || typeof keywordEntry != "object")
+      throw new Error("Input should be a valid object");
+
+    if (!("keyword" in keywordEntry) || !keywordEntry.keyword ||
+        typeof(keywordEntry.keyword) != "string")
+      throw new Error("Invalid keyword");
+    if (("postData" in keywordEntry) && keywordEntry.postData &&
+                                        typeof(keywordEntry.postData) != "string")
+      throw new Error("Invalid POST data");
+    if (!("url" in keywordEntry))
+      throw new Error("undefined is not a valid URL");
+    let { keyword, url } = keywordEntry;
+    keyword = keyword.trim().toLowerCase();
+    let postData = keywordEntry.postData || null;
+    // This also checks href for validity
+    url = new URL(url);
+
+    return Task.spawn(function* () {
+      let cache = yield gKeywordsCachePromise;
+
+      // Trying to set the same keyword is a no-op.
+      let oldEntry = cache.get(keyword);
+      if (oldEntry && oldEntry.url.href == url.href &&
+                      oldEntry.postData == keywordEntry.postData) {
+        return;
+      }
+
+      // A keyword can only be associated to a single page.
+      // If another page is using the new keyword, we must update the keyword
+      // entry.
+      // Note we cannot use INSERT OR REPLACE cause it wouldn't invoke the delete
+      // trigger.
+      let db = yield PlacesUtils.promiseWrappedConnection();
+      if (oldEntry) {
+        yield db.executeCached(
+          `UPDATE moz_keywords
+           SET place_id = (SELECT id FROM moz_places WHERE url = :url),
+               post_data = :post_data
+           WHERE keyword = :keyword
+          `, { url: url.href, keyword: keyword, post_data: postData });
+        yield notifyKeywordChange(oldEntry.url.href, "");
+      } else {
+        // An entry for the given page could be missing, in such a case we need to
+        // create it.
+        yield db.executeCached(
+          `INSERT OR IGNORE INTO moz_places (url, rev_host, hidden, frecency, guid)
+           VALUES (:url, :rev_host, 0, :frecency, GENERATE_GUID())
+          `, { url: url.href, rev_host: PlacesUtils.getReversedHost(url),
+               frecency: url.protocol == "place:" ? 0 : -1 });
+        yield db.executeCached(
+          `INSERT INTO moz_keywords (keyword, place_id, post_data)
+           VALUES (:keyword, (SELECT id FROM moz_places WHERE url = :url), :post_data)
+          `, { url: url.href, keyword: keyword, post_data: postData });
+      }
+
+      cache.set(keyword, { keyword, url, postData });
+
+      // In any case, notify about the new keyword.
+      yield notifyKeywordChange(url.href, keyword);
+    }.bind(this));
+  },
+
+  /**
+   * Removes a keyword.
+   *
+   * @param keyword
+   *        The keyword to remove.
+   * @return {Promise}
+   * @resolves when the removal is complete.
+   */
+  remove(keyword) {
+    if (!keyword || typeof(keyword) != "string")
+      throw new Error("Invalid keyword");
+    keyword = keyword.trim().toLowerCase();
+    return Task.spawn(function* () {
+      let cache = yield gKeywordsCachePromise;
+      if (!cache.has(keyword))
+        return;
+      let { url } = cache.get(keyword);
+      cache.delete(keyword);
+
+      let db = yield PlacesUtils.promiseWrappedConnection();
+      yield db.execute(`DELETE FROM moz_keywords WHERE keyword = :keyword`,
+                       { keyword });
+
+      // Notify bookmarks about the removal.
+      yield notifyKeywordChange(url.href, "");
+    }.bind(this));
+  }
+};
+
+// Set by the keywords API to distinguish notifications fired by the old API.
+// Once the old API will be gone, we can remove this and stop observing.
+let gIgnoreKeywordNotifications = false;
+
+XPCOMUtils.defineLazyGetter(this, "gKeywordsCachePromise", Task.async(function* () {
+  let cache = new Map();
+  let db = yield PlacesUtils.promiseWrappedConnection();
+  let rows = yield db.execute(
+    `SELECT keyword, url, post_data
+     FROM moz_keywords k
+     JOIN moz_places h ON h.id = k.place_id
+    `);
+  for (let row of rows) {
+    let keyword = row.getResultByName("keyword");
+    let entry = { keyword,
+                  url: new URL(row.getResultByName("url")),
+                  postData: row.getResultByName("post_data") };
+    cache.set(keyword, entry);
+  }
+
+  // Helper to get a keyword from an href.
+  function keywordsForHref(href) {
+    let keywords = [];
+    for (let [ key, val ] of cache) {
+      if (val.url.href == href)
+        keywords.push(key);
     }
-    catch(ex) {
+    return keywords;
-      // It's too late to block shutdown, just close the connection.
+  }
-      return conn.close();
+
-      throw (ex);
+  // Start observing changes to bookmarks. For now we are going to keep that
+  // relation for backwards compatibility reasons, but mostly because we are
+  // lacking a UI to manage keywords directly.
+  let observer = {
+    QueryInterface: XPCOMUtils.generateQI(Ci.nsINavBookmarkObserver),
+    onBeginUpdateBatch() {},
+    onEndUpdateBatch() {},
+    onItemAdded() {},
+    onItemVisited() {},
+    onItemMoved() {},
+
+    onItemRemoved(id, parentId, index, itemType, uri, guid, parentGuid) {
+      if (itemType != PlacesUtils.bookmarks.TYPE_BOOKMARK)
+        return;
+
+      let keywords = keywordsForHref(uri.spec);
+      // This uri has no keywords associated, so there's nothing to do.
+      if (keywords.length == 0)
+        return;
+
+      Task.spawn(function* () {
+        // If the uri is not bookmarked anymore, we can remove this keyword.
+        let bookmark = yield PlacesUtils.bookmarks.fetch({ url: uri });
+        if (!bookmark) {
+          for (let keyword of keywords) {
+            yield PlacesUtils.keywords.remove(keyword);
+          }
+        }
+      }).catch(Cu.reportError);
+    },
+
+    onItemChanged(id, prop, isAnno, val, lastMod, itemType, parentId, guid) {
+      if (gIgnoreKeywordNotifications ||
+          prop != "keyword")
+        return;
+
+      Task.spawn(function* () {
+        let bookmark = yield PlacesUtils.bookmarks.fetch(guid);
+        // By this time the bookmark could have gone, there's nothing we can do.
+        if (!bookmark)
+          return;
+
+        if (val.length == 0) {
+          // We are removing a keyword.
+          let keywords = keywordsForHref(bookmark.url.href)
+          for (let keyword of keywords) {
+            cache.delete(keyword);
+          }
+        } else {
+          // We are adding a new keyword.
+          cache.set(val, { keyword: val, url: bookmark.url });
+        }
+      }).catch(Cu.reportError);
     }
-    return Promise.resolve();
+  };
-  }).then(null, Cu.reportError);
+
-  return connPromised;
+  PlacesUtils.bookmarks.addObserver(observer, false);
-});
+  PlacesUtils.registerShutdownFunction(() => {
+    PlacesUtils.bookmarks.removeObserver(observer);
+  });
+  return cache;
+}));
 
 // Sometime soon, likely as part of the transition to mozIAsyncBookmarks,
 // itemIds will be deprecated in favour of GUIDs, which play much better
@@ -2899,15 +3242,19 @@ this.PlacesEditBookmarkPostDataTransaction =
 PlacesEditBookmarkPostDataTransaction.prototype = {
   __proto__: BaseTransaction.prototype,
 
-  doTransaction: function EBPDTXN_doTransaction()
+  doTransaction() {
-  {
+    // Setting null postData is not supported by the current schema.
-    this.item.postData = PlacesUtils.getPostDataForBookmark(this.item.id);
+    if (this.new.postData) {
-    PlacesUtils.setPostDataForBookmark(this.item.id, this.new.postData);
+      this.item.postData = PlacesUtils.getPostDataForBookmark(this.item.id);
+      PlacesUtils.setPostDataForBookmark(this.item.id, this.new.postData);
+    }
   },
 
-  undoTransaction: function EBPDTXN_undoTransaction()
+  undoTransaction() {
-  {
+    // Setting null postData is not supported by the current schema.
-    PlacesUtils.setPostDataForBookmark(this.item.id, this.item.postData);
+    if (this.item.postData) {
+      PlacesUtils.setPostDataForBookmark(this.item.id, this.item.postData);
+    }
   }
 };
 
@@ -3069,7 +3416,7 @@ PlacesSortFolderByNameTransaction.prototype = {
     let callback = {
       _self: this,
       runBatched: function() {
-        for (item in this._self._oldOrder)
+        for (let item in this._self._oldOrder)
           PlacesUtils.bookmarks.setItemIndex(item, this._self._oldOrder[item]);
       }
     };

toolkit/components/places/UnifiedComplete.js

@@ -152,24 +152,20 @@ const SQL_ADAPTIVE_QUERY =
 const SQL_KEYWORD_QUERY =
   `/* do not warn (bug 487787) */
    SELECT :query_type,
-     (SELECT REPLACE(url, '%s', :query_string) FROM moz_places WHERE id = b.fk)
+          REPLACE(h.url, '%s', :query_string) AS search_url, h.title,
-     AS search_url, h.title,
      IFNULL(f.url, (SELECT f.url
                     FROM moz_places
                     JOIN moz_favicons f ON f.id = favicon_id
-                    WHERE rev_host = (SELECT rev_host FROM moz_places WHERE id = b.fk)
+                    WHERE rev_host = h.rev_host
                     ORDER BY frecency DESC
                     LIMIT 1)
            ),
-     1, b.title, NULL, h.visit_count, h.typed, IFNULL(h.id, b.fk),
+     1, NULL, NULL, h.visit_count, h.typed, h.id, t.open_count, h.frecency
-     t.open_count, h.frecency
    FROM moz_keywords k
-   JOIN moz_bookmarks b ON b.keyword_id = k.id
+   JOIN moz_places h ON k.place_id = h.id
-   LEFT JOIN moz_places h ON h.url = search_url
    LEFT JOIN moz_favicons f ON f.id = h.favicon_id
    LEFT JOIN moz_openpages_temp t ON t.url = search_url
-   WHERE LOWER(k.keyword) = LOWER(:keyword)
+   WHERE k.keyword = LOWER(:keyword)`;
-   ORDER BY h.frecency DESC`;
 
 function hostQuery(conditions = "") {
   let query =
@@ -1241,24 +1237,13 @@ Search.prototype = {
     let title = bookmarkTitle || historyTitle;
 
     if (queryType == QUERYTYPE_KEYWORD) {
+      match.style = "keyword";
       if (this._enableActions) {
-        match.style = "keyword";
         url = makeActionURL("keyword", {
           url: escapedURL,
           input: this._originalSearchString,
         });
         action = "keyword";
-      } else {
-        // If we do not have a title, then we must have a keyword, so let the UI
-        // know it is a keyword.  Otherwise, we found an exact page match, so just
-        // show the page like a regular result.  Because the page title is likely
-        // going to be more specific than the bookmark title (keyword title).
-        if (!historyTitle) {
-          match.style = "keyword"
-        }
-        else {
-          title = historyTitle;
-        }
       }
     }
 

toolkit/components/places/nsINavBookmarksService.idl

@@ -222,7 +222,7 @@ interface nsINavBookmarkObserver : nsISupports
  * folders.  A URI in history can be contained in one or more such folders.
  */
 
-[scriptable, uuid(b0f9a80a-d7f0-4421-8513-444125f0d828)]
+[scriptable, uuid(24533891-afa6-4663-b72d-3143d03f1b04)]
 interface nsINavBookmarksService : nsISupports
 {
   /**
@@ -536,12 +536,6 @@ interface nsINavBookmarksService : nsISupports
    */
   void setKeywordForBookmark(in long long aItemId, in AString aKeyword);
 
-  /**
-   * Retrieves the keyword for the given URI. Will be void string
-   * (null in JS) if no such keyword is found.
-   */
-  AString getKeywordForURI(in nsIURI aURI);
-
   /**
    * Retrieves the keyword for the given bookmark. Will be void string
    * (null in JS) if no such keyword is found.

toolkit/components/places/nsNavBookmarks.cpp

@@ -19,8 +19,6 @@
 
 #include "GeckoProfiler.h"
 
-#define BOOKMARKS_TO_KEYWORDS_INITIAL_CACHE_LENGTH 32
-
 using namespace mozilla;
 
 // These columns sit to the right of the kGetInfoIndex_* columns.
@@ -40,25 +38,6 @@ PLACES_FACTORY_SINGLETON_IMPLEMENTATION(nsNavBookmarks, gBookmarksService)
 
 namespace {
 
-struct keywordSearchData
-{
-  int64_t itemId;
-  nsString keyword;
-};
-
-PLDHashOperator
-SearchBookmarkForKeyword(nsTrimInt64HashKey::KeyType aKey,
-                         const nsString aValue,
-                         void* aUserArg)
-{
-  keywordSearchData* data = reinterpret_cast<keywordSearchData*>(aUserArg);
-  if (data->keyword.Equals(aValue)) {
-    data->itemId = aKey;
-    return PL_DHASH_STOP;
-  }
-  return PL_DHASH_NEXT;
-}
-
 template<typename Method, typename DataType>
 class AsyncGetBookmarksForURI : public AsyncStatementCallback
 {
@@ -143,8 +122,6 @@ nsNavBookmarks::nsNavBookmarks()
   , mCanNotify(false)
   , mCacheObservers("bookmark-observers")
   , mBatching(false)
-  , mBookmarkToKeywordHash(BOOKMARKS_TO_KEYWORDS_INITIAL_CACHE_LENGTH)
-  , mBookmarkToKeywordHashInitialized(false)
 {
   NS_ASSERTION(!gBookmarksService,
                "Attempting to create two instances of the service!");
@@ -645,10 +622,6 @@ nsNavBookmarks::RemoveItem(int64_t aItemId)
       rv = history->UpdateFrecency(bookmark.placeId);
       NS_ENSURE_SUCCESS(rv, rv);
     }
-
-    rv = UpdateKeywordsHashForRemovedBookmark(aItemId);
-    NS_ENSURE_SUCCESS(rv, rv);
-
     // A broken url should not interrupt the removal process.
     (void)NS_NewURI(getter_AddRefs(uri), bookmark.url);
   }
@@ -1108,8 +1081,14 @@ nsNavBookmarks::RemoveFolderChildren(int64_t aFolderId)
   rv = SetItemDateInternal(LAST_MODIFIED, folder.id, RoundedPRNow());
   NS_ENSURE_SUCCESS(rv, rv);
 
-  for (uint32_t i = 0; i < folderChildrenArray.Length(); i++) {
+  rv = transaction.Commit();
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // Call observers in reverse order to serve children before their parent.
+  for (int32_t i = folderChildrenArray.Length() - 1; i >= 0; --i) {
     BookmarkData& child = folderChildrenArray[i];
+
+    nsCOMPtr<nsIURI> uri;
     if (child.type == TYPE_BOOKMARK) {
       // If not a tag, recalculate frecency for this entry, since it changed.
       if (child.grandParentId != mTagsRoot) {
@@ -1118,20 +1097,6 @@ nsNavBookmarks::RemoveFolderChildren(int64_t aFolderId)
         rv = history->UpdateFrecency(child.placeId);
         NS_ENSURE_SUCCESS(rv, rv);
       }
-
-      rv = UpdateKeywordsHashForRemovedBookmark(child.id);
-      NS_ENSURE_SUCCESS(rv, rv);
-    }
-  }
-
-  rv = transaction.Commit();
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  // Call observers in reverse order to serve children before their parent.
-  for (int32_t i = folderChildrenArray.Length() - 1; i >= 0; --i) {
-    BookmarkData& child = folderChildrenArray[i];
-    nsCOMPtr<nsIURI> uri;
-    if (child.type == TYPE_BOOKMARK) {
       // A broken url should not interrupt the removal process.
       (void)NS_NewURI(getter_AddRefs(uri), child.url);
     }
@@ -2263,44 +2228,6 @@ nsNavBookmarks::SetItemIndex(int64_t aItemId, int32_t aNewIndex)
 }
 
 
-nsresult
-nsNavBookmarks::UpdateKeywordsHashForRemovedBookmark(int64_t aItemId)
-{
-  nsAutoString keyword;
-  if (NS_SUCCEEDED(GetKeywordForBookmark(aItemId, keyword)) &&
-      !keyword.IsEmpty()) {
-    nsresult rv = EnsureKeywordsHash();
-    NS_ENSURE_SUCCESS(rv, rv);
-    mBookmarkToKeywordHash.Remove(aItemId);
-
-    // If the keyword is unused, remove it from the database.
-    keywordSearchData searchData;
-    searchData.keyword.Assign(keyword);
-    searchData.itemId = -1;
-    mBookmarkToKeywordHash.EnumerateRead(SearchBookmarkForKeyword, &searchData);
-    if (searchData.itemId == -1) {
-      nsCOMPtr<mozIStorageAsyncStatement> stmt = mDB->GetAsyncStatement(
-        "DELETE FROM moz_keywords "
-        "WHERE keyword = :keyword "
-        "AND NOT EXISTS ( "
-          "SELECT id "
-          "FROM moz_bookmarks "
-          "WHERE keyword_id = moz_keywords.id "
-        ")"
-      );
-      NS_ENSURE_STATE(stmt);
-
-      rv = stmt->BindStringByName(NS_LITERAL_CSTRING("keyword"), keyword);
-      NS_ENSURE_SUCCESS(rv, rv);
-      nsCOMPtr<mozIStoragePendingStatement> pendingStmt;
-      rv = stmt->ExecuteAsync(nullptr, getter_AddRefs(pendingStmt));
-      NS_ENSURE_SUCCESS(rv, rv);
-    }
-  }
-  return NS_OK;
-}
-
-
 NS_IMETHODIMP
 nsNavBookmarks::SetKeywordForBookmark(int64_t aBookmarkId,
                                       const nsAString& aUserCasedKeyword)
@@ -2311,121 +2238,163 @@ nsNavBookmarks::SetKeywordForBookmark(int64_t aBookmarkId,
   BookmarkData bookmark;
   nsresult rv = FetchItemInfo(aBookmarkId, bookmark);
   NS_ENSURE_SUCCESS(rv, rv);
-
+  nsCOMPtr<nsIURI> uri;
-  rv = EnsureKeywordsHash();
+  rv = NS_NewURI(getter_AddRefs(uri), bookmark.url);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Shortcuts are always lowercased internally.
   nsAutoString keyword(aUserCasedKeyword);
   ToLowerCase(keyword);
 
-  // Check if bookmark was already associated to a keyword.
+  // The same URI can be associated to more than one keyword, provided the post
-  nsAutoString oldKeyword;
+  // data differs.  Check if there are already keywords associated to this uri.
-  rv = GetKeywordForBookmark(bookmark.id, oldKeyword);
+  nsTArray<nsString> oldKeywords;
-  NS_ENSURE_SUCCESS(rv, rv);
+  {
+    nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
+      "SELECT keyword FROM moz_keywords WHERE place_id = :place_id"
+    );
+    NS_ENSURE_STATE(stmt);
+    mozStorageStatementScoper scoper(stmt);
+    rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("place_id"), bookmark.placeId);
+    NS_ENSURE_SUCCESS(rv, rv);
 
-  // Trying to set the same value or to remove a nonexistent keyword is a no-op.
+    bool hasMore;
-  if (keyword.Equals(oldKeyword) || (keyword.IsEmpty() && oldKeyword.IsEmpty()))
+    while (NS_SUCCEEDED(stmt->ExecuteStep(&hasMore)) && hasMore) {
+      nsString oldKeyword;
+      rv = stmt->GetString(0, oldKeyword);
+      NS_ENSURE_SUCCESS(rv, rv);
+      oldKeywords.AppendElement(oldKeyword);
+    }
+  }
+
+  // Trying to remove a non-existent keyword is a no-op.
+  if (keyword.IsEmpty() && oldKeywords.Length() == 0) {
     return NS_OK;
-
+  }
-  mozStorageTransaction transaction(mDB->MainConn(), false);
-
-  nsCOMPtr<mozIStorageStatement> updateBookmarkStmt = mDB->GetStatement(
-    "UPDATE moz_bookmarks "
-    "SET keyword_id = (SELECT id FROM moz_keywords WHERE keyword = :keyword), "
-        "lastModified = :date "
-    "WHERE id = :item_id "
-  );
-  NS_ENSURE_STATE(updateBookmarkStmt);
-  mozStorageStatementScoper updateBookmarkScoper(updateBookmarkStmt);
 
   if (keyword.IsEmpty()) {
-    // Remove keyword association from the hash.
+    // We are removing the existing keywords.
-    mBookmarkToKeywordHash.Remove(bookmark.id);
+    for (uint32_t i = 0; i < oldKeywords.Length(); ++i) {
-    rv = updateBookmarkStmt->BindNullByName(NS_LITERAL_CSTRING("keyword"));
+      nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
+        "DELETE FROM moz_keywords WHERE keyword = :old_keyword"
+      );
+      NS_ENSURE_STATE(stmt);
+      mozStorageStatementScoper scoper(stmt);
+      rv = stmt->BindStringByName(NS_LITERAL_CSTRING("old_keyword"),
+                                  oldKeywords[i]);
+      NS_ENSURE_SUCCESS(rv, rv);
+      rv = stmt->Execute();
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
+
+    nsTArray<BookmarkData> bookmarks;
+    rv = GetBookmarksForURI(uri, bookmarks);
+    NS_ENSURE_SUCCESS(rv, rv);
+    for (uint32_t i = 0; i < bookmarks.Length(); ++i) {
+      NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
+                       nsINavBookmarkObserver,
+                       OnItemChanged(bookmarks[i].id,
+                                     NS_LITERAL_CSTRING("keyword"),
+                                     false,
+                                     EmptyCString(),
+                                     bookmarks[i].lastModified,
+                                     TYPE_BOOKMARK,
+                                     bookmarks[i].parentId,
+                                     bookmarks[i].guid,
+                                     bookmarks[i].parentGuid));
+    }
+
+    return NS_OK;
   }
-   else {
+
-    // We are associating bookmark to a new keyword. Create a new keyword
+  // A keyword can only be associated to a single URI.  Check if the requested
-    // record if needed.
+  // keyword was already associated, in such a case we will need to notify about
-    nsCOMPtr<mozIStorageStatement> newKeywordStmt = mDB->GetStatement(
+  // the change.
-      "INSERT OR IGNORE INTO moz_keywords (keyword) VALUES (:keyword)"
+  nsCOMPtr<nsIURI> oldUri;
+  {
+    nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
+      "SELECT url "
+      "FROM moz_keywords "
+      "JOIN moz_places h ON h.id = place_id "
+      "WHERE keyword = :keyword"
     );
-    NS_ENSURE_STATE(newKeywordStmt);
+    NS_ENSURE_STATE(stmt);
-    mozStorageStatementScoper newKeywordScoper(newKeywordStmt);
+    mozStorageStatementScoper scoper(stmt);
-
+    rv = stmt->BindStringByName(NS_LITERAL_CSTRING("keyword"), keyword);
-    rv = newKeywordStmt->BindStringByName(NS_LITERAL_CSTRING("keyword"),
-                                          keyword);
-    NS_ENSURE_SUCCESS(rv, rv);
-    rv = newKeywordStmt->Execute();
     NS_ENSURE_SUCCESS(rv, rv);
 
-    // Add new keyword association to the hash, removing the old one if needed.
+    bool hasMore;
-    if (!oldKeyword.IsEmpty())
+    if (NS_SUCCEEDED(stmt->ExecuteStep(&hasMore)) && hasMore) {
-      mBookmarkToKeywordHash.Remove(bookmark.id);
+      nsAutoCString spec;
-    mBookmarkToKeywordHash.Put(bookmark.id, keyword);
+      rv = stmt->GetUTF8String(0, spec);
-    rv = updateBookmarkStmt->BindStringByName(NS_LITERAL_CSTRING("keyword"), keyword);
+      NS_ENSURE_SUCCESS(rv, rv);
+      rv = NS_NewURI(getter_AddRefs(oldUri), spec);
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
   }
-  NS_ENSURE_SUCCESS(rv, rv);
-  bookmark.lastModified = RoundedPRNow();
-  rv = updateBookmarkStmt->BindInt64ByName(NS_LITERAL_CSTRING("date"),
-                                           bookmark.lastModified);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = updateBookmarkStmt->BindInt64ByName(NS_LITERAL_CSTRING("item_id"),
-                                           bookmark.id);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = updateBookmarkStmt->Execute();
-  NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = transaction.Commit();
+  // If another uri is using the new keyword, we must update the keyword entry.
-  NS_ENSURE_SUCCESS(rv, rv);
+  // Note we cannot use INSERT OR REPLACE cause it wouldn't invoke the delete
+  // trigger.
+  nsCOMPtr<mozIStorageStatement> stmt;
+  if (oldUri) {
+    // In both cases, notify about the change.
+    nsTArray<BookmarkData> bookmarks;
+    rv = GetBookmarksForURI(oldUri, bookmarks);
+    NS_ENSURE_SUCCESS(rv, rv);
+    for (uint32_t i = 0; i < bookmarks.Length(); ++i) {
+      NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
+                       nsINavBookmarkObserver,
+                       OnItemChanged(bookmarks[i].id,
+                                     NS_LITERAL_CSTRING("keyword"),
+                                     false,
+                                     EmptyCString(),
+                                     bookmarks[i].lastModified,
+                                     TYPE_BOOKMARK,
+                                     bookmarks[i].parentId,
+                                     bookmarks[i].guid,
+                                     bookmarks[i].parentGuid));
+    }
 
-  NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
+    stmt = mDB->GetStatement(
-                   nsINavBookmarkObserver,
+      "UPDATE moz_keywords SET place_id = :place_id WHERE keyword = :keyword"
-                   OnItemChanged(bookmark.id,
+    );
-                                 NS_LITERAL_CSTRING("keyword"),
+    NS_ENSURE_STATE(stmt);
-                                 false,
+  }
-                                 NS_ConvertUTF16toUTF8(keyword),
+  else {
-                                 bookmark.lastModified,
+    stmt = mDB->GetStatement(
-                                 bookmark.type,
+      "INSERT INTO moz_keywords (keyword, place_id) "
-                                 bookmark.parentId,
+      "VALUES (:keyword, :place_id)"
-                                 bookmark.guid,
+    );
-                                 bookmark.parentGuid));
+  }
-
-  return NS_OK;
-}
-
-
-NS_IMETHODIMP
-nsNavBookmarks::GetKeywordForURI(nsIURI* aURI, nsAString& aKeyword)
-{
-  PLACES_WARN_DEPRECATED();
-
-  NS_ENSURE_ARG(aURI);
-  aKeyword.Truncate(0);
-
-  nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
-    "SELECT k.keyword "
-    "FROM moz_places h "
-    "JOIN moz_bookmarks b ON b.fk = h.id "
-    "JOIN moz_keywords k ON k.id = b.keyword_id "
-    "WHERE h.url = :page_url "
-  );
   NS_ENSURE_STATE(stmt);
   mozStorageStatementScoper scoper(stmt);
 
-  nsresult rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), aURI);
+  rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("place_id"), bookmark.placeId);
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = stmt->BindStringByName(NS_LITERAL_CSTRING("keyword"), keyword);
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = stmt->Execute();
   NS_ENSURE_SUCCESS(rv, rv);
 
-  bool hasMore = false;
+  // In both cases, notify about the change.
-  rv = stmt->ExecuteStep(&hasMore);
+  nsTArray<BookmarkData> bookmarks;
-  if (NS_FAILED(rv) || !hasMore) {
+  rv = GetBookmarksForURI(uri, bookmarks);
-    aKeyword.SetIsVoid(true);
+  NS_ENSURE_SUCCESS(rv, rv);
-    return NS_OK; // not found: return void keyword string
+  for (uint32_t i = 0; i < bookmarks.Length(); ++i) {
+    NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
+                     nsINavBookmarkObserver,
+                     OnItemChanged(bookmarks[i].id,
+                                   NS_LITERAL_CSTRING("keyword"),
+                                   false,
+                                   NS_ConvertUTF16toUTF8(keyword),
+                                   bookmarks[i].lastModified,
+                                   TYPE_BOOKMARK,
+                                   bookmarks[i].parentId,
+                                   bookmarks[i].guid,
+                                   bookmarks[i].parentGuid));
   }
 
-  // found, get the keyword
-  rv = stmt->GetString(0, aKeyword);
-  NS_ENSURE_SUCCESS(rv, rv);
   return NS_OK;
 }
 
@@ -2436,17 +2405,34 @@ nsNavBookmarks::GetKeywordForBookmark(int64_t aBookmarkId, nsAString& aKeyword)
   NS_ENSURE_ARG_MIN(aBookmarkId, 1);
   aKeyword.Truncate(0);
 
-  nsresult rv = EnsureKeywordsHash();
+  // We can have multiple keywords for the same uri, here we'll just return the
+  // last created one.
+  nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(NS_LITERAL_CSTRING(
+    "SELECT k.keyword "
+    "FROM moz_bookmarks b "
+    "JOIN moz_keywords k ON k.place_id = b.fk "
+    "WHERE b.id = :item_id "
+    "ORDER BY k.ROWID DESC "
+    "LIMIT 1"
+  ));
+  NS_ENSURE_STATE(stmt);
+  mozStorageStatementScoper scoper(stmt);
+
+  nsresult rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("item_id"),
+                             aBookmarkId);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  nsAutoString keyword;
+  bool hasMore;
-  if (!mBookmarkToKeywordHash.Get(aBookmarkId, &keyword)) {
+  if (NS_SUCCEEDED(stmt->ExecuteStep(&hasMore)) && hasMore) {
-    aKeyword.SetIsVoid(true);
+    nsAutoString keyword;
-  }
+    rv = stmt->GetString(0, keyword);
-  else {
+    NS_ENSURE_SUCCESS(rv, rv);
-    aKeyword.Assign(keyword);
+    aKeyword = keyword;
+    return NS_OK;
   }
 
+  aKeyword.SetIsVoid(true);
+
   return NS_OK;
 }
 
@@ -2463,51 +2449,27 @@ nsNavBookmarks::GetURIForKeyword(const nsAString& aUserCasedKeyword,
   nsAutoString keyword(aUserCasedKeyword);
   ToLowerCase(keyword);
 
-  nsresult rv = EnsureKeywordsHash();
+  nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(NS_LITERAL_CSTRING(
-  NS_ENSURE_SUCCESS(rv, rv);
+    "SELECT h.url "
+    "FROM moz_places h "
+    "JOIN moz_keywords k ON k.place_id = h.id "
+    "WHERE k.keyword = :keyword"
+  ));
+  NS_ENSURE_STATE(stmt);
+  mozStorageStatementScoper scoper(stmt);
 
-  keywordSearchData searchData;
+  nsresult rv = stmt->BindStringByName(NS_LITERAL_CSTRING("keyword"), keyword);
-  searchData.keyword.Assign(keyword);
-  searchData.itemId = -1;
-  mBookmarkToKeywordHash.EnumerateRead(SearchBookmarkForKeyword, &searchData);
-
-  if (searchData.itemId == -1) {
-    // Not found.
-    return NS_OK;
-  }
-
-  rv = GetBookmarkURI(searchData.itemId, aURI);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  return NS_OK;
-}
-
-
-nsresult
-nsNavBookmarks::EnsureKeywordsHash() {
-  if (mBookmarkToKeywordHashInitialized) {
-    return NS_OK;
-  }
-  mBookmarkToKeywordHashInitialized = true;
-
-  nsCOMPtr<mozIStorageStatement> stmt;
-  nsresult rv = mDB->MainConn()->CreateStatement(NS_LITERAL_CSTRING(
-    "SELECT b.id, k.keyword "
-    "FROM moz_bookmarks b "
-    "JOIN moz_keywords k ON k.id = b.keyword_id "
-  ), getter_AddRefs(stmt));
   NS_ENSURE_SUCCESS(rv, rv);
 
   bool hasMore;
-  while (NS_SUCCEEDED(stmt->ExecuteStep(&hasMore)) && hasMore) {
+  if (NS_SUCCEEDED(stmt->ExecuteStep(&hasMore)) && hasMore) {
-    int64_t itemId;
+    nsAutoCString spec;
-    rv = stmt->GetInt64(0, &itemId);
+    rv = stmt->GetUTF8String(0, spec);
     NS_ENSURE_SUCCESS(rv, rv);
-    nsAutoString keyword;
+    nsCOMPtr<nsIURI> uri;
-    rv = stmt->GetString(1, keyword);
+    rv = NS_NewURI(getter_AddRefs(uri), spec);
     NS_ENSURE_SUCCESS(rv, rv);
-
+    uri.forget(aURI);
-    mBookmarkToKeywordHash.Put(itemId, keyword);
   }
 
   return NS_OK;

toolkit/components/places/nsNavBookmarks.h

@@ -421,21 +421,13 @@ private:
   // Note: this is only tracking bookmarks batches, not history ones.
   bool mBatching;
 
-  /**
-   * Always call EnsureKeywordsHash() and check it for errors before actually
-   * using the hash.  Internal keyword methods are already doing that.
-   */
-  nsresult EnsureKeywordsHash();
-  nsDataHashtable<nsTrimInt64HashKey, nsString> mBookmarkToKeywordHash;
-  bool mBookmarkToKeywordHashInitialized;
-
   /**
    * This function must be called every time a bookmark is removed.
    *
    * @param aURI
    *        Uri to test.
    */
-  nsresult UpdateKeywordsHashForRemovedBookmark(int64_t aItemId);
+  nsresult UpdateKeywordsForRemovedBookmark(const BookmarkData& aBookmark);
 };
 
 #endif // nsNavBookmarks_h_

toolkit/components/places/nsPlacesAutoComplete.js

@@ -414,24 +414,20 @@ function nsPlacesAutoComplete()
   XPCOMUtils.defineLazyGetter(this, "_keywordQuery", function() {
     return this._db.createAsyncStatement(
       `/* do not warn (bug 487787) */
-       SELECT
+       SELECT REPLACE(h.url, '%s', :query_string) AS search_url, h.title,
-       (SELECT REPLACE(url, '%s', :query_string) FROM moz_places WHERE id = b.fk)
-       AS search_url, h.title,
        IFNULL(f.url, (SELECT f.url
                       FROM moz_places
                       JOIN moz_favicons f ON f.id = favicon_id
-                      WHERE rev_host = (SELECT rev_host FROM moz_places WHERE id = b.fk)
+                      WHERE rev_host = h.rev_host
                       ORDER BY frecency DESC
                       LIMIT 1)
-       ), 1, b.title, NULL, h.visit_count, h.typed, IFNULL(h.id, b.fk),
+       ), 1, NULL, NULL, h.visit_count, h.typed, h.id,
        :query_type, t.open_count
        FROM moz_keywords k
-       JOIN moz_bookmarks b ON b.keyword_id = k.id
+       JOIN moz_places h ON k.place_id = h.id
-       LEFT JOIN moz_places h ON h.url = search_url
        LEFT JOIN moz_favicons f ON f.id = h.favicon_id
        LEFT JOIN moz_openpages_temp t ON t.url = search_url
-       WHERE LOWER(k.keyword) = LOWER(:keyword)
+       WHERE k.keyword = LOWER(:keyword)`
-       ORDER BY h.frecency DESC`
     );
   });
 

toolkit/components/places/nsPlacesIndexes.h

@@ -121,4 +121,13 @@
     "guid_uniqueindex", "moz_favicons", "guid", "UNIQUE" \
   )
 
+/**
+ * moz_keywords
+ */
+
+#define CREATE_IDX_MOZ_KEYWORDS_PLACEPOSTDATA \
+  CREATE_PLACES_IDX( \
+    "placepostdata_uniqueindex", "moz_keywords", "place_id, post_data", "UNIQUE" \
+  )
+
 #endif // nsPlacesIndexes_h__

toolkit/components/places/nsPlacesTables.h

@@ -121,6 +121,8 @@
   "CREATE TABLE moz_keywords (" \
     "  id INTEGER PRIMARY KEY AUTOINCREMENT" \
     ", keyword TEXT UNIQUE" \
+    ", place_id INTEGER" \
+    ", post_data TEXT" \
   ")" \
 )
 

toolkit/components/places/nsPlacesTriggers.h

@@ -175,7 +175,7 @@
   "END" \
 )
 
-#define CREATE_FOREIGNCOUNT_AFTERDELETE_TRIGGER NS_LITERAL_CSTRING( \
+#define CREATE_BOOKMARKS_FOREIGNCOUNT_AFTERDELETE_TRIGGER NS_LITERAL_CSTRING( \
   "CREATE TEMP TRIGGER moz_bookmarks_foreign_count_afterdelete_trigger " \
   "AFTER DELETE ON moz_bookmarks FOR EACH ROW " \
   "BEGIN " \
@@ -185,7 +185,7 @@
   "END" \
 )
 
-#define CREATE_FOREIGNCOUNT_AFTERINSERT_TRIGGER NS_LITERAL_CSTRING( \
+#define CREATE_BOOKMARKS_FOREIGNCOUNT_AFTERINSERT_TRIGGER NS_LITERAL_CSTRING( \
   "CREATE TEMP TRIGGER moz_bookmarks_foreign_count_afterinsert_trigger " \
   "AFTER INSERT ON moz_bookmarks FOR EACH ROW " \
   "BEGIN " \
@@ -195,7 +195,7 @@
   "END" \
 )
 
-#define CREATE_FOREIGNCOUNT_AFTERUPDATE_TRIGGER NS_LITERAL_CSTRING( \
+#define CREATE_BOOKMARKS_FOREIGNCOUNT_AFTERUPDATE_TRIGGER NS_LITERAL_CSTRING( \
   "CREATE TEMP TRIGGER moz_bookmarks_foreign_count_afterupdate_trigger " \
   "AFTER UPDATE OF fk ON moz_bookmarks FOR EACH ROW " \
   "BEGIN " \
@@ -207,4 +207,38 @@
     "WHERE id = OLD.fk;" \
   "END" \
 )
+
+#define CREATE_KEYWORDS_FOREIGNCOUNT_AFTERDELETE_TRIGGER NS_LITERAL_CSTRING( \
+  "CREATE TEMP TRIGGER moz_keywords_foreign_count_afterdelete_trigger " \
+  "AFTER DELETE ON moz_keywords FOR EACH ROW " \
+  "BEGIN " \
+    "UPDATE moz_places " \
+    "SET foreign_count = foreign_count - 1 " \
+    "WHERE id = OLD.place_id;" \
+  "END" \
+)
+
+#define CREATE_KEYWORDS_FOREIGNCOUNT_AFTERINSERT_TRIGGER NS_LITERAL_CSTRING( \
+  "CREATE TEMP TRIGGER moz_keyords_foreign_count_afterinsert_trigger " \
+  "AFTER INSERT ON moz_keywords FOR EACH ROW " \
+  "BEGIN " \
+    "UPDATE moz_places " \
+    "SET foreign_count = foreign_count + 1 " \
+    "WHERE id = NEW.place_id;" \
+  "END" \
+)
+
+#define CREATE_KEYWORDS_FOREIGNCOUNT_AFTERUPDATE_TRIGGER NS_LITERAL_CSTRING( \
+  "CREATE TEMP TRIGGER moz_keywords_foreign_count_afterupdate_trigger " \
+  "AFTER UPDATE OF place_id ON moz_keywords FOR EACH ROW " \
+  "BEGIN " \
+    "UPDATE moz_places " \
+    "SET foreign_count = foreign_count + 1 " \
+    "WHERE id = NEW.place_id; " \
+    "UPDATE moz_places " \
+    "SET foreign_count = foreign_count - 1 " \
+    "WHERE id = OLD.place_id; " \
+  "END" \
+)
+
 #endif // __nsPlacesTriggers_h__

toolkit/components/places/tests/autocomplete/test_keyword_search.js

@@ -43,13 +43,13 @@ let kTitles = [
 // Add the keyword bookmark
 addPageBook(0, 0, 1, [], keyKey);
 // Add in the "fake pages" for keyword searches
-gPages[1] = [1,1];
+gPages[1] = [1,0];
-gPages[2] = [2,1];
+gPages[2] = [2,0];
-gPages[3] = [3,1];
+gPages[3] = [3,0];
-gPages[4] = [4,1];
+gPages[4] = [4,0];
 // Add a page into history
 addPageBook(5, 0);
-gPages[6] = [6,1];
+gPages[6] = [6,0];
 
 // Provide for each test: description; search terms; array of gPages indices of
 // pages that should match; optional function to be run before the test
@@ -68,14 +68,4 @@ let gTests = [
    keyKey, [6]],
   ["6: Keyword without query (with space)",
    keyKey + " ", [6]],
-
-  // This adds a second keyword so anything after this will match 2 keywords
-  ["7: Two keywords matched",
-   keyKey + " twoKey", [8,9],
-   function() {
-     // Add the keyword search as well as search results
-     addPageBook(7, 0, 1, [], keyKey);
-     gPages[8] = [8,1];
-     gPages[9] = [9,1];
-   }]
 ];

toolkit/components/places/tests/bookmarks/test_bookmarks.js

@@ -321,33 +321,8 @@ add_task(function test_bookmarks() {
   // test setKeywordForBookmark
   let kwTestItemId = bs.insertBookmark(testRoot, uri("http://keywordtest.com"),
                                        bs.DEFAULT_INDEX, "");
-  try {
+  bs.setKeywordForBookmark(kwTestItemId, "bar");
-    let dateAdded = bs.getItemDateAdded(kwTestItemId);
-    // after just inserting, modified should not be set
-    let lastModified = bs.getItemLastModified(kwTestItemId);
-    do_check_eq(lastModified, dateAdded);
 
-    // Workaround possible VM timers issues moving lastModified and dateAdded
-    // to the past.
-    lastModified -= 1000;
-    bs.setItemLastModified(kwTestItemId, --lastModified);
-    dateAdded -= 1000;
-    bs.setItemDateAdded(kwTestItemId, dateAdded);
-
-    bs.setKeywordForBookmark(kwTestItemId, "bar");
-
-    let lastModified2 = bs.getItemLastModified(kwTestItemId);
-    LOG("test setKeywordForBookmark");
-    LOG("dateAdded = " + dateAdded);
-    LOG("lastModified = " + lastModified);
-    LOG("lastModified2 = " + lastModified2);
-    do_check_true(is_time_ordered(lastModified, lastModified2));
-    do_check_true(is_time_ordered(dateAdded, lastModified2));
-  } catch(ex) {
-    do_throw("setKeywordForBookmark: " + ex);
-  }
-
-  let lastModified3 = bs.getItemLastModified(kwTestItemId);
   // test getKeywordForBookmark
   let k = bs.getKeywordForBookmark(kwTestItemId);
   do_check_eq("bar", k);

toolkit/components/places/tests/bookmarks/test_keywords.js

@@ -1,169 +1,302 @@
-/* Any copyright is dedicated to the Public Domain.
+const URI1 = NetUtil.newURI("http://test1.mozilla.org/");
-   http://creativecommons.org/publicdomain/zero/1.0/ */
+const URI2 = NetUtil.newURI("http://test2.mozilla.org/");
+const URI3 = NetUtil.newURI("http://test3.mozilla.org/");
 
-function check_bookmark_keyword(aItemId, aKeyword)
+function check_keyword(aURI, aKeyword) {
-{
+  if (aKeyword)
-  let keyword = aKeyword ? aKeyword.toLowerCase() : null;
+    aKeyword = aKeyword.toLowerCase();
-  do_check_eq(PlacesUtils.bookmarks.getKeywordForBookmark(aItemId),
-              keyword);
-}
-
-function check_uri_keyword(aURI, aKeyword)
-{
-  let keyword = aKeyword ? aKeyword.toLowerCase() : null;
 
   for (let bm of PlacesUtils.getBookmarksForURI(aURI)) {
-    let kid = PlacesUtils.bookmarks.getKeywordForBookmark(bm);
+    let keyword = PlacesUtils.bookmarks.getKeywordForBookmark(bm);
-    if (kid && !keyword) {
+    if (keyword && !aKeyword) {
-      Assert.ok(false, `${aURI.spec} should not have a keyword`);
+      throw(`${aURI.spec} should not have a keyword`);
-    } else if (keyword && kid == keyword) {
+    } else if (aKeyword && keyword == aKeyword) {
-      Assert.equal(kid, keyword, "Found the keyword");
+      Assert.equal(keyword, aKeyword);
-      break;
     }
   }
 
   if (aKeyword) {
-    // This API can't tell which uri the user wants, so it returns a random one.
+    let uri = PlacesUtils.bookmarks.getURIForKeyword(aKeyword);
-    let re = /http:\/\/test[0-9]\.mozilla\.org/;
+    Assert.equal(uri.spec, aURI.spec);
-    let url = PlacesUtils.bookmarks.getURIForKeyword(aKeyword).spec;
-    do_check_true(re.test(url));
     // Check case insensitivity.
-    url = PlacesUtils.bookmarks.getURIForKeyword(aKeyword.toUpperCase()).spec
+    uri = PlacesUtils.bookmarks.getURIForKeyword(aKeyword.toUpperCase());
-    do_check_true(re.test(url));
+    Assert.equal(uri.spec, aURI.spec);
   }
 }
 
-function check_orphans()
+function check_orphans() {
-{
+  let db = yield PlacesUtils.promiseDBConnection();
-  let stmt = DBConn().createStatement(
+  let rows = yield db.executeCached(
-    `SELECT id FROM moz_keywords k WHERE NOT EXISTS (
+    `SELECT id FROM moz_keywords k
-        SELECT id FROM moz_bookmarks WHERE keyword_id = k.id
+     WHERE NOT EXISTS (SELECT 1 FROM moz_places WHERE id = k.place_id)
-     )`
+    `);
-  );
+  Assert.equal(rows.length, 0);
-  try {
-    do_check_false(stmt.executeStep());
-  } finally {
-    stmt.finalize();
-  }
-
-  print("Check there are no orphan database entries");
-  stmt = DBConn().createStatement(
-    `SELECT b.id FROM moz_bookmarks b
-     LEFT JOIN moz_keywords k ON b.keyword_id = k.id
-     WHERE keyword_id NOTNULL AND k.id ISNULL`
-  );
-  try {
-    do_check_false(stmt.executeStep());
-  } finally {
-    stmt.finalize();
-  }
 }
 
-const URIS = [
+function expectNotifications() {
-  uri("http://test1.mozilla.org/"),
+  let notifications = [];
-  uri("http://test2.mozilla.org/"),
+  let observer = new Proxy(NavBookmarkObserver, {
-];
+    get(target, name) {
+      if (name == "check") {
+        PlacesUtils.bookmarks.removeObserver(observer);
+        return expectedNotifications =>
+          Assert.deepEqual(notifications, expectedNotifications);
+      }
 
-add_test(function test_addBookmarkWithKeyword()
+      if (name.startsWith("onItemChanged")) {
-{
+        return (id, prop, isAnno, val, lastMod, itemType, parentId, guid, parentGuid) => {
-  check_uri_keyword(URIS[0], null);
+          if (prop != "keyword")
+            return;
+          let args = Array.from(arguments, arg => {
+            if (arg && arg instanceof Ci.nsIURI)
+              return new URL(arg.spec);
+            if (arg && typeof(arg) == "number" && arg >= Date.now() * 1000)
+              return new Date(parseInt(arg/1000));
+            return arg;
+          });
+          notifications.push({ name: name, arguments: args });
+        }
+      }
+
+      return target[name];
+    }
+  });
+  PlacesUtils.bookmarks.addObserver(observer, false);
+  return observer;
+}
+
+add_task(function test_invalid_input() {
+  Assert.throws(() => PlacesUtils.bookmarks.getURIForKeyword(null),
+                /NS_ERROR_ILLEGAL_VALUE/);
+  Assert.throws(() => PlacesUtils.bookmarks.getURIForKeyword(""),
+                /NS_ERROR_ILLEGAL_VALUE/);
+  Assert.throws(() => PlacesUtils.bookmarks.getKeywordForBookmark(null),
+                /NS_ERROR_ILLEGAL_VALUE/);
+  Assert.throws(() => PlacesUtils.bookmarks.getKeywordForBookmark(0),
+                /NS_ERROR_ILLEGAL_VALUE/);
+  Assert.throws(() => PlacesUtils.bookmarks.setKeywordForBookmark(null, "k"),
+                /NS_ERROR_ILLEGAL_VALUE/);
+  Assert.throws(() => PlacesUtils.bookmarks.setKeywordForBookmark(0, "k"),
+                /NS_ERROR_ILLEGAL_VALUE/);
+});
+
+add_task(function test_addBookmarkAndKeyword() {
+  check_keyword(URI1, null);
+  let fc = yield foreign_count(URI1);
+  let observer = expectNotifications();
 
   let itemId =
     PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
-                                         URIS[0],
+                                         URI1,
                                          PlacesUtils.bookmarks.DEFAULT_INDEX,
                                          "test");
+
   PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "keyword");
-  check_bookmark_keyword(itemId, "keyword");
+  let bookmark = yield PlacesUtils.bookmarks.fetch({ url: URI1 });
-  check_uri_keyword(URIS[0], "keyword");
+  observer.check([ { name: "onItemChanged",
+                     arguments: [ itemId, "keyword", false, "keyword",
+                                  bookmark.lastModified, bookmark.type,
+                                  (yield PlacesUtils.promiseItemId(bookmark.parentGuid)),
+                                  bookmark.guid, bookmark.parentGuid ] }
+                 ]);
+  yield PlacesTestUtils.promiseAsyncUpdates();
 
-  PlacesTestUtils.promiseAsyncUpdates().then(() => {
+  check_keyword(URI1, "keyword");
-    check_orphans();
+  Assert.equal((yield foreign_count(URI1)), fc + 2); // + 1 bookmark + 1 keyword
-    run_next_test();
+
-  });
+  yield PlacesTestUtils.promiseAsyncUpdates();
+  yield check_orphans();
 });
 
-add_test(function test_addBookmarkToURIHavingKeyword()
+add_task(function test_addBookmarkToURIHavingKeyword() {
-{
+  // The uri has already a keyword.
+  check_keyword(URI1, "keyword");
+  let fc = yield foreign_count(URI1);
+
   let itemId =
     PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
-                                         URIS[0],
+                                         URI1,
                                          PlacesUtils.bookmarks.DEFAULT_INDEX,
                                          "test");
-  // The uri has a keyword, but this specific bookmark has not.
+  check_keyword(URI1, "keyword");
-  check_bookmark_keyword(itemId, null);
+  Assert.equal((yield foreign_count(URI1)), fc + 1); // + 1 bookmark
-  check_uri_keyword(URIS[0], "keyword");
 
-  PlacesTestUtils.promiseAsyncUpdates().then(() => {
+  PlacesUtils.bookmarks.removeItem(itemId);
-    check_orphans();
+  yield PlacesTestUtils.promiseAsyncUpdates();
-    run_next_test();
+  check_orphans();
-  });
 });
 
-add_test(function test_addSameKeywordToOtherURI()
+add_task(function test_sameKeywordDifferentURI() {
-{
+  let fc1 = yield foreign_count(URI1);
+  let fc2 = yield foreign_count(URI2);
+  let observer = expectNotifications();
+
   let itemId =
     PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
-                                         URIS[1],
+                                         URI2,
                                          PlacesUtils.bookmarks.DEFAULT_INDEX,
                                          "test2");
-  check_bookmark_keyword(itemId, null);
+  check_keyword(URI1, "keyword");
-  check_uri_keyword(URIS[1], null);
+  check_keyword(URI2, null);
 
   PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "kEyWoRd");
-  check_bookmark_keyword(itemId, "kEyWoRd");
-  check_uri_keyword(URIS[1], "kEyWoRd");
 
-  // Check case insensitivity.
+  let bookmark1 = yield PlacesUtils.bookmarks.fetch({ url: URI1 });
-  check_uri_keyword(URIS[0], "kEyWoRd");
+  let bookmark2 = yield PlacesUtils.bookmarks.fetch({ url: URI2 });
-  check_bookmark_keyword(itemId, "keyword");
+  observer.check([ { name: "onItemChanged",
-  check_uri_keyword(URIS[1], "keyword");
+                     arguments: [ (yield PlacesUtils.promiseItemId(bookmark1.guid)),
-  check_uri_keyword(URIS[0], "keyword");
+                                  "keyword", false, "",
+                                  bookmark1.lastModified, bookmark1.type,
+                                  (yield PlacesUtils.promiseItemId(bookmark1.parentGuid)),
+                                  bookmark1.guid, bookmark1.parentGuid ] },
+                    { name: "onItemChanged",
+                     arguments: [ itemId, "keyword", false, "keyword",
+                                  bookmark2.lastModified, bookmark2.type,
+                                  (yield PlacesUtils.promiseItemId(bookmark2.parentGuid)),
+                                  bookmark2.guid, bookmark2.parentGuid ] }
+                 ]);
+  yield PlacesTestUtils.promiseAsyncUpdates();
 
-  PlacesTestUtils.promiseAsyncUpdates().then(() => {
+  // The keyword should have been "moved" to the new URI.
-    check_orphans();
+  check_keyword(URI1, null);
-    run_next_test();
+  Assert.equal((yield foreign_count(URI1)), fc1 - 1); // - 1 keyword
-  });
+  check_keyword(URI2, "keyword");
+  Assert.equal((yield foreign_count(URI2)), fc2 + 2); // + 1 bookmark + 1 keyword
+
+  yield PlacesTestUtils.promiseAsyncUpdates();
+  check_orphans();
 });
 
-add_test(function test_removeBookmarkWithKeyword()
+add_task(function test_sameURIDifferentKeyword() {
-{
+  let fc = yield foreign_count(URI2);
+  let observer = expectNotifications();
+
   let itemId =
     PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
-                                         URIS[1],
+                                         URI2,
+                                         PlacesUtils.bookmarks.DEFAULT_INDEX,
+                                         "test2");
+  check_keyword(URI2, "keyword");
+
+  PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "keyword2");
+
+  let bookmarks = [];
+  yield PlacesUtils.bookmarks.fetch({ url: URI2 }, bookmark => bookmarks.push(bookmark));
+  observer.check([ { name: "onItemChanged",
+                     arguments: [ (yield PlacesUtils.promiseItemId(bookmarks[0].guid)),
+                                  "keyword", false, "keyword2",
+                                  bookmarks[0].lastModified, bookmarks[0].type,
+                                  (yield PlacesUtils.promiseItemId(bookmarks[0].parentGuid)),
+                                  bookmarks[0].guid, bookmarks[0].parentGuid ] },
+                    { name: "onItemChanged",
+                     arguments: [ (yield PlacesUtils.promiseItemId(bookmarks[1].guid)),
+                                  "keyword", false, "keyword2",
+                                  bookmarks[1].lastModified, bookmarks[1].type,
+                                  (yield PlacesUtils.promiseItemId(bookmarks[1].parentGuid)),
+                                  bookmarks[1].guid, bookmarks[1].parentGuid ] }
+                 ]);
+  yield PlacesTestUtils.promiseAsyncUpdates();
+
+  check_keyword(URI2, "keyword2");
+  Assert.equal((yield foreign_count(URI2)), fc + 2); // + 1 bookmark + 1 keyword
+
+  yield PlacesTestUtils.promiseAsyncUpdates();
+  check_orphans();
+});
+
+add_task(function test_removeBookmarkWithKeyword() {
+  let fc = yield foreign_count(URI2);
+  let itemId =
+    PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
+                                         URI2,
                                          PlacesUtils.bookmarks.DEFAULT_INDEX,
                                          "test");
-  PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "keyword");
-  check_bookmark_keyword(itemId, "keyword");
-  check_uri_keyword(URIS[1], "keyword");
 
-  // The keyword should not be removed from other bookmarks.
+   // The keyword should not be removed, since there are other bookmarks yet.
+   PlacesUtils.bookmarks.removeItem(itemId);
+
+  check_keyword(URI2, "keyword2");
+  Assert.equal((yield foreign_count(URI2)), fc); // + 1 bookmark - 1 bookmark
+
+  yield PlacesTestUtils.promiseAsyncUpdates();
+  check_orphans();
+});
+
+add_task(function test_unsetKeyword() {
+  let fc = yield foreign_count(URI2);
+  let observer = expectNotifications();
+
+  let itemId =
+    PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
+                                         URI2,
+                                         PlacesUtils.bookmarks.DEFAULT_INDEX,
+                                         "test");
+
+  // The keyword should be removed from any bookmark.
+  PlacesUtils.bookmarks.setKeywordForBookmark(itemId, null);
+
+  let bookmarks = [];
+  yield PlacesUtils.bookmarks.fetch({ url: URI2 }, bookmark => bookmarks.push(bookmark));
+  do_print(bookmarks.length);
+  observer.check([ { name: "onItemChanged",
+                     arguments: [ (yield PlacesUtils.promiseItemId(bookmarks[0].guid)),
+                                  "keyword", false, "",
+                                  bookmarks[0].lastModified, bookmarks[0].type,
+                                  (yield PlacesUtils.promiseItemId(bookmarks[0].parentGuid)),
+                                  bookmarks[0].guid, bookmarks[0].parentGuid ] },
+                    { name: "onItemChanged",
+                     arguments: [ (yield PlacesUtils.promiseItemId(bookmarks[1].guid)),
+                                  "keyword", false, "",
+                                  bookmarks[1].lastModified, bookmarks[1].type,
+                                  (yield PlacesUtils.promiseItemId(bookmarks[1].parentGuid)),
+                                  bookmarks[1].guid, bookmarks[1].parentGuid ] },
+                    { name: "onItemChanged",
+                     arguments: [ (yield PlacesUtils.promiseItemId(bookmarks[2].guid)),
+                                  "keyword", false, "",
+                                  bookmarks[2].lastModified, bookmarks[2].type,
+                                  (yield PlacesUtils.promiseItemId(bookmarks[2].parentGuid)),
+                                  bookmarks[2].guid, bookmarks[2].parentGuid ] }
+                 ]);
+
+  check_keyword(URI1, null);
+  check_keyword(URI2, null);
+  Assert.equal((yield foreign_count(URI2)), fc - 1); // + 1 bookmark - 2 keyword
+
+  yield PlacesTestUtils.promiseAsyncUpdates();
+  check_orphans();
+});
+
+add_task(function test_addRemoveBookmark() {
+  let fc = yield foreign_count(URI3);
+  let observer = expectNotifications();
+
+  let itemId =
+    PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
+                                         URI3,
+                                         PlacesUtils.bookmarks.DEFAULT_INDEX,
+                                         "test3");
+
+  PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "keyword");
+  let bookmark = yield PlacesUtils.bookmarks.fetch({ url: URI3 });
+  let parentId = yield PlacesUtils.promiseItemId(bookmark.parentGuid);
   PlacesUtils.bookmarks.removeItem(itemId);
 
-  check_uri_keyword(URIS[1], "keyword");
+  observer.check([ { name: "onItemChanged",
-  check_uri_keyword(URIS[0], "keyword");
+                     arguments: [ itemId,
+                                  "keyword", false, "keyword",
+                                  bookmark.lastModified, bookmark.type,
+                                  parentId,
+                                  bookmark.guid, bookmark.parentGuid ] }
+                 ]);
 
-  PlacesTestUtils.promiseAsyncUpdates().then(() => {
+  check_keyword(URI3, null);
-    check_orphans();
+  // Don't check the foreign count since the process is async.
-    run_next_test();
+  // The new test_keywords.js in unit is checking this though.
-  });
+
+  yield PlacesTestUtils.promiseAsyncUpdates();
+  check_orphans();
 });
 
-add_test(function test_removeFolderWithKeywordedBookmarks()
+function run_test() {
-{
-  // Keyword should be removed as well.
-  PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.unfiledBookmarksFolderId);
-
-  check_uri_keyword(URIS[1], null);
-  check_uri_keyword(URIS[0], null);
-
-  PlacesTestUtils.promiseAsyncUpdates().then(() => {
-    check_orphans();
-    run_next_test();
-  });
-});
-
-function run_test()
-{
   run_next_test();
 }

toolkit/components/places/tests/head_common.js

@@ -3,7 +3,7 @@
  * 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/. */
 
-const CURRENT_SCHEMA_VERSION = 26;
+const CURRENT_SCHEMA_VERSION = 27;
 const FIRST_UPGRADABLE_SCHEMA_VERSION = 11;
 
 const NS_APP_USER_PROFILE_50_DIR = "ProfD";
@@ -851,3 +851,17 @@ function checkBookmarkObject(info) {
   Assert.ok(info.lastModified >= info.dateAdded, "lastModified should never be smaller than dateAdded");
   Assert.ok(typeof info.type == "number", "type should be a number");
 }
+
+/**
+ * Reads foreign_count value for a given url.
+ */
+function* foreign_count(url) {
+  if (url instanceof Ci.nsIURI)
+    url = url.spec;
+  let db = yield PlacesUtils.promiseDBConnection();
+  let rows = yield db.executeCached(
+    `SELECT foreign_count FROM moz_places
+     WHERE url = :url
+    `, { url });
+  return rows.length == 0 ? 0 : rows[0].getResultByName("foreign_count");
+}

toolkit/components/places/tests/migration/test_current_from_v26.js

@@ -0,0 +1,75 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+add_task(function* setup() {
+  yield setupPlacesDatabase("places_v26.sqlite");
+  // Setup database contents to be migrated.
+  let path = OS.Path.join(OS.Constants.Path.profileDir, DB_FILENAME);
+  let db = yield Sqlite.openConnection({ path });
+  // Add pages.
+  yield db.execute(`INSERT INTO moz_places (url, guid)
+                    VALUES ("http://test1.com/", "test1_______")
+                         , ("http://test2.com/", "test2_______")
+                   `);
+  // Add keywords.
+  yield db.execute(`INSERT INTO moz_keywords (keyword)
+                    VALUES ("kw1")
+                         , ("kw2")
+                         , ("kw3")
+                   `);
+  // Add bookmarks.
+  let now = Date.now() * 1000;
+  let index = 0;
+  yield db.execute(`INSERT INTO moz_bookmarks (type, fk, parent, position, dateAdded, lastModified, keyword_id, guid)
+                    VALUES (1, (SELECT id FROM moz_places WHERE guid = 'test1_______'), 3, ${index++}, ${now}, ${now},
+                             (SELECT id FROM moz_keywords WHERE keyword = 'kw1'), "bookmark1___")
+                            /* same uri, different keyword */
+                         , (1, (SELECT id FROM moz_places WHERE guid = 'test1_______'), 3, ${index++}, ${now}, ${now},
+                             (SELECT id FROM moz_keywords WHERE keyword = 'kw2'), "bookmark2___")
+                           /* different uri, same keyword as 1 */
+                         , (1, (SELECT id FROM moz_places WHERE guid = 'test2_______'), 3, ${index++}, ${now}, ${now},
+                             (SELECT id FROM moz_keywords WHERE keyword = 'kw1'), "bookmark3___")
+                           /* same uri, same keyword as 1 */
+                         , (1, (SELECT id FROM moz_places WHERE guid = 'test1_______'), 3, ${index++}, ${now}, ${now},
+                             (SELECT id FROM moz_keywords WHERE keyword = 'kw1'), "bookmark4___")
+                           /* same uri, same keyword as 2 */
+                         , (1, (SELECT id FROM moz_places WHERE guid = 'test2_______'), 3, ${index++}, ${now}, ${now},
+                             (SELECT id FROM moz_keywords WHERE keyword = 'kw2'), "bookmark5___")
+                           /* different uri, same keyword as 1 */
+                         , (1, (SELECT id FROM moz_places WHERE guid = 'test1_______'), 3, ${index++}, ${now}, ${now},
+                             (SELECT id FROM moz_keywords WHERE keyword = 'kw3'), "bookmark6___")
+                   `);
+  // Add postData.
+  yield db.execute(`INSERT INTO moz_anno_attributes (name)
+                    VALUES ("bookmarkProperties/POSTData")`);
+  yield db.execute(`INSERT INTO moz_items_annos(anno_attribute_id, item_id, content)
+                    VALUES ((SELECT id FROM moz_anno_attributes where name = "bookmarkProperties/POSTData"),
+                            (SELECT id FROM moz_bookmarks WHERE guid = "bookmark3___"), "postData1")
+                         , ((SELECT id FROM moz_anno_attributes where name = "bookmarkProperties/POSTData"),
+                            (SELECT id FROM moz_bookmarks WHERE guid = "bookmark5___"), "postData2")`);
+  yield db.close();
+});
+
+add_task(function* database_is_valid() {
+  Assert.equal(PlacesUtils.history.databaseStatus,
+               PlacesUtils.history.DATABASE_STATUS_UPGRADED);
+
+  let db = yield PlacesUtils.promiseDBConnection();
+  Assert.equal((yield db.getSchemaVersion()), CURRENT_SCHEMA_VERSION);
+});
+
+add_task(function* test_keywords() {
+  // When 2 urls have the same keyword, if one has postData it will be
+  // preferred.
+  let [ url1, postData1 ] = PlacesUtils.getURLAndPostDataForKeyword("kw1");
+  Assert.equal(url1, "http://test2.com/");
+  Assert.equal(postData1, "postData1");
+  let [ url2, postData2 ] = PlacesUtils.getURLAndPostDataForKeyword("kw2");
+  Assert.equal(url2, "http://test2.com/");
+  Assert.equal(postData2, "postData2");
+  let [ url3, postData3 ] = PlacesUtils.getURLAndPostDataForKeyword("kw3");
+  Assert.equal(url3, "http://test1.com/");
+
+  Assert.equal((yield foreign_count("http://test1.com/")), 5); // 4 bookmark2 + 1 keywords
+  Assert.equal((yield foreign_count("http://test2.com/")), 4); // 2 bookmark2 + 2 keywords
+});

toolkit/components/places/tests/migration/xpcshell.ini

@@ -15,6 +15,7 @@ support-files =
   places_v24.sqlite
   places_v25.sqlite
   places_v26.sqlite
+  places_v27.sqlite
 
 [test_current_from_downgraded.js]
 [test_current_from_v6.js]
@@ -22,3 +23,4 @@ support-files =
 [test_current_from_v19.js]
 [test_current_from_v24.js]
 [test_current_from_v25.js]
+[test_current_from_v26.js]

toolkit/components/places/tests/queries/test_sorting.js

@@ -504,7 +504,7 @@ tests.push({
 
       // if keywords are equal, should fall back to title
       { isBookmark: true,
-        uri: "http://example.com/b2",
+        uri: "http://example.com/b1",
         parentFolder: PlacesUtils.bookmarks.toolbarFolder,
         index: PlacesUtils.bookmarks.DEFAULT_INDEX,
         title: "y8",

toolkit/components/places/tests/unifiedcomplete/test_keyword_search.js

@@ -19,60 +19,48 @@ add_task(function* test_keyword_searc() {
     { uri: uri1, title: "Generic page title" },
     { uri: uri2, title: "Generic page title" }
   ]);
-  yield addBookmark({ uri: uri1, title: "Keyword title", keyword: "key"});
+  yield addBookmark({ uri: uri1, title: "Bookmark title", keyword: "key"});
 
   do_print("Plain keyword query");
   yield check_autocomplete({
     search: "key term",
-    matches: [ { uri: NetUtil.newURI("http://abc/?search=term"), title: "Keyword title", style: ["keyword"] } ]
+    matches: [ { uri: NetUtil.newURI("http://abc/?search=term"), title: "Generic page title", style: ["keyword"] } ]
   });
 
   do_print("Multi-word keyword query");
   yield check_autocomplete({
     search: "key multi word",
-    matches: [ { uri: NetUtil.newURI("http://abc/?search=multi+word"), title: "Keyword title", style: ["keyword"] } ]
+    matches: [ { uri: NetUtil.newURI("http://abc/?search=multi+word"), title: "Generic page title", style: ["keyword"] } ]
   });
 
   do_print("Keyword query with +");
   yield check_autocomplete({
     search: "key blocking+",
-    matches: [ { uri: NetUtil.newURI("http://abc/?search=blocking%2B"), title: "Keyword title", style: ["keyword"] } ]
+    matches: [ { uri: NetUtil.newURI("http://abc/?search=blocking%2B"), title: "Generic page title", style: ["keyword"] } ]
   });
 
   do_print("Unescaped term in query");
   yield check_autocomplete({
     search: "key ユニコード",
-    matches: [ { uri: NetUtil.newURI("http://abc/?search=ユニコード"), title: "Keyword title", style: ["keyword"] } ]
+    matches: [ { uri: NetUtil.newURI("http://abc/?search=ユニコード"), title: "Generic page title", style: ["keyword"] } ]
   });
 
   do_print("Keyword that happens to match a page");
   yield check_autocomplete({
     search: "key ThisPageIsInHistory",
-    matches: [ { uri: NetUtil.newURI("http://abc/?search=ThisPageIsInHistory"), title: "Generic page title", style: ["bookmark"] } ]
+    matches: [ { uri: NetUtil.newURI("http://abc/?search=ThisPageIsInHistory"), title: "Generic page title", style: ["keyword"] } ]
   });
 
   do_print("Keyword without query (without space)");
   yield check_autocomplete({
     search: "key",
-    matches: [ { uri: NetUtil.newURI("http://abc/?search="), title: "Keyword title", style: ["keyword"] } ]
+    matches: [ { uri: NetUtil.newURI("http://abc/?search="), title: "Generic page title", style: ["keyword"] } ]
   });
 
   do_print("Keyword without query (with space)");
   yield check_autocomplete({
     search: "key ",
-    matches: [ { uri: NetUtil.newURI("http://abc/?search="), title: "Keyword title", style: ["keyword"] } ]
+    matches: [ { uri: NetUtil.newURI("http://abc/?search="), title: "Generic page title", style: ["keyword"] } ]
-  });
-
-  // This adds a second keyword so anything after this will match 2 keywords
-  let uri3 = NetUtil.newURI("http://xyz/?foo=%s");
-  yield PlacesTestUtils.addVisits([ { uri: uri3, title: "Generic page title" } ]);
-  yield addBookmark({ uri: uri3, title: "Keyword title", keyword: "key", style: ["keyword"] });
-
-  do_print("Two keywords matched");
-  yield check_autocomplete({
-    search: "key twoKey",
-    matches: [ { uri: NetUtil.newURI("http://abc/?search=twoKey"), title: "Keyword title", style: ["keyword"] },
-               { uri: NetUtil.newURI("http://xyz/?foo=twoKey"), title: "Keyword title", style: ["keyword"] } ]
   });
 
   yield cleanup();