Bug 1131416 - Desktop syncing module for reading list service (prepatory changes). r=markh
authorDrew Willcoxon <adw@mozilla.com>
Fri, 20 Mar 2015 12:12:10 -0700
changeset 263717 c7897f709ff2c1e48dbd3a02c957d6cf5c299e5c
parent 263716 b11aaab4a248e4fec35f76dea393af7782af33f6
child 263718 6305033b955e0fb3220301e196e02fe8ee3e82ec
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh
bugs1131416
milestone39.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1131416 - Desktop syncing module for reading list service (prepatory changes). r=markh
browser/base/content/browser-readinglist.js
browser/components/readinglist/ReadingList.jsm
browser/components/readinglist/SQLiteStore.jsm
browser/components/readinglist/sidebar.js
browser/components/readinglist/test/ReadingListTestUtils.jsm
browser/components/readinglist/test/xpcshell/test_ReadingList.js
browser/modules/ReaderParent.jsm
--- a/browser/base/content/browser-readinglist.js
+++ b/browser/base/content/browser-readinglist.js
@@ -245,17 +245,17 @@ let ReadingListUI = {
     let msg = {topic: "UpdateActiveItem", url: null};
     if (!uri) {
       this.toolbarButton.setAttribute("hidden", true);
       if (this.isSidebarOpen)
         document.getElementById("sidebar").contentWindow.postMessage(msg, "*");
       return;
     }
 
-    let isInList = yield ReadingList.containsURL(uri);
+    let isInList = yield ReadingList.hasItemForURL(uri);
     if (this.isSidebarOpen) {
       if (isInList)
         msg.url = typeof uri == "string" ? uri : uri.spec;
       document.getElementById("sidebar").contentWindow.postMessage(msg, "*");
     }
     this.setToolbarButtonState(isInList);
   }),
 
--- a/browser/components/readinglist/ReadingList.jsm
+++ b/browser/components/readinglist/ReadingList.jsm
@@ -28,18 +28,20 @@ XPCOMUtils.defineLazyModuleGetter(this, 
   });
   let formatter = new Log.BasicFormatter();
   parentLog.addAppender(new Log.ConsoleAppender(formatter));
   parentLog.addAppender(new Log.DumpAppender(formatter));
 }
 let log = Log.repository.getLogger("readinglist.api");
 
 
-// Names of basic properties on ReadingListItem.
-const ITEM_BASIC_PROPERTY_NAMES = `
+// Each ReadingListItem has a _record property, an object containing the raw
+// data from the server and local store.  These are the names of the properties
+// in that object.
+const ITEM_RECORD_PROPERTIES = `
   guid
   lastModified
   url
   title
   resolvedURL
   resolvedTitle
   excerpt
   preview
@@ -66,17 +68,17 @@ const ITEM_BASIC_PROPERTY_NAMES = `
  * Options Objects
  * ---------------
  *
  * Some methods on ReadingList take an "optsList", a variable number of
  * arguments, each of which is an "options object".  Options objects let you
  * control the items that the method acts on.
  *
  * Each options object is a simple object with properties whose names are drawn
- * from ITEM_BASIC_PROPERTY_NAMES.  For an item to match an options object, the
+ * from ITEM_RECORD_PROPERTIES.  For an item to match an options object, the
  * properties of the item must match all the properties in the object.  For
  * example, an object { guid: "123" } matches any item whose GUID is 123.  An
  * object { guid: "123", title: "foo" } matches any item whose GUID is 123 *and*
  * whose title is foo.
  *
  * You can pass multiple options objects as separate arguments.  For an item to
  * match multiple objects, its properties must match all the properties in at
  * least one of the objects.  For example, a list of objects { guid: "123" } and
@@ -84,17 +86,17 @@ const ITEM_BASIC_PROPERTY_NAMES = `
  * foo.
  *
  * The properties in an options object can be arrays, not only scalars.  When a
  * property is an array, then for an item to match, its corresponding property
  * must have a value that matches any value in the array.  For example, an
  * options object { guid: ["123", "456"] } matches any item whose GUID is either
  * 123 *or* 456.
  *
- * In addition to properties with names from ITEM_BASIC_PROPERTY_NAMES, options
+ * In addition to properties with names from ITEM_RECORD_PROPERTIES, options
  * objects can also have the following special properties:
  *
  *   * sort: The name of a property to sort on.
  *   * descending: A boolean, true to sort descending, false to sort ascending.
  *     If `sort` is given but `descending` isn't, the sort is ascending (since
  *     `descending` is falsey).
  *   * limit: Limits the number of matching items to this number.
  *   * offset: Starts matching items at this index in the results.
