Open Bug 1492748 Opened 6 years ago Updated 2 years ago

Align resize event and fullscreenchange event to be in the same tick

Categories

(Core :: DOM: Core & HTML, enhancement, P2)

enhancement

Tracking

()

Tracking Status
firefox64 --- affected

People

(Reporter: xidorn, Unassigned)

References

(Blocks 2 open bugs)

Details

fullscreen/api/document-exit-fullscreen-timing-manual.html fails for this. Ideally, fullscreen changes should be applied before any related event is dispatched to the content, so that we can minimize unnecessary flush.
So... the reason that test is failing is different from what I thought before.

We are actually consistently fire resize event *after* fullscreen change event on content process (while we do the reverse for parent process). The resize event comes one or two transaction later than the fullscreen change event.

The test fails because after we get the first fullscreenchange event, we will get the resize event for the change of entering fullscreen first, in which event we are still in fullscreen.

Per spec, the event should be like this:
> requestFullscreen()
< "resize" event
< "fullscreenchange" event
> exitFullscreen()
< "resize" event
< "fullscreenchange" event

However, in this test, what's happening is:
> requestFullscreen()
< "fullscreenchange" event
> exitFullscreen()
< "resize" event
< "fullscreenchange" event
< "resize" event

The test incorrectly count the first "resize" event as the one in response to exitFullscreen, while it's actually a late one for the "requestFullscreen".

To fix this, we need to really align the resize and the fullscreen event.
Summary: Investigate why resize event is dispatched before fullscreen changes get applied when exiting fullscreen → Align resize event and fullscreenchange event to be in the same tick
It's funny that, currently we make the view manager set delayed resize, and such delayed resize is flushed in PresShell::DoFlushPendingNotifications which is invoked from Tick *after* the steps of dispatching resize and fullscreenchange. The resize event is queued by resize reflow invoked by nsViewManager::DoSetWindowDimensions from the delayed resize flush. Because of this settings, we are consistently dispatching resize event after the flush for fullscreen change.

This raise a question, then, should we queue the resize event before resize reflow happens, i.e. whenever the delayed resize is set? Would that cause some inconsistency or observable behavior change?
There is an even more aggressive question, should we ever try to do resize reflow synchronously? Maybe we can have nsViewManager::SetWindowDimensions always just record a delayed resize and queue a resize event, and then let the next tick do its work?
smaug, what do you think about the questions in comment 2 and comment 3?
Flags: needinfo?(bugs)
Not doing resize reflow sync ever would be web visible no? If one resizes an iframe using scripts and asks then information about the sizes of the elements in it.


But queuing resize event before we try to reflow sounds reasonable to me. Next flush (either during tick or explicit layout flush) would force the resize reflow.



(random note, looks like we aren't doing too good job with the old testcase https://bug457862.bmoattachments.org/attachment.cgi?id=355244 when resizing the frame)
Flags: needinfo?(bugs)
Blocks: 1494236
Blocks: 1494237
(In reply to Olli Pettay [:smaug] (r- if the bug doesn't explain what the change(s) are about.) from comment #5)
> Not doing resize reflow sync ever would be web visible no? If one resizes an
> iframe using scripts and asks then information about the sizes of the
> elements in it.

If script asks the information about the sizes of elements in it, we should flush the layout of the document, shouldn't we? When that happens, we should flush the delayed resize I suppose.
ok, then I misinterpreted the proposal. And yes, if we do then synchronously flush the pending resize reflow, that sounds reasonable.
No longer blocks: 1494237
Priority: -- → P2
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.