Bug 1007409 - Cache reading list articles in files, not indexedDB. r=rnewman
authorMargaret Leibovic <margaret.leibovic@gmail.com>
Tue, 04 Nov 2014 13:34:45 -0800
changeset 213838 7075ce6196357d86e0a9e32a25d108212540864a
parent 213837 6ddcc5266799d444b64ff1256bc2209bb7f389f1
child 213839 3084582bbe17176a40008091191e7527d5f30e22
push id9759
push usermleibovic@mozilla.com
push dateTue, 04 Nov 2014 21:35:11 +0000
treeherderfx-team@3084582bbe17 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs1007409
milestone36.0a1
Bug 1007409 - Cache reading list articles in files, not indexedDB. r=rnewman
mobile/android/chrome/content/Reader.js
mobile/android/chrome/content/browser.js
--- a/mobile/android/chrome/content/Reader.js
+++ b/mobile/android/chrome/content/Reader.js
@@ -1,17 +1,20 @@
 // -*- indent-tabs-mode: nil; js-indent-level: 2 -*-
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
+XPCOMUtils.defineLazyModuleGetter(this, "CommonUtils",
+                                  "resource://services-common/utils.js");
+
 let Reader = {
-  // Version of the cache database schema
-  DB_VERSION: 1,
+  // Version of the cache schema.
+  CACHE_VERSION: 1,
 
   DEBUG: 0,
 
   // Don't try to parse the page if it has too many elements (for memory and
   // performance reasons)
   MAX_ELEMS_TO_PARSE: 3000,
 
   _requests: {},
@@ -71,72 +74,76 @@ let Reader = {
         important: true
       });
     }
   },
 
   observe: function(aMessage, aTopic, aData) {
     switch(aTopic) {
       case "Reader:Removed": {
-        this.removeArticleFromCache(aData);
+        let uri = Services.io.newURI(aData, null, null);
+        this.removeArticleFromCache(uri).catch(e => Cu.reportError("Error removing article from cache: " + e));
         break;
       }
 
       case "nsPref:changed": {
         if (aData.startsWith("reader.parse-on-load.")) {
           this.isEnabledForParseOnLoad = this.getStateForParseOnLoad();
         }
         break;
       }
     }
   },
 
   _addTabToReadingList: function(tabID) {
     let tab = BrowserApp.getTabForId(tabID);
-    let currentURI = tab.browser.currentURI;
-    let urlWithoutRef = currentURI.specIgnoringRef;
+    if (!tab) {
+      Cu.reportError("Can't add tab to reading list because no tab found for ID: " + tabID);
+      return;
+    }
+    let uri = tab.browser.currentURI;
 
-    this.getArticleFromCache(urlWithoutRef, (article) => {
+    this.getArticleFromCache(uri).then(article => {
       // If the article is already in the cache, just use that.
       if (article) {
         this.addArticleToReadingList(article);
         return;
       }
 
       // Otherwise, get the article data from the tab.
-      this.getArticleForTab(tabID, urlWithoutRef, (article) => {
+      this.getArticleForTab(tabID, uri.specIgnoringRef, 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,
           });
         }
       });
-    });
+    }, e => Cu.reportError("Error trying to get article from cache: " + e));
   },
 
   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);
+    this.storeArticleInCache(article).catch(e => Cu.reportError("Error storing article in cache: " + e));
   },
 
   getStateForParseOnLoad: function Reader_getStateForParseOnLoad() {
     let isEnabled = Services.prefs.getBoolPref("reader.parse-on-load.enabled");
     let isForceEnabled = Services.prefs.getBoolPref("reader.parse-on-load.force-enabled");
     // For low-memory devices, don't allow reader mode since it takes up a lot of memory.
     // See https://bugzilla.mozilla.org/show_bug.cgi?id=792603 for details.
     return isForceEnabled || (isEnabled && !BrowserApp.isOnLowMemoryPlatform);
@@ -149,179 +156,144 @@ let Reader = {
       let request = this._requests[url];
       request.callbacks.push(callback);
       return;
     }
 
     let request = { url: url, callbacks: [callback] };
     this._requests[url] = request;
 
-    try {
-      this.log("parseDocumentFromURL: " + url);
+    let uri = Services.io.newURI(url, null, null);
 
-      // First, try to find a cached parsed article in the DB
-      this.getArticleFromCache(url, function(article) {
-        if (article) {
-          this.log("Page found in cache, return article immediately");
-          this._runCallbacksAndFinish(request, article);
-          return;
-        }
+    // First, try to find a parsed article in the cache.
+    this.getArticleFromCache(uri).then(article => {
+      if (article) {
+        this.log("Page found in cache, return article immediately");
+        this._runCallbacksAndFinish(request, article);
+        return;
+      }
 
-        if (!this._requests) {
-          this.log("Reader has been destroyed, abort");
-          return;
-        }
+      if (!this._requests) {
+        this.log("Reader has been destroyed, abort");
+        return;
+      }
 
-        // Article hasn't been found in the cache DB, we need to
-        // download the page and parse the article out of it.
-        this._downloadAndParseDocument(url, request);
-      }.bind(this));
-    } catch (e) {
-      this.log("Error parsing document from URL: " + e);
+      // Article hasn't been found in the cache, we need to
+      // download the page and parse the article out of it.
+      this._downloadAndParseDocument(url, request);
+    }, e => {
+      Cu.reportError("Error trying to get article from cache: " + e);
       this._runCallbacksAndFinish(request, null);
-    }
+    });
   },
 
   getArticleForTab: function Reader_getArticleForTab(tabId, url, callback) {
     let tab = BrowserApp.getTabForId(tabId);
     if (tab) {
       let article = tab.savedArticle;
       if (article && article.url == url) {
         this.log("Saved article found in tab");
         callback(article);
         return;
       }
     }
 
     this.parseDocumentFromURL(url, callback);
   },
 
