Bug 1142217 - Make the '+-' button in reader view work. r=markh a=readinglist
authorFlorian Quèze <florian@queze.net>
Fri, 13 Mar 2015 17:04:56 +1100
changeset 248424 c8f5fabac3693dca7d0f8bf5a4855cd333bf05bc
parent 248423 47992ba93d42af911f841011c7306c8763e8d9e0
child 248425 a7946db30b62636c1c893a22d6b9c831893199db
push id7837
push userjwein@mozilla.com
push dateFri, 27 Mar 2015 00:27:16 +0000
treeherdermozilla-aurora@cb0db44ce60e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh, readinglist
bugs1142217
milestone38.0a2
Bug 1142217 - Make the '+-' button in reader view work. r=markh a=readinglist
browser/components/readinglist/ReadingList.jsm
browser/components/readinglist/test/xpcshell/test_ReadingList.js
browser/modules/ReaderParent.jsm
--- a/browser/components/readinglist/ReadingList.jsm
+++ b/browser/components/readinglist/ReadingList.jsm
@@ -189,16 +189,18 @@ ReadingListImpl.prototype = {
    *         is updated.  Rejected with an Error on error.
    */
   addItem: Task.async(function* (obj) {
     obj = stripNonItemProperties(obj);
     yield this._store.addItem(obj);
     this._invalidateIterators();
     let item = this._itemFromObject(obj);
     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.
    *
    * 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
@@ -229,20 +231,34 @@ 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();
+    let mm = Cc["@mozilla.org/globalmessagemanager;1"].getService(Ci.nsIMessageListenerManager);
+    mm.broadcastAsyncMessage("Reader:Removed", item);
     this._callListeners("onItemDeleted", item);
   }),
 
   /**
+   * 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.
+   */
+  getItemForURL: Task.async(function* (uri) {
+    let url = this._normalizeURI(uri).spec;
+    let [item] = yield this.iterator({url: url}, {resolvedURL: url}).items(1);
+    return 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.
@@ -284,16 +300,32 @@ ReadingListImpl.prototype = {
   // A Set containing nsIWeakReferences that refer to valid iterators produced
   // by the list.
   _iterators: null,
 
   // A Set containing listener objects.
   _listeners: null,
 
   /**
+   * 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.
+   */
+  _normalizeURI(uri) {
+    if (typeof uri == "string") {
+      uri = Services.io.newURI(uri, "", null);
+    }
+    uri = uri.cloneIgnoringRef();
+    uri.userPass = "";
+    return uri;
+  },
+
+  /**
    * 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) {
     let itemWeakRef = this._itemsByURL.get(obj.url);
@@ -344,26 +376,40 @@ ReadingListImpl.prototype = {
 
   _ensureItemBelongsToList(item) {
     if (item.list != this) {
       throw new Error("The item does not belong to this list");
     }
   },
 };
 
+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.
  */
 function ReadingListItem(props={}) {
   this._properties = {};
+
+  // |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.
+  // 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 = {
 
   /**
    * Item's unique ID.
    * @type string
@@ -825,17 +871,17 @@ function hash(str) {
                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)].
+    [("0" + binaryStr.charCodeAt(i).toString(16)).slice(-2) for (i in binaryStr)].
     join("");
   return hexStr;
 }
 
 function clone(obj) {
   return Cu.cloneInto(obj, {}, { cloneFunctions: false });
 }
 
--- a/browser/components/readinglist/test/xpcshell/test_ReadingList.js
+++ b/browser/components/readinglist/test/xpcshell/test_ReadingList.js
@@ -692,12 +692,12 @@ function hash(str) {
                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)].
+    [("0" + binaryStr.charCodeAt(i).toString(16)).slice(-2) for (i in binaryStr)].
     join("");
   return hexStr;
 }
--- a/browser/modules/ReaderParent.jsm
+++ b/browser/modules/ReaderParent.jsm
@@ -9,16 +9,17 @@ const { classes: Cc, interfaces: Ci, uti
 
 this.EXPORTED_SYMBOLS = [ "ReaderParent" ];
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "ReaderMode", "resource://gre/modules/ReaderMode.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "ReadingList", "resource:///modules/readinglist/ReadingList.jsm");
 
 const gStringBundle = Services.strings.createBundle("chrome://global/locale/aboutReader.properties");
 
 let ReaderParent = {
 
   MESSAGES: [
     "Reader:AddToList",
     "Reader:ArticleGet",
@@ -37,38 +38,48 @@ let ReaderParent = {
     for (let msg of this.MESSAGES) {
       mm.addMessageListener(msg, this);
     }
   },
 
   receiveMessage: function(message) {
     switch (message.name) {
       case "Reader:AddToList":
-        // XXX: To implement.
+        ReadingList.addItem(message.data.article);
         break;
 
       case "Reader:ArticleGet":
         this._getArticle(message.data.url, message.target).then((article) => {
           // Make sure the target browser is still alive before trying to send data back.
           if (message.target.messageManager) {
             message.target.messageManager.sendAsyncMessage("Reader:ArticleData", { article: article });
           }
         });
         break;
 
       case "Reader:FaviconRequest": {
         // XXX: To implement.
         break;
       }
       case "Reader:ListStatusRequest":
-        // XXX: To implement.
+        ReadingList.count(message.data).then(count => {
+          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: !!count, url: message.data.url });
+          }
+        });
         break;
 
       case "Reader:RemoveFromList":
-        // XXX: To implement.
+        // We need to get the "real" item to delete it.
+        ReadingList.getItemForURL(message.data.url).then(item => {
+          ReadingList.deleteItem(item)
+        });
         break;
 
       case "Reader:Share":
         // XXX: To implement.
         break;
 
       case "Reader:SystemUIVisibility":
         // XXX: To implement.