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

perf: consolidate DBW pass into defaultPass #2160

Merged
merged 8 commits into from
May 10, 2017
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@

<!-- PASS: preload that's activated later does not block rendering. -->
<link rel="preload" href="./dbw_tester.css?delay=2000&async=true" as="style" onload="this.rel = 'stylesheet'">
<!-- PASS: async stylesheet that loads after FCP -->
<link rel="stylesheet" href="./dbw_tester.css?delay=3000&async=true" disabled onload="this.disabled = false">

<!-- FAIL: block rendering -->
<script src="./dbw_tester.js"></script>
Expand Down
32 changes: 2 additions & 30 deletions lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ module.exports = [
extendedInfo: {
value: {
results: {
length: 13
length: 14
}
}
}
Expand Down Expand Up @@ -64,26 +64,6 @@ module.exports = [
}
}
},
'no-console-time': {
score: false,
extendedInfo: {
value: {
results: {
length: 3
}
}
}
},
'no-datenow': {
score: false,
extendedInfo: {
value: {
results: {
length: 5
}
}
}
},
'no-document-write': {
score: false,
extendedInfo: {
Expand Down Expand Up @@ -135,9 +115,7 @@ module.exports = [
value: {
// Note: This would normally be 7 but M56 defaults document-level
// listeners to passive. See https://www.chromestatus.com/features/5093566007214080
results: {
length: 4
}
length: 4
}
}
},
Expand Down Expand Up @@ -232,12 +210,6 @@ module.exports = [
'link-blocking-first-paint': {
score: 100
},
'no-console-time': {
score: true
},
'no-datenow': {
score: true
},
'no-document-write': {
score: true
},
Expand Down
15 changes: 5 additions & 10 deletions lighthouse-core/audits/dobetterweb/geolocation-on-start.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@

'use strict';

const Audit = require('../audit');
const ViolationAudit = require('../violation-audit');
const Formatter = require('../../report/formatter');

class GeolocationOnStart extends Audit {
class GeolocationOnStart extends ViolationAudit {
/**
* @return {!AuditMeta}
*/
Expand All @@ -37,7 +37,7 @@ class GeolocationOnStart extends Audit {
helpText: 'Users are mistrustful of or confused by sites that request their ' +
'location without context. Consider tying the request to user gestures instead. ' +
'[Learn more](https://developers.google.com/web/tools/lighthouse/audits/geolocation-on-load).',
requiredArtifacts: ['GeolocationOnStart']
requiredArtifacts: ['ChromeConsoleMessages']
};
}

Expand All @@ -46,18 +46,13 @@ class GeolocationOnStart extends Audit {
* @return {!AuditResult}
*/
static audit(artifacts) {
const results = artifacts.GeolocationOnStart.map(err => {
return Object.assign({
label: `line: ${err.line}, col: ${err.col}`
}, err);
});

const results = ViolationAudit.getViolationResults(artifacts, /geolocation/);

const headings = [
{key: 'url', itemType: 'url', text: 'URL'},
{key: 'label', itemType: 'text', text: 'Location'},
];
const details = Audit.makeV2TableDetails(headings, results);
const details = ViolationAudit.makeV2TableDetails(headings, results);

return {
rawValue: results.length === 0,
Expand Down
18 changes: 13 additions & 5 deletions lighthouse-core/audits/dobetterweb/link-blocking-first-paint.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,23 +46,27 @@ class LinkBlockingFirstPaintAudit extends Audit {
helpText: 'Link elements are blocking the first paint of your page. Consider ' +
'inlining critical links and deferring non-critical ones. ' +
'[Learn more](https://developers.google.com/web/tools/lighthouse/audits/blocking-resources).',
requiredArtifacts: ['TagsBlockingFirstPaint']
requiredArtifacts: ['TagsBlockingFirstPaint', 'traces']
};
}

/**
* @param {!Artifacts} artifacts
* @param {string} tagFilter The tagName to filter on
* @param {number=} loadThreshold Filter to resources that took at least this
* @param {number=} endTimeMax The trace milisecond timestamp that offending tags must have ended
* before (typically first contentful paint).
* @param {number=} loadDurationThreshold Filter to resources that took at least this
* many milliseconds to load.
* @return {!AuditResult} The object to pass to `generateAuditResult`
*/
static computeAuditResultForTags(artifacts, tagFilter, loadThreshold = 0) {
static computeAuditResultForTags(artifacts, tagFilter, endTimeMax = Infinity,
loadDurationThreshold = 0) {
const artifact = artifacts.TagsBlockingFirstPaint;

const filtered = artifact.filter(item => {
return item.tag.tagName === tagFilter &&
(item.endTime - item.startTime) * 1000 >= loadThreshold;
(item.endTime - item.startTime) * 1000 >= loadDurationThreshold &&
item.endTime * 1000 < endTimeMax;
});

const startTime = filtered.length === 0 ? 0 :
Expand Down Expand Up @@ -117,7 +121,11 @@ class LinkBlockingFirstPaintAudit extends Audit {
* @return {!AuditResult}
*/
static audit(artifacts) {
return this.computeAuditResultForTags(artifacts, 'LINK', LOAD_THRESHOLD_IN_MS);
const trace = artifacts.traces[Audit.DEFAULT_PASS];
return artifacts.requestTraceOfTab(trace).then(traceOfTab => {
const fcp = traceOfTab.timestamps.firstContentfulPaint;
return this.computeAuditResultForTags(artifacts, 'LINK', fcp, LOAD_THRESHOLD_IN_MS);
});
}
}

Expand Down
82 changes: 0 additions & 82 deletions lighthouse-core/audits/dobetterweb/no-console-time.js

This file was deleted.

82 changes: 0 additions & 82 deletions lighthouse-core/audits/dobetterweb/no-datenow.js

This file was deleted.

14 changes: 5 additions & 9 deletions lighthouse-core/audits/dobetterweb/no-document-write.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@

'use strict';

const Audit = require('../audit');
const ViolationAudit = require('../violation-audit');
const Formatter = require('../../report/formatter');

class NoDocWriteAudit extends Audit {
class NoDocWriteAudit extends ViolationAudit {

/**
* @return {!AuditMeta}
Expand All @@ -37,7 +37,7 @@ class NoDocWriteAudit extends Audit {
helpText: 'For users on slow connections, external scripts dynamically injected via ' +
'`document.write()` can delay page load by tens of seconds. ' +
'[Learn more](https://developers.google.com/web/tools/lighthouse/audits/document-write).',
requiredArtifacts: ['DocWriteUse']
requiredArtifacts: ['ChromeConsoleMessages']
};
}

Expand All @@ -46,17 +46,13 @@ class NoDocWriteAudit extends Audit {
* @return {!AuditResult}
*/
static audit(artifacts) {
const results = artifacts.DocWriteUse.map(err => {
return Object.assign({
label: `line: ${err.line}, col: ${err.col}`
}, err);
});
const results = ViolationAudit.getViolationResults(artifacts, /document\.write/);

const headings = [
{key: 'url', itemType: 'url', text: 'URL'},
{key: 'label', itemType: 'text', text: 'Location'},
];
const details = Audit.makeV2TableDetails(headings, results);
const details = ViolationAudit.makeV2TableDetails(headings, results);

return {
rawValue: results.length === 0,
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/audits/dobetterweb/no-mutation-events.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class NoMutationEventsAudit extends Audit {
const isMutationEvent = this.MUTATION_EVENTS.includes(loc.type);
let sameHost = URL.hostsMatch(artifacts.URL.finalUrl, loc.url);

if (!URL.isValid(loc.url)) {
if (!URL.isValid(loc.url) && isMutationEvent) {
sameHost = true;
debugString = URL.INVALID_URL_DEBUG_STRING;
}
Expand Down
Loading