Skip to content

Commit

Permalink
core(tsc): gather type-checking cleanup (GoogleChrome#5019)
Browse files Browse the repository at this point in the history
  • Loading branch information
brendankenny authored and kdzwinel committed Aug 16, 2018
1 parent d21fdc3 commit 7c96b7d
Show file tree
Hide file tree
Showing 7 changed files with 145 additions and 63 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,13 @@ const compressionTypes = ['gzip', 'br', 'deflate'];
const binaryMimeTypes = ['image', 'audio', 'video'];
const CHROME_EXTENSION_PROTOCOL = 'chrome-extension:';

/** @typedef {{requestId: string, url: string, mimeType: string, transferSize: number, resourceSize: number, gzipSize: number}} ResponseInfo */

class ResponseCompression extends Gatherer {
/**
* @param {Array<LH.WebInspector.NetworkRequest>} networkRecords
* @return {Array<ResponseInfo>}
* @return {LH.Artifacts['ResponseCompression']}
*/
static filterUnoptimizedResponses(networkRecords) {
/** @type {Array<ResponseInfo>} */
/** @type {LH.Artifacts['ResponseCompression']} */
const unoptimizedResponses = [];

networkRecords.forEach(record => {
Expand Down Expand Up @@ -66,6 +64,7 @@ class ResponseCompression extends Gatherer {
/**
* @param {LH.Gatherer.PassContext} passContext
* @param {LH.Gatherer.LoadData} loadData
* @return {Promise<LH.Artifacts['ResponseCompression']>}
*/
afterPass(passContext, loadData) {
const networkRecords = loadData.networkRecords;
Expand Down
99 changes: 60 additions & 39 deletions lighthouse-core/gather/gatherers/image-usage.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
*/
// @ts-nocheck
/**
* @fileoverview Gathers all images used on the page with their src, size,
* and attribute information. Executes script in the context of the page.
Expand All @@ -12,11 +11,16 @@

const Gatherer = require('./gatherer');
const DOMHelpers = require('../../lib/dom-helpers.js');
const Driver = require('../driver.js'); // eslint-disable-line no-unused-vars

/* global window, getElementsInDocument, Image */

/** @typedef {MakeOptional<LH.Artifacts.SingleImageUsage, 'networkRecord'>} ImageSizeInfo */

/** @return {Array<ImageSizeInfo>} */
/* istanbul ignore next */
function collectImageElementInfo() {
/** @param {Element} element */
function getClientRect(element) {
const clientRect = element.getBoundingClientRect();
return {
Expand All @@ -28,9 +32,14 @@ function collectImageElementInfo() {
};
}

/** @type {Array<Element>} */
// @ts-ignore - added by getElementsInDocumentFnString
const allElements = getElementsInDocument();
const allImageElements = allElements.filter(element => element.localName === 'img');
const allImageElements = /** @type {Array<HTMLImageElement>} */ (allElements.filter(element => {
return element.localName === 'img';
}));

/** @type {Array<ImageSizeInfo>} */
const htmlImages = allImageElements.map(element => {
const computedStyle = window.getComputedStyle(element);
return {
Expand Down Expand Up @@ -59,12 +68,13 @@ function collectImageElementInfo() {

const cssImages = allElements.reduce((images, element) => {
const style = window.getComputedStyle(element);
if (!CSS_URL_REGEX.test(style.backgroundImage) ||
!CSS_SIZE_REGEX.test(style.backgroundSize)) {
if (!style.backgroundImage || !CSS_URL_REGEX.test(style.backgroundImage) ||
!style.backgroundSize || !CSS_SIZE_REGEX.test(style.backgroundSize)) {
return images;
}

const imageMatch = style.backgroundImage.match(CSS_URL_REGEX);
// @ts-ignore test() above ensures that there is a match.
const url = imageMatch[1];

// Heuristic to filter out sprite sheets
Expand All @@ -87,11 +97,15 @@ function collectImageElementInfo() {
});

return images;
}, []);
}, /** @type {Array<ImageSizeInfo>} */ ([]));

return htmlImages.concat(cssImages);
}

/**
* @param {string} url
* @return {Promise<{naturalWidth: number, naturalHeight: number}>}
*/
/* istanbul ignore next */
function determineNaturalSize(url) {
return new Promise((resolve, reject) => {
Expand All @@ -110,63 +124,70 @@ function determineNaturalSize(url) {

class ImageUsage extends Gatherer {
/**
* @param {{src: string}} element
* @return {!Promise<!Object>}
* @param {Driver} driver
* @param {ImageSizeInfo} element
* @return {Promise<ImageSizeInfo>}
*/
fetchElementWithSizeInformation(element) {
async fetchElementWithSizeInformation(driver, element) {
const url = JSON.stringify(element.src);
return this.driver.evaluateAsync(`(${determineNaturalSize.toString()})(${url})`)
.then(size => {
return Object.assign(element, size);
}).catch(_ => {
// determineNaturalSize fails on invalid images, which we treat as non-visible
return Object.assign(element, {naturalWidth: 0, naturalHeight: 0});
});
try {
/** @type {{naturalWidth: number, naturalHeight: number}} */
const size = await driver.evaluateAsync(`(${determineNaturalSize.toString()})(${url})`);
return Object.assign(element, size);
} catch (_) {
// determineNaturalSize fails on invalid images, which we treat as non-visible
return Object.assign(element, {naturalWidth: 0, naturalHeight: 0});
}
}

afterPass(options, traceData) {
const driver = this.driver = options.driver;
const indexedNetworkRecords = traceData.networkRecords.reduce((map, record) => {
/**
* @param {LH.Gatherer.PassContext} passContext
* @param {LH.Gatherer.LoadData} loadData
* @return {Promise<LH.Artifacts['ImageUsage']>}
*/
afterPass(passContext, loadData) {
const driver = passContext.driver;
const indexedNetworkRecords = loadData.networkRecords.reduce((map, record) => {
if (/^image/.test(record._mimeType) && record.finished) {
map[record._url] = {
url: record.url,
resourceSize: Math.min(record.resourceSize, record.transferSize),
resourceSize: Math.min(record._resourceSize || 0, record.transferSize),
startTime: record.startTime,
endTime: record.endTime,
responseReceivedTime: record.responseReceivedTime,
responseReceivedTime: record._responseReceivedTime,
mimeType: record._mimeType,
};
}

return map;
}, {});
}, /** @type {Object<string, LH.Artifacts.SingleImageUsage['networkRecord']>} */ ({}));

const expression = `(function() {
${DOMHelpers.getElementsInDocumentFnString}; // define function on page
return (${collectImageElementInfo.toString()})();
})()`;

return driver.evaluateAsync(expression)
.then(elements => {
return elements.reduce((promise, element) => {
return promise.then(collector => {
/** @type {Promise<Array<ImageSizeInfo>>} */
const evaluatePromise = driver.evaluateAsync(expression);
return evaluatePromise.then(elements => {
return elements.reduce((promise, element) => {
return promise.then(collector => {
// Images within `picture` behave strangely and natural size information isn't accurate,
// CSS images have no natural size information at all.
// Try to get the actual size if we can.
const elementPromise = (element.isPicture || element.isCss) && element.networkRecord ?
this.fetchElementWithSizeInformation(driver, element) :
Promise.resolve(element);

return elementPromise.then(element => {
// link up the image with its network record
element.networkRecord = indexedNetworkRecords[element.src];

// Images within `picture` behave strangely and natural size information isn't accurate,
// CSS images have no natural size information at all.
// Try to get the actual size if we can.
const elementPromise = (element.isPicture || element.isCss) && element.networkRecord ?
this.fetchElementWithSizeInformation(element) :
Promise.resolve(element);

return elementPromise.then(element => {
collector.push(element);
return collector;
});
collector.push(/** @type {LH.Artifacts.SingleImageUsage} */ (element));
return collector;
});
}, Promise.resolve([]));
});
});
}, Promise.resolve(/** @type {LH.Artifacts['ImageUsage']} */ ([])));
});
}
}

Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/gather/gatherers/js-usage.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class JsUsage extends Gatherer {

/**
* @param {LH.Gatherer.PassContext} passContext
* @return {Promise<LH.Artifacts['JsUsageArtifact']>}
* @return {Promise<LH.Artifacts['JsUsage']>}
*/
async afterPass(passContext) {
const driver = passContext.driver;
Expand Down
37 changes: 20 additions & 17 deletions lighthouse-core/gather/gatherers/start-url.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,39 +3,39 @@
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
*/
// @ts-nocheck
'use strict';

const Gatherer = require('./gatherer');
const manifestParser = require('../../lib/manifest-parser');
const Driver = require('../driver.js'); // eslint-disable-line no-unused-vars

class StartUrl extends Gatherer {
/**
* Grab the manifest, extract it's start_url, attempt to `fetch()` it while offline
* @param {*} options
* @return {{statusCode: number, debugString?: string}}
* @param {LH.Gatherer.PassContext} passContext
* @return {Promise<LH.Artifacts['StartUrl']>}
*/
afterPass(options) {
const driver = options.driver;
return driver.goOnline(options)
afterPass(passContext) {
const driver = passContext.driver;
return driver.goOnline(passContext)
.then(() => driver.getAppManifest())
.then(response => driver.goOffline(options).then(() => response))
.then(response => response && manifestParser(response.data, response.url, options.url))
.then(response => driver.goOffline().then(() => response))
.then(response => response && manifestParser(response.data, response.url, passContext.url))
.then(manifest => {
const {isReadFailure, reason, startUrl} = this._readManifestStartUrl(manifest);
if (isReadFailure) {
return {statusCode: -1, debugString: reason};
const startUrlInfo = this._readManifestStartUrl(manifest);
if (startUrlInfo.isReadFailure) {
return {statusCode: -1, debugString: startUrlInfo.reason};
}

return this._attemptManifestFetch(options.driver, startUrl);
return this._attemptManifestFetch(passContext.driver, startUrlInfo.startUrl);
}).catch(() => {
return {statusCode: -1, debugString: 'Unable to fetch start URL via service worker'};
});
}

/**
* Read the parsed manifest and return failure reasons or the startUrl
* @param {Manifest} manifest
* @param {?{value?: {start_url: {value?: string, debugString?: string}}, debugString?: string}} manifest
* @return {{isReadFailure: true, reason: string}|{isReadFailure: false, startUrl: string}}
*/
_readManifestStartUrl(manifest) {
Expand All @@ -55,15 +55,16 @@ class StartUrl extends Gatherer {
return {isReadFailure: true, reason: manifest.value.start_url.debugString};
}

// @ts-ignore - TODO(bckenny): should actually be testing value above, not debugString
return {isReadFailure: false, startUrl: manifest.value.start_url.value};
}

/**
* Try to `fetch(start_url)`, return true if fetched by SW
* Resolves when we have a matched network request
* @param {!Driver} driver
* @param {!string} startUrl
* @return {Promise<{statusCode: ?number, debugString: ?string}>}
* @param {Driver} driver
* @param {string} startUrl
* @return {Promise<{statusCode: number, debugString: string}>}
*/
_attemptManifestFetch(driver, startUrl) {
// Wait up to 3s to get a matched network request from the fetch() to work
Expand All @@ -77,7 +78,9 @@ class StartUrl extends Gatherer {
const fetchPromise = new Promise(resolve => {
driver.on('Network.responseReceived', onResponseReceived);

function onResponseReceived({response}) {
/** @param {LH.Crdp.Network.ResponseReceivedEvent} responseEvent */
function onResponseReceived(responseEvent) {
const {response} = responseEvent;
// ignore mismatched URLs
if (response.url !== startUrl) return;
driver.off('Network.responseReceived', onResponseReceived);
Expand Down
Loading

0 comments on commit 7c96b7d

Please sign in to comment.