Skip to content

Commit

Permalink
Moves from XHR to DevTools Protocol for manifest retrieval (#600)
Browse files Browse the repository at this point in the history
* Moves from XHR to DevTools Protocol for manifest
  • Loading branch information
paullewis authored and brendankenny committed Aug 17, 2016
1 parent 3f3e5c2 commit 74690f1
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 57 deletions.
59 changes: 10 additions & 49 deletions lighthouse-core/gather/gatherers/manifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,40 +19,6 @@
const Gatherer = require('./gatherer');
const manifestParser = require('../../lib/manifest-parser');

/* global document, XMLHttpRequest, __returnResults */

/* istanbul ignore next */
function getManifestContent() {
function post(response) {
// __returnResults is magically inserted by driver.evaluateAsync
__returnResults(response);
}

const manifestNode = document.querySelector('link[rel=manifest]');
if (!manifestNode) {
return post({error: 'No <link rel="manifest"> found in DOM.'});
}

const manifestURL = manifestNode.href;
if (!manifestURL) {
return post({error: 'No href found on <link rel="manifest">.'});
}

const req = new XMLHttpRequest();
req.open('GET', manifestURL);
req.onload = function() {
if (req.status !== 200) {
return post({
error: `Unable to fetch manifest at \
${manifestURL}: ${req.status} - ${req.statusText}`
});
}

post({manifestContent: req.response});
};
req.send();
}

class Manifest extends Gatherer {

static _errorManifest(errorString) {
Expand All @@ -70,23 +36,18 @@ class Manifest extends Gatherer {
* potentially lead to a different asset. Using the original manifest
* resource is tracked in issue #83
*/
return driver.evaluateAsync(`(${getManifestContent.toString()}())`)

.then(returnedValue => {
if (!returnedValue) {
return driver.sendCommand('Page.getAppManifest')
.then(response => {
if (response.errors.length) {
this.artifact = Manifest._errorManifest(response.errors.join(', '));
return;
}

this.artifact = manifestParser(response.data);
}, _ => {
this.artifact = Manifest._errorManifest('Unable to retrieve manifest');
return;
}

if (returnedValue.error) {
this.artifact = Manifest._errorManifest(returnedValue.error);
} else {
this.artifact = manifestParser(returnedValue.manifestContent);
}
}, _ => {
this.artifact = Manifest._errorManifest('Unable to retrieve manifest');
return;
});
});
}
}

Expand Down
17 changes: 9 additions & 8 deletions lighthouse-core/test/gather/gatherers/manifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ describe('Manifest gatherer', () => {
it('returns an artifact', () => {
return manifestGather.afterPass({
driver: {
evaluateAsync() {
return Promise.resolve('');
sendCommand() {
return Promise.resolve({data: '', errors: [], url: 'https://example.com/manifest.json'});
}
}
}).then(_ => {
Expand All @@ -46,7 +46,7 @@ describe('Manifest gatherer', () => {
it('handles driver failure', () => {
return manifestGather.afterPass({
driver: {
evaluateAsync() {
sendCommand() {
return Promise.reject('such a fail');
}
}
Expand All @@ -61,9 +61,9 @@ describe('Manifest gatherer', () => {
const error = 'There was an error.';
return manifestGather.afterPass({
driver: {
evaluateAsync() {
sendCommand() {
return Promise.resolve({
error
errors: [error]
});
}
}
Expand All @@ -73,14 +73,15 @@ describe('Manifest gatherer', () => {
});

it('creates a manifest object for valid manifest content', () => {
const manifestContent = JSON.stringify({
const data = JSON.stringify({
name: 'App'
});
return manifestGather.afterPass({
driver: {
evaluateAsync() {
sendCommand() {
return Promise.resolve({
manifestContent
errors: [],
data
});
}
}
Expand Down

0 comments on commit 74690f1

Please sign in to comment.