Closed Bug 350023 Opened 18 years ago Closed 11 years ago

Named items of window.frames collection can include frames that have been removed from the document

Categories

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

x86
All
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 170799
Tracking Status
blocking2.0 --- -

People

(Reporter: bluesky, Unassigned)

References

()

Details

(Keywords: memory-leak, testcase)

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; cs; rv:1.8.0.6) Gecko/20060728 Firefox/1.5.0.6
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; cs; rv:1.8.0.6) Gecko/20060728 Firefox/1.5.0.6

After creating a frame with the same name for the second time it's location is read-only and set to null. So, it's not possible to set location.href or use location.assign(); Means nothing is loaded in the frame.
Other Gecko based browsers suffer this problem as well (tried on Flock and Epiphany). Flock (Gecko/20060731 Firefox/1.5.0.5 Flock/0.7.4.1) gets to loadFrame() in the example, Firefox stops at newFrame().

Reproducible: Always

Steps to Reproduce:
1. Create an iframe with javascript, put it in the document and load some page in it
2. Remove the iframe from DOM (document)
3. Create an iframe with the same name and try to load some page in it
4. Nothing is loaded

Actual Results:  
There's no content loaded in the frame. Location is set to null and it's read-only. So, location.toString fails.

Expected Results:  
Content of the frame should be loaded every time it's created.

Using iframes in this way is quite common technique for javascript enhanced webs (Web 2.0).
I think this is might be somehow connected with bug 325290.
Attached file testcase
It seems like named items of the frames collection, still give access to the window.
After clicking on the first button, the second gives 'undefined', while the third button gives 'object Window'.
Assignee: nobody → general
Status: UNCONFIRMED → NEW
Component: General → DOM
Ever confirmed: true
Keywords: testcase
Product: Firefox → Core
QA Contact: general → ian
Version: unspecified → Trunk
Related to bug 170799?
Summary: iframes don't load content when they're created, removed and created again → Named items of window.frames collection can include frames that have been removed from the document
OS: Linux → All
Problem is recreatable with SeaMonkey trunk 2006082209/Win-2K.
 (1) Load the test page in URL link of this bug.
 (2) Click link of "create frame foo" => 'frame-foo' is displayed
 (3) Check iframe objects in document.
      javascript:var obj=document.getElementsByTagName('iframe');
      alert((typeof obj)+','+String(obj)+','+obj.length);
      => object,[object HTMLCollection],1
 (4) Check window.frames object.
      javascript:var obj=window.frames;alert((typeof obj)+','+String(obj)
       +','+obj.frames.length+','+obj['frame-foo']);
       => object,[object Window],1,[object Window]
 (5) Check window.frames['frame-foo'] object.
      javascript:var obj=window.frames['frame-foo'];
       var msg='';for(var a in obj){msg+='#'+a+'#'+'\n';}alert(msg);
       => Nothing is displayed ('alert(msg)' is not executed)
          This is also true when window.frames[0] is used.
       (Q1) Creation of window.frames['frame-foo'], which is DOM-0 object,
            is incomplete for createElement'ed IFRAME
            when 'NAME' attribute is added?
       (Q2) No property is defined for window.frames['frame-foo']?
       (Q3) JavaScript execution terminates on first "for (var a in obj)"?  
 (6) Click link of "delete frame foo" => 'frame-foo' disappered
 (7) Check iframe objects in document.
      javascript:var obj=document.getElementsByTagName('iframe');
      alert((typeof obj)+','+String(obj)+','+obj.length);
      => object,[object HTMLCollection],0
 (8) Check window.frames object.
       javascript:var obj=window.frames;alert((typeof obj)+','+String(obj)
        +','+obj.frames.length+','+obj['frame-foo']);
       => object,[object Window],0,[object Window]
          (window.frames.length is reset to ZERO properly,)
          (but window.frames['frame-foo'] is not removed. )

Seems to be same issue as bug 170799 Jesse Ruderman says.
Assignee: general → nobody
QA Contact: ian → general
Attached file exploratory testcase
I've convinced myself that this bug is a serious MEMORY LEAK causing nsGlobalWindow objects to live longer than they should.  In particular,

1. Click "Add a named frame" 5 times.
2. Click "List named frames".
3. Click "Remove" on all 5 frames.
4. Click "Collect garbage" (assuming you have domFuzzLite installed).

The total number of nsGlobalWindow does not drop significantly at step 4; it only drops after I close the tab.
This memory leak affects any page that is left open for a long time, adds and removes frames, and references frames by name.  I am not aware of any specific pages that fit this description but I'd be surprised if nothing popular does.
blocking2.0: --- → ?
Keywords: mlk
We originally found it making a webmail with virtual tabs. Each tab had an iframe inside. And many users kept this app open for hours.
Bluesky's site reused names, so instead of getting a leak, it got brokenness.
Could this be the cause of the infamous Gmail leak (bug 497808)?
Does gmail create lots of frames on the same window that have different names and get them via window.frames[name]?

