Skip to content

Commit

Permalink
Improve error handling when injecting the sidebar
Browse files Browse the repository at this point in the history
If the mini-script injected into the page to determine
the content type failed to execute, the returned
'frameResults' value will be null. Unfortunately
Chrome does not expose the actual exception details.

Additionally, several Chrome async APIs were used without
checking chrome.extension.lastError in the callback.

 - Handle the case where frameResults is null or not
   an array.
 - Use utils.promisify() to wrap several Chrome async APIs.
   This checks chrome.extension.lastError and rejects the
   promise if it fails.
  • Loading branch information
robertknight committed Feb 10, 2016
1 parent 590b6a8 commit 2073cc1
Showing 1 changed file with 31 additions and 26 deletions.
57 changes: 31 additions & 26 deletions h/browser/chrome/lib/sidebar-injector.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,14 @@ function SidebarInjector(chromeTabs, dependencies) {
return canInject;
}

function guessContentTypeFromURL(url) {
if (url.indexOf('.pdf') !== -1) {
return CONTENT_TYPE_PDF;
} else {
return CONTENT_TYPE_HTML;
}
}

function detectTabContentType(tab) {
if (isPDFViewerURL(tab.url)) {
return Promise.resolve(CONTENT_TYPE_PDF);
Expand All @@ -106,16 +114,21 @@ function SidebarInjector(chromeTabs, dependencies) {
return executeScriptFn(tab.id, {
code: toIIFEString(detectContentType)
}).then(function (frameResults) {
return frameResults[0].type;
if (Array.isArray(frameResults)) {
return frameResults[0].type;
} else {
// if the content script threw an exception,
// frameResults may be null or undefined.
//
// In that case, fall back to guessing based on the
// tab URL
return guessContentTypeFromURL(tab.url);
}
});
} else {
// we cannot inject a content script in order to determine the
// file type, so fall back to a URL-based mechanism
if (tab.url.indexOf('.pdf') !== -1) {
return Promise.resolve(CONTENT_TYPE_PDF);
} else {
return Promise.resolve(CONTENT_TYPE_HTML);
}
return Promise.resolve(guessContentTypeFromURL(tab.url));
}
});
}
Expand Down Expand Up @@ -163,15 +176,11 @@ function SidebarInjector(chromeTabs, dependencies) {
}

function injectIntoPDF(tab) {
return new Promise(function (resolve, reject) {
if (!isPDFViewerURL(tab.url)) {
chromeTabs.update(tab.id, {url: getPDFViewerURL(tab.url)}, function () {
resolve();
});
} else {
resolve();
}
});
if (isPDFViewerURL(tab.url)) {
return Promise.resolve();
}
var updateFn = util.promisify(chromeTabs.update);
return updateFn(tab.id, {url: getPDFViewerURL(tab.url)});
}

function injectIntoLocalPDF(tab) {
Expand Down Expand Up @@ -252,17 +261,13 @@ function SidebarInjector(chromeTabs, dependencies) {
* injected script via the 'window.HYPOTHESIS_ENV' object.
*/
function injectScript(tabId, path, env) {
return new Promise(function (resolve) {
var src = extensionURL(path);

var code = generateMetaTagCode(env) +
'var script = document.createElement("script");' +
'script.src = "{}";' +
'document.body.appendChild(script);';
var code = code.replace('{}', src);

chromeTabs.executeScript(tabId, {code: code}, resolve);
});
var src = extensionURL(path);
var code = generateMetaTagCode(env) +
'var script = document.createElement("script");' +
'script.src = "{}";' +
'document.body.appendChild(script);';
var code = code.replace('{}', src);
return executeScriptFn(tabId, {code: code});
}
}

Expand Down

0 comments on commit 2073cc1

Please sign in to comment.