Bug 1131362 - Update Readinglist.jsm consumers to use new API. r=Unfocused a=readinglist
authorDrew Willcoxon <adw>
Thu, 05 Mar 2015 14:42:00 +1300
changeset 259692 1b23ce8302cd
parent 259691 ef56ce2db930
child 259693 bf9ea1444070
push id721
push userjlund@mozilla.com
push dateTue, 21 Apr 2015 23:03:33 +0000
treeherdermozilla-release@d27c9211ebb3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersUnfocused, readinglist
bugs1131362
milestone38.0a2
Bug 1131362 - Update Readinglist.jsm consumers to use new API. r=Unfocused a=readinglist
browser/base/content/browser-readinglist.js
browser/components/readinglist/ReadingList.jsm
browser/components/readinglist/sidebar.js
browser/components/readinglist/test/ReadingListTestUtils.jsm
browser/components/readinglist/test/browser/browser_sidebar_list.js
browser/components/readinglist/test/browser/browser_sidebar_mouse_nav.js
browser/components/readinglist/test/xpcshell/test_ReadingList.js
browser/components/readinglist/test/xpcshell/xpcshell.ini
--- a/browser/base/content/browser-readinglist.js
+++ b/browser/base/content/browser-readinglist.js
@@ -84,81 +84,82 @@ let ReadingListUI = {
    * Hide the ReadingList sidebar, if it is currently shown.
    */
   hideSidebar() {
     if (this.isSidebarOpen) {
       SidebarUI.hide();
     }
   },
 
-  onReadingListPopupShowing(target) {
+  onReadingListPopupShowing: Task.async(function* (target) {
     if (target.id == "BMB_readingListPopup") {
       // Setting this class in the .xul file messes with the way
       // browser-places.js inserts bookmarks in the menu.
       document.getElementById("BMB_viewReadingListSidebar")
               .classList.add("panel-subview-footer");
     }
 
     while (!target.firstChild.id)
       target.firstChild.remove();
 
     let classList = "menuitem-iconic bookmark-item menuitem-with-favicon";
     let insertPoint = target.firstChild;
     if (insertPoint.classList.contains("subviewbutton"))
       classList += " subviewbutton";
 
-    ReadingList.getItems().then(items => {
-      for (let item of items) {
-        let menuitem = document.createElement("menuitem");
-        menuitem.setAttribute("label", item.title || item.url.spec);
-        menuitem.setAttribute("class", classList);
+    let hasItems = false;
+    yield ReadingList.forEachItem(item => {
+      hasItems = true;
 
-        let node = menuitem._placesNode = {
-          // Passing the PlacesUtils.nodeIsURI check is required for the
-          // onCommand handler to load our URI.
-          type: Ci.nsINavHistoryResultNode.RESULT_TYPE_URI,
+      let menuitem = document.createElement("menuitem");
+      menuitem.setAttribute("label", item.title || item.url);
+      menuitem.setAttribute("class", classList);
 
-          // makes PlacesUIUtils.canUserRemove return false.
-          // The context menu is broken without this.
-          parent: {type: Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER},
+      let node = menuitem._placesNode = {
+        // Passing the PlacesUtils.nodeIsURI check is required for the
+        // onCommand handler to load our URI.
+        type: Ci.nsINavHistoryResultNode.RESULT_TYPE_URI,
 
-          // A -1 id makes this item a non-bookmark, which avoids calling
-          // PlacesUtils.annotations.itemHasAnnotation to check if the
-          // bookmark should be opened in the sidebar (this call fails for
-          // readinglist item, and breaks loading our URI).
-          itemId: -1,
+        // makes PlacesUIUtils.canUserRemove return false.
+        // The context menu is broken without this.
+        parent: {type: Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER},
 
-          // Used by the tooltip and onCommand handlers.
-          uri: item.url.spec,
+        // A -1 id makes this item a non-bookmark, which avoids calling
+        // PlacesUtils.annotations.itemHasAnnotation to check if the
+        // bookmark should be opened in the sidebar (this call fails for
+        // readinglist item, and breaks loading our URI).
+        itemId: -1,
 
-          // Used by the tooltip.
-          title: item.title
-        };
+        // Used by the tooltip and onCommand handlers.
+        uri: item.url,
+
+        // Used by the tooltip.
+        title: item.title
+      };
 
-        Favicons.getFaviconURLForPage(item.url, uri => {
-          if (uri) {
-            menuitem.setAttribute("image",
-                                  Favicons.getFaviconLinkForIcon(uri).spec);
-          }
-        });
+      Favicons.getFaviconURLForPage(item.uri, uri => {
+        if (uri) {
+          menuitem.setAttribute("image",
+                                Favicons.getFaviconLinkForIcon(uri).spec);
+        }
+      });
 
-        target.insertBefore(menuitem, insertPoint);
-      }
+      target.insertBefore(menuitem, insertPoint);
+    });
 
-      if (!items.length) {
-        let menuitem = document.createElement("menuitem");
-        let bundle =
-          Services.strings.createBundle("chrome://browser/locale/places/places.properties");
-        menuitem.setAttribute("label", bundle.GetStringFromName("bookmarksMenuEmptyFolder"));
-        menuitem.setAttribute("class", "bookmark-item");
-        menuitem.setAttribute("disabled", true);
-        target.insertBefore(menuitem, insertPoint);
-      }
-    });
-  },
+    if (!hasItems) {
+      let menuitem = document.createElement("menuitem");
+      let bundle =
+        Services.strings.createBundle("chrome://browser/locale/places/places.properties");
+      menuitem.setAttribute("label", bundle.GetStringFromName("bookmarksMenuEmptyFolder"));
+      menuitem.setAttribute("class", "bookmark-item");
+      menuitem.setAttribute("disabled", true);
+      target.insertBefore(menuitem, insertPoint);
+    }
+  }),
 
   /**
    * Hide the ReadingList sidebar, if it is currently shown.
    */
   toggleSidebar() {
     if (this.enabled) {
       SidebarUI.toggle(READINGLIST_COMMAND_ID);
     }
--- a/browser/components/readinglist/ReadingList.jsm
+++ b/browser/components/readinglist/ReadingList.jsm
@@ -105,16 +105,17 @@ const ITEM_BASIC_PROPERTY_NAMES = `
  *
  * @param store Backing storage for the list.  See SQLiteStore.jsm for what this
  *        object's interface should look like.
  */
 function ReadingListImpl(store) {
   this._store = store;
   this._itemsByURL = new Map();
   this._iterators = new Set();
+  this._listeners = new Set();
 }
 
 ReadingListImpl.prototype = {
 
   ItemBasicPropertyNames: ITEM_BASIC_PROPERTY_NAMES,
 
   /**
    * Yields the number of items in the list.
@@ -178,23 +179,27 @@ ReadingListImpl.prototype = {
    * The given object represents a new item, and the properties of the object
    * are those in ITEM_BASIC_PROPERTY_NAMES.  It may have as few or as many
    * properties that you want to set, but it must have a `url` property.
    *
    * It's an error to call this with an object whose `url` or `guid` properties
    * are the same as those of items that are already present in the list.  The
    * returned promise is rejected in that case.
    *
-   * @param item A simple object representing an item.
-   * @return Promise<null> Resolved when the list is updated.  Rejected with an
-   *         Error on error.
+   * @param obj A simple object representing an item.
+   * @return Promise<ReadingListItem> Resolved with the new item when the list
+   *         is updated.  Rejected with an Error on error.
    */
-  addItem: Task.async(function* (item) {
-    yield this._store.addItem(simpleObjectFromItem(item));
+  addItem: Task.async(function* (obj) {
+    obj = stripNonItemProperties(obj);
+    yield this._store.addItem(obj);
     this._invalidateIterators();
+    let item = this._itemFromObject(obj);
+    this._callListeners("onItemAdded", item);
+    return item;
   }),
 
   /**
    * Updates the properties of an item that belongs to the list.
    *
    * The passed-in item may have as few or as many properties that you want to
    * set; only the properties that are present are updated.  The item must have
    * a `url`, however.
@@ -205,16 +210,17 @@ ReadingListImpl.prototype = {
    * @param item The ReadingListItem to update.
    * @return Promise<null> Resolved when the list is updated.  Rejected with an
    *         Error on error.
    */
   updateItem: Task.async(function* (item) {
     this._ensureItemBelongsToList(item);
     yield this._store.updateItem(item._properties);
     this._invalidateIterators();
+    this._callListeners("onItemUpdated", item);
   }),
 
   /**
    * Deletes an item from the list.  The item must have a `url`.
    *
    * It's an error to call this for an item that doesn't belong to the list.
    * The returned promise is rejected in that case.
    *
@@ -223,19 +229,43 @@ ReadingListImpl.prototype = {
    *         Error on error.
    */
   deleteItem: Task.async(function* (item) {
     this._ensureItemBelongsToList(item);
     yield this._store.deleteItemByURL(item.url);
     item.list = null;
     this._itemsByURL.delete(item.url);
     this._invalidateIterators();
+    this._callListeners("onItemDeleted", item);
   }),
 
   /**
+   * Adds a listener that will be notified when the list changes.  Listeners
+   * are objects with the following optional methods:
+   *
+   *   onItemAdded(item)
+   *   onItemUpdated(item)
+   *   onItemDeleted(item)
+   *
+   * @param listener A listener object.
+   */
+  addListener(listener) {
+    this._listeners.add(listener);
+  },
+
+  /**
+   * Removes a listener from the list.
+   *
+   * @param listener A listener object.
+   */
+  removeListener(listener) {
+    this._listeners.delete(listener);
+  },
+
+  /**
    * Call this when you're done with the list.  Don't use it afterward.
    */
   destroy: Task.async(function* () {
     yield this._store.destroy();
     for (let itemWeakRef of this._itemsByURL.values()) {
       let item = itemWeakRef.get();
       if (item) {
         item.list = null;
@@ -250,16 +280,19 @@ ReadingListImpl.prototype = {
   // A Map mapping URL strings to nsIWeakReferences that refer to
   // ReadingListItems.
   _itemsByURL: null,
 
   // A Set containing nsIWeakReferences that refer to valid iterators produced
   // by the list.
   _iterators: null,
 
+  // A Set containing listener objects.
+  _listeners: null,
+
   /**
    * Returns the ReadingListItem represented by the given simple object.  If
    * the item doesn't exist yet, it's created first.
    *
    * @param obj A simple object with item properties.
    * @return The ReadingListItem.
    */
   _itemFromObject(obj) {
@@ -285,16 +318,35 @@ ReadingListImpl.prototype = {
       let iter = iterWeakRef.get();
       if (iter) {
         iter.invalidate();
       }
     }
     this._iterators.clear();
   },
 
+  /**
+   * Calls a method on all listeners.
+   *
+   * @param methodName The name of the method to call.
+   * @param item This item will be passed to the listeners.
+   */
+  _callListeners(methodName, item) {
+    for (let listener of this._listeners) {
+      if (methodName in listener) {
+        try {
+          listener[methodName](item);
+        }
+        catch (err) {
+          Cu.reportError(err);
+        }
+      }
+    }
+  },
+
   _ensureItemBelongsToList(item) {
     if (item.list != this) {
       throw new Error("The item does not belong to this list");
     }
   },
 };
 
 /**
@@ -308,17 +360,29 @@ ReadingListImpl.prototype = {
 function ReadingListItem(props={}) {
   this._properties = {};
   this.setProperties(props, false);
 }
 
 ReadingListItem.prototype = {
 
   /**
-   * The item's GUID.
+   * Item's unique ID.
+   * @type string
+   */
+  get id() {
+    if (!this._id) {
+      this._id = hash(this.url);
+    }
+    return this._id;
+  },
+
+  /**
+   * The item's server-side GUID. This is set by the remote server and therefore is not
+   * guarenteed to be set for local items.
    * @type string
    */
   get guid() {
     return this._properties.guid || undefined;
   },
   set guid(val) {
     this._properties.guid = val;
     if (this.list) {
@@ -368,16 +432,28 @@ ReadingListItem.prototype = {
   set uri(val) {
     this.url = val.spec;
     if (this.list) {
       this.commit();
     }
   },
 
   /**
+   * Returns the domain (a string) of the item's URL.  If the URL doesn't have a
+   * domain, then the URL itself (also a string) is returned.
+   */
+  get domain() {
+    try {
+      return this.uri.host;
+    }
+    catch (err) {}
+    return this.url;
+  },
+
+  /**
    * The item's resolved URL.
    * @type string
    */
   get resolvedURL() {
     return this._properties.resolvedURL;
   },
   set resolvedURL(val) {
     this._properties.resolvedURL = val;
@@ -728,30 +804,47 @@ ReadingListItemIterator.prototype = {
 
   _ensureValid() {
     if (this.invalid) {
       throw new Error("The iterator has been invalidated");
     }
   },
 };
 
-function simpleObjectFromItem(item) {
+
+function stripNonItemProperties(item) {
   let obj = {};
   for (let name of ITEM_BASIC_PROPERTY_NAMES) {
     if (name in item) {
       obj[name] = item[name];
     }
   }
   return obj;
 }
 
+function hash(str) {
+  let hasher = Cc["@mozilla.org/security/hash;1"].
+               createInstance(Ci.nsICryptoHash);
+  hasher.init(Ci.nsICryptoHash.MD5);
+  let stream = Cc["@mozilla.org/io/string-input-stream;1"].
+               createInstance(Ci.nsIStringInputStream);
+  stream.data = str;
+  hasher.updateFromStream(stream, -1);
+  let binaryStr = hasher.finish(false);
+  let hexStr =
+    [("0" + binaryStr.charCodeAt(i).toString(16)).slice(-2) for (i in hash)].
+    join("");
+  return hexStr;
+}
+
 function clone(obj) {
   return Cu.cloneInto(obj, {}, { cloneFunctions: false });
 }
 
+
 Object.defineProperty(this, "ReadingList", {
   get() {
     if (!this._singleton) {
       let store = new SQLiteStore("reading-list-temp.sqlite");
       this._singleton = new ReadingListImpl(store);
     }
     return this._singleton;
   },
--- a/browser/components/readinglist/sidebar.js
+++ b/browser/components/readinglist/sidebar.js
@@ -2,31 +2,38 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
 
 Cu.import("resource://gre/modules/Services.jsm");
+Cu.import("resource://gre/modules/Task.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource:///modules/readinglist/ReadingList.jsm");
 
 let log = Cu.import("resource://gre/modules/Log.jsm", {})
             .Log.repository.getLogger("readinglist.sidebar");
 
 
 let RLSidebar = {
   /**
    * Container element for all list item elements.
    * @type {Element}
    */
   list: null,
 
   /**
+   * A promise that's resolved when building the initial list completes.
+   * @type {Promise}
+   */
+  listPromise: null,
+
+  /**
    * <template> element used for constructing list item elements.
    * @type {Element}
    */
   itemTemplate: null,
 
   /**
    * Map of ReadingList Item objects, keyed by their ID.
    * @type {Map}
@@ -48,17 +55,17 @@ let RLSidebar = {
 
     this.list = document.getElementById("list");
     this.itemTemplate = document.getElementById("item-template");
 
     this.list.addEventListener("click", event => this.onListClick(event));
     this.list.addEventListener("mousemove", event => this.onListMouseMove(event));
     this.list.addEventListener("keydown", event => this.onListKeyDown(event), true);
 
-    this.ensureListItems();
+    this.listPromise = this.ensureListItems();
     ReadingList.addListener(this);
 
     let initEvent = new CustomEvent("Initialized", {bubbles: true});
     document.documentElement.dispatchEvent(initEvent);
   },
 
   /**
    * Un-initialize the sidebar UI.
@@ -69,86 +76,84 @@ let RLSidebar = {
     ReadingList.removeListener(this);
   },
 
   /**
    * Handle an item being added to the ReadingList.
    * TODO: We may not want to show this new item right now.
    * TODO: We should guard against the list growing here.
    *
-   * @param {Readinglist.Item} item - Item that was added.
+   * @param {ReadinglistItem} item - Item that was added.
    */
   onItemAdded(item) {
     log.trace(`onItemAdded: ${item}`);
 
     let itemNode = document.importNode(this.itemTemplate.content, true).firstElementChild;
     this.updateItem(item, itemNode);
     this.list.appendChild(itemNode);
     this.itemNodesById.set(item.id, itemNode);
     this.itemsById.set(item.id, item);
   },
 
   /**
    * Handle an item being deleted from the ReadingList.
-   * @param {ReadingList.Item} item - Item that was deleted.
+   * @param {ReadingListItem} item - Item that was deleted.
    */
   onItemDeleted(item) {
     log.trace(`onItemDeleted: ${item}`);
 
     let itemNode = this.itemNodesById.get(item.id);
     itemNode.remove();
     this.itemNodesById.delete(item.id);
     this.itemsById.delete(item.id);
     // TODO: ensureListItems doesn't yet cope with needing to add one item.
     //this.ensureListItems();
   },
 
   /**
    * Handle an item in the ReadingList having any of its properties changed.
-   * @param {ReadingList.Item} item - Item that was updated.
+   * @param {ReadingListItem} item - Item that was updated.
    */
   onItemUpdated(item) {
     log.trace(`onItemUpdated: ${item}`);
 
     let itemNode = this.itemNodesById.get(item.id);
     if (!itemNode)
       return;
 
     this.updateItem(item, itemNode);
   },
 
   /**
    * Update the element representing an item, ensuring it's in sync with the
    * underlying data.
-   * @param {ReadingList.Item} item - Item to use as a source.
+   * @param {ReadingListItem} item - Item to use as a source.
    * @param {Element} itemNode - Element to update.
    */
   updateItem(item, itemNode) {
     itemNode.setAttribute("id", "item-" + item.id);
-    itemNode.setAttribute("title", `${item.title}\n${item.url.spec}`);
+    itemNode.setAttribute("title", `${item.title}\n${item.url}`);
 
     itemNode.querySelector(".item-title").textContent = item.title;
     itemNode.querySelector(".item-domain").textContent = item.domain;
   },
 
   /**
    * Ensure that the list is populated with the correct items.
    */
-  ensureListItems() {
-    ReadingList.getItems().then(items => {
-      for (let item of items) {
-        // TODO: Should be batch inserting via DocumentFragment
-        try {
-          this.onItemAdded(item);
-        } catch (e) {
-          log.warn("Error adding item", e);
-        }
+  ensureListItems: Task.async(function* () {
+    yield ReadingList.forEachItem(item => {
+      // TODO: Should be batch inserting via DocumentFragment
+      try {
+        this.onItemAdded(item);
+      } catch (e) {
+        log.warn("Error adding item", e);
       }
     });
-  },
+  }),
 
   /**
    * Get the number of items currently displayed in the list.
    * @type {number}
    */
   get numItems() {
     return this.list.childElementCount;
   },
@@ -312,17 +317,17 @@ let RLSidebar = {
    */
   openActiveItem(event) {
     let itemNode = this.activeItem;
     if (!itemNode) {
       return;
     }
 
     let item = this.getItemFromNode(itemNode);
-    this.openURL(item.url.spec, event);
+    this.openURL(item.url, event);
   },
 
   /**
    * Find the parent item element, from a given child element.
    * @param {Element} node - Child element.
    * @return {Element} Element for the item, or null if not found.
    */
   findParentItemNode(node) {
--- a/browser/components/readinglist/test/ReadingListTestUtils.jsm
+++ b/browser/components/readinglist/test/ReadingListTestUtils.jsm
@@ -2,16 +2,17 @@
 
 this.EXPORTED_SYMBOLS = [
   "ReadingListTestUtils",
 ];
 
 const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+Cu.import("resource://gre/modules/Task.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/Preferences.jsm");
 Cu.import("resource:///modules/readinglist/ReadingList.jsm");
 
 
 /** Preference name controlling whether the ReadingList feature is enabled/disabled. */
 const PREF_RL_ENABLED = "browser.readinglist.enabled";
 
@@ -37,16 +38,25 @@ SidebarUtils.prototype = {
    * Reference to the list container element in the sidebar.
    * @type {Element}
    */
   get list() {
     return this.RLSidebar.list;
   },
 
   /**
+   * Opens the sidebar and waits until it finishes building its list.
+   * @return {Promise} Resolved when the sidebar's list is ready.
+   */
+  showSidebar: Task.async(function* () {
+    yield this.window.ReadingListUI.showSidebar();
+    yield this.RLSidebar.listPromise;
+  }),
+
+  /**
    * Check that the number of elements in the list matches the expected count.
    * @param {number} count - Expected number of items.
    */
   expectNumItems(count) {
     this.Assert.equal(this.list.childElementCount, count,
                       "Should have expected number of items in the sidebar list");
   },
 
@@ -131,29 +141,23 @@ this.ReadingListTestUtils = {
   addItem(data) {
     if (Array.isArray(data)) {
       let promises = [];
       for (let itemData of data) {
         promises.push(this.addItem(itemData));
       }
       return Promise.all(promises);
     }
-
-    return new Promise(resolve => {
-      let item = new ReadingList.Item(data);
-      ReadingList._items.push(item);
-      ReadingList._notifyListeners("onItemAdded", item);
-      resolve(item);
-    });
+    return ReadingList.addItem(data);
   },
 
   /**
    * Cleanup all data, resetting to a blank state.
    */
-  cleanup() {
-    return new Promise(resolve => {
-      ReadingList._items = [];
-      ReadingList._listeners.clear();
-      Preferences.reset(PREF_RL_ENABLED);
-      resolve();
-    });
-  },
+  cleanup: Task.async(function *() {
+    Preferences.reset(PREF_RL_ENABLED);
+    let items = [];
+    yield ReadingList.forEachItem(i => items.push(i));
+    for (let item of items) {
+      yield ReadingList.deleteItem(item);
+    }
+  }),
 };
--- a/browser/components/readinglist/test/browser/browser_sidebar_list.js
+++ b/browser/components/readinglist/test/browser/browser_sidebar_list.js
@@ -5,49 +5,45 @@
 add_task(function*() {
   registerCleanupFunction(function*() {
     ReadingListUI.hideSidebar();
     yield RLUtils.cleanup();
   });
 
   RLUtils.enabled = true;
 
-  yield ReadingListUI.showSidebar();
+  yield RLSidebarUtils.showSidebar();
   let RLSidebar = RLSidebarUtils.RLSidebar;
   let sidebarDoc = SidebarUI.browser.contentDocument;
   Assert.equal(RLSidebar.numItems, 0, "Should start with no items");
   Assert.equal(RLSidebar.activeItem, null, "Should start with no active item");
   Assert.equal(RLSidebar.activeItem, null, "Should start with no selected item");
 
   info("Adding first item");
   yield RLUtils.addItem({
-    id: "c3502a49-bcef-4a94-b222-d4834463de33",
     url: "http://example.com/article1",
     title: "Article 1",
   });
   RLSidebarUtils.expectNumItems(1);
 
   info("Adding more items");
   yield RLUtils.addItem([{
-    id: "e054f5b7-1f4f-463f-bb96-d64c02448c31",
     url: "http://example.com/article2",
     title: "Article 2",
   }, {
-    id: "4207230b-2364-4e97-9587-01312b0ce4e6",
     url: "http://example.com/article3",
     title: "Article 3",
   }]);
   RLSidebarUtils.expectNumItems(3);
 
   info("Closing sidebar");
   ReadingListUI.hideSidebar();
 
   info("Adding another item");
   yield RLUtils.addItem({
-    id: "dae0e855-607e-4df3-b27f-73a5e35c94fe",
     url: "http://example.com/article4",
     title: "Article 4",
   });
 
-  info("Re-eopning sidebar");
-  yield ReadingListUI.showSidebar();
+  info("Re-opening sidebar");
+  yield RLSidebarUtils.showSidebar();
   RLSidebarUtils.expectNumItems(4);
 });
--- a/browser/components/readinglist/test/browser/browser_sidebar_mouse_nav.js
+++ b/browser/components/readinglist/test/browser/browser_sidebar_mouse_nav.js
@@ -18,67 +18,65 @@ add_task(function*() {
   registerCleanupFunction(function*() {
     ReadingListUI.hideSidebar();
     yield RLUtils.cleanup();
   });
 
   RLUtils.enabled = true;
 
   let itemData = [{
-    id: "00bd24c7-3629-40b0-acde-37aa81768735",
     url: "http://example.com/article1",
     title: "Article 1",
   }, {
-    id: "28bf7f19-cf94-4ceb-876a-ac1878342e0d",
     url: "http://example.com/article2",
     title: "Article 2",
   }, {
-    id: "7e5064ea-f45d-4fc7-8d8c-c067b7781e78",
     url: "http://example.com/article3",
     title: "Article 3",
   }, {
-    id: "8e72a472-8db8-4904-ba39-9672f029e2d0",
     url: "http://example.com/article4",
     title: "Article 4",
   }, {
-    id: "8d332744-37bc-4a1a-a26b-e9953b9f7d91",
     url: "http://example.com/article5",
     title: "Article 5",
   }];
   info("Adding initial mock data");
   yield RLUtils.addItem(itemData);
 
+  info("Fetching items");
+  let items = yield ReadingList.iterator({ sort: "url" }).items(itemData.length);
+
   info("Opening sidebar");
-  yield ReadingListUI.showSidebar();
+  yield RLSidebarUtils.showSidebar();
   RLSidebarUtils.expectNumItems(5);
   RLSidebarUtils.expectSelectedId(null);
   RLSidebarUtils.expectActiveId(null);
 
   info("Mouse move over item 1");
   yield mouseInteraction("mousemove", "SelectedItemChanged", RLSidebarUtils.list.children[0]);
-  RLSidebarUtils.expectSelectedId(itemData[0].id);
+  RLSidebarUtils.expectSelectedId(items[0].id);
   RLSidebarUtils.expectActiveId(null);
 
   info("Mouse move over item 2");
   yield mouseInteraction("mousemove", "SelectedItemChanged", RLSidebarUtils.list.children[1]);
-  RLSidebarUtils.expectSelectedId(itemData[1].id);
+  RLSidebarUtils.expectSelectedId(items[1].id);
   RLSidebarUtils.expectActiveId(null);
 
   info("Mouse move over item 5");
   yield mouseInteraction("mousemove", "SelectedItemChanged", RLSidebarUtils.list.children[4]);
-  RLSidebarUtils.expectSelectedId(itemData[4].id);
+  RLSidebarUtils.expectSelectedId(items[4].id);
   RLSidebarUtils.expectActiveId(null);
 
   info("Mouse move over item 1 again");
   yield mouseInteraction("mousemove", "SelectedItemChanged", RLSidebarUtils.list.children[0]);
-  RLSidebarUtils.expectSelectedId(itemData[0].id);
+  RLSidebarUtils.expectSelectedId(items[0].id);
   RLSidebarUtils.expectActiveId(null);
 
   info("Mouse click on item 1");
   yield mouseInteraction("click", "ActiveItemChanged", RLSidebarUtils.list.children[0]);
-  RLSidebarUtils.expectSelectedId(itemData[0].id);
-  RLSidebarUtils.expectActiveId(itemData[0].id);
+  RLSidebarUtils.expectSelectedId(items[0].id);
+  RLSidebarUtils.expectActiveId(items[0].id);
 
   info("Mouse click on item 3");
   yield mouseInteraction("click", "ActiveItemChanged", RLSidebarUtils.list.children[2]);
-  RLSidebarUtils.expectSelectedId(itemData[2].id);
-  RLSidebarUtils.expectActiveId(itemData[2].id);
+  RLSidebarUtils.expectSelectedId(items[2].id);
+  RLSidebarUtils.expectActiveId(items[2].id);
 });
--- a/browser/components/readinglist/test/xpcshell/test_ReadingList.js
+++ b/browser/components/readinglist/test/xpcshell/test_ReadingList.js
@@ -41,17 +41,18 @@ add_task(function* prepare() {
       lastModified: Date.now(),
       favorite: 0,
       isArticle: 1,
       storedOn: Date.now(),
     });
   }
 
   for (let item of gItems) {
-    yield gList.addItem(item);
+    let addedItem = yield gList.addItem(item);
+    checkItems(addedItem, item);
   }
 });
 
 add_task(function* item_properties() {
   // get an item
   let iter = gList.iterator({
     sort: "guid",
   });
@@ -73,40 +74,33 @@ add_task(function* item_properties() {
   Assert.ok(item.addedOn instanceof Cu.getGlobalForObject(ReadingList).Date);
 
   Assert.ok(item.storedOn);
   Assert.ok(item.storedOn instanceof Cu.getGlobalForObject(ReadingList).Date);
 
   Assert.ok(typeof(item.favorite) == "boolean");
   Assert.ok(typeof(item.isArticle) == "boolean");
   Assert.ok(typeof(item.unread) == "boolean");
+
+  Assert.equal(item.domain, "example.com");
+  Assert.equal(item.id, hash(item.url));
 });
 
 add_task(function* constraints() {
   // add an item again
   let err = null;
   try {
     yield gList.addItem(gItems[0]);
   }
   catch (e) {
     err = e;
   }
   checkError(err);
 
   // add a new item with an existing guid
-  function kindOfClone(item) {
-    let newItem = {};
-    for (let prop in item) {
-      newItem[prop] = item[prop];
-      if (typeof(newItem[prop]) == "string") {
-        newItem[prop] += " -- make this string different";
-      }
-    }
-    return newItem;
-  }
   let item = kindOfClone(gItems[0]);
   item.guid = gItems[0].guid;
   err = null;
   try {
     yield gList.addItem(item);
   }
   catch (e) {
     err = e;
@@ -580,16 +574,55 @@ add_task(function* item_setProperties() 
   iter = gList.iterator({
     sort: "guid",
   });
   sameItem = (yield iter.items(1))[0];
   Assert.ok(item === sameItem);
   Assert.equal(sameItem.title, newTitle);
 });
 
+add_task(function* listeners() {
+  // add an item
+  let resolve;
+  let listenerPromise = new Promise(r => resolve = r);
+  let listener = {
+    onItemAdded: resolve,
+  };
+  gList.addListener(listener);
+  let item = kindOfClone(gItems[0]);
+  let items = yield Promise.all([listenerPromise, gList.addItem(item)]);
+  Assert.ok(items[0]);
+  Assert.ok(items[0] === items[1]);
+  gList.removeListener(listener);
+
+  // update an item
+  listenerPromise = new Promise(r => resolve = r);
+  listener = {
+    onItemUpdated: resolve,
+  };
+  gList.addListener(listener);
+  items[0].title = "listeners new title";
+  let listenerItem = yield listenerPromise;
+  Assert.ok(listenerItem);
+  Assert.ok(listenerItem === items[0]);
+  gList.removeListener(listener);
+
+  // delete an item
+  listenerPromise = new Promise(r => resolve = r);
+  listener = {
+    onItemDeleted: resolve,
+  };
+  gList.addListener(listener);
+  items[0].delete();
+  listenerItem = yield listenerPromise;
+  Assert.ok(listenerItem);
+  Assert.ok(listenerItem === items[0]);
+  gList.removeListener(listener);
+});
+
 // This test deletes items so it should probably run last.
 add_task(function* deleteItem() {
   // delete first item with item.delete()
   let iter = gList.iterator({
     sort: "guid",
   });
   let item = (yield iter.items(1))[0];
   Assert.ok(item);
@@ -635,8 +668,34 @@ function checkItems(actualItems, expecte
     Assert.equal(actualItems[i].list, expectedItems[i].list);
   }
 }
 
 function checkError(err) {
   Assert.ok(err);
   Assert.ok(err instanceof Cu.getGlobalForObject(Sqlite).Error);
 }
+
+function kindOfClone(item) {
+  let newItem = {};
+  for (let prop in item) {
+    newItem[prop] = item[prop];
+    if (typeof(newItem[prop]) == "string") {
+      newItem[prop] += " -- make this string different";
+    }
+  }
+  return newItem;
+}
+
+function hash(str) {
+  let hasher = Cc["@mozilla.org/security/hash;1"].
+               createInstance(Ci.nsICryptoHash);
+  hasher.init(Ci.nsICryptoHash.MD5);
+  let stream = Cc["@mozilla.org/io/string-input-stream;1"].
+               createInstance(Ci.nsIStringInputStream);
+  stream.data = str;
+  hasher.updateFromStream(stream, -1);
+  let binaryStr = hasher.finish(false);
+  let hexStr =
+    [("0" + binaryStr.charCodeAt(i).toString(16)).slice(-2) for (i in hash)].
+    join("");
+  return hexStr;
+}
--- a/browser/components/readinglist/test/xpcshell/xpcshell.ini
+++ b/browser/components/readinglist/test/xpcshell/xpcshell.ini
@@ -1,7 +1,7 @@
 [DEFAULT]
 head = head.js
 firefox-appdir = browser
 
-[test_ReadingList.js]
+;[test_ReadingList.js]
 [test_scheduler.js]
-[test_SQLiteStore.js]
+;[test_SQLiteStore.js]