Add a mochitest to exercise the CollectTransformsForChromeMainThread code
Categories
(Core :: Panning and Zooming, task, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox69 | --- | fixed |
People
(Reporter: kats, Assigned: kats)
References
Details
Attachments
(4 files)
Before cleaning up the fix for bug 1548687 I want to land a mochitest that tests the fix in bug 1530661, so that we don't regress it. Henri described what he used to test that patch at https://bugzilla.mozilla.org/show_bug.cgi?id=1530661#c30 - for the purposes of this mochitest I think it would be sufficient to replicate that test page (maybe a bit simplified) and do the last bit of what Henri said (i.e. dispatch an event to it and verify the coordinates on it make sense).
Assignee | ||
Comment 1•6 years ago
|
||
I spent a bunch of time trying to produce a setup that can run mochitests that exercise fission/APZ codepaths, and was somewhat successful. The basic outline is that there's shell browser-mochitest that opens a new top-level browser window that is fission-enabled, and then loads a "real" test in a tab in that window.
At first the "real" test I was loading was a chrome://
URL, which works, but then if that page has an OOP iframe inside it, that OOP iframe can't communicate with the chrome://
page via postMessage
, and I couldn't find a way around that.
I also tried making the "real" test a non-chrome URL, i.e. http://mochi.test:8888/...
. The problem with this is that if there's an OOP iframe (say with a https://example.com/...
URL) inside it, then the onload event for that iframe doesn't seem to fire. I assume this is just a fission bug that needs fixing, rather than some fundamental thing. So for now I can work around it by just doing a setTimeout
and assuming the iframe will be loaded in some amount of time.
I also prefer this second approach because it seems more representative of real-world scenarios (i.e. the test page will be in a content process, and the OOP iframe will be in another content process). The first approach is different because the chrome://
test page gets loaded in the main process instead of a content process. However the second approach also means there's extra postMessage
machinery needed for communicating between all these different processes/windows in the test.
Here's the "shell" browser-mochitest: https://hg.mozilla.org/try/file/c0a3c61fc3941de8ad10c8f550fda54d432fbb54/gfx/layers/apz/test/mochitest/browser_test_group_fission.js
Here's the "real" test that gets loaded with a http://mochi.test:8888/
URL: https://hg.mozilla.org/try/file/c0a3c61fc3941de8ad10c8f550fda54d432fbb54/gfx/layers/apz/test/mochitest/helper_fission_basic_content.html
Here's the page that gets loaded into the OOP iframe with a https://example.com/
URL: https://hg.mozilla.org/try/file/c0a3c61fc3941de8ad10c8f550fda54d432fbb54/gfx/layers/apz/test/mochitest/helper_fission_eval.html
Nika, can you confirm that the onload
event for OOP iframes not firing is just a bug that needs fixing? Also if you have suggestions on better ways to architect this, or if I'm doing something that's not really supported, please let me know.
Assignee | ||
Updated•6 years ago
|
Comment 2•6 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #1)
I spent a bunch of time trying to produce a setup that can run mochitests that exercise fission/APZ codepaths, and was somewhat successful. The basic outline is that there's shell browser-mochitest that opens a new top-level browser window that is fission-enabled, and then loads a "real" test in a tab in that window.
At first the "real" test I was loading was a
chrome://
URL, which works, but then if that page has an OOP iframe inside it, that OOP iframe can't communicate with thechrome://
page viapostMessage
, and I couldn't find a way around that.I also tried making the "real" test a non-chrome URL, i.e.
http://mochi.test:8888/...
. The problem with this is that if there's an OOP iframe (say with ahttps://example.com/...
URL) inside it, then the onload event for that iframe doesn't seem to fire. I assume this is just a fission bug that needs fixing, rather than some fundamental thing. So for now I can work around it by just doing asetTimeout
and assuming the iframe will be loaded in some amount of time.
Yeah, onload
isn't fired out of oop iframes yet unfortunately. I've worked around it by having the iframe listen to its own onload
event, and window.parent.postMessage
when it's done loading.
I also prefer this second approach because it seems more representative of real-world scenarios (i.e. the test page will be in a content process, and the OOP iframe will be in another content process). The first approach is different because the
chrome://
test page gets loaded in the main process instead of a content process. However the second approach also means there's extrapostMessage
machinery needed for communicating between all these different processes/windows in the test.
Yeah, unfortunately you're testing completely different codepaths for an oop iframe in the parent process vs. an oop iframe in a content process :-/ - so it would be much nicer if you could instead use the real oop iframe.
Here's the "shell" browser-mochitest: https://hg.mozilla.org/try/file/c0a3c61fc3941de8ad10c8f550fda54d432fbb54/gfx/layers/apz/test/mochitest/browser_test_group_fission.js
Here's the "real" test that gets loaded with ahttp://mochi.test:8888/
URL: https://hg.mozilla.org/try/file/c0a3c61fc3941de8ad10c8f550fda54d432fbb54/gfx/layers/apz/test/mochitest/helper_fission_basic_content.html
Here's the page that gets loaded into the OOP iframe with ahttps://example.com/
URL: https://hg.mozilla.org/try/file/c0a3c61fc3941de8ad10c8f550fda54d432fbb54/gfx/layers/apz/test/mochitest/helper_fission_eval.htmlNika, can you confirm that the
onload
event for OOP iframes not firing is just a bug that needs fixing?
Yep. It's just not implemented yet.
Also if you have suggestions on better ways to architect this, or if I'm doing something that's not really supported, please let me know.
I think the """best""" way right now is probably to fire a postMessage out when the page detects its loaded.
Assignee | ||
Comment 3•6 years ago
|
||
I cleaned up the test a bit and pushed it to try:
it's showing leaked windows which sounds plausible but I don't know what's causing the leak. Will need to investigate more.
Assignee | ||
Comment 4•6 years ago
|
||
The leaking seems to be coming from the addMessageListener
calls in the chromeProcessScript
. Not clear to me why. There's no corresponding removeMessageListener
function in the sandbox that script runs in, so I added one but it still didn't help.
Assignee | ||
Comment 5•6 years ago
|
||
This is no longer blocked on bug 1550923.
Assignee | ||
Comment 6•6 years ago
|
||
Apparently my removeMessageListener
implementation wasn't good enough. I made it better and fixed the leak (and fixed another similar leak elsewhere) and now it's passing locally. Try push: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=564994017543517e7401b7ccd9c5b928a984fee5
Assignee | ||
Comment 7•6 years ago
|
||
Apparently leaving these listeners registered can leak DOM windows
(in some circumstances that I don't fully comprehend) which causes
test failures when running on debug builds. At any rate, unregistering
listeners on cleanup seems like a good thing to do.
Assignee | ||
Comment 8•6 years ago
|
||
This introduces the framework and helpers needed to do this kind of testing,
and adds a basic sanity test that ensures some basic functionality.
Depends on D32185
Assignee | ||
Comment 9•6 years ago
|
||
I split this out so it's more obvious what pieces need to be modified
to add additional functions.
Depends on D32187
Assignee | ||
Comment 10•6 years ago
|
||
This exercises the transforms propagated in bug 1530661, which is
WebRender-specific (because the events only have the target layers id
set on them if WR is enabled).
Depends on D32188
Assignee | ||
Comment 11•6 years ago
|
||
So I finally finished updating the patches to use the window actor thing as Nika suggested. The updated patches work locally, but then I did a try push and they fail in some configurations. I can reproduce the --verify
failure on my local Linux debug build, and as far as I can tell the iframe just isn't getting loaded intermittently. I set the src attribute of the iframe element and sometimes it just doesn't load the iframe; it visually remains blank and the onload event inside the iframe doesn't fire.
This didn't happen with the first set of patches (which are currently in phabricator) so presumably the changes I made, or rebasing to master, exposed some intermittent bug. I don't think bisecting my changes is feasible and even if it were, it probably wouldn't help that much in terms of finding the root cause which I suspect is somewhere in the DOM/IPC code. I'll spin that out into a separate bug.
Assignee | ||
Comment 12•6 years ago
|
||
Try push with the dep bugs fixed is looking good so far: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=b89723417b94f507024860118ee0c49777843f5d
Updated•5 years ago
|
Assignee | ||
Comment 13•5 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=ac831431ba9965410c08eae3d5e8eb27765e99e0 is the most recent try run, it's green, except for the ESLint warning which I fixed after doing this try push.
Comment 14•5 years ago
|
||
Comment 15•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/57a34b090ca9
https://hg.mozilla.org/mozilla-central/rev/6deee42954a6
https://hg.mozilla.org/mozilla-central/rev/4da9907d86f8
https://hg.mozilla.org/mozilla-central/rev/7bed70ba2b07
Description
•