From 8a1e12656a489974b24daadc06309e9cf1edcf2c Mon Sep 17 00:00:00 2001 From: Margaret Leibovic Date: Tue, 11 Nov 2014 17:11:30 -0800 Subject: [PATCH] Bug 1096612 - Properly handle exceptions thrown while parsing article. r=bnicholson --- mobile/android/chrome/content/Reader.js | 23 +++++----- mobile/android/chrome/content/aboutReader.js | 5 ++- mobile/android/chrome/content/browser.js | 46 ++++++++++++-------- 3 files changed, 43 insertions(+), 31 deletions(-) diff --git a/mobile/android/chrome/content/Reader.js b/mobile/android/chrome/content/Reader.js index a030b3a689dd..836001dfef3d 100644 --- a/mobile/android/chrome/content/Reader.js +++ b/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 { diff --git a/mobile/android/chrome/content/aboutReader.js b/mobile/android/chrome/content/aboutReader.js index 322cd9921b63..8ea53c0c014a 100644 --- a/mobile/android/chrome/content/aboutReader.js +++ b/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 { diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js index 9d119fe8ab9a..8892f30b598a 100644 --- a/mobile/android/chrome/content/browser.js +++ b/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); + }); } } },