1191112 - window.close() immediately after mozRequestFullScreen leads to bad state and assertion failure

Status: RESOLVED FIXED

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

Description

[Jesse Ruderman]

Attachment: manual testcase

1. Load the testcase
2. Click "Open second window"
3. Click "Full screen & close window"
       Result: Nothing is full-screen, but dock & menubar are hidden!
4. Quit Firefox
       Result: Assertion failure: isEmpty(), at LinkedList.h:308

Comment 1

[Jesse Ruderman]

Attachment: stack

It's a LinkedList<FullscreenRequest>. That matches the declaration of sPendingFullscreenRequests, which was added in:

changeset:   https://hg.mozilla.org/mozilla-central/rev/10d47ad00e7a
user:        Xidorn Quan
date:        Wed Jun 10 23:13:12 2015 +1200
summary:     Bug 1161802 part 7 - Put document into fullscreen state after window becomes fullscreen. r=smaug,dao

This could be a more recent regression, though.

Comment 2

[Xidorn Quan [:xidorn] UTC+11]

It is a non-e10s-only bug, but from this I actually notice another e10s bug.

The assertion and the dock & menubar hidden are probably two bugs: one is in DOM core which is relatively easier to solve, and not too harmful; the other is the Cocoa widget which really makes me worry.

Comment 3

[Xidorn Quan [:xidorn] UTC+11]

It seems to be a regreession of bug 1160014.

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

Attachment: patch 1 - flush pending fullscreen requests on page close to avoid leak

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

Attachment: patch 2 - set the destroyed flag properly on cocoa window

Comment 6

[Xidorn Quan [:xidorn] UTC+11]

Attachment: patch 3 - avoid making widget fullscreen if it has been destroyed

Comment 7

[Xidorn Quan [:xidorn] UTC+11]

Attachment: patch 3 - avoid touch widget if it has been destroyed

Comment 8

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8643484 [details] [diff] [review]
patch 2 - set the destroyed flag properly on cocoa window

Looks good to me.  Good catch!

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8643468 [details] [diff] [review]
patch 1 - flush pending fullscreen requests on page close to avoid leak

looks wrong to me. Shouldn't we cancel all the fullscreen requests related to 'this'.

Comment 10

[Xidorn Quan [:xidorn] UTC+11]

Attachment: patch 1 - clear pending fullscreen requests on page close

Comment 11

[Xidorn Quan [:xidorn] UTC+11]

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

Comment 12

[Xidorn Quan [:xidorn] UTC+11]

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/1f5f32b97e0ddd31c267a491f4c36bc8ddbe29d2
changeset:  1f5f32b97e0ddd31c267a491f4c36bc8ddbe29d2
user:       Xidorn Quan <[e-mail]>
date:       Thu Aug 06 15:37:48 2015 +1000
description:
Bug 1191112 part 1 - Clear pending fullscreen requests on page close. r=smaug

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/988184cd378ec56523dbcdfcffefe93d27ea08bf
changeset:  988184cd378ec56523dbcdfcffefe93d27ea08bf
user:       Xidorn Quan <[e-mail]>
date:       Thu Aug 06 15:37:48 2015 +1000
description:
Bug 1191112 part 2 - Set the destroyed flag on nsCocoaWindow when destroyed. r=smichaud

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/d5813935e9a33c27be2391a1cd8617b1e8a9e6f4
changeset:  d5813935e9a33c27be2391a1cd8617b1e8a9e6f4
user:       Xidorn Quan <[e-mail]>
date:       Thu Aug 06 15:37:48 2015 +1000
description:
Bug 1191112 part 3 - Don't touch the widget anymore if it has been destroyed during the fullscreen transition. r=smaug

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1f5f32b97e0d
https://hg.mozilla.org/mozilla-central/rev/988184cd378e
https://hg.mozilla.org/mozilla-central/rev/d5813935e9a3