new_audit: bf-cache #14465
new_audit: bf-cache #14465
Conversation
| import FRGatherer from '../base-gatherer.js'; | ||
| import {waitForFrameNavigated, waitForLoadEvent} from '../driver/wait-for-condition.js'; | ||
|
|
||
| class BFCacheErrors extends FRGatherer { |
There was a problem hiding this comment.
maybe name to just BFCache or BFCacheReasonsNotRestored?
There was a problem hiding this comment.
I wanted to differentiate from the BFCache audit, and BFCacheReasonsNotRestored feels too long. This also mirrors the InstallabilityErrors artifact.
| const entry = history.entries[history.currentIndex]; | ||
|
|
||
| await Promise.all([ | ||
| session.sendCommand('Page.navigate', {url: 'chrome://terms'}), |
There was a problem hiding this comment.
- need to verify this url works in edge and brave
- need to confirm we can't just use about:blank (does the devtools CL suggest why, exactly?)
There was a problem hiding this comment.
need to verify this url works in edge and brave
It works but the contents are different for each browser. (Doesn't work in opera if that matters)
need to confirm we can't just use about:blank (does the devtools CL suggest why, exactly?)
Seems like about:blank would fit the bill as well?
There was a problem hiding this comment.
Pending response from chrome peeps about if about:blank would suffice here.
| @@ -13,6 +13,10 @@ const config = { | |||
| // webpages present here, hence the inclusion of 'best-practices'. | |||
| onlyCategories: ['performance', 'best-practices'], | |||
|
|
|||
| // BF cache will request the page again, initiating additional network requests. | |||
| // Disable the audit so we only detect requests from the normal page load. | |||
| skipAudits: ['bf-cache'], | |||
There was a problem hiding this comment.
Maybe we should have a test-definition base config we extend from? we could also specify only-categories: [] in it, forcing smoke configs that extend it to be more specific (I assume that works...)
There was a problem hiding this comment.
Seems like overkill plus it would be confusing when writing a test to bounce between the smokehouse base config and the test specific config.
This issue is only present on a handful of tests.
core/audits/bf-cache.js
Outdated
| } | ||
|
|
||
| return { | ||
| score: 0, |
There was a problem hiding this comment.
may want to mark n/a if supportPendingTable or notActionableTable is >0
There was a problem hiding this comment.
If there are still actionable failures I think it's worth surfacing them
| const entry = history.entries[history.currentIndex]; | ||
|
|
||
| await Promise.all([ | ||
| session.sendCommand('Page.navigate', {url: 'chrome://terms'}), |
There was a problem hiding this comment.
Pending response from chrome peeps about if about:blank would suffice here.
Required for bfcache testing in Lighthouse to work: GoogleChrome/lighthouse#14465 Bug: None Change-Id: I4faf54e5dee9acdf8ffde4ee8762c21ed7fb50b5 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4117673 Commit-Queue: Adam Raine <asraine@chromium.org> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org> Reviewed-by: Paul Irish <paulirish@chromium.org>
|
Looks like we might need a longer timeout than 50ms: |
Example result on https://www.espn.com/:
Changes we should consider:
BFCacheErrorscomputed artifact and get the event from the DT log for timespan mode.Ref
#13960