Conversation
Automatically download mojojs.zip and turn on relevant flags for Chrome Dev. Follow-up to #24739.
stephenmcgruer
left a comment
stephenmcgruer
left a comment
There was a problem hiding this comment.
LGTM, though I'm concerned that re-running mojoJS tests is going to needlessly grab them from the network
|
|
||
| def install_mojojs(self, dest): | ||
| url = self._latest_chromium_snapshot_url() + "mojojs.zip" | ||
| def install_mojojs(self, dest, channel, browser_binary): |
There was a problem hiding this comment.
Should this check if the mojojs bindings already exist before trying to download them?
There was a problem hiding this comment.
We could, but we'd need to create a local lock file to record the version of the downloaded mojojs.zip; and it would only save us ~200KiB. WDYT?
There was a problem hiding this comment.
Oh for some reason I assumed the unzipped contents would be in a version-specific folder, and we could just check for that.
I'd still like to see us do this, but I'm ok to see it done in a future CL.
There was a problem hiding this comment.
I'd still like to see us do this, but I'm ok to see it done in a future CL.
I just realized that this will happen for all tests that anyone runs on Chrome, rather than just mojojs tests (right?). Assuming I'm right, I will double down to I think we should do such caching... but again I'm ok with landing this as long as the follow-up happens :)
There was a problem hiding this comment.
Ok, this looks fine to me. So if someone goes between channels (or Chrome updates) they will re-download, but thats probably rare and thus fine.
There was a problem hiding this comment.
Yeah. If I'm reading the source code correctly, the seed for message scrambling is the full version string; if so, then one doesn't need to redownload if they already have the exact same version of mojojs.zip regardless of the builder or channel. But that's a big if, and it also depends on an implementation detail, so I decided to be safe here.
|
|
||
| def install_mojojs(self, dest): | ||
| url = self._latest_chromium_snapshot_url() + "mojojs.zip" | ||
| def install_mojojs(self, dest, channel, browser_binary): |
There was a problem hiding this comment.
Ok, this looks fine to me. So if someone goes between channels (or Chrome updates) they will re-download, but thats probably rare and thus fine.
4 participants
Automatically download mojojs.zip and turn on relevant flags for Chrome
Dev. Follow-up to #24739.
Test:
./wpt run --channel dev --log-mach - chrome webxr