@@ -104,24 +106,24 @@ const ITEM_BASIC_PROPERTY_NAMES = `
  * really make sense to do so.  The last property in the list is the one that's
  * used.
  *
  * @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._itemsByNormalizedURL = new Map();
   this._iterators = new Set();
   this._listeners = new Set();
 }
 
 ReadingListImpl.prototype = {
 
-  ItemBasicPropertyNames: ITEM_BASIC_PROPERTY_NAMES,
+  ItemRecordProperties: ITEM_RECORD_PROPERTIES,
 
   /**
    * Yields the number of items in the list.
    *
    * @param optsList A variable number of options objects that control the
    *        items that are matched.  See Options Objects.
    * @return Promise<number> The number of matching items in the list.  Rejected
    *         with an Error on error.
@@ -132,30 +134,30 @@ ReadingListImpl.prototype = {
 
   /**
    * Checks whether a given URL is in the ReadingList already.
    *
    * @param {String/nsIURI} url - URL to check.
    * @returns {Promise} Promise that is fulfilled with a boolean indicating
    *                    whether the URL is in the list or not.
    */
-  containsURL: Task.async(function* (url) {
+  hasItemForURL: Task.async(function* (url) {
     url = normalizeURI(url).spec;
 
     // This is used on every tab switch and page load of the current tab, so we
     // want it to be quick and avoid a DB query whenever possible.
 
     // First check if any cached items have a direct match.
-    if (this._itemsByURL.has(url)) {
+    if (this._itemsByNormalizedURL.has(url)) {
       return true;
     }
 
     // Then check if any cached items may have a different resolved URL
     // that matches.
-    for (let itemWeakRef of this._itemsByURL.values()) {
+    for (let itemWeakRef of this._itemsByNormalizedURL.values()) {
       let item = itemWeakRef.get();
       if (item && item.resolvedURL == url) {
         return true;
       }
     }
 
     // Finally, fall back to the DB.
     let count = yield this.count({url: url}, {resolvedURL: url});
@@ -172,20 +174,20 @@ ReadingListImpl.prototype = {
    * @param optsList A variable number of options objects that control the
    *        items that are matched.  See Options Objects.
    * @return Promise<null> Resolved when the enumeration completes *and* the
    *         last promise returned by the callback is resolved.  Rejected with
    *         an Error on error.
    */
   forEachItem: Task.async(function* (callback, ...optsList) {
     let promiseChain = Promise.resolve();
-    yield this._store.forEachItem(obj => {
+    yield this._store.forEachItem(record => {
       promiseChain = promiseChain.then(() => {
         return new Promise((resolve, reject) => {
-          let promise = callback(this._itemFromObject(obj));
+          let promise = callback(this._itemFromRecord(record));
           if (promise instanceof Promise) {
             return promise.then(resolve, reject);
           }
           resolve();
           return undefined;
         });
       });
     }, ...optsList);
@@ -205,38 +207,36 @@ ReadingListImpl.prototype = {
     this._iterators.add(Cu.getWeakReference(iter));
     return iter;
   },
 
   /**
    * Adds an item to the list that isn't already present.
    *
    * 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
+   * are those in ITEM_RECORD_PROPERTIES.  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 obj A simple object representing an item.
+   * @param record 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* (obj) {
-    obj = stripNonItemProperties(obj);
-    normalizeReadingListProperties(obj);
-
-    obj.addedOn = Date.now();
-    if (Services.prefs.prefHasUserValue("services.sync.client.name"))
-      obj.addedBy = Services.prefs.getCharPref("services.sync.client.name");
-
-    yield this._store.addItem(obj);
+  addItem: Task.async(function* (record) {
+    record = normalizeRecord(record);
+    record.addedOn = Date.now();
+    if (Services.prefs.prefHasUserValue("services.sync.client.name")) {
+      record.addedBy = Services.prefs.getCharPref("services.sync.client.name");
+    }
+    yield this._store.addItem(record);
     this._invalidateIterators();
-    let item = this._itemFromObject(obj);
+    let item = this._itemFromRecord(record);
     this._callListeners("onItemAdded", item);
     let mm = Cc["@mozilla.org/globalmessagemanager;1"].getService(Ci.nsIMessageListenerManager);
     mm.broadcastAsyncMessage("Reader:Added", item);
     return item;
   }),
 
   /**
    * Updates the properties of an item that belongs to the list.
@@ -249,17 +249,17 @@ ReadingListImpl.prototype = {
    * The returned promise is rejected in that case.
    *
    * @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);
+    yield this._store.updateItem(item._record);
     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.
@@ -268,58 +268,67 @@ ReadingListImpl.prototype = {
    * @param item The ReadingListItem to delete.
    * @return Promise<null> Resolved when the list is updated.  Rejected with an
    *         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._itemsByNormalizedURL.delete(item.url);
     this._invalidateIterators();
     let mm = Cc["@mozilla.org/globalmessagemanager;1"].getService(Ci.nsIMessageListenerManager);
     mm.broadcastAsyncMessage("Reader:Removed", item);
     this._callListeners("onItemDeleted", item);
   }),
 
   /**
+   * Finds the first item that matches the given options.
+   *
+   * @param optsList See Options Objects.
+   * @return The first matching item, or null if there are no matching items.
+   */
+  item: Task.async(function* (...optsList) {
+    return (yield this.iterator(...optsList).items(1))[0] || null;
+  }),
+
+  /**
    * Find any item that matches a given URL - either the item's URL, or its
    * resolved URL.
    *
    * @param {String/nsIURI} uri - URI to match against. This will be normalized.
+   * @return The first matching item, or null if there are no matching items.
    */
-  getItemForURL: Task.async(function* (uri) {
+  itemForURL: Task.async(function* (uri) {
     let url = normalizeURI(uri).spec;
-    let [item] = yield this.iterator({url: url}, {resolvedURL: url}).items(1);
-    return item;
+    return (yield this.item({ url: url }, { resolvedURL: url }));
   }),
 
-   /**
+  /**
    * Add to the ReadingList the page that is loaded in a given browser.
    *
    * @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, url) {
     let metadata = yield getMetadataFromBrowser(browser);
-    let itemData = {
+    let record = {
       url: url,
       title: metadata.title,
       resolvedURL: metadata.url,
       excerpt: metadata.description,
     };
 
     if (metadata.previews.length > 0) {
       itemData.preview = metadata.previews[0];
     }
 
-    let item = yield ReadingList.addItem(itemData);
-    return item;
+    return (yield this.addItem(record));
   }),
 
   /**
    * Adds a listener that will be notified when the list changes.  Listeners
    * are objects with the following optional methods:
    *
    *   onItemAdded(item)
    *   onItemUpdated(item)
@@ -340,56 +349,56 @@ ReadingListImpl.prototype = {
     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()) {
+    for (let itemWeakRef of this._itemsByNormalizedURL.values()) {
       let item = itemWeakRef.get();
       if (item) {
         item.list = null;
       }
     }
-    this._itemsByURL.clear();
+    this._itemsByNormalizedURL.clear();
   }),
 
   // The list's backing store.
   _store: null,
 
-  // A Map mapping URL strings to nsIWeakReferences that refer to
+  // A Map mapping *normalized* URL strings to nsIWeakReferences that refer to
   // ReadingListItems.
-  _itemsByURL: null,
+  _itemsByNormalizedURL: 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
+   * Returns the ReadingListItem represented by the given record object.  If
    * the item doesn't exist yet, it's created first.
    *
-   * @param obj A simple object with item properties.
+   * @param record A simple object with *normalized* item record properties.
    * @return The ReadingListItem.
    */
-  _itemFromObject(obj) {
-    let itemWeakRef = this._itemsByURL.get(obj.url);
+  _itemFromRecord(record) {
+    let itemWeakRef = this._itemsByNormalizedURL.get(record.url);
     let item = itemWeakRef ? itemWeakRef.get() : null;
     if (item) {
-      item.setProperties(obj, false);
+      item._record = record;
     }
     else {
-      item = new ReadingListItem(obj);
+      item = new ReadingListItem(record);
       item.list = this;
-      this._itemsByURL.set(obj.url, Cu.getWeakReference(item));
+      this._itemsByNormalizedURL.set(record.url, Cu.getWeakReference(item));
     }
     return item;
   },
 
   /**
    * Marks all the list's iterators as invalid, meaning it's not safe to use
    * them anymore.
    */
@@ -425,343 +434,316 @@ ReadingListImpl.prototype = {
   _ensureItemBelongsToList(item) {
     if (!item || !item._ensureBelongsToList) {
       throw new Error("The item is not a ReadingListItem");
     }
     item._ensureBelongsToList();
   },
 };
 
-/*
- * normalize the properties of a "regular" object that reflects a ReadingListItem
- */
-function normalizeReadingListProperties(obj) {
-  if (obj.url) {
-    obj.url = normalizeURI(obj.url).spec;
-  }
-  if (obj.resolvedURL) {
-    obj.resolvedURL = normalizeURI(obj.resolvedURL).spec;
-  }
-}
-
 
 let _unserializable = () => {}; // See comments in the ReadingListItem ctor.
 
 /**
  * An item in a reading list.
  *
  * Each item belongs to a list, and it's an error to use an item with a
  * ReadingList that the item doesn't belong to.
  *
- * @param props The properties of the item, as few or many as you want.
+ * @param record A simple object with the properties of the item, as few or many
+ *        as you want.  This will be normalized.
  */
-function ReadingListItem(props={}) {
-  this._properties = {};
+function ReadingListItem(record={}) {
+  this._record = record;
 
   // |this._unserializable| works around a problem when sending one of these
   // items via a message manager. If |this.list| is set, the item can't be
   // transferred directly, so .toJSON is implicitly called and the object
   // returned via that is sent. However, once the item is deleted and |this.list|
   // is null, the item *can* be directly serialized - so the message handler
-  // sees the "raw" object - ie, it sees "_properties" etc.
+  // sees the "raw" object - ie, it sees "_record" etc.
   // We work around this problem by *always* having an unserializable property
   // on the object - this way the implicit .toJSON call is always made, even
   // when |this.list| is null.
   this._unserializable = _unserializable;
-
-  this.setProperties(props, false);
 }
 
 ReadingListItem.prototype = {
 
+  // Be careful when caching properties.  If you cache a property that depends
+  // on a mutable _record property, then you need to recache your property after
+  // _record is set.
+
   /**
    * 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.
+   * guaranteed to be set for local items.
    * @type string
    */
   get guid() {
-    return this._properties.guid || undefined;
-  },
-  set guid(val) {
-    this._properties.guid = val;
-  },
-
-  /**
-   * The date the item was last modified.
-   * @type Date
-   */
-  get lastModified() {
-    return this._properties.lastModified ?
-           new Date(this._properties.lastModified) :
-           undefined;
-  },
-  set lastModified(val) {
-    this._properties.lastModified = val.valueOf();
+    return this._record.guid || undefined;
   },
 
   /**
    * The item's URL.
    * @type string
    */
   get url() {
-    return this._properties.url;
-  },
-  set url(val) {
-    this._properties.url = normalizeURI(val).spec;
+    return this._record.url;
   },
 
   /**
    * The item's URL as an nsIURI.
    * @type nsIURI
    */
   get uri() {
-    return this._properties.url ?
-           Services.io.newURI(this._properties.url, "", null) :
-           undefined;
-  },
-  set uri(val) {
-    this.url = normalizeURI(val).spec;
-  },
-
-  /**
-   * 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;
+    if (!this._uri) {
+      this._uri = this._record.url ?
+                  Services.io.newURI(this._record.url, "", null) :
+                  undefined;
     }
-    catch (err) {}
-    return this.url;
+    return this._uri;
   },
 
   /**
    * The item's resolved URL.
    * @type string
    */
   get resolvedURL() {
-    return this._properties.resolvedURL;
+    return this._record.resolvedURL;
   },
   set resolvedURL(val) {
-    this._properties.resolvedURL = normalizeURI(val).spec;
+    this._updateRecord({ resolvedURL: val });
   },
 
   /**
-   * The item's resolved URL as an nsIURI.
+   * The item's resolved URL as an nsIURI.  The setter takes an nsIURI or a
+   * string spec.
    * @type nsIURI
    */
   get resolvedURI() {
-    return this._properties.resolvedURL ?
-           Services.io.newURI(this._properties.resolvedURL, "", null) :
+    return this._record.resolvedURL ?
+           Services.io.newURI(this._record.resolvedURL, "", null) :
            undefined;
   },
   set resolvedURI(val) {
-    this.resolvedURL = val.spec;
+    this._updateRecord({ resolvedURL: val });
   },
 
   /**
    * The item's title.
    * @type string
    */
   get title() {
-    return this._properties.title;
+    return this._record.title;
   },
   set title(val) {
-    this._properties.title = val;
+    this._updateRecord({ title: val });
   },
 
   /**
    * The item's resolved title.
    * @type string
    */
   get resolvedTitle() {
-    return this._properties.resolvedTitle;
+    return this._record.resolvedTitle;
   },
   set resolvedTitle(val) {
-    this._properties.resolvedTitle = val;
+    this._updateRecord({ resolvedTitle: val });
   },
 
   /**
    * The item's excerpt.
    * @type string
    */
   get excerpt() {
-    return this._properties.excerpt;
+    return this._record.excerpt;
   },
   set excerpt(val) {
-    this._properties.excerpt = val;
+    this._updateRecord({ excerpt: val });
   },
 
   /**
    * The item's status.
    * @type integer
    */
   get status() {
-    return this._properties.status;
+    return this._record.status;
   },
   set status(val) {
-    this._properties.status = val;
+    this._updateRecord({ status: val });
   },
 
   /**
    * Whether the item is a favorite.
    * @type boolean
    */
   get favorite() {
-    return !!this._properties.favorite;
+    return !!this._record.favorite;
   },
   set favorite(val) {
-    this._properties.favorite = !!val;
+    this._updateRecord({ favorite: !!val });
   },
 
   /**
    * Whether the item is an article.
    * @type boolean
    */
   get isArticle() {
-    return !!this._properties.isArticle;
+    return !!this._record.isArticle;
   },
   set isArticle(val) {
-    this._properties.isArticle = !!val;
+    this._updateRecord({ isArticle: !!val });
   },
 
   /**
    * The item's word count.
    * @type integer
    */
   get wordCount() {
-    return this._properties.wordCount;
+    return this._record.wordCount;
   },
   set wordCount(val) {
-    this._properties.wordCount = val;
+    this._updateRecord({ wordCount: val });
   },
 
   /**
    * Whether the item is unread.
    * @type boolean
    */
   get unread() {
-    return !!this._properties.unread;
+    return !!this._record.unread;
   },
   set unread(val) {
-    this._properties.unread = !!val;
+    this._updateRecord({ unread: !!val });
   },
 
   /**
    * The date the item was added.
    * @type Date
    */
   get addedOn() {
-    return this._properties.addedOn ?
-           new Date(this._properties.addedOn) :
+    return this._record.addedOn ?
+           new Date(this._record.addedOn) :
            undefined;
   },
   set addedOn(val) {
-    this._properties.addedOn = val.valueOf();
+    this._updateRecord({ addedOn: val.valueOf() });
   },
 
   /**
    * The date the item was stored.
    * @type Date
    */
   get storedOn() {
-    return this._properties.storedOn ?
-           new Date(this._properties.storedOn) :
+    return this._record.storedOn ?
+           new Date(this._record.storedOn) :
            undefined;
   },
   set storedOn(val) {
-    this._properties.storedOn = val.valueOf();
+    this._updateRecord({ storedOn: val.valueOf() });
   },
 
   /**
    * The GUID of the device that marked the item read.
    * @type string
    */
   get markedReadBy() {
-    return this._properties.markedReadBy;
+    return this._record.markedReadBy;
   },
   set markedReadBy(val) {
-    this._properties.markedReadBy = val;
+    this._updateRecord({ markedReadBy: val });
   },
 
   /**
    * The date the item marked read.
    * @type Date
    */
   get markedReadOn() {
-    return this._properties.markedReadOn ?
-           new Date(this._properties.markedReadOn) :
+    return this._record.markedReadOn ?
+           new Date(this._record.markedReadOn) :
            undefined;
   },
   set markedReadOn(val) {
-    this._properties.markedReadOn = val.valueOf();
+    this._updateRecord({ markedReadOn: val.valueOf() });
   },
 
   /**
    * The item's read position.
    * @param integer
    */
   get readPosition() {
-    return this._properties.readPosition;
+    return this._record.readPosition;
   },
   set readPosition(val) {
-    this._properties.readPosition = val;
+    this._updateRecord({ readPosition: val });
   },
 
   /**
    * The URL to a preview image.
    * @type string
    */
    get preview() {
-     return this._properties.preview;
+     return this._record.preview;
    },
 
   /**
-   * Sets the given properties of the item, optionally calling list.updateItem().
-   *
-   * @param props A simple object containing the properties to set.
-   * @param update If true, updateItem() is called for this item.
-   * @return Promise<null> If update is true, resolved when the update
-   *         completes; otherwise resolved immediately.
-   */
-  setProperties: Task.async(function* (props, update=true) {
-    for (let name in props) {
-      this._properties[name] = props[name];
-    }
-    // make sure everything is normalized.
-    normalizeReadingListProperties(this._properties);
-    if (update) {
-      yield this.list.updateItem(this);
-    }
-  }),
-
-  /**
    * Deletes the item from its list.
    *
    * @return Promise<null> Resolved when the list has been updated.
    */
   delete: Task.async(function* () {
     this._ensureBelongsToList();
     yield this.list.deleteItem(this);
     this.delete = () => Promise.reject("The item has already been deleted");
   }),
 
   toJSON() {
-    return this._properties;
+    return this._record;
+  },
+
+  /**
+   * Do not use this at all unless you know what you're doing.  Use the public
+   * getters and setters, above, instead.
+   *
+   * A simple object that contains the item's normalized data in the same format
+   * that the local store and server use.  Records passed in by the consumer are
+   * not normalized, but everywhere else, records are always normalized unless
+   * otherwise stated.  The setter normalizes the passed-in value, so it will
+   * throw an error if the value is not a valid record.
+   */
+  get _record() {
+    return this.__record;
+  },
+  set _record(val) {
+    this.__record = normalizeRecord(val);
+  },
+
+  /**
+   * Updates the item's record.  This calls the _record setter, so it will throw
+   * an error if the partial record is not valid.
+   *
+   * @param partialRecord An object containing any of the record properties.
+   */
+  _updateRecord(partialRecord) {
+    let record = this._record;
+    for (let prop in partialRecord) {
+      record[prop] = partialRecord[prop];
+    }
+    this._record = record;
   },
 
   _ensureBelongsToList() {
     if (!this.list) {
       throw new Error("The item must belong to a reading list");
     }
   },
 };
@@ -854,16 +836,42 @@ ReadingListItemIterator.prototype = {
 
   _ensureValid() {
     if (this.invalid) {
       throw new Error("The iterator has been invalidated");
     }
   },
 };
 
+
+/**
+ * Normalizes the properties of a record object, which represents a
+ * ReadingListItem.  Throws an error if the record contains properties that
+ * aren't in ITEM_RECORD_PROPERTIES.
+ *
+ * @param record A non-normalized record object.
+ * @return The new normalized record.
+ */
+function normalizeRecord(nonNormalizedRecord) {
+  for (let prop in nonNormalizedRecord) {
+    if (!ITEM_RECORD_PROPERTIES.includes(prop)) {
+      throw new Error("Unrecognized item property: " + prop);
+    }
+  }
+
+  let record = clone(nonNormalizedRecord);
+  if (record.url) {
+    record.url = normalizeURI(record.url).spec;
+  }
+  if (record.resolvedURL) {
+    record.resolvedURL = normalizeURI(record.resolvedURL).spec;
+  }
+  return record;
+}
+
 /**
  * Normalize a URI, stripping away extraneous parts we don't want to store
  * or compare against.
  *
  * @param {nsIURI/String} uri - URI to normalize.
  * @returns {nsIURI} Cloned and normalized version of the input URI.
  */
 function normalizeURI(uri) {
@@ -872,26 +880,16 @@ function normalizeURI(uri) {
   }
   uri = uri.cloneIgnoringRef();
   try {
     uri.userPass = "";
   } catch (ex) {} // Not all nsURI impls (eg, nsSimpleURI) support .userPass
   return uri;
 };
 
-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);
--- a/browser/components/readinglist/SQLiteStore.jsm
+++ b/browser/components/readinglist/SQLiteStore.jsm
@@ -57,17 +57,17 @@ this.SQLiteStore.prototype = {
    *        single object, an item.
    * @param optsList A variable number of options objects that control the
    *        items that are matched.  See Options Objects in ReadingList.jsm.
    * @return Promise<null> Resolved when the enumeration completes.  Rejected
    *         with an Error on error.
    */
   forEachItem: Task.async(function* (callback, ...optsList) {
     let [sql, args] = sqlFromOptions(optsList);
-    let colNames = ReadingList.ItemBasicPropertyNames;
+    let colNames = ReadingList.ItemRecordProperties;
     let conn = yield this._connectionPromise;
     yield conn.executeCached(`
       SELECT ${colNames} FROM items ${sql};
     `, args, row => callback(itemFromRow(row)));
   }),
 
   /**
    * Adds an item to the store that isn't already present.  See
@@ -214,24 +214,24 @@ this.SQLiteStore.prototype = {
     yield conn.execute(`
       CREATE INDEX items_unread ON items (unread);
     `);
   }),
 };
 
 /**
  * Returns a simple object whose properties are the
- * ReadingList.ItemBasicPropertyNames properties lifted from the given row.
+ * ReadingList.ItemRecordProperties lifted from the given row.
  *
  * @param row A mozIStorageRow.
  * @return The item.
  */
 function itemFromRow(row) {
   let item = {};
-  for (let name of ReadingList.ItemBasicPropertyNames) {
+  for (let name of ReadingList.ItemRecordProperties) {
     item[name] = row.getResultByName(name);
   }
   return item;
 }
 
 /**
  * Returns the back part of a SELECT statement generated from the given list of
  * options.
--- a/browser/components/readinglist/sidebar.js
+++ b/browser/components/readinglist/sidebar.js
@@ -140,17 +140,24 @@ let RLSidebar = {
    * @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}`);
 
     itemNode.querySelector(".item-title").textContent = item.title;