-  parseDocumentFromTab: function(tabId, callback) {
-    try {
-      this.log("parseDocumentFromTab: " + tabId);
-
-      let tab = BrowserApp.getTabForId(tabId);
-      let url = tab.browser.contentWindow.location.href;
-      let uri = Services.io.newURI(url, null, null);
-
-      if (!this._shouldCheckUri(uri)) {
-        callback(null);
-        return;
-      }
-
-      // First, try to find a cached parsed article in the DB
-      this.getArticleFromCache(url, function(article) {
-        if (article) {
-          this.log("Page found in cache, return article immediately");
-          callback(article);
-          return;
-        }
+  parseDocumentFromTab: function (tab, callback) {
+    let uri = tab.browser.currentURI;
+    if (!this._shouldCheckUri(uri)) {
+      callback(null);
+      return;
+    }
 
-        let doc = tab.browser.contentWindow.document;
-        this._readerParse(uri, doc, function (article) {
-          if (!article) {
-            this.log("Failed to parse page");
-            callback(null);
-            return;
-          }
-
-          callback(article);
-        }.bind(this));
-      }.bind(this));
-    } catch (e) {
-      this.log("Error parsing document from tab: " + e);
-      callback(null);
-    }
-  },
-
-  getArticleFromCache: function Reader_getArticleFromCache(url, callback) {
-    this._getCacheDB(function(cacheDB) {
-      if (!cacheDB) {
-        callback(false);
+    // First, try to find a parsed article in the cache.
+    this.getArticleFromCache(uri).then(article => {
+      if (article) {
+        this.log("Page found in cache, return article immediately");
+        callback(article);
         return;
       }
 
-      let transaction = cacheDB.transaction(cacheDB.objectStoreNames);
-      let articles = transaction.objectStore(cacheDB.objectStoreNames[0]);
-
-      let request = articles.get(url);
-
-      request.onerror = function(event) {
-        this.log("Error getting article from the cache DB: " + url);
-        callback(null);
-      }.bind(this);
-
-      request.onsuccess = function(event) {
-        this.log("Got article from the cache DB: " + event.target.result);
-        callback(event.target.result);
-      }.bind(this);
-    }.bind(this));
+      let doc = tab.browser.contentWindow.document;
+      this._readerParse(uri, doc, article => {
+        if (!article) {
+          this.log("Failed to parse page");
+          callback(null);
+          return;
+        }
+        callback(article);
+      });
+    }, e => {
+      Cu.reportError("Error trying to get article from cache: " + e);
+      callback(null);
+    });
   },
 
-  storeArticleInCache: function Reader_storeArticleInCache(article) {
-    this._getCacheDB(function(cacheDB) {
-      if (!cacheDB) {
-        return;
-      }
-
-      let transaction = cacheDB.transaction(cacheDB.objectStoreNames, "readwrite");
-      let articles = transaction.objectStore(cacheDB.objectStoreNames[0]);
-
-      let request = articles.add(article);
-
-      request.onerror = function(event) {
-        this.log("Error storing article in the cache DB: " + article.url);
-      }.bind(this);
-
-      request.onsuccess = function(event) {
-        this.log("Stored article in the cache DB: " + article.url);
-      }.bind(this);
-    }.bind(this));
-  },
+  /**
+   * Retrieves an article from the cache given an article URI.
+   *
+   * @param uri The article URI.
+   * @return Promise
+   * @resolve JS object representing the article, or null if no article is found.
+   * @rejects OS.File.Error
+   */
+  getArticleFromCache: Task.async(function* (uri) {
+    let path = this._toHashedPath(uri.specIgnoringRef);
+    try {
+      let array = yield OS.File.read(path);
+      return JSON.parse(new TextDecoder().decode(array));
+    } catch (e if e instanceof OS.File.Error && e.becauseNoSuchFile) {
+      return null;
+    }
+  }),
 
-  removeArticleFromCache: function Reader_removeArticleFromCache(url) {
-    this._getCacheDB(function(cacheDB) {
-      if (!cacheDB) {
-        return;
-      }
-
-      let transaction = cacheDB.transaction(cacheDB.objectStoreNames, "readwrite");
-      let articles = transaction.objectStore(cacheDB.objectStoreNames[0]);
+  /**
+   * Stores an article in the cache.
+   *
+   * @param article JS object representing article.
+   * @return Promise
+   * @resolve When the article is stored.
+   * @rejects OS.File.Error
+   */
+  storeArticleInCache: Task.async(function* (article) {
+    let array = new TextEncoder().encode(JSON.stringify(article));
+    let path = this._toHashedPath(article.url);
+    yield this._ensureCacheDir();
+    yield OS.File.writeAtomic(path, array, { tmpPath: path + ".tmp" });
+  }),
 
-      let request = articles.delete(url);
-
-      request.onerror = function(event) {
-        this.log("Error removing article from the cache DB: " + url);
-      }.bind(this);
-
-      request.onsuccess = function(event) {
-        this.log("Removed article from the cache DB: " + url);
-      }.bind(this);
-    }.bind(this));
-  },
+  /**
+   * Removes an article from the cache given an article URI.
+   *
+   * @param uri The article URI.
+   * @return Promise
+   * @resolve When the article is removed.
+   * @rejects OS.File.Error
+   */
+  removeArticleFromCache: Task.async(function* (uri) {
+    let path = this._toHashedPath(uri.specIgnoringRef);
+    yield OS.File.remove(path);
+  }),
 
   uninit: function Reader_uninit() {
     Services.prefs.removeObserver("reader.parse-on-load.", this);
 
     Services.obs.removeObserver(this, "Reader:Removed");
 
     let requests = this._requests;
     for (let url in requests) {
       let request = requests[url];
       if (request.browser) {
         let browser = request.browser;
         browser.parentNode.removeChild(browser);
       }
     }
     delete this._requests;
-
-    if (this._cacheDB) {
-      this._cacheDB.close();
-      delete this._cacheDB;
-    }
   },
 
   log: function(msg) {
     if (this.DEBUG)
       dump("Reader: " + msg);
   },
 
   _shouldCheckUri: function Reader_shouldCheckUri(uri) {
@@ -369,17 +341,17 @@ let Reader = {
           host: uri.host,
           prePath: uri.prePath,
           scheme: uri.scheme,
           pathBase: Services.io.newURI(".", null, uri).spec
         },
         doc: new XMLSerializer().serializeToString(doc)
       });
     } catch (e) {
-      dump("Reader: could not build Readability arguments: " + e);
+      Cu.reportError("Reader: could not build Readability arguments: " + e);
       callback(null);
     }
   },
 
   _runCallbacksAndFinish: function Reader_runCallbacksAndFinish(request, result) {
     delete this._requests[request.url];
 
     request.callbacks.forEach(function(callback) {
@@ -401,108 +373,118 @@ let Reader = {
     browser.stop();
 
     browser.webNavigation.allowAuth = false;
     browser.webNavigation.allowImages = false;
     browser.webNavigation.allowJavascript = false;
     browser.webNavigation.allowMetaRedirects = true;
     browser.webNavigation.allowPlugins = false;
 
-    browser.addEventListener("DOMContentLoaded", function (event) {
+    browser.addEventListener("DOMContentLoaded", event => {
       let doc = event.originalTarget;
 
       // ignore on frames and other documents
       if (doc != browser.contentDocument)
         return;
 
       this.log("Done loading: " + doc);
       if (doc.location.href == "about:blank") {
         callback(null);
 
         // Request has finished with error, remove browser element
         browser.parentNode.removeChild(browser);
         return;
       }
 
       callback(doc);
-    }.bind(this));
+    });
 
     browser.loadURIWithFlags(url, Ci.nsIWebNavigation.LOAD_FLAGS_NONE,
                              null, null, null);
 
     return browser;
   },
 
   _downloadAndParseDocument: function Reader_downloadAndParseDocument(url, request) {
     try {
       this.log("Needs to fetch page, creating request: " + url);
 
-      request.browser = this._downloadDocument(url, function(doc) {
+      request.browser = this._downloadDocument(url, doc => {
         this.log("Finished loading page: " + doc);
 
         if (!doc) {
           this.log("Error loading page");
           this._runCallbacksAndFinish(request, null);
           return;
         }
 
         this.log("Parsing response with Readability");
 
         let uri = Services.io.newURI(url, null, null);
-        this._readerParse(uri, doc, function (article) {
+        this._readerParse(uri, doc, article => {
           // Delete reference to the browser element as we've finished parsing.
           let browser = request.browser;
           if (browser) {
             browser.parentNode.removeChild(browser);
             delete request.browser;
           }
 
           if (!article) {
             this.log("Failed to parse page");
             this._runCallbacksAndFinish(request, null);
             return;
           }
 
           this.log("Parsing has been successful");
-
           this._runCallbacksAndFinish(request, article);
-        }.bind(this));
-      }.bind(this));
+        });
+      });
     } catch (e) {
       this.log("Error downloading and parsing document: " + e);
       this._runCallbacksAndFinish(request, null);
     }
   },
 
-  _getCacheDB: function Reader_getCacheDB(callback) {
-    if (this._cacheDB) {
-      callback(this._cacheDB);
-      return;
-    }
+  get _cryptoHash() {
+    delete this._cryptoHash;
+    return this._cryptoHash = Cc["@mozilla.org/security/hash;1"].createInstance(Ci.nsICryptoHash);
+  },
 
-    let request = window.indexedDB.open("about:reader", this.DB_VERSION);
-
-    request.onerror = function(event) {
-      this.log("Error connecting to the cache DB");
-      this._cacheDB = null;
-      callback(null);
-    }.bind(this);
+  get _unicodeConverter() {
+    delete this._unicodeConverter;
+    this._unicodeConverter = Cc["@mozilla.org/intl/scriptableunicodeconverter"]
+                              .createInstance(Ci.nsIScriptableUnicodeConverter);
+    this._unicodeConverter.charset = "utf8";
+    return this._unicodeConverter;
+  },
 
-    request.onsuccess = function(event) {
-      this.log("Successfully connected to the cache DB");
-      this._cacheDB = event.target.result;
-      callback(this._cacheDB);
-    }.bind(this);
+  /**
+   * Calculate the hashed path for a stripped article URL.
+   *
+   * @param url The article URL. This should have referrers removed.
+   * @return The file path to the cached article.
+   */
+  _toHashedPath: function (url) {
+    let value = this._unicodeConverter.convertToByteArray(url);
+    this._cryptoHash.init(this._cryptoHash.MD5);
+    this._cryptoHash.update(value, value.length);
 
-    request.onupgradeneeded = function(event) {
-      this.log("Database schema upgrade from " +
-           event.oldVersion + " to " + event.newVersion);
-
-      let cacheDB = event.target.result;
+    let hash = CommonUtils.encodeBase32(this._cryptoHash.finish(false));
+    let fileName = hash.substring(0, hash.indexOf("=")) + ".json";
+    return OS.Path.join(OS.Constants.Path.profileDir, "readercache", fileName);
+  },
 
-      // Create the articles object store
-      this.log("Creating articles object store");
-      cacheDB.createObjectStore("articles", { keyPath: "url" });
-
-      this.log("Database upgrade done: " + this.DB_VERSION);
-    }.bind(this);
+  /**
+   * Ensures the cache directory exists.
+   *
+   * @return Promise
+   * @resolves When the cache directory exists.
+   * @rejects OS.File.Error
+   */
+  _ensureCacheDir: function () {
+    let dir = OS.Path.join(OS.Constants.Path.profileDir, "readercache");
+    return OS.File.exists(dir).then(exists => {
+      if (!exists) {
+        return OS.File.makeDir(dir);
+      }
+    });
   }
 };
--- a/mobile/android/chrome/content/browser.js
+++ b/mobile/android/chrome/content/browser.js
@@ -4220,17 +4220,17 @@ Tab.prototype = {
             ExternalApps.clearPageAction();
           }
         }
 
         if (!Reader.isEnabledForParseOnLoad)
           return;
 
         // Once document is fully loaded, parse it
-        Reader.parseDocumentFromTab(this.id, function (article) {
+        Reader.parseDocumentFromTab(this, function (article) {
           // The loaded page may have changed while we were parsing the document. 
           // Make sure we've got the current one.
           let uri = this.browser.currentURI;
           let tabURL = uri.specIgnoringRef;
           // Do nothing if there's no article or the page in this tab has
           // changed
           if (article == null || (article.url != tabURL)) {
             // Don't clear the article for about:reader pages since we want to