534149 - [FIX]Give DOM windows a unique id

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

I believe Firebug could make good use of this, as could various other extensions.  Basically, have a 64-bit counter that counts up as we create windows.  Make its value readable for a given window via windowutils.

Seem reasonable?

Comment 1

[John J. Barton]

Yes, Firebug could use this to be more certain about its window-equality tests. I have wondered many times if they worked.

Comment 2

[Simon Bünzli]

This would also have been useful for SessionStore (cf. bug 452961).

Comment 3

[Boris Zbarsky [:bzbarsky]]

John, another question.  Does Firebug want to test for _outer_ window equality (i.e. navigation context equality), or does it want to test for _inner_ window equality (i.e. script global object equality)?  This matters for the bfcache case, in particular, which is one of the things I'd like to use this for.  For bfcache, you need inner window equality testing, basically.

Simon, same question.

Comment 4

[John J. Barton]

I don't understand the question. We compare values that come from  aWebProgress.DOMWindow in:
nsIWebProgressListener.onLocationChange(in nsIWebProgress aWebProgress,
                        in nsIRequest aRequest,
                        in nsIURI aLocation);

Two MDC documents discuss inner or outer windows:
https://developer.mozilla.org/En/Working_with_BFCache,
https://developer.mozilla.org/en/SpiderMonkey/Split_object
but they do not tell us how to get either kind. The API documents like
https://developer.mozilla.org/en/NsIWebProgress
don't specify if the window is inner or outer.

When we want the script global object we get it from |win.wrappedJSObject| where |win| ultimately comes from webProgress.DOMWindow. According to my understanding the wrappedJSObject is an alternative API, not an accessor for a possibly changeable property. None of the documents describe wrappedJSObject as being involved with inner or outer windows. Or script global objects, I just heard it from you.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> We compare values that come from  aWebProgress.DOMWindow 

Those are outer windows.  Any time you're comparing objects that you're actually holding, those are outer windows.  You never have a reference to the inner window directly.

> When we want the script global object we get it from |win.wrappedJSObject|

What you get there is the outer window.  That is NOT the script global object, in fact.  It's just something that currently happens to have a script global object embedded in it.

> According to my understanding the wrappedJSObject is an alternative API, not
> an accessor for a possibly changeable property.

That's more or less correct.  You have a handle to the outer window.  All gets/sets on it get forwarded to the inner window, etc.

Here's a simple experiment you can do.  Load a web page.  Get its window object.  Set some global properties, etc.  Then load a new web page.  You still have the same window object (compares == and ===, etc).  But all the properties that were there are now gone, and new ones are in their place (from the new page).  Now go back.  If you don't have Firebug installed and the page doesn't disable bfcache itself, all your old properties will come back.  What's happening there is the inner window inside the outer window changing.

So again, the outer window corresponds to a user-visible frame in the browser; something that can be navigated without changing its object identity.  The inner window is the actual script compilation scope and the place where global properties are actually set; there is a new one for every pageload, to a first approximation.

When a page is evicted from bfcache, what gets destroyed is its inner window. Presumably you want to know _which_ inner window it was, and cache per-page data based on inner window id, so you can evict it in that situation, right?

Comment 6

[John J. Barton]

