Bug 1096612 - Properly handle exceptions thrown while parsing article. r=bnicholson

mobile/android/chrome/content/Reader.js

@@ -102,7 +102,10 @@ let Reader = {
     let uri = tab.browser.currentURI;
     let urlWithoutRef = uri.specIgnoringRef;
 
-    let article = yield this.getArticle(urlWithoutRef, tabID);
+    let article = yield this.getArticle(urlWithoutRef, tabID).catch(e => {
+      Cu.reportError("Error getting article for tab: " + e);
+      return null;
+    });
     if (!article) {
       // If there was a problem getting the article, just store the
       // URL and title from the tab.
@@ -145,7 +148,6 @@ let Reader = {
    * @param tabId (optional) The id of the tab where we can look for a saved article.
    * @return {Promise}
    * @resolves JS object representing the article, or null if no article is found.
-   * @rejects Never.
    */
   getArticle: Task.async(function* (url, tabId) {
     // First, look for an article object stored on the tab.
@@ -168,10 +170,7 @@ let Reader = {
 
     // Article hasn't been found in the cache, we need to
     // download the page and parse the article out of it.
-    return yield this._downloadAndParseDocument(url).catch(e => {
-      Cu.reportError("Error downloading and parsing article: " + e);
-      return null;
-    });
+    return yield this._downloadAndParseDocument(url);
   }),
 
   /**
@@ -269,16 +268,18 @@ let Reader = {
     return new Promise((resolve, reject) => {
       let numTags = doc.getElementsByTagName("*").length;
       if (numTags > this.MAX_ELEMS_TO_PARSE) {
-        reject("Aborting parse for " + uri.spec + "; " + numTags + " elements found");
+        this.log("Aborting parse for " + uri.spec + "; " + numTags + " elements found");
+        resolve(null);
         return;
       }
 
       let worker = new ChromeWorker("readerWorker.js");
-      worker.onmessage = function (evt) {
+      worker.onmessage = evt => {
         let article = evt.data;
 
         if (!article) {
-          reject("Worker did not return an article");
+          this.log("Worker did not return an article");
+          resolve(null);
           return;
         }
 
@@ -291,8 +292,8 @@ let Reader = {
         resolve(article);
       };
 
-      worker.onerror = function (evt) {
-        reject(evt.message);
+      worker.onerror = evt => {
+        reject("Error in worker: " + evt.message);
       };
 
       try {

mobile/android/chrome/content/aboutReader.js

@@ -507,7 +507,10 @@ AboutReader.prototype = {
   _loadArticle: Task.async(function* (url, tabId) {
     this._showProgressDelayed();
 
-    let article = yield gChromeWin.Reader.getArticle(url, tabId);
+    let article = yield gChromeWin.Reader.getArticle(url, tabId).catch(e => {
+      Cu.reportError("Error loading article: " + e);
+      return null;
+    });
     if (article) {
       this._showContent(article);
     } else {

mobile/android/chrome/content/browser.js

@@ -4215,27 +4215,31 @@ Tab.prototype = {
           this.tilesData = null;
         }
 
-        if (!Reader.isEnabledForParseOnLoad)
+        if (!Reader.isEnabledForParseOnLoad) {
           return;
+        }
+
+        let resetReaderFlags = currentURL => {
+          // Don't clear the article for about:reader pages since we want to
+          // use the article from the previous page.
+          if (!currentURL.startsWith("about:reader")) {
+            this.savedArticle = null;
+            this.readerEnabled = false;
+            this.readerActive = false;
+          } else {
+            this.readerActive = true;
+          }
+        };
 
         // Once document is fully loaded, parse it
         Reader.parseDocumentFromTab(this).then(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
-            // use the article from the previous page
-            if (!tabURL.startsWith("about:reader")) {
-              this.savedArticle = null;
-              this.readerEnabled = false;
-              this.readerActive = false;
-            } else {
-              this.readerActive = true;
-            }
+          let currentURL = this.browser.currentURI.specIgnoringRef;
+
+          // Do nothing if there's no article or the page in this tab has changed.
+          if (article == null || (article.url != currentURL)) {
+            resetReaderFlags(currentURL);
             return;
           }
 
@@ -4246,12 +4250,16 @@ Tab.prototype = {
             tabID: this.id
           });
 
-          if(this.readerActive)
+          if (this.readerActive) {
             this.readerActive = false;
-
-          if(!this.readerEnabled)
+          }
+          if (!this.readerEnabled) {
             this.readerEnabled = true;
-        }, e => Cu.reportError("Error parsing document from tab: " + e));
+          }
+        }).catch(e => {
+          Cu.reportError("Error parsing document from tab: " + e);
+          resetReaderFlags(this.browser.currentURI.specIgnoringRef);
+        });
       }
     }
   },