Bug 1265004 - Try to save in-memory reader view article instead of downloading r?margaret draft
authorAndrzej Hunt <ahunt@mozilla.com>
Mon, 02 May 2016 15:47:46 -0700
changeset 362674 af009b20384d9d55adbea25ebe429d0f6ea35efa
parent 362646 00ecd5d8bbe78c6f6969edaa54ab1ce0a722dbbc
child 519858 2a3bdc88c7a05acb5c02390f7f96f4c4e5cb9b6e
push id17011
push userahunt@mozilla.com
push dateMon, 02 May 2016 22:48:13 +0000
reviewersmargaret
bugs1265004
milestone49.0a1
Bug 1265004 - Try to save in-memory reader view article instead of downloading r?margaret This should guarantee that any reader view page will also be successfully saved offline. MozReview-Commit-ID: 4J8cY6hYeyU
mobile/android/base/java/org/mozilla/gecko/Tab.java
mobile/android/base/java/org/mozilla/gecko/reader/ReadingListHelper.java
mobile/android/chrome/content/Reader.js
toolkit/components/reader/AboutReader.jsm
--- a/mobile/android/base/java/org/mozilla/gecko/Tab.java
+++ b/mobile/android/base/java/org/mozilla/gecko/Tab.java
@@ -548,17 +548,17 @@ public class Tab {
             @Override
             public void run() {
                 mDB.addBookmark(getContentResolver(), mTitle, pageUrl);
                 Tabs.getInstance().notifyListeners(Tab.this, Tabs.TabEvents.BOOKMARK_ADDED);
             }
         });
 
         if (AboutPages.isAboutReader(url)) {
-            ReadingListHelper.cacheReaderItem(pageUrl, mAppContext);
+            ReadingListHelper.cacheReaderItem(pageUrl, mId, mAppContext);
         }
     }
 
     public void removeBookmark() {
         final String url = getURL();
         if (url == null) {
             return;
         }
--- a/mobile/android/base/java/org/mozilla/gecko/reader/ReadingListHelper.java
+++ b/mobile/android/base/java/org/mozilla/gecko/reader/ReadingListHelper.java
@@ -86,25 +86,34 @@ public final class ReadingListHelper imp
     }
 
     private void handleAddedToCache(final String url, final String path, final int size) {
         final SavedReaderViewHelper rch = SavedReaderViewHelper.getSavedReaderViewHelper(context);
 
         rch.put(url, path, size);
     }
 