This might be fixable by defining a custom getter when we resolve a frame name instead of sticking the frame in the slot; the getter would return undefined or null or something if there's no longer a frame with that name....

Note that there's a lot more weirdness here, if you dig a bit, and it all has to do with the fact that window === window.frames.  So |var x| conflicts with <frame name="x">, etc, and the interactions get ... complicated.
Seems to me that accumulating getters would also be a leak, just a smaller one.
Yes.  But so is just doing |window[count++] = count| over and over again.

The only way to do this in constant memory is to implement virtual named properties, so we don't add an actual property to the JS object.  That might happen, but not for 2.0.
I added a printf to http://mxr.mozilla.org/mozilla-central/source/dom/base/nsDOMClassInfo.cpp#6658 to see whether Gmail uses window.framename. It doesn't. (But the Instapaper bookmarklet does!)
While this could be used to construct pages that hold on to lots and lots of memory, so could many other techniques. Given that this is how Firefox has worked since day one, I can't justify needing to fix this right now. This is likely to change if we redo our dom bindings using proxies after 4...
blocking2.0: ? → -
People are using long-lived web apps more than they were in 1998, and in Firefox 4, we're encouraging that trend with App Tabs.  But the long-lived pages I've tested aren't using window.framename, so I guess blocking- is ok.
We are running into this problem with BrowserID.  Because we are supporting IE8/9, both of which have a bug in them which do not allow window.postMessage to a window opened using window.open, we are creating a messaging proxy iframe whenever we need to do communication, the idea is that the popup window where you interact with BrowserID sends/receives messages via this named iframe that lives in its window.opener.  So that we do not have multiple code paths depending on the browser, we have decided to use this proxy iframe in all browsers.  When BrowserID has completed its business, the proxy iframe is removed from the document.  If the user then decides that they need to go into BrowserID again, we attach a new iframe with the same name to the document.  Unfortunately, on occasion, the dialog cannot find the proxy functions it needs to call that are supposed to be in the iframe.  In FF6, this happens around 1 in every 4 times.  On Nightly, the iframe proxy functions cannot be found by the dialog on the second opening of the dialog.
You can work around that by using opener.document.getElementById("myIframe").contentWindow instead of opener.myIframe and using an <iframe id="myIframe"> instead of an <iframe name="myIframe">....

Fixing this probably involves making the _inner_ window a proxy or something.  Or doing the slotless getter thing... but I think that the proxy approach is needed to really match webidl here.
Would it be too ugly to delete the property (using JS_DeleteProperty) whenever we remove an iframe? We can't make the inner window a proxy since it's the global object and, by definition, those need to be native objects...

Is there anything saying that frames have to be "own" properties? We could hack another object into the inner window's prototype chain if need be.
> Is there anything saying that frames have to be "own" properties? 

I think the spec does right now (assuming it's actually been written down and not just in various W3C bugs), but maybe that should be changed.... But note that the exact order of property lookups for global variables, iframes, other stuff hanging off the window, the global scope polluter, etc, is pretty fragile.

Deleting is perhaps an option if we can guarantee that the property is one that was set up for this iframe.  Which is a separate fun question.  I do wonder what the spec says for this part.
(In reply to Boris Zbarsky (:bz) from comment #22)
> I think the spec does right now (assuming it's actually been written down
> and not just in various W3C bugs), but maybe that should be changed....

Correct, the spec requires this through the [[GetOwnProperty]] algorithm, which exposes named properties as own properties on the object.

> But
> note that the exact order of property lookups for global variables, iframes,
> other stuff hanging off the window, the global scope polluter, etc, is
> pretty fragile.

The order is well defined by specs currently.  Whether it is a good ordering is up for discussion, of course.

> Deleting is perhaps an option if we can guarantee that the property is one
> that was set up for this iframe.  Which is a separate fun question.  I do
> wonder what the spec says for this part.

So per the final resolution of http://www.w3.org/Bugs/Public/show_bug.cgi?id=8241, own expando properties on window (including variables) and properties from the prototype prevent named properties with the same names from being exposed.


The bits of specs that require all of this are http://dev.w3.org/2006/webapi/WebIDL/#getownproperty, which references http://dev.w3.org/2006/webapi/WebIDL/#dfn-named-property-visibility, and the HTML bit http://www.whatwg.org/specs/web-apps/current-work/multipage/browsers.html#named-access-on-the-window-object for the named property getter on Window (which is not marked [OverrideBuiltins], note).
OK, so my questions are:

1)  Is it possible to implement this behavior with a non-proxy ES object?  Only if certain
    "host object" hooks are overridden?
2)  If so, is it possible to override those hooks in spidermonkey?  Is the class get hook
    what corresponds to [[GetOwnProperty]]?
3)  How does all this work with "in" and the property descriptor gunk?  Do those use
    [[GetOwnProperty]] or Spidermonkey equivalent?

Jeff?  ;)
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → DUPLICATE
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: