mozilla :: #apz

17 Mar 2017
13:38katspetruta: ping
16:40botondkats: ping?
16:41katsbotond: pong
16:43botondkats: when you have a sec, could you have a look at https://reviewboard.mozilla.org/r/120722/diff/1#index_header to see if it's doing something obvious that would cause an intermittent timeout, like a place where we should be doing some flushApzRepaints type stuff? just wondering if there's an obvious problem that can be spotted by inspection, before i dive into debugging it
16:44katsbotond: do you know where it times out?
16:47katsbotond: you should probably use document.scrollingElement instead of document.documentElement (although if that's the problem it would be a permafail, not intermittent)
16:47botondkats: so, in the test output, i see the assertion at line 33 being reached and passing
16:47botondkats: which is really weird, because the next line calls subtestDone(), so i'm not sure what is left to time out...
16:48katsbotond: hm, it might be that the scroll event fires before you're done synthesizing all your events
16:48katsbotond: i.e. during the second move synth
16:48katsthat might try to end the subtest before the continuation is done running
16:48katsi'm not sure what effect that would have
16:49katsit might leave the mouse in a down state for the next text, which might trigger a timeout
16:49katsit might be better to add some flags or handling to make sure that subtestDone only gets called after all the events have been synthesized
16:49katsthat's about the only potential problem i can see
16:50botondkats: hmm, interesting
17:31rbarkerkats: ping? Do you have a minute to talk about animating the dynamic tool bar?
17:32katsrbarker: sure
17:32rbarkerkats: If I remember correctly you said something about using Vsync to tick the animation. I've looked at the code and I'm not clear how to use it. Did I miss understand?
17:33katsrbarker: where does the animation code live?
17:34rbarkerI would like it to live some where in the compositor thread. it doesn't live any where yet :)
17:34katsrbarker: ok. so yes, it would be good if the animation was ticked by the vsync signal
17:35rbarkerkats: but it looks like the vsync notify comes in on different threads depending on if it is in GPU process or not...
17:35katsrbarker: the composition always happens on the compositor thread, and that's driven by vsync
17:35katsrbarker: the vsync thread sends a message to the compositor thread to compose
17:37katsrbarker: http://searchfox.org/mozilla-central/source/gfx/layers/ipc/CompositorVsyncScheduler.cpp#126
17:38rbarkerkats: so when not running with GPU process, the vsync notify is in it's own thread I assume. But with GPU process, the IPC ends up invoking the notify on the compositor thread?
17:39rbarkerkats: yeah, I've been looking at the CompositorVsyncScheduler.
17:39rbarkerkats: I wasn't sure if that was the best place to hook in, but I guess it is.
17:39katsrbarker: better to hook in a little lower down from there
17:40katslike maybe in AsyncCompositionManager where it ticks APZ animatinos
17:40katsrbarker: i've been assuming the vsync thread lives in the gpu process, if gpu process is enabled
17:40katsi guess i never verified that
17:41rbarkerkats there is an assert that says the notify is in the compositor thread when in GPU process.
17:42katsrbarker: which assert are you talking about?
17:42rbarkerkats: http://searchfox.org/mozilla-central/source/gfx/layers/ipc/CompositorVsyncScheduler.cpp#205
17:43katsrbarker: ok, that makes sense
17:44rbarkerkats: yeah I understand why it is that way since the notify comes from IPC that gets delivered in the compositor thread.
17:45katsrbarker: yup
17:45rbarkerkats: but I like hooking into the AsyncCompositionManager better as I've already have some code in there already.
17:45katsok
17:45rbarkerkats: and I learned the hard way that AsyncCompositionManager is not thread safe and can only be used in the compositor thread :)
18:01rbarkerkats: okay, I think I get it, thanks.
18:01rbarkerkats: I know I asked this before but I don't remember the answer: Is there a reason I can't remove this comment and close the referenced bug? http://searchfox.org/mozilla-central/source/gfx/layers/composite/AsyncCompositionManager.cpp#1360
18:02katsi don't think so, i consider the code sufficiently unified now
18:02katsrbarker: feel free to close it
18:05rbarkerkats: Okay, that's what I suspected but I wasn't sure if there was something I was missing. Also, the SyncFrameMetric call is now gone :) No more hard sync between UI thread and compositor thread during composition (I believe?)
18:06rbarkerstill have first paint but it is now async ipc call.
18:06katsok cool
21:59botondtnikkel: ping
21:59tnikkelbotond: hey
22:00botondtnikkel: hey. have you had a chance to look at https://bugzilla.mozilla.org/show_bug.cgi?id=1312697#c24 at all?
22:00firebotBug 1312697 NEW, botond@mozilla.com Scroll position not saved on local page ( file:///)
22:04botondtnikkel: basically, the test is not exercising the intended failure scenario
22:05botondtnikkel: because the failure scenario involves ScrollToRestoredPosition() being called while the page is in the stopped state, and that's not happening
22:05tnikkelbotond: do you remember what was calling ScrollToRestoredPosition in the original bug?
22:06botondtnikkel: i'm pretty sure it was ReflowFinished
22:11tnikkelbotond: we could maybe just force a reflow
22:12tnikkellike use a setTimeout that fires after we know the page is stopped
22:13botondtnikkel: use a setTimeout to do what?
22:13tnikkelbotond: tweak some content in the page that would cause a reflow
22:14botondtnikkel: can i just do that after calling stop()? do i need a setTimeout()?
22:15tnikkelbotond: yeah, if you can do it
22:18botondtnikkel: so that would be e.g. inserting some text into the page?
22:19tnikkelbotond: yep
22:28botondtnikkel: that does seem to have worked!
22:32botondtnikkel: there is one remaining issue with the test
22:33botondtnikkel: in between the first reload() and the stop(), i currently use a setTimeout(1000)
22:34botondtnikkel: because if i call stop() right away, sometimes the entire reload()/stop() pair has no effect, and i don't get the partially loaded page i need
22:35botondtnikkel: but the setTimeout(1000) is also bad, since it only guarantees that we'll wake up _some time after_ 1000 ms have elapsed
22:35botondtnikkel: and sometimes, we wake up too late, by which time the page has completely reloaded (the slow-loading page script currently has a 5000 ms delay in it)
22:36botondtnikkel: is there a reliable way to catch the page in the partially-loaded state, and call stop() then>
22:36botond?
22:51tnikkelbotond: can we insert anything into the page that can call into the test code?
22:54botondtnikkel: the page has an onload handler that calls a function in the test via window.opener. perhaps we could do something similar
22:54tnikkelbotond: you could send a script element after a delay (but before the page is loaded) that calls into the test
23:03botondtnikkel: that seems to have worked as well. thanks for the suggestions!
23:23botondtnikkel: the test is now up for review
23:30tnikkelcool, thanks
18 Mar 2017
No messages
   
Last message: 157 days and 10 hours ago