The ordering between DOMFullscreen:Entered and UpdateDimensions is not enforced
Categories
(Core :: DOM: Events, defect, P3)
Tracking
()
People
(Reporter: mstange, Unassigned, NeedInfo)
References
(Blocks 1 open bug)
Details
Attachments
(1 obsolete file)
test_videocontrols.html
has a subtest at the end which assumes that, at the time at which the mozfullscreenchange
event fires in the content process, the content document covers the entire screen.
However, this is not necessarily the case: Content process documents only learn about their size via UpdateDimensions
messages, and those are dispatched by the parent from nsSubDocumentFrame::ReflowFinished
, so they only happen if the parent process window document is reflown.
The parent process tells the content process about the full screen change with a DOMFullscreen:Entered
message, which is sent from a MozDOMFullscreen:Entered
event handler, which is called during the parent process's Document::ApplyFullscreen
call.
In the past, on macOS, in the parent process, Document::ApplyFullscreen
always ran after a reflow of the parent process window document, because Document::ApplyFullscreen
runs after nsBaseWidget::InfallibleMakeFullScreen
, which triggers a window resize, and in the past, window resizes would cause synchronous paints, and the synchronous paint flushed layout and triggered the reflow.
After bug 1574538, there will not be a synchronous paint during window resize operations. The paint will happen asynchronously. (The screen still updates atomically though.)
On Linux, resizes always cause asynchronous paints, as far as I know. And bug 1514060 shows that test_videocontrols.html was already failing intermittently, almost exclusively on Linux.
What should we do here? Relying on synchronous paints during window resizes is not a good idea in my opinion. I think we have two options:
- Enforce the ordering between
UpdateDimensions
andDOMFullscreen:Entered
, for example by flushing layout in the parent process before sendingDOMFullscreen:Entered
to content. - Do not enforce any ordering, and make content deal with the fact that
fullscreen
may fire at a time at which the document still has the old size.
I prefer the first option, but it's slightly risky. It also means we may end up reflowing the parent process window document twice, which runs counter to bug 1267568. (Unless mechanisms from bug 1267568 prevent a double reflow, I'm not sure about that.)
Until this bug is fixed, I'll try to work around this issue in the test, by using the document size instead of the screen size.
Reporter | ||
Comment 1•5 years ago
|
||
Reporter | ||
Comment 2•5 years ago
|
||
Reporter | ||
Comment 3•5 years ago
|
||
Looks like this patch on its own doesn't fix test_videocontrols.html
on Linux: There's still a failure on a Linux64 asan job.
Updated•5 years ago
|
Reporter | ||
Comment 4•5 years ago
|
||
The approach in this fix was no good. In the review Emilio had a better idea:
Would it be better to make
MozDOMFullScreen:Entered
more async, like eitherDispatchFullscreenChange
(which uses the refresh driver), orDispatchFullscreenNewOriginEvent
(which posts to the event loop viaAsyncEventDispatcher
).
(This would give us a safer place to flush. And if we ran this event from the refresh driver after the layout flush phase, we'd already have flushed layout.)
I'm going to unassign this bug from myself for now, because my immediate problem is fixed by the workaround in the test, but I may pick this up again in the future.
Updated•5 years ago
|
Comment 5•5 years ago
|
||
Regardless of the internal ordering, I indeed want to ensure that when fullscreenchange
event comes, the window that content see has been resized (but not yet reflowed). I had put lots effort to minimize the number of reflows happen in the content when going fullscreen.
So, yeah I think it makes sense to ensure that UpdateDimensions
should come before fullscreen notification to content.
I'm unsure about CoreAnimation, but the story in Linux is more complicated than that. Basically on Linux there is no way you can reliable know when the window state has been changed to fullscreen and the window has been resized. See discussion in bug 1254448 as well as the related gtk bug I filed for that.
As far as it's not the case for CoreAnimation, and we can still know when the window state is stable, I think it's possible to guarantee the state content observes.
Reporter | ||
Comment 6•5 years ago
|
||
Would it make sense to update dimensions and fullscreen state atomically, with the same message? Maybe renaming UpdateDimensions
to UpdateDimensionProperties
with size+fullscreenstate+sizemode would work.
Comment 7•5 years ago
|
||
Moving to "DOM: Events" component (though most of the fullscreen logic is implemented in frontend code).
Updated•5 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•2 years ago
|
Description
•