Bug 1131416 - Desktop syncing module for reading list service (prepatory changes). r=markh a=readinglist
authorDrew Willcoxon <adw@mozilla.com>
Fri, 20 Mar 2015 12:12:10 -0700
changeset 259725 de26a5644014
parent 259724 2dbde197a04e
child 259726 599e67dd8b26
push id721
push userjlund@mozilla.com
push date2015-04-21 23:03 +0000
treeherdermozilla-release@d27c9211ebb3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh, readinglist
bugs1131416
milestone38.0a2
Bug 1131416 - Desktop syncing module for reading list service (prepatory changes). r=markh a=readinglist
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;