core: don't load blank page twice in first pass #6369
Conversation
| // Enable emulation based on settings | ||
| // In the devtools/extension case, we can't still be on the site while trying to clear state | ||
| // So we first navigate to a blank page, then apply our emulation & setup | ||
| await GatherRunner.loadBlank(driver); |
There was a problem hiding this comment.
should this use the config for the first pass (for blank page and duration)?
There was a problem hiding this comment.
Nah, I think @paulirish was even borderline on the way to removing the blankPage and blankDuration params entirely from the passes anyway
There was a problem hiding this comment.
I'm a bit confused, aren't we still loading blank twice now? Seems like this line should go away and the comments moved into the loadBlank call in run
There was a problem hiding this comment.
sorry if I confused you with my "two blank calls" comment I meant for the pass and for before setup driver, not for setup driver + before setup driver :)
| await GatherRunner.setupDriver(driver, options); | ||
| baseArtifacts.BenchmarkIndex = await options.driver.getBenchmarkIndex(); |
There was a problem hiding this comment.
wait I take it back this won't work sorry!! :)
It needs to go before we setup emulation since its purpose is establishing the right multiplier (see #6162)
it doesn't seem like it'd be the end of the world to have the two loadBlank calls in this function, was there another reason I'm forgetting that it's inside setupDriver now?
There was a problem hiding this comment.
+1 to restoring the original loadBlank line
| // Enable emulation based on settings | ||
| // In the devtools/extension case, we can't still be on the site while trying to clear state | ||
| // So we first navigate to a blank page, then apply our emulation & setup | ||
| await GatherRunner.loadBlank(driver); |
There was a problem hiding this comment.
I'm a bit confused, aren't we still loading blank twice now? Seems like this line should go away and the comments moved into the loadBlank call in run
| @@ -26,22 +26,22 @@ const Driver = require('../gather/driver.js'); // eslint-disable-line no-unused- | |||
| * Execution sequence when GatherRunner.run() is called: | |||
| * | |||
| * 1. Setup | |||
| * A. navigate to about:blank | |||
| * B. driver.connect() | |||
There was a problem hiding this comment.
I think this is now
A. driver.connect()
B. navigate to about:blank
| * v. register a performance observer | ||
| * vi. register dialog dismisser | ||
| * vii. clearDataForOrigin | ||
| * i. navigate to a blank page |
| * i. navigate to about:blank | ||
| * ii. Enable network request blocking for specified patterns | ||
| * iii. all gatherers' beforePass() | ||
| * i. Enable network request blocking for specified patterns |
There was a problem hiding this comment.
still need i. navigate to about:blank from master (we could maybe add a parenthetical about the first pass? but I don't think it's needed for just the overview up here)
Split from #6366.
Skip loading the blank page a second time on the first pass. It's already been loaded.