mirror of
https://github.com/mozilla/gecko-dev.git
synced 2024-10-14 22:05:44 +00:00
Bug 1087722 - Convert reader mode API to use promises. r=rnewman,bnicholson
This commit is contained in:
Margaret Leibovic
parent
6bb92c84ac
commit
e753d47841
@ -21,14 +21,14 @@ const ARTICLE = {
|
||||
|
||||
const ARTICLE_URI = Services.io.newURI(ARTICLE.url, null, null);
|
||||
|
||||
add_task(function test_article_not_found() {
|
||||
add_task(function* test_article_not_found() {
|
||||
let Reader = Services.wm.getMostRecentWindow("navigator:browser").Reader;
|
||||
|
||||
let article = yield Reader.getArticleFromCache(ARTICLE_URI);
|
||||
do_check_eq(article, null);
|
||||
});
|
||||
|
||||
add_task(function test_store_article() {
|
||||
add_task(function* test_store_article() {
|
||||
let Reader = Services.wm.getMostRecentWindow("navigator:browser").Reader;
|
||||
|
||||
yield Reader.storeArticleInCache(ARTICLE);
|
||||
@ -37,7 +37,7 @@ add_task(function test_store_article() {
|
||||
checkArticle(article);
|
||||
});
|
||||
|
||||
add_task(function test_remove_article() {
|
||||
add_task(function* test_remove_article() {
|
||||
let Reader = Services.wm.getMostRecentWindow("navigator:browser").Reader;
|
||||
|
||||
yield Reader.removeArticleFromCache(ARTICLE_URI);
|
||||
@ -46,13 +46,11 @@ add_task(function test_remove_article() {
|
||||
do_check_eq(article, null);
|
||||
});
|
||||
|
||||
add_test(function test_parse_article() {
|
||||
add_task(function* test_parse_article() {
|
||||
let Reader = Services.wm.getMostRecentWindow("navigator:browser").Reader;
|
||||
|
||||
Reader.parseDocumentFromURL(ARTICLE.url, function parseCallback(article) {
|
||||
checkArticle(article);
|
||||
run_next_test();
|
||||
});
|
||||
let article = yield Reader._downloadAndParseDocument(ARTICLE.url);
|
||||
checkArticle(article);
|
||||
});
|
||||
|
||||
function checkArticle(article) {
|
||||
|
||||
@ -25,7 +25,7 @@ let Reader = {
|
||||
// Listen for future pref changes.
|
||||
Services.prefs.addObserver("reader.parse-on-load.", this, false);
|
||||
|
||||
return this.isEnabledForParseOnLoad = this.getStateForParseOnLoad();
|
||||
return this.isEnabledForParseOnLoad = this._getStateForParseOnLoad();
|
||||
},
|
||||
|
||||
pageAction: {
|
||||
@ -37,7 +37,7 @@ let Reader = {
|
||||
},
|
||||
|
||||
readerModeActiveCallback: function(tabID) {
|
||||
Reader._addTabToReadingList(tabID);
|
||||
Reader._addTabToReadingList(tabID).catch(e => Cu.reportError("Error adding tab to reading list: " + e));
|
||||
UITelemetry.addEvent("save.1", "pageaction", null, "reader");
|
||||
},
|
||||
},
|
||||
@ -86,43 +86,31 @@ let Reader = {
|
||||
|
||||
case "nsPref:changed": {
|
||||
if (aData.startsWith("reader.parse-on-load.")) {
|
||||
this.isEnabledForParseOnLoad = this.getStateForParseOnLoad();
|
||||
this.isEnabledForParseOnLoad = this._getStateForParseOnLoad();
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
_addTabToReadingList: function(tabID) {
|
||||
_addTabToReadingList: Task.async(function* (tabID) {
|
||||
let tab = BrowserApp.getTabForId(tabID);
|
||||
if (!tab) {
|
||||
Cu.reportError("Can't add tab to reading list because no tab found for ID: " + tabID);
|
||||
return;
|
||||
throw new Error("Can't add tab to reading list because no tab found for ID: " + tabID);
|
||||
}
|
||||
|
||||
let uri = tab.browser.currentURI;
|
||||
let urlWithoutRef = uri.specIgnoringRef;
|
||||
|
||||
this.getArticleFromCache(uri).then(article => {
|
||||
// If the article is already in the cache, just use that.
|
||||
if (article) {
|
||||
this.addArticleToReadingList(article);
|
||||
return;
|
||||
}
|
||||
let article = yield this.getArticle(urlWithoutRef, tabID);
|
||||
if (!article) {
|
||||
// If there was a problem getting the article, just store the
|
||||
// URL and title from the tab.
|
||||
article = { url: urlWithoutRef, title: tab.browser.contentDocument.title };
|
||||
}
|
||||
|
||||
// Otherwise, get the article data from the tab.
|
||||
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));
|
||||
},
|
||||
this.addArticleToReadingList(article);
|
||||
}),
|
||||
|
||||
addArticleToReadingList: function(article) {
|
||||
if (!article || !article.url) {
|
||||
@ -141,7 +129,7 @@ let Reader = {
|
||||
this.storeArticleInCache(article).catch(e => Cu.reportError("Error storing article in cache: " + e));
|
||||
},
|
||||
|
||||
getStateForParseOnLoad: function Reader_getStateForParseOnLoad() {
|
||||
_getStateForParseOnLoad: function () {
|
||||
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.
|
||||
@ -149,92 +137,71 @@ let Reader = {
|
||||
return isForceEnabled || (isEnabled && !BrowserApp.isOnLowMemoryPlatform);
|
||||
},
|
||||
|
||||
parseDocumentFromURL: function Reader_parseDocumentFromURL(url, callback) {
|
||||
// If there's an on-going request for the same URL, simply append one
|
||||
// more callback to it to be called when the request is done.
|
||||
if (url in this._requests) {
|
||||
let request = this._requests[url];
|
||||
request.callbacks.push(callback);
|
||||
return;
|
||||
}
|
||||
|
||||
let request = { url: url, callbacks: [callback] };
|
||||
this._requests[url] = request;
|
||||
|
||||
let uri = Services.io.newURI(url, null, null);
|
||||
|
||||
// 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;
|
||||
}
|
||||
|
||||
// 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) {
|
||||
/**
|
||||
* Gets an article for a given URL. This method will download and parse a document
|
||||
* if it does not find the article in the tab data or the cache.
|
||||
*
|
||||
* @param url The article URL.
|
||||
* @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.
|
||||
*/
|
||||
getArticle: Task.async(function* (url, tabId) {
|
||||
// First, look for an article object stored on the tab.
|
||||
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;
|
||||
return article;
|
||||
}
|
||||
}
|
||||
|
||||
this.parseDocumentFromURL(url, callback);
|
||||
},
|
||||
// Next, try to find a parsed article in the cache.
|
||||
let uri = Services.io.newURI(url, null, null);
|
||||
let article = yield this.getArticleFromCache(uri);
|
||||
if (article) {
|
||||
this.log("Saved article found in cache");
|
||||
return article;
|
||||
}
|
||||
|
||||
parseDocumentFromTab: function (tab, callback) {
|
||||
// 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);
|
||||
}),
|
||||
|
||||
/**
|
||||
* Gets an article from a loaded tab's document. This method will parse the document
|
||||
* if it does not find the article in the tab data or the cache.
|
||||
*
|
||||
* @param tab The loaded tab.
|
||||
* @return {Promise}
|
||||
* @resolves JS object representing the article, or null if no article is found.
|
||||
*/
|
||||
parseDocumentFromTab: Task.async(function* (tab) {
|
||||
let uri = tab.browser.currentURI;
|
||||
if (!this._shouldCheckUri(uri)) {
|
||||
callback(null);
|
||||
return;
|
||||
this.log("Reader mode disabled for URI");
|
||||
return null;
|
||||
}
|
||||
|
||||
// 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 article = yield this.getArticleFromCache(uri);
|
||||
if (article) {
|
||||
this.log("Page found in cache, return article immediately");
|
||||
return article;
|
||||
}
|
||||
|
||||
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);
|
||||
});
|
||||
},
|
||||
let doc = tab.browser.contentWindow.document;
|
||||
return yield this._readerParse(uri, doc);
|
||||
}),
|
||||
|
||||
/**
|
||||
* 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.
|
||||
* @return {Promise}
|
||||
* @resolves JS object representing the article, or null if no article is found.
|
||||
* @rejects OS.File.Error
|
||||
*/
|
||||
getArticleFromCache: Task.async(function* (uri) {
|
||||
@ -251,8 +218,8 @@ let Reader = {
|
||||
* Stores an article in the cache.
|
||||
*
|
||||
* @param article JS object representing article.
|
||||
* @return Promise
|
||||
* @resolve When the article is stored.
|
||||
* @return {Promise}
|
||||
* @resolves When the article is stored.
|
||||
* @rejects OS.File.Error
|
||||
*/
|
||||
storeArticleInCache: Task.async(function* (article) {
|
||||
@ -266,8 +233,8 @@ let Reader = {
|
||||
* Removes an article from the cache given an article URI.
|
||||
*
|
||||
* @param uri The article URI.
|
||||
* @return Promise
|
||||
* @resolve When the article is removed.
|
||||
* @return {Promise}
|
||||
* @resolves When the article is removed.
|
||||
* @rejects OS.File.Error
|
||||
*/
|
||||
removeArticleFromCache: Task.async(function* (uri) {
|
||||
@ -280,7 +247,7 @@ let Reader = {
|
||||
dump("Reader: " + msg);
|
||||
},
|
||||
|
||||
_shouldCheckUri: function Reader_shouldCheckUri(uri) {
|
||||
_shouldCheckUri: function (uri) {
|
||||
if ((uri.prePath + "/") === uri.spec) {
|
||||
this.log("Not parsing home page: " + uri.spec);
|
||||
return false;
|
||||
@ -294,139 +261,109 @@ let Reader = {
|
||||
return true;
|
||||
},
|
||||
|
||||
_readerParse: function Reader_readerParse(uri, doc, callback) {
|
||||
let numTags = doc.getElementsByTagName("*").length;
|
||||
if (numTags > this.MAX_ELEMS_TO_PARSE) {
|
||||
this.log("Aborting parse for " + uri.spec + "; " + numTags + " elements found");
|
||||
callback(null);
|
||||
return;
|
||||
}
|
||||
_readerParse: function (uri, doc) {
|
||||
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");
|
||||
return;
|
||||
}
|
||||
|
||||
let worker = new ChromeWorker("readerWorker.js");
|
||||
worker.onmessage = function (evt) {
|
||||
let article = evt.data;
|
||||
let worker = new ChromeWorker("readerWorker.js");
|
||||
worker.onmessage = function (evt) {
|
||||
let article = evt.data;
|
||||
|
||||
// Append URL to the article data. specIgnoringRef will ignore any hash
|
||||
// in the URL.
|
||||
if (article) {
|
||||
if (!article) {
|
||||
reject("Worker did not return an article");
|
||||
return;
|
||||
}
|
||||
|
||||
// Append URL to the article data. specIgnoringRef will ignore any hash
|
||||
// in the URL.
|
||||
article.url = uri.specIgnoringRef;
|
||||
let flags = Ci.nsIDocumentEncoder.OutputSelectionOnly | Ci.nsIDocumentEncoder.OutputAbsoluteLinks;
|
||||
article.title = Cc["@mozilla.org/parserutils;1"].getService(Ci.nsIParserUtils)
|
||||
.convertToPlainText(article.title, flags, 0);
|
||||
resolve(article);
|
||||
};
|
||||
|
||||
try {
|
||||
worker.postMessage({
|
||||
uri: {
|
||||
spec: uri.spec,
|
||||
host: uri.host,
|
||||
prePath: uri.prePath,
|
||||
scheme: uri.scheme,
|
||||
pathBase: Services.io.newURI(".", null, uri).spec
|
||||
},
|
||||
doc: new XMLSerializer().serializeToString(doc)
|
||||
});
|
||||
} catch (e) {
|
||||
reject("Reader: could not build Readability arguments: " + e);
|
||||
}
|
||||
|
||||
callback(article);
|
||||
};
|
||||
|
||||
try {
|
||||
worker.postMessage({
|
||||
uri: {
|
||||
spec: uri.spec,
|
||||
host: uri.host,
|
||||
prePath: uri.prePath,
|
||||
scheme: uri.scheme,
|
||||
pathBase: Services.io.newURI(".", null, uri).spec
|
||||
},
|
||||
doc: new XMLSerializer().serializeToString(doc)
|
||||
});
|
||||
} catch (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) {
|
||||
callback(result);
|
||||
});
|
||||
},
|
||||
|
||||
_downloadDocument: function Reader_downloadDocument(url, callback) {
|
||||
// We want to parse those arbitrary pages safely, outside the privileged
|
||||
// context of chrome. We create a hidden browser element to fetch the
|
||||
// loaded page's document object then discard the browser element.
|
||||
_downloadDocument: function (url) {
|
||||
return new Promise((resolve, reject) => {
|
||||
// We want to parse those arbitrary pages safely, outside the privileged
|
||||
// context of chrome. We create a hidden browser element to fetch the
|
||||
// loaded page's document object then discard the browser element.
|
||||
let browser = document.createElement("browser");
|
||||
browser.setAttribute("type", "content");
|
||||
browser.setAttribute("collapsed", "true");
|
||||
browser.setAttribute("disablehistory", "true");
|
||||
|
||||
let browser = document.createElement("browser");
|
||||
browser.setAttribute("type", "content");
|
||||
browser.setAttribute("collapsed", "true");
|
||||
browser.setAttribute("disablehistory", "true");
|
||||
document.documentElement.appendChild(browser);
|
||||
browser.stop();
|
||||
|
||||
document.documentElement.appendChild(browser);
|
||||
browser.stop();
|
||||
browser.webNavigation.allowAuth = false;
|
||||
browser.webNavigation.allowImages = false;
|
||||
browser.webNavigation.allowJavascript = false;
|
||||
browser.webNavigation.allowMetaRedirects = true;
|
||||
browser.webNavigation.allowPlugins = false;
|
||||
|
||||
browser.webNavigation.allowAuth = false;
|
||||
browser.webNavigation.allowImages = false;
|
||||
browser.webNavigation.allowJavascript = false;
|
||||
browser.webNavigation.allowMetaRedirects = true;
|
||||
browser.webNavigation.allowPlugins = false;
|
||||
browser.addEventListener("DOMContentLoaded", event => {
|
||||
let doc = event.originalTarget;
|
||||
|
||||
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);
|
||||
});
|
||||
|
||||
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, doc => {
|
||||
this.log("Finished loading page: " + doc);
|
||||
|
||||
if (!doc) {
|
||||
this.log("Error loading page");
|
||||
this._runCallbacksAndFinish(request, null);
|
||||
// ignore on frames and other documents
|
||||
if (doc != browser.contentDocument) {
|
||||
return;
|
||||
}
|
||||
|
||||
this.log("Parsing response with Readability");
|
||||
this.log("Done loading: " + doc);
|
||||
if (doc.location.href == "about:blank") {
|
||||
reject("about:blank loaded; aborting");
|
||||
|
||||
let uri = Services.io.newURI(url, null, null);
|
||||
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;
|
||||
}
|
||||
// Request has finished with error, remove browser element
|
||||
browser.parentNode.removeChild(browser);
|
||||
return;
|
||||
}
|
||||
|
||||
if (!article) {
|
||||
this.log("Failed to parse page");
|
||||
this._runCallbacksAndFinish(request, null);
|
||||
return;
|
||||
}
|
||||
|
||||
this.log("Parsing has been successful");
|
||||
this._runCallbacksAndFinish(request, article);
|
||||
});
|
||||
resolve({ browser, doc });
|
||||
});
|
||||
} catch (e) {
|
||||
this.log("Error downloading and parsing document: " + e);
|
||||
this._runCallbacksAndFinish(request, null);
|
||||
}
|
||||
|
||||
browser.loadURIWithFlags(url, Ci.nsIWebNavigation.LOAD_FLAGS_NONE,
|
||||
null, null, null);
|
||||
});
|
||||
},
|
||||
|
||||
_downloadAndParseDocument: Task.async(function* (url) {
|
||||
this.log("Needs to fetch page, creating request: " + url);
|
||||
let { browser, doc } = yield this._downloadDocument(url);
|
||||
this.log("Finished loading page: " + doc);
|
||||
|
||||
try {
|
||||
this.log("Parsing response with Readability");
|
||||
let uri = Services.io.newURI(url, null, null);
|
||||
let article = yield this._readerParse(uri, doc);
|
||||
this.log("Document parsed successfully");
|
||||
return article;
|
||||
} finally {
|
||||
browser.parentNode.removeChild(browser);
|
||||
}
|
||||
}),
|
||||
|
||||
get _cryptoHash() {
|
||||
delete this._cryptoHash;
|
||||
return this._cryptoHash = Cc["@mozilla.org/security/hash;1"].createInstance(Ci.nsICryptoHash);
|
||||
|
||||
@ -130,13 +130,7 @@ let AboutReader = function(doc, win) {
|
||||
|
||||
let url = queryArgs.url;
|
||||
let tabId = queryArgs.tabId;
|
||||
if (tabId) {
|
||||
dump("Loading from tab with ID: " + tabId + ", URL: " + url);
|
||||
this._loadFromTab(tabId, url);
|
||||
} else {
|
||||
dump("Fetching page with URL: " + url);
|
||||
this._loadFromURL(url);
|
||||
}
|
||||
this._loadArticle(url, tabId);
|
||||
}
|
||||
|
||||
AboutReader.prototype = {
|
||||
@ -515,27 +509,16 @@ AboutReader.prototype = {
|
||||
});
|
||||
},
|
||||
|
||||
_loadFromURL: function Reader_loadFromURL(url) {
|
||||
_loadArticle: Task.async(function* (url, tabId) {
|
||||
this._showProgressDelayed();
|
||||
|
||||
gChromeWin.Reader.parseDocumentFromURL(url, function(article) {
|
||||
if (article)
|
||||
this._showContent(article);
|
||||
else
|
||||
this._win.location.href = url;
|
||||
}.bind(this));
|
||||
},
|
||||
|
||||
_loadFromTab: function Reader_loadFromTab(tabId, url) {
|
||||
this._showProgressDelayed();
|
||||
|
||||
gChromeWin.Reader.getArticleForTab(tabId, url, function(article) {
|
||||
if (article)
|
||||
this._showContent(article);
|
||||
else
|
||||
this._showError(gStrings.GetStringFromName("aboutReader.loadError"));
|
||||
}.bind(this));
|
||||
},
|
||||
try {
|
||||
let article = yield gChromeWin.Reader.getArticle(url, tabId);
|
||||
this._showContent(article);
|
||||
} catch (e) {
|
||||
this._win.location.href = url;
|
||||
}
|
||||
}),
|
||||
|
||||
_requestFavicon: function Reader_requestFavicon() {
|
||||
Messaging.sendRequest({
|
||||
|
||||
@ -4218,7 +4218,7 @@ Tab.prototype = {
|
||||
return;
|
||||
|
||||
// Once document is fully loaded, parse it
|
||||
Reader.parseDocumentFromTab(this, function (article) {
|
||||
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;
|
||||
@ -4250,7 +4250,7 @@ Tab.prototype = {
|
||||
|
||||
if(!this.readerEnabled)
|
||||
this.readerEnabled = true;
|
||||
}.bind(this));
|
||||
}, e => Cu.reportError(e));
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
Loading…
Reference in New Issue
Block a user