Merge in scripting API and es module v119 webcompat addon updates early to get some more testing during the nightly cycle
Categories
(Web Compatibility :: Interventions, task)
Tracking
(firefox119 fixed)
| Tracking | Status | |
|---|---|---|
| firefox119 | --- | fixed |
People
(Reporter: twisniewski, Assigned: twisniewski)
References
Details
(Keywords: perf-alert)
Attachments
(3 files)
This includes these pull requests:
|
Assignee | |
Comment 1•2 years ago
|
||
|
||
Updated•2 years ago
|
|
||
Comment 2•2 years ago
|
||
|
|
||
Comment 4•2 years ago
|
||
Authored by https://github.com/wisniewskit
https://github.com/mozilla-mobile/firefox-android/commit/092b70fd8f8cc1cb967e234257d7cae1ef84ee0e
[main] Bug 1853013 - Ship 118.2.0 of feature-webcompat
|
||
Comment 5•2 years ago
|
||
Backed out changeset 8771fccb94fa (Bug 1853013) for causing several mochitest failures.
Failure logs:
- https://treeherder.mozilla.org/logviewer?job_id=429026520&repo=autoland
- https://treeherder.mozilla.org/logviewer?job_id=429028490&repo=autoland
- https://treeherder.mozilla.org/logviewer?job_id=429024926&repo=autoland
- https://treeherder.mozilla.org/logviewer?job_id=429028917&repo=autoland
- https://treeherder.mozilla.org/logviewer?job_id=429029273&repo=autoland
Backout link: https://hg.mozilla.org/integration/autoland/rev/ab101a976da356370e4d513e38e73a1fa55b1597
|
|
Assignee | |
Comment 6•2 years ago
|
||
Hmm. Indeed, toolkit/components/passwordmgr/test/browser/browser_autofill_hidden_document.js also fails for me locally. It seems like all of the content scripts we're registering with the scripting API somehow end up with matchAboutBlank perma-set to true in makeInternalContentScript.
This strikes me as a pretty weird thing to do when add-ons probably don't want to match about:blank, which we definitely do not want here. It causes all of the interventions to load during the test, and worse, it loads them all in one execution context, meaning that const variables end up being re-declared. This causes the test to fail as it does not expect the resulting extra console spam.
We could work around this by disabling the compat addon for these tests, but that strikes me as a bad solution. The older contentScripts.register API had an matchAboutBlank option rather than "forcing" addon content scripts to match about:blank when they almost certainly don't actually want to. I think it's best for the addons team to chime in before I try landing this one again.
:rpl, what are your thoughts?
|
|
Assignee | |
Comment 7•2 years ago
|
||
For the record, we've roughly decided on a course of action here. rpl is working on the required changes in this PR (and possibly other related changes to Gecko in other bugs). Once that work lands, we'll update this patch and try again.
|
||
Comment 8•2 years ago
|
||
Bug 1853412 has been merged to mozilla-central, and so I thought to push to try an updated patch "vendoring webcompat with the changes from the previous attempt plus the changes from the new PR" along with an additional one line patch that is "setting extensions.webcompat.useScriptingAPI to true from modules/libpref/init/all.js" (which makes the webcompat builtin to opt-in on using scripting API for injections and shims, while it would be falling back to contentScripts API as before in builds where that pref isn't set):
|
|
||
Comment 9•2 years ago
|
||
|
|
||
Comment 10•2 years ago
|
||
Authored by https://github.com/wisniewskit
https://github.com/mozilla-mobile/firefox-android/commit/4fe5f5e398ce97ca2146541ee582507e47df1685
[main] Bug 1853013 - Ship 118.3.0 of feature-webcompat
|
||
Comment 11•2 years ago
|
||
|
||
Comment 12•2 years ago
|
||
| bugherder | ||
|
|
||
Comment 13•2 years ago
|
||
(In reply to Pulsebot from comment #11)
Pushed by twisniewski@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f36c23c0efa6
Merge in scripting API and es module webcompat addon updates;
r=denschub,webcompat-reviewers,ksenia
== Change summary for alert #39620 (as of Wed, 20 Sep 2023 10:14:33 GMT) ==
Improvements:
| Ratio | Test | Platform | Options | Absolute values (old vs new) | Performance Profiles |
|---|---|---|---|---|---|
| 38% | office LastVisualChange | windows10-64-shippable-qr | cold fission webrender | 1,888.65 -> 1,172.36 | Before/After |
| 30% | office LastVisualChange | windows10-64-shippable-qr | fission warm webrender | 1,144.13 -> 803.09 | Before/After |
| 30% | office LastVisualChange | macosx1015-64-shippable-qr | cold fission webrender | 1,717.64 -> 1,208.10 | Before/After |
| 26% | office LastVisualChange | linux1804-64-shippable-qr | fission warm webrender | 1,213.28 -> 896.02 | Before/After |
| 26% | office LastVisualChange | macosx1015-64-shippable-qr | fission warm webrender | 931.23 -> 689.87 | Before/After |
| ... | ... | ... | ... | ... | ... |
| 4% | office SpeedIndex | windows10-64-shippable-qr | fission warm webrender | 339.63 -> 327.18 | Before/After |
For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=39620
|
|
||
Updated•2 years ago
|
|
||
Comment 14•2 years ago
|
||
jftr, I was a bit skeptical about an improvement this large. However, in my quick tests with the Excel overscroll intervention, and the Outlook registerProtocolHandler intervention, everything appears to be working fine. I tried reloading a couple of times with different cache states and simulated network speeds in an attempt to provoke a race condition, but I couldn't break the intervention. So this might just be an improvement, not a bug.
Description
•