Closed Bug 1241763 Opened 4 years ago Closed 4 years ago

Don't fire dom-window-destroyed on outer windows

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: khuey, Assigned: khuey)

References

Details

Attachments

(1 file)

Attached patch PatchSplinter Review
This is a problem once inner and outer windows become different types.  The good news is that every JS observer will (effectively) get the inner anyways and that there's only one C++ consumer that actually wants the outer.  We can just call that one manually.
Attachment #8710866 - Flags: review?(bzbarsky)
> The good news is that every JS observer will (effectively) get the inner anyways

Wait, what?  They'll get the outer, no?  That's what JS always sees.

What do the JS consumers of this notification do with the subject?
Flags: needinfo?(khuey)
Er, yes, that's what I meant.

The only in-tree JS consumer is http://mxr.mozilla.org/mozilla-central/source/dom/identity/nsDOMIdentity.js#653, which is an nsIDOMGlobalPropertyInitializer that cleans up on dom-window-destroyed (so it wants to be notified of the inner's death).
Flags: needinfo?(khuey)
And to be clear, what I was going for was that since JS always gets the outer window it's not possible to notify it of the death of both inner and outer windows with a single notification disambiguated by what type of object it's passed.  So JS won't be relying on the ability to see both.
Yeah, so that JS consumer is just broken, in that it will uninit when _any_ inner for its outer is destroyed, right?  Possibly while the actual inner it cares about is still alive.

That seems totally and utterly bogus.  Please file a bug on it.  :(  It should be watching for the "inner-window-destroyed" notification for the relevant window id.  That's what it used to do until bug 1065128 "fixed" it to do the wrong thing...

I really wish we could just prevent JS observers getting the dom-window-destroyed notification or something.  There's just nothing remotely useful they can do with it.
Comment on attachment 8710866 [details] [diff] [review]
Patch

r=me I guess, but do file that bug on the JS consumer.
Attachment #8710866 - Flags: review?(bzbarsky) → review+
You're saying it's broken in the presence, of say, the bfcache?  It would seem so.
Flags: needinfo?(bzbarsky)
> You're saying it's broken in the presence, of say, the bfcache?

Exactly.
Flags: needinfo?(bzbarsky)
https://hg.mozilla.org/mozilla-central/rev/29690442872d
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.