-    public static void cacheReaderItem(final String url, Context context) {
+    public static void cacheReaderItem(final String url, final int tabID, Context context) {
         if (AboutPages.isAboutReader(url)) {
             throw new IllegalArgumentException("Page url must be original (not about:reader) url");
         }
 
         SavedReaderViewHelper rch = SavedReaderViewHelper.getSavedReaderViewHelper(context);
 
         if (!rch.isURLCached(url)) {
-            GeckoAppShell.notifyObservers("Reader:AddToCache", url);
+            JSONObject object = new JSONObject();
+            try {
+                object.put("url", url);
+                object.put("tabID", tabID);
+            } catch (JSONException e) {
+                // This should never happen
+                throw new IllegalStateException("Couldn't create JSON to store reader cache item", e);
+            }
+
+            GeckoAppShell.notifyObservers("Reader:AddToCache", object.toString());
         }
     }
 
     public static void removeCachedReaderItem(final String url, Context context) {
         if (AboutPages.isAboutReader(url)) {
             throw new IllegalArgumentException("Page url must be original (not about:reader) url");
         }
 
--- a/mobile/android/chrome/content/Reader.js
+++ b/mobile/android/chrome/content/Reader.js
@@ -77,18 +77,24 @@ var Reader = {
       }
 
       case "Reader:RemoveFromCache": {
         ReaderMode.removeArticleFromCache(aData).catch(e => Cu.reportError("Error removing article from cache: " + e));
         break;
       }
 
       case "Reader:AddToCache": {
+        let data = JSON.parse(aData);
+        let tab = BrowserApp.getTabForId(data.tabID);
+        if (!tab) {
+          throw new Error("No tab for tabID = " + tabID);
+        }
+
         // If the article is coming from reader mode, we must have fetched it already.
-        this._getArticle(aData).then((article) => {
+        this._getArticle(data.url, tab.browser).then((article) => {
           ReaderMode.storeArticleInCache(article);
         }).catch(e => Cu.reportError("Error storing article in cache: " + e));
         break;
       }
     }
   },
 
   receiveMessage: function(message) {
@@ -257,35 +263,56 @@ var Reader = {
   /**
    * Gets an article for a given URL. This method will download and parse a document
    * if it does not find the article in the cache.
    *
    * @param url The article URL.
    * @return {Promise}
    * @resolves JS object representing the article, or null if no article is found.
    */
-  _getArticle: Task.async(function* (url) {
+  _getArticle: Task.async(function* (url, browser) {
+    let article;
+    if (browser != null) {
+      article = yield this._getArticleData(browser);
+      if (article) {
+        return article;
+      }
+    }
+
     // First try to find a parsed article in the cache.
-    let article = yield ReaderMode.getArticleFromCache(url);
+    article = yield ReaderMode.getArticleFromCache(url);
     if (article) {
       return article;
     }
 
     // Article hasn't been found in the cache, we need to
     // download the page and parse the article out of it.
     return yield ReaderMode.downloadAndParseDocument(url).catch(e => {
       if (e && e.newURL) {
         // Pass up the error so we can navigate the browser in question to the new URL:
         throw e;
       }
       Cu.reportError("Error downloading and parsing document: " + e);
       return null;
     });
   }),
 
+  _getArticleData: function(browser) {
+    return new Promise((resolve, reject) => {
+      let mm = browser.messageManager;
+      let listener = (message) => {
+        mm.removeMessageListener("Reader:StoredArticleData", listener);
+        resolve(message.data.article);
+      };
+      mm.addMessageListener("Reader:StoredArticleData", listener);
+      mm.sendAsyncMessage("Reader:GetStoredArticleData");
+    });
+  },
+
+
   /**
    * Migrates old indexedDB reader mode cache to new JSON cache.
    */
   migrateCache: Task.async(function* () {
     let cacheDB = yield new Promise((resolve, reject) => {
       let request = window.indexedDB.open("about:reader", 1);
       request.onsuccess = event => resolve(event.target.result);
       request.onerror = event => reject(request.error);
--- a/toolkit/components/reader/AboutReader.jsm
+++ b/toolkit/components/reader/AboutReader.jsm
@@ -32,16 +32,17 @@ var AboutReader = function(mm, win, arti
   }
 
   let doc = win.document;
 
   this._mm = mm;
   this._mm.addMessageListener("Reader:CloseDropdown", this);
   this._mm.addMessageListener("Reader:AddButton", this);
   this._mm.addMessageListener("Reader:RemoveButton", this);
+  this._mm.addMessageListener("Reader:GetStoredArticleData", this);
 
   this._docRef = Cu.getWeakReference(doc);
   this._winRef = Cu.getWeakReference(win);
 
   this._article = null;
 
   if (articlePromise) {
     this._articlePromise = articlePromise;
@@ -200,16 +201,19 @@ AboutReader.prototype = {
       case "Reader:RemoveButton": {
         if (message.data.id) {
           let btn = this._doc.getElementById(message.data.id);
           if (btn)
             btn.remove();
         }
         break;
       }
+      case "Reader:GetStoredArticleData": {
+        this._mm.sendAsyncMessage("Reader:StoredArticleData", { article: this._article });
+      }
     }
   },
 
   handleEvent: function(aEvent) {
     if (!aEvent.isTrusted)
       return;
 
     switch (aEvent.type) {
@@ -248,16 +252,17 @@ AboutReader.prototype = {
 
       case "unload":
         // Close the Banners Font-dropdown, cleanup Android BackPressListener.
         this._closeDropdowns();
 
         this._mm.removeMessageListener("Reader:CloseDropdown", this);
         this._mm.removeMessageListener("Reader:AddButton", this);
         this._mm.removeMessageListener("Reader:RemoveButton", this);
+        this._mm.removeMessageListener("Reader:GetStoredArticleData", this);
         this._windowUnloaded = true;
         break;
     }
   },
 
   _onReaderClose: function() {
     ReaderMode.leaveReaderMode(this._mm.docShell, this._win);
   },