-    itemNode.querySelector(".item-domain").textContent = item.domain;
+
+    let domain = item.uri.spec;
+    try {
+      domain = item.uri.host;
+    }
+    catch (err) {}
+    itemNode.querySelector(".item-domain").textContent = domain;
+
     let thumb = itemNode.querySelector(".item-thumb-container");
     if (item.preview) {
       thumb.style.backgroundImage = "url(" + item.preview + ")";
     } else {
       thumb.style.removeProperty("background-image");
     }
   },
 
--- a/browser/components/readinglist/test/ReadingListTestUtils.jsm
+++ b/browser/components/readinglist/test/ReadingListTestUtils.jsm
@@ -79,17 +79,23 @@ SidebarUtils.prototype = {
     this.Assert.ok(node.classList.contains("item"),
                    "Node should have .item class");
     this.Assert.equal(node.id, "item-" + item.id,
                       "Node should have correct ID");
     this.Assert.equal(node.getAttribute("title"), item.title + "\n" + item.url.spec,
                       "Node should have correct title attribute");
     this.Assert.equal(node.querySelector(".item-title").textContent, item.title,
                       "Node's title element's text should match item title");
-    this.Assert.equal(node.querySelector(".item-domain").textContent, item.domain,
+
+    let domain = item.uri.spec;
+    try {
+      domain = item.uri.host;
+    }
+    catch (err) {}
+    this.Assert.equal(node.querySelector(".item-domain").textContent, domain,
                       "Node's domain element's text should match item title");
   },
 
   expectSelectedId(itemId) {
     let selectedItem = this.RLSidebar.selectedItem;
     if (itemId == null) {
       this.Assert.equal(selectedItem, null, "Should have no selected item");
     } else {
--- a/browser/components/readinglist/test/xpcshell/test_ReadingList.js
+++ b/browser/components/readinglist/test/xpcshell/test_ReadingList.js
@@ -27,17 +27,16 @@ add_task(function* prepare() {
     if (gDBFile.exists()) {
       gDBFile.remove(true);
     }
   });
 
   gItems = [];
   for (let i = 0; i < 3; i++) {
     gItems.push({
-      list: gList,
       guid: `guid${i}`,
       url: `http://example.com/${i}`,
       resolvedURL: `http://example.com/resolved/${i}`,
       title: `title ${i}`,
       excerpt: `excerpt ${i}`,
       unread: 0,
       addedOn: Date.now(),
       lastModified: Date.now(),
@@ -58,37 +57,33 @@ add_task(function* item_properties() {
   let iter = gList.iterator({
     sort: "guid",
   });
   let item = (yield iter.items(1))[0];
   Assert.ok(item);
 
   Assert.ok(item.uri);
   Assert.ok(item.uri instanceof Ci.nsIURI);
-  Assert.equal(item.uri.spec, item.url);
+  Assert.equal(item.uri.spec, item._record.url);
 
   Assert.ok(item.resolvedURI);
   Assert.ok(item.resolvedURI instanceof Ci.nsIURI);
-  Assert.equal(item.resolvedURI.spec, item.resolvedURL);
-
-  Assert.ok(item.lastModified);
-  Assert.ok(item.lastModified instanceof Cu.getGlobalForObject(ReadingList).Date);
+  Assert.equal(item.resolvedURI.spec, item._record.resolvedURL);
 
   Assert.ok(item.addedOn);
   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));
+  Assert.equal(item.id, hash(item._record.url));
 });
 
 add_task(function* constraints() {
   // add an item again
   let err = null;
   try {
     yield gList.addItem(gItems[0]);
   }
@@ -116,52 +111,54 @@ add_task(function* constraints() {
   try {
     yield gList.addItem(item);
   }
   catch (e) {
     err = e;
   }
   checkError(err);
 
-  // update an item with an existing url
-  let rlitem = yield gList.getItemForURL(gItems[0].url);
-  rlitem.guid = gItems[1].guid;
-  err = null;
-  try {
-    yield gList.updateItem(rlitem);
-  }
-  catch (e) {
-    err = e;
-  }
-  checkError(err);
-
   // add a new item with an existing resolvedURL
   item = kindOfClone(gItems[0]);
   item.resolvedURL = gItems[0].resolvedURL;
   err = null;
   try {
     yield gList.addItem(item);
   }
   catch (e) {
     err = e;
   }
   checkError(err);
 
-  // update an item with an existing resolvedURL
-  rlitem = yield gList.getItemForURL(gItems[0].url);
-  rlitem.url = gItems[1].url;
+  // add a new item with no url
+  item = kindOfClone(gItems[0]);
+  delete item.url;
   err = null;
   try {
-    yield gList.updateItem(rlitem);
+    yield gList.addItem(item);
   }
   catch (e) {
     err = e;
   }
   checkError(err);
 
+  // add an item with a bogus property
+  item = kindOfClone(gItems[0]);
+  item.bogus = "gnarly";
+  err = null;
+  try {
+    yield gList.addItem(item);
+  }
+  catch (e) {
+    err = e;
+  }
+  Assert.ok(err);
+  Assert.ok(err.message);
+  Assert.ok(err.message.indexOf("Unrecognized item property:") >= 0);
+
   // add a new item with no guid, which is allowed
   item = kindOfClone(gItems[0]);
   delete item.guid;
   err = null;
   let rlitem1;
   try {
     rlitem1 = yield gList.addItem(item);
   }
@@ -178,34 +175,23 @@ add_task(function* constraints() {
   try {
     rlitem2 = yield gList.addItem(item);
   }
   catch (e) {
     err = e;
   }
   Assert.ok(!err, err ? err.message : undefined);
 
-  // Delete both items since other tests assume the store contains only gItems.
+  // Delete the two previous items since other tests assume the store contains
+  // only gItems.
   yield gList.deleteItem(rlitem1);
   yield gList.deleteItem(rlitem2);
   let items = [];
-  yield gList.forEachItem(i => items.push(i), { url: [rlitem1.url, rlitem2.url] });
+  yield gList.forEachItem(i => items.push(i), { url: [rlitem1.uri.spec, rlitem2.uri.spec] });
   Assert.equal(items.length, 0);
-
-  // add a new item with no url
-  item = kindOfClone(gItems[0]);
-  delete item.url;
-  err = null;
-  try {
-    yield gList.addItem(item);
-  }
-  catch (e) {
-    err = e;
-  }
-  checkError(err);
 });
 
 add_task(function* count() {
   let count = yield gList.count();
   Assert.equal(count, gItems.length);
 
   count = yield gList.count({
     guid: gItems[0].guid,
@@ -501,16 +487,32 @@ add_task(function* iterator_forEach_prom
       }, 0);
     });
     promises.push(promise);
     return promise;
   });
   checkItems(items, gItems);
 });
 
+add_task(function* item() {
+  let item = yield gList.item({ guid: gItems[0].guid });
+  checkItems([item], [gItems[0]]);
+
+  item = yield gList.item({ guid: gItems[1].guid });
+  checkItems([item], [gItems[1]]);
+});
+
+add_task(function* itemForURL() {
+  let item = yield gList.itemForURL(gItems[0].url);
+  checkItems([item], [gItems[0]]);
+
+  item = yield gList.itemForURL(gItems[1].url);
+  checkItems([item], [gItems[1]]);
+});
+
 add_task(function* updateItem() {
   // get an item
   let items = [];
   yield gList.forEachItem(i => items.push(i), {
     guid: gItems[0].guid,
   });
   Assert.equal(items.length, 1);
   let item = items[0];
@@ -526,62 +528,75 @@ add_task(function* updateItem() {
   yield gList.forEachItem(i => items.push(i), {
     guid: gItems[0].guid,
   });
   Assert.equal(items.length, 1);
   item = items[0];
   Assert.equal(item.title, newTitle);
 });
 
-add_task(function* item_setProperties() {
+add_task(function* item_setRecord() {
   // get an item
   let iter = gList.iterator({
     sort: "guid",
   });
   let item = (yield iter.items(1))[0];
   Assert.ok(item);
 
-  // item.setProperties(update=false).  After fetching the item again, its title
-  // should be the old title.
+  // Set item._record without an updateItem.  After fetching the item again, its
+  // title should be the old title.
   let oldTitle = item.title;
-  let newTitle = "item_setProperties title 1";
+  let newTitle = "item_setRecord title 1";
   Assert.notEqual(oldTitle, newTitle);
-  item.setProperties({ title: newTitle }, false);
+  item._record.title = newTitle;
   Assert.equal(item.title, newTitle);
   iter = gList.iterator({
     sort: "guid",
   });
   let sameItem = (yield iter.items(1))[0];
   Assert.ok(item === sameItem);
   Assert.equal(sameItem.title, oldTitle);
 
-  // item.setProperties(update=true).  After fetching the item again, its title
-  // should be the new title.
-  newTitle = "item_setProperties title 2";
-  item.setProperties({ title: newTitle }, true);
+  // Set item._record followed by an updateItem.  After fetching the item again,
+  // its title should be the new title.
+  newTitle = "item_setRecord title 2";
+  item._record.title = newTitle;
+  yield gList.updateItem(item);
   Assert.equal(item.title, newTitle);
   iter = gList.iterator({
     sort: "guid",
   });
   sameItem = (yield iter.items(1))[0];
   Assert.ok(item === sameItem);
   Assert.equal(sameItem.title, newTitle);
 
-  // Set item.title directly.  After fetching the item again, its title should
-  // be the new title.
-  newTitle = "item_setProperties title 3";
+  // Set item.title directly and call updateItem.  After fetching the item
+  // again, its title should be the new title.
+  newTitle = "item_setRecord title 3";
   item.title = newTitle;
-  gList.updateItem(item);
+  yield gList.updateItem(item);
   Assert.equal(item.title, newTitle);
   iter = gList.iterator({
     sort: "guid",
   });
   sameItem = (yield iter.items(1))[0];
   Assert.ok(item === sameItem);
   Assert.equal(sameItem.title, newTitle);
+
+  // Setting _record to an object with a bogus property should throw.
+  let err = null;
+  try {
+    item._record = { bogus: "gnarly" };
+  }
+  catch (e) {
+    err = e;
+  }
+  Assert.ok(err);
+  Assert.ok(err.message);
+  Assert.ok(err.message.indexOf("Unrecognized item property:") >= 0);
 });
 
 add_task(function* listeners() {
   Assert.equal((yield gList.count()), gItems.length);
   // add an item
   let resolve;
   let listenerPromise = new Promise(r => resolve = r);
   let listener = {
@@ -597,17 +612,17 @@ add_task(function* listeners() {
 
   // update an item
   listenerPromise = new Promise(r => resolve = r);
   listener = {
     onItemUpdated: resolve,
   };
   gList.addListener(listener);
   items[0].title = "listeners new title";
-  gList.updateItem(items[0]);
+  yield gList.updateItem(items[0]);
   let listenerItem = yield listenerPromise;
   Assert.ok(listenerItem);
   Assert.ok(listenerItem === items[0]);
   gList.removeListener(listener);
   Assert.equal((yield gList.count()), gItems.length + 1);
 
   // delete an item
   listenerPromise = new Promise(r => resolve = r);
@@ -661,21 +676,20 @@ add_task(function* deleteItem() {
   checkItems(items, gItems.slice(3));
 });
 
 function checkItems(actualItems, expectedItems) {
   Assert.equal(actualItems.length, expectedItems.length);
   for (let i = 0; i < expectedItems.length; i++) {
     for (let prop in expectedItems[i]) {
       if (prop != "list") {
-        Assert.ok(prop in actualItems[i]._properties, prop);
-        Assert.equal(actualItems[i]._properties[prop], expectedItems[i][prop]);
+        Assert.ok(prop in actualItems[i]._record, prop);
+        Assert.equal(actualItems[i]._record[prop], expectedItems[i][prop]);
       }
     }
-    Assert.equal(actualItems[i].list, expectedItems[i].list);
   }
 }
 
 function checkError(err) {
   Assert.ok(err);
   Assert.ok(err instanceof Cu.getGlobalForObject(Sqlite).Error, err);
 }
 
--- a/browser/modules/ReaderParent.jsm
+++ b/browser/modules/ReaderParent.jsm
@@ -55,29 +55,29 @@ let ReaderParent = {
         });
         break;
 
       case "Reader:FaviconRequest": {
         // XXX: To implement.
         break;
       }
       case "Reader:ListStatusRequest":
-        ReadingList.containsURL(message.data.url).then(inList => {
+        ReadingList.hasItemForURL(message.data.url).then(inList => {
           let mm = message.target.messageManager
           // Make sure the target browser is still alive before trying to send data back.
           if (mm) {
             mm.sendAsyncMessage("Reader:ListStatusData",
                                 { inReadingList: inList, url: message.data.url });
           }
         });
         break;
 
       case "Reader:RemoveFromList":
         // We need to get the "real" item to delete it.
-        ReadingList.getItemForURL(message.data.url).then(item => {
+        ReadingList.itemForURL(message.data.url).then(item => {
           ReadingList.deleteItem(item)
         });
         break;
 
       case "Reader:Share":
         // XXX: To implement.
         break;