Closed Bug 1240643 Opened 8 years ago Closed 7 years ago

Intermittent test_fullscreen-api.html | [scrollbar] Should have vertical scrollbar when [object HTMLHtmlElement] is in fullscreen - got 487, expected 1587

Categories

(Core :: DOM: Core & HTML, defect, P5)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
RESOLVED WORKSFORME
Tracking Flags:
Tracking Status
e10s + ---

People

(Reporter: philor, Unassigned)

References

(Depends on 2 open bugs, Blocks 1 open bug)

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Bug 1240643 - Dispatch a pending resize event before fetching screen bounds on GTK.
58 bytes, text/x-review-board-request
karlt
: review-
Details 
This bug indicates that when fullscreenchange event is triggered, we may still have the old window dimension, which sounds pretty serious. I currently have no idea how that could happen. I suppose several intermittents on pointerlock and fullscreen-api tests are caused by this issue as well.
Depends on: 1240696
So with e10s, it could sometimes happen that the window dimension has not been changed when fullscreenchange event is dispatched. I suspect this could relate to APZ or something.
tracking-e10s: --- → ?
I think this should be fixed by the patch I'm testing in bug 1254448.
Depends on: 1254448
Intermittent e10s test failure
Priority: -- → P5
This is sort of to be expected; the 'fullscreen' window state reported by GDK is setting a property associated with a window (_NET_WM_STATE), that doesn't necessarily imply the window has been resized (only that the window manager has acknowledged a request). Depending on how quickly the window manager handles the request, we may not have received the resize when we set _NET_WM_STATE.

> -1241028800[7fb1b4ca44a0]: nsWindow::MakeFullScreen [7fb18d781c00] aFullScreen 1
> Gdk-Message: property notify:	window: 260, atom(423): "_NET_CLIENT_LIST"
> Gdk-Message: property notify:	window: 260, atom(424): "_NET_CLIENT_LIST_STACKING"
> Gdk-Message: property notify:	window: 41943071, atom(432): "_NET_WM_ALLOWED_ACTIONS"
> Gdk-Message: property notify:	window: 41943071, atom(322): "_NET_WM_STATE"
> -1241028800[7fb1b4ca44a0]: nsWindow::OnWindowStateEvent [7fb18d781c00] changed 16 new_window_state 144
> -1241028800[7fb1b4ca44a0]: 	Fullscreen
> -1241028800[7fb1b4ca44a0]: nsWindow::SetSizeMode [7fb18d781c00] 3
> -1241028800[7fb1b4ca44a0]: nsWindow::OnWindowStateEvent [7fb18d781c00] changed 16 new_window_state 144
> Gdk-Message: property notify:	window: 41943071, atom(313): "_NET_FRAME_EXTENTS"
> Gdk-Message: configure notify:	window: 41943071  x,y: 0 0	w,h: 2880 1620  b-w: 0  above: 0	 ovr: 0
> -1241028800[7fb1b4ca44a0]: configure event [7fb18d781c00] 0 0 1440 810

Observe how the WindowStateEvent gets delivered before the window receives a ConfigureNotify event, the latter of which is responsible for informing us about a resize.

We could potentially coalesce the fullscreen window-state-event and the following resize into a single event, but a fullscreen state change could potentially result in no resize.
Never mind, the code actually tries to be robust to this (waiting for a resize after a fullscreen event). I'll continue looking into this.
The issue here appears to be mBounds being out of sync with layout when we query the screen bounds in test_fullscreen-api.html before an expose event flushes the pending resize. As a result, we'll assume that a reflow has occurred since window.inner{Width,Height} is equal to the screen bounds, and the test will fail.

Initial results on m3.large instances have shown positive results (see tc-M-e10s-3);

https://treeherder.mozilla.org/#/jobs?repo=try&revision=db5aa72f3a0bbb852d1c87a8dc5e50a7a6d3469b

(this used to fail much more frequently prior to the patch- see https://treeherder.mozilla.org/#/jobs?repo=try&revision=1777b364e97874a5673b4d8fef85f0a7baa66911)

Thanks!
Comment on attachment 8774557 [details]
Bug 1240643 - Dispatch a pending resize event before fetching screen bounds on GTK.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67022/diff/1-2/
Comment on attachment 8774557 [details]
Bug 1240643 - Dispatch a pending resize event before fetching screen bounds on GTK.

https://reviewboard.mozilla.org/r/67022/#review89966

Having a getter method trigger dispatch of an event can cause arbitrary code
to run, contrary to assumptions of the caller about object lifetimes, etc.
Gecko's use of nested event loops means this extends to anything happening
during the call, but merely running the event handlers can be problem enough.
Attachment #8774557 - Flags: review?(karlt) → review-
If the problem is layout inconsistent with widget, then perhaps
FlushPendingNotifications() needs to use the new value of mBounds somehow.  I
guess that might require widget notifying layout of the change in a way that
does not trigger JS events.  The existing DispatchResized() would be fine to
call synchronously from widget if the handler were changed so as to dispatch
JS events asynchronously.  That could be a fairly fundamental changes though
and I don't know what else might be depending on current behaviour.

Perhaps the test could wait for the resize notification, given it is assuming
the resize will happen, but there have been no development branch failures since August on infra as currently configured , so perhaps something has already fixed this test failure.
The last time this intermittent happened on our inbound repo was Augest 4th, which has been 3 months ago. I guess we can close this bug as WORKSFORME now.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
Let's track the new one in bug 1350875.
Status: REOPENED → RESOLVED
Closed: 8 years ago → 7 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: