Bug 1085685 - Consolidate logic to add articles to reading list. r=rnewman

mobile/android/base/ReadingListHelper.java

@@ -27,13 +27,8 @@ import android.widget.Toast;
 public final class ReadingListHelper implements GeckoEventListener, NativeEventListener {
     private static final String LOGTAG = "ReadingListHelper";
 
-    private static final int READER_ADD_SUCCESS = 0;
-    private static final int READER_ADD_FAILED = 1;
-    private static final int READER_ADD_DUPLICATE = 2;
-
     protected final Context context;
 
-
     public ReadingListHelper(Context context) {
         this.context = context;
 
@@ -86,16 +81,6 @@ public final class ReadingListHelper implements GeckoEventListener, NativeEventL
      * icon, or by tapping the readinglist-add icon in the ReaderMode banner.
      */
     private void handleAddToList(JSONObject message) {
-        final int result = message.optInt("result", READER_ADD_FAILED);
-        if (result != READER_ADD_SUCCESS) {
-            if (result == READER_ADD_FAILED) {
-                showToast(R.string.reading_list_failed, Toast.LENGTH_SHORT);
-            } else if (result == READER_ADD_DUPLICATE) {
-                showToast(R.string.reading_list_duplicate, Toast.LENGTH_SHORT);
-            }
-            return;
-        }
-
         final ContentValues values = new ContentValues();
         final String url = message.optString("url");
 

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

@@ -359,8 +359,6 @@ size. -->
 <!ENTITY site_settings_no_settings  "There are no settings to clear.">
 
 <!ENTITY reading_list_added "Page added to your Reading List">
-<!ENTITY reading_list_failed "Failed to add page to your Reading List">
-<!ENTITY reading_list_duplicate "Page already in your Reading List">
 
 <!-- Localization note (reading_list_time_minutes) : This string is used in the "Reading List"
      panel on the home page to give the user an estimate of how many minutes it will take to

mobile/android/base/strings.xml.in

@@ -294,8 +294,6 @@
   <string name="site_settings_no_settings">&site_settings_no_settings;</string>
 
   <string name="reading_list_added">&reading_list_added;</string>
-  <string name="reading_list_failed">&reading_list_failed;</string>
-  <string name="reading_list_duplicate">&reading_list_duplicate;</string>
   <string name="reading_list_time_minutes">&reading_list_time_minutes;</string>
   <string name="reading_list_time_over_an_hour">&reading_list_time_over_an_hour;</string>
 

mobile/android/chrome/content/Reader.js

@@ -10,10 +10,6 @@ let Reader = {
 
   DEBUG: 0,
 
-  READER_ADD_SUCCESS: 0,
-  READER_ADD_FAILED: 1,
-  READER_ADD_DUPLICATE: 2,
-
   // Don't try to parse the page if it has too many elements (for memory and
   // performance reasons)
   MAX_ELEMS_TO_PARSE: 3000,
@@ -38,7 +34,7 @@ let Reader = {
     },
 
     readerModeActiveCallback: function(tabID) {
-      Reader.addTabToReadingList(tabID);
+      Reader._addTabToReadingList(tabID);
       UITelemetry.addEvent("save.1", "pageaction", null, "reader");
     },
   },
@@ -93,50 +89,49 @@ let Reader = {
     }
   },
 
-  addTabToReadingList: function(tabID) {
+  _addTabToReadingList: function(tabID) {
     let tab = BrowserApp.getTabForId(tabID);
     let currentURI = tab.browser.currentURI;
-    let url = currentURI.spec;
     let urlWithoutRef = currentURI.specIgnoringRef;
 
-    let sendResult = function(result, article) {
-      article = article || {};
-
-      Messaging.sendRequest({
-        type: "Reader:AddToList",
-        result: result,
-        title: truncate(article.title, MAX_TITLE_LENGTH),
-        url: truncate(url, MAX_URI_LENGTH),
-        length: article.length,
-        excerpt: article.excerpt
-      });
-    }.bind(this);
-
-    let handleArticle = function(article) {
-      if (!article) {
-        sendResult(this.READER_ADD_FAILED, null);
-        return;
-      }
-
-      this.storeArticleInCache(article, function(success) {
-        let result = (success ? this.READER_ADD_SUCCESS : this.READER_ADD_FAILED);
-        sendResult(result, article);
-      }.bind(this));
-    }.bind(this);
-
-    this.getArticleFromCache(urlWithoutRef, function (article) {
-      // If the article is already in reading list, bail
+    this.getArticleFromCache(urlWithoutRef, (article) => {
+      // If the article is already in the cache, just use that.
       if (article) {
-        sendResult(this.READER_ADD_DUPLICATE, null);
+        this.addArticleToReadingList(article);
         return;
       }
 
-      if (tabID != null) {
-        this.getArticleForTab(tabID, urlWithoutRef, handleArticle);
-      } else {
-        this.parseDocumentFromURL(urlWithoutRef, handleArticle);
-      }
-    }.bind(this));
+      // Otherwise, get the article data from the tab.
+      this.getArticleForTab(tabID, urlWithoutRef, (article) => {
+        if (article) {
+          this.addArticleToReadingList(article);
+        } else {
+          // If there was a problem getting the article, just store the
+          // URL and title from the tab.
+          this.addArticleToReadingList({
+            url: urlWithoutRef,
+            title: tab.browser.contentDocument.title,
+          });
+        }
+      });
+    });
+  },
+
+  addArticleToReadingList: function(article) {
+    if (!article || !article.url) {
+      Cu.reportError("addArticleToReadingList requires article with valid URL");
+      return;
+    }
+
+    Messaging.sendRequest({
+      type: "Reader:AddToList",
+      url: truncate(article.url, MAX_URI_LENGTH),
+      title: truncate(article.title || "", MAX_TITLE_LENGTH),
+      length: article.length || 0,
+      excerpt: article.excerpt || "",
+    });
+
+    this.storeArticleInCache(article);
   },
 
   getStateForParseOnLoad: function Reader_getStateForParseOnLoad() {
@@ -261,10 +256,9 @@ let Reader = {
     }.bind(this));
   },
 
-  storeArticleInCache: function Reader_storeArticleInCache(article, callback) {
+  storeArticleInCache: function Reader_storeArticleInCache(article) {
     this._getCacheDB(function(cacheDB) {
       if (!cacheDB) {
-        callback(false);
         return;
       }
 
@@ -275,12 +269,10 @@ let Reader = {
 
       request.onerror = function(event) {
         this.log("Error storing article in the cache DB: " + article.url);
-        callback(false);
       }.bind(this);
 
       request.onsuccess = function(event) {
         this.log("Stored article in the cache DB: " + article.url);
-        callback(true);
       }.bind(this);
     }.bind(this));
   },

mobile/android/chrome/content/aboutReader.js

@@ -320,25 +320,9 @@ AboutReader.prototype = {
       return;
 
     if (this._isReadingListItem == 0) {
-      let uptime = UITelemetry.uptimeMillis();
-      gChromeWin.Reader.storeArticleInCache(this._article, function(success) {
-        let result = gChromeWin.Reader.READER_ADD_FAILED;
-        if (success) {
-          result = gChromeWin.Reader.READER_ADD_SUCCESS;
-          UITelemetry.addEvent("save.1", "button", uptime, "reader");
-        }
+      gChromeWin.Reader.addArticleToReadingList(this._article);
 
-        let json = JSON.stringify({ fromAboutReader: true, url: this._article.url });
-
-        Messaging.sendRequest({
-          type: "Reader:AddToList",
-          result: result,
-          title: this._article.title,
-          url: this._article.url,
-          length: this._article.length,
-          excerpt: this._article.excerpt
-        });
-      }.bind(this));
+      UITelemetry.addEvent("save.1", "button", null, "reader");
     } else {
       Messaging.sendRequest({
         type: "Reader:RemoveFromList",