Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: remove WebInspector.resourceTypes references #5556

Merged
merged 6 commits into from
Jun 27, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class UnusedBytes extends Audit {
*
* @param {LH.WebInspector.NetworkRequest=} networkRecord
* @param {number} totalBytes Uncompressed size of the resource
* @param {string=} resourceType
* @param {LH.Crdp.Page.ResourceType=} resourceType
* @param {number=} compressionRatio
* @return {number}
*/
Expand All @@ -79,7 +79,7 @@ class UnusedBytes extends Audit {
// roughly the size of the content gzipped.
// See https://discuss.httparchive.org/t/file-size-and-compression-savings/145 for multipliers
return Math.round(totalBytes * compressionRatio);
} else if (networkRecord._resourceType && networkRecord._resourceType._name === resourceType) {
} else if (networkRecord._resourceType === resourceType) {
// This was a regular standalone asset, just use the transfer size.
return networkRecord.transferSize || 0;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
*/
'use strict';

const WebInspector = require('../../lib/web-inspector');
const NetworkRequest = require('../../lib/network-request');
const ByteEfficiencyAudit = require('./byte-efficiency-audit');

// If GIFs are above this size, we'll flag them
Expand Down Expand Up @@ -49,7 +49,7 @@ class EfficientAnimatedContent extends ByteEfficiencyAudit {
static audit_(artifacts, networkRecords) {
const unoptimizedContent = networkRecords.filter(
record => record._mimeType === 'image/gif' &&
record._resourceType === WebInspector.resourceTypes.Image &&
record._resourceType === NetworkRequest.TYPES.Image &&
(record._resourceSize || 0) > GIF_BYTE_THRESHOLD
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const Audit = require('../audit');
const BaseNode = require('../../lib/dependency-graph/base-node');
const ByteEfficiencyAudit = require('./byte-efficiency-audit');
const UnusedCSS = require('./unused-css-rules');
const WebInspector = require('../../lib/web-inspector');
const NetworkRequest = require('../../lib/network-request');

/** @typedef {import('../../lib/dependency-graph/simulator/simulator')} Simulator */
/** @typedef {import('../../lib/dependency-graph/base-node.js').Node} Node */
Expand Down Expand Up @@ -152,7 +152,7 @@ class RenderBlockingResources extends Audit {
if (node.type !== BaseNode.TYPES.NETWORK) return !canDeferRequest;

const isStylesheet =
node.record._resourceType === WebInspector.resourceTypes.Stylesheet;
node.record._resourceType === NetworkRequest.TYPES.Stylesheet;
if (canDeferRequest && isStylesheet) {
// We'll inline the used bytes of the stylesheet and assume the rest can be deferred
const wastedBytes = wastedCssBytesByUrl.get(node.record.url) || 0;
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/audits/byte-efficiency/unminified-css.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class UnminifiedCSS extends ByteEfficiencyAudit {
}

const totalBytes = ByteEfficiencyAudit.estimateTransferSize(networkRecord, content.length,
'stylesheet');
'Stylesheet');
const wastedRatio = 1 - totalTokenLength / content.length;
const wastedBytes = Math.round(totalBytes * wastedRatio);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class UnminifiedJavaScript extends ByteEfficiencyAudit {
}

const totalBytes = ByteEfficiencyAudit.estimateTransferSize(networkRecord, contentLength,
'script');
'Script');
const wastedRatio = 1 - totalTokenLength / contentLength;
const wastedBytes = Math.round(totalBytes * wastedRatio);

Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/audits/byte-efficiency/unused-css-rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class UnusedCSSRules extends ByteEfficiencyAudit {
}

const totalTransferredBytes = ByteEfficiencyAudit.estimateTransferSize(
stylesheetInfo.networkRecord, totalUncompressedBytes, 'stylesheet');
stylesheetInfo.networkRecord, totalUncompressedBytes, 'Stylesheet');
const percentUnused = (totalUncompressedBytes - usedUncompressedBytes) / totalUncompressedBytes;
const wastedBytes = Math.round(percentUnused * totalTransferredBytes);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class UnusedJavaScript extends ByteEfficiencyAudit {
}

const totalBytes = ByteEfficiencyAudit.estimateTransferSize(networkRecord, contentLength,
'script');
'Script');
const wastedRatio = (unusedLength / contentLength) || 0;
const wastedBytes = Math.round(totalBytes * wastedRatio);

Expand Down
14 changes: 7 additions & 7 deletions lighthouse-core/audits/byte-efficiency/uses-long-cache-ttl.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const assert = require('assert');
// @ts-ignore - typed where used.
const parseCacheControl = require('parse-cache-control');
const Audit = require('../audit');
const WebInspector = require('../../lib/web-inspector');
const NetworkRequest = require('../../lib/network-request');
const URL = require('../../lib/url-shim');
const linearInterpolation = require('../../lib/statistics').linearInterpolation;

Expand Down Expand Up @@ -135,17 +135,17 @@ class CacheHeaders extends Audit {
const CACHEABLE_STATUS_CODES = new Set([200, 203, 206]);

const STATIC_RESOURCE_TYPES = new Set([
WebInspector.resourceTypes.Font,
WebInspector.resourceTypes.Image,
WebInspector.resourceTypes.Media,
WebInspector.resourceTypes.Script,
WebInspector.resourceTypes.Stylesheet,
NetworkRequest.TYPES.Font,
NetworkRequest.TYPES.Image,
NetworkRequest.TYPES.Media,
NetworkRequest.TYPES.Script,
NetworkRequest.TYPES.Stylesheet,
]);

const resourceUrl = record._url;
return (
CACHEABLE_STATUS_CODES.has(record.statusCode) &&
STATIC_RESOURCE_TYPES.has(record._resourceType) &&
STATIC_RESOURCE_TYPES.has(record._resourceType || 'Other') &&
!resourceUrl.includes('data:')
);
}
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/audits/font-display.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
'use strict';

const Audit = require('./audit');
const WebInspector = require('../lib/web-inspector');
const NetworkRequest = require('../lib/network-request');
const allowedFontFaceDisplays = ['block', 'fallback', 'optional', 'swap'];

class FontDisplay extends Audit {
Expand Down Expand Up @@ -40,7 +40,7 @@ class FontDisplay extends Audit {

return artifacts.requestNetworkRecords(devtoolsLogs).then((networkRecords) => {
const results = networkRecords.filter(record => {
const isFont = record._resourceType === WebInspector.resourceTypes.Font;
const isFont = record._resourceType === NetworkRequest.TYPES.Font;

return isFont;
})
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/audits/network-requests.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class NetworkRequests extends Audit {
transferSize: record.transferSize,
statusCode: record.statusCode,
mimeType: record._mimeType,
resourceType: record._resourceType && record._resourceType._name,
resourceType: (record._resourceType || 'other').toLowerCase(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lowercase is confusing here, but I'm assuming this is to keep compatibility with old ResourceType.name...is it worth keeping it that way or should we just switch to the same capitalization as Page.ResourceType?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah this was for compat, but we're pre 3.0 official so I'm game to remove if others think it makes sense :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have strong opinions, so consistency makes sense to me, but I haven't used this for anything. I guess the use cases are for

  1. programmatic consumption (maybe makes sense to match Page.ResourceType? But if I were creating the values for the first time myself I would never have capitalized the strings :)
  2. table display in the report. Which looks/reads better?

I don't really care :) @paulirish?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 doesn't really apply since network-requests audit is never displayed, just for programmatic consumption.

that leaves 1. I agree it makes sense to match Page.ResourceType and that if I were creating them myself I wouldn't have capitalized them, so... yeah @paulirish :D

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eh. let's nuke the toLowerCase. 🔥

};
});

Expand Down
16 changes: 8 additions & 8 deletions lighthouse-core/gather/computed/critical-request-chains.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
'use strict';

const ComputedArtifact = require('./computed-artifact');
const WebInspector = require('../../lib/web-inspector');
const NetworkRequest = require('../../lib/network-request');
const assert = require('assert');

class CriticalRequestChains extends ComputedArtifact {
Expand All @@ -30,19 +30,19 @@ class CriticalRequestChains extends ComputedArtifact {
return false;
}

const resourceTypeCategory = request._resourceType && request._resourceType._category;

// Iframes are considered High Priority but they are not render blocking
const isIframe = request._resourceType === WebInspector.resourceTypes.Document
const isIframe = request._resourceType === NetworkRequest.TYPES.Document
&& request._frameId !== mainResource._frameId;
// XHRs are fetched at High priority, but we exclude them, as they are unlikely to be critical
// Images are also non-critical.
// Treat any images missed by category, primarily favicons, as non-critical resources
// Treat any missed images, primarily favicons, as non-critical resources
const nonCriticalResourceTypes = [
WebInspector.resourceTypes.Image._category,
WebInspector.resourceTypes.XHR._category,
NetworkRequest.TYPES.Image,
NetworkRequest.TYPES.XHR,
NetworkRequest.TYPES.Fetch,
NetworkRequest.TYPES.EventSource,
];
if (nonCriticalResourceTypes.includes(resourceTypeCategory) ||
if (nonCriticalResourceTypes.includes(request._resourceType || 'Other') ||
isIframe ||
request._mimeType && request._mimeType.startsWith('image/')) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

const MetricArtifact = require('./lantern-metric');
const BaseNode = require('../../../lib/dependency-graph/base-node');
const WebInspector = require('../../../lib/web-inspector');
const NetworkRequest = require('../../../lib/network-request');

/** @typedef {BaseNode.Node} Node */

Expand Down Expand Up @@ -45,8 +45,8 @@ class Interactive extends MetricArtifact {
}

// Include all scripts and high priority requests, exclude all images
const isImage = node.record._resourceType === WebInspector.resourceTypes.Image;
const isScript = node.record._resourceType === WebInspector.resourceTypes.Script;
const isImage = node.record._resourceType === NetworkRequest.TYPES.Image;
const isScript = node.record._resourceType === NetworkRequest.TYPES.Script;
return (
!isImage &&
(isScript ||
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/gather/computed/metrics/lantern-metric.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

const ComputedArtifact = require('../computed-artifact');
const BaseNode = require('../../../lib/dependency-graph/base-node');
const ResourceType = require('../../../../third-party/devtools/ResourceType');
const NetworkRequest = require('../../../lib/network-request');

/** @typedef {BaseNode.Node} Node */
/** @typedef {import('../../../lib/dependency-graph/network-node')} NetworkNode */
Expand All @@ -25,7 +25,7 @@ class LanternMetricArtifact extends ComputedArtifact {

dependencyGraph.traverse(node => {
if (node.type === BaseNode.TYPES.CPU) return;
if (node.record._resourceType !== ResourceType.TYPES.Script) return;
if (node.record._resourceType !== NetworkRequest.TYPES.Script) return;
if (condition && !condition(node)) return;
scriptUrls.add(node.record.url);
});
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/gather/computed/page-dependency-graph.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const NetworkNode = require('../../lib/dependency-graph/network-node');
const CPUNode = require('../../lib/dependency-graph/cpu-node');
const NetworkAnalyzer = require('../../lib/dependency-graph/simulator/network-analyzer');
const TracingProcessor = require('../../lib/traces/tracing-processor');
const WebInspector = require('../../lib/web-inspector');
const NetworkRequest = require('../../lib/network-request');

/** @typedef {import('../../lib/dependency-graph/base-node.js').Node} Node */

Expand Down Expand Up @@ -155,7 +155,7 @@ class PageDependencyGraphArtifact extends ComputedArtifact {
const networkNode = networkNodeOutput.idToNodeMap.get(reqId);
if (!networkNode ||
// Ignore all non-XHRs
networkNode.record._resourceType !== WebInspector.resourceTypes.XHR ||
networkNode.record._resourceType !== NetworkRequest.TYPES.XHR ||
// Ignore all network nodes that started before this CPU task started
// A network request that started earlier could not possibly have been started by this task
networkNode.startTime <= cpuNode.startTime) return;
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/gather/computed/trace-of-tab.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const Sentry = require('../../lib/sentry');
// Bring in web-inspector for side effect of adding [].stableSort
// See https://github.com/GoogleChrome/lighthouse/pull/2415
// eslint-disable-next-line no-unused-vars
const WebInspector = require('../../lib/web-inspector');
const NetworkRequest = require('../../lib/network-request');

class TraceOfTab extends ComputedArtifact {
get name() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class OptimizedImages extends Gatherer {

seenUrls.add(record._url);
const isOptimizableImage = record._resourceType &&
record._resourceType._name === 'image' &&
record._resourceType === 'Image' &&
/image\/(png|bmp|jpeg)/.test(record._mimeType);
const isSameOrigin = URL.originsMatch(pageUrl, record._url);
const isBase64DataUri = /^data:.{2,40}base64\s*,/.test(record._url);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,21 @@
'use strict';

const Gatherer = require('../gatherer');
const NetworkRequest = require('../../../lib/network-request');
const gzip = require('zlib').gzip;

const CHROME_EXTENSION_PROTOCOL = 'chrome-extension:';
const compressionHeaders = ['content-encoding', 'x-original-content-encoding'];
const compressionTypes = ['gzip', 'br', 'deflate'];
const binaryMimeTypes = ['image', 'audio', 'video'];
const CHROME_EXTENSION_PROTOCOL = 'chrome-extension:';
const textResourceTypes = [
NetworkRequest.TYPES.Document,
NetworkRequest.TYPES.Script,
NetworkRequest.TYPES.Stylesheet,
NetworkRequest.TYPES.XHR,
NetworkRequest.TYPES.Fetch,
NetworkRequest.TYPES.EventSource,
];

class ResponseCompression extends Gatherer {
/**
Expand All @@ -29,14 +38,14 @@ class ResponseCompression extends Gatherer {

networkRecords.forEach(record => {
const mimeType = record._mimeType;
const resourceType = record._resourceType;
const resourceType = record._resourceType || NetworkRequest.TYPES.Other;
const resourceSize = record._resourceSize;

const isBinaryResource = mimeType && binaryMimeTypes.some(type => mimeType.startsWith(type));
const isTextBasedResource = !isBinaryResource && resourceType && resourceType.isTextType();
const isTextResource = !isBinaryResource && textResourceTypes.includes(resourceType);
const isChromeExtensionResource = record.url.startsWith(CHROME_EXTENSION_PROTOCOL);

if (!isTextBasedResource || !resourceSize || !record.finished ||
if (!isTextResource || !resourceSize || !record.finished ||
isChromeExtensionResource || !record.transferSize || record.statusCode === 304) {
return;
}
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/gather/gatherers/scripts.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
'use strict';

const Gatherer = require('./gatherer');
const WebInspector = require('../../lib/web-inspector');
const NetworkRequest = require('../../lib/network-request');

/**
* @fileoverview Gets JavaScript file contents.
Expand All @@ -23,7 +23,7 @@ class Scripts extends Gatherer {
/** @type {Object<string, string>} */
const scriptContentMap = {};
const scriptRecords = loadData.networkRecords
.filter(record => record._resourceType === WebInspector.resourceTypes.Script);
.filter(record => record._resourceType === NetworkRequest.TYPES.Script);

for (const record of scriptRecords) {
try {
Expand Down
6 changes: 3 additions & 3 deletions lighthouse-core/lib/dependency-graph/network-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
'use strict';

const BaseNode = require('./base-node');
const WebInspector = require('../web-inspector');
const NetworkRequest = require('../network-request');

class NetworkNode extends BaseNode {
/**
Expand Down Expand Up @@ -62,8 +62,8 @@ class NetworkNode extends BaseNode {
*/
hasRenderBlockingPriority() {
const priority = this._record.priority();
const isScript = this._record._resourceType === WebInspector.resourceTypes.Script;
const isDocument = this._record._resourceType === WebInspector.resourceTypes.Document;
const isScript = this._record._resourceType === NetworkRequest.TYPES.Script;
const isDocument = this._record._resourceType === NetworkRequest.TYPES.Document;
const isBlockingScript = priority === 'High' && isScript;
const isBlockingHtmlImport = priority === 'High' && isDocument;
return priority === 'VeryHigh' || isBlockingScript || isBlockingHtmlImport;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
'use strict';

const INITIAL_CWD = 14 * 1024;
const WebInspector = require('../../web-inspector');
const NetworkRequest = require('../../network-request');

class NetworkAnalyzer {
/**
Expand Down Expand Up @@ -328,7 +328,7 @@ class NetworkAnalyzer {
static findMainDocument(records) {
// TODO(phulce): handle more edge cases like client redirects, or plumb through finalUrl
const documentRequests = records.filter(record => record._resourceType ===
WebInspector.resourceTypes.Document);
NetworkRequest.TYPES.Document);
return documentRequests.sort((a, b) => a.startTime - b.startTime)[0];
}
}
Expand Down
Loading