(In reply to comment #5)
>  You never have a reference to the
> inner window directly.

So I still don't understand the question. If I can't have a reference, I can't compare for equality.

Based on what you are saying, Firebug has just be incredibly and ridiculously lucky. We only ever have outer windows and yet we are only ever interested in equality comparisons of inner windows.  Perhaps this explains why we have to be very careful with the code in Firebug's tabWatcher. I believe now that this code is using the window events to ensure that our outer windows are only compared when the inner windows are what we expect them to be.

Comment 7

[Boris Zbarsky [:bzbarsky]]

> If I can't have a reference, I can't compare for equality.

You can't compare for object equality.  However, if we add an API to give every window (inner and outer) a unique id, and we add an API for you to get the id of the current inner window for a given outer, then you can compare equality of those integers, right?  Would that work for whatever you use window object equality compares for right now?  Would it let you get rid of your unload listeners, if we fired "window destroyed" event with the id of the window that got destroyed?

Comment 8

[Simon Bünzli]

(In reply to comment #3)
I guess for chrome windows we'd need the outer one (for associating tab-related data with the user-visible window frame) while for content windows the inner one (for associating form data with a specific webpage). In the chrome case we might get by with the inner window as well, though, since we hardly ever load more than one XUL document into any chrome window during its lifetime.

Comment 9

[John J. Barton]

(In reply to comment #7)
> > If I can't have a reference, I can't compare for equality.
> 
> You can't compare for object equality.  However, if we add an API to give every
> window (inner and outer) a unique id, and we add an API for you to get the id
> of the current inner window for a given outer, then you can compare equality of
> those integers, right?  Would that work for whatever you use window object
> equality compares for right now?  Would it let you get rid of your unload
> listeners, if we fired "window destroyed" event with the id of the window that
> got destroyed?

But the tag or id is not useful unless we know when to do the comparison, and if we know when to do the comparison, we don't need the tag.

To be clear let's consider a new nsIDOMWindow property "pageWindowTag", which is unique for every inner window.  We can hold only references to outer windows, let's call it |domWindow|. 

Now if the inner window changes, the value of domWindow.pageWindowTag changes, but we don't know it. 

Suppose we have an event that tells us that the inner window has changed. Then yes in that event handler we can use the domWindow.pageWindowTag for comparisons, but we can just as well use the |domWindow| for comparisons. Because this is the only code path where the value of domWindow.pageWindowTag can change, this is place we need to worry about the difference between domWindow and inner window. 

This is the logic that Firebug uses in its TabWatcher. The bug it has is using onLocationChange/unload/pagehide for the event. That is why I am so keen on Bug 342715: in my view it is supposed to give us exactly the event we need to solve the problem.

There is one case where we could use the |domWindow.pageWindowTag| to improve performance: when Firefox reads from the bfCache it recovers a complete inner window object. Firebug has discarded its context for the page, so it can't use this cached object as far as I know. But we cannot use the cache because we have no way to know when to discard our context object. Thus we cannot use the pageWindowTag until we get bug 484710 fixed.

Comment 10

[Boris Zbarsky [:bzbarsky]]

> and if we know when to do the comparison, we don't need the tag.

If that were true, you wouldn't be adding unload handlers, now would you?

> Thus we cannot use the pageWindowTag until we get bug 484710 fixed.

Right, and that event is useless to you until windows have an id, since that event wouldn't be able to tell you _which_ window got evicted.  The plan is to have that event include the id of the window that got evicted.

Simon, sounds like you want both inner and outer id.  Thanks.  Will implement.

Comment 11

[John J. Barton]

(In reply to comment #10)
> > and if we know when to do the comparison, we don't need the tag.
> 
> If that were true, you wouldn't be adding unload handlers, now would you?

The unload handler allow us to delete the Firebug context for a page. If we have the tag, when do we test the tag to know it is time to delete our context?

Comment 12

[Boris Zbarsky [:bzbarsky]]

You will get an event saying "Window #2376 is now gone".  At that point you delete the context for window 2376.  And you store your contexts by window id.  Make sense?

Comment 13

[John J. Barton]

(In reply to comment #12)
> You will get an event saying "Window #2376 is now gone".  At that point you
> delete the context for window 2376.  And you store your contexts by window id. 
> Make sense?

If you mean, "after bug 484710 and bug 342715 land you will get an event..." then yes it makes sense. Else not, since this bug is not about events.

Maybe the problem you are trying to solve here is: "what are the event arguments for bug 342715 and 484710?" The typical nsIDOMWindow argument may be akward because the event involves the inner (page) window; the outer DOMWindow may not be changed.

Comment 14

[Boris Zbarsky [:bzbarsky]]

> "what are the event arguments for bug 342715 and 484710?"

Yes, precisely.

> The typical nsIDOMWindow argument may be akward

Not awkward.  Impossible, for exactly the reason you described.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Attachment: Like so

Comment 16

[Boris Zbarsky [:bzbarsky]]

Oh, I was torn between "ID" and "Id" throughout.  Can change if desired.

Comment 17

[Rob Campbell [:rc] (:robcee)]

This looks great. I think ID is fine here. Very nice!

Comment 18

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/7adb6aca38be

Comment 19

[John J. Barton]

Just a suggestion on doc: the outer.inner pair looks something like:
   load a URL into a tab  82.83  (no location), loading...
                          82.84 URL now shows in tab
   reload page            82.85 URL again shows in tab

The sequence for a new XUL window is more complicated.

Comment 20

[Eric Shepherd [:sheppy]]

(In reply to comment #19)
> Just a suggestion on doc: the outer.inner pair looks something like:
>    load a URL into a tab  82.83  (no location), loading...
>                           82.84 URL now shows in tab
>    reload page            82.85 URL again shows in tab
> 
> The sequence for a new XUL window is more complicated.

Not entirely sure what your point is here at the moment. These attributes are actually already in the reference documentation, although I've cleaned them up a bit:

https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIDOMWindowUtils#Attributes

I've added a little code snippet and some explanatory text here:

https://developer.mozilla.org/en/Code_snippets/Windows#Uniquely_identifying_DOM_windows

That said, I'm sure someone here can make that text more useful and descriptive, which would be helpful.