Closed
Bug 311022
Opened 19 years ago
Closed 19 years ago
Greasemonkey 0.6.2 exposes window wrapper proliferation when enumerating
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla1.8beta5
People
(Reporter: brendan, Assigned: mrbkap)
References
Details
(Keywords: fixed1.8)
Attachments
(1 file, 1 obsolete file)
1.39 KB,
patch
|
bzbarsky
:
review+
jst
:
superreview+
brendan
:
approval1.8b5+
|
Details | Diff | Splinter Review |
This is probably a split window bug exposed by the fix for bug 308856. We see stacks overflowing via MarkSharpObjects, starting with a window.arguments Array, descending through 0 to a window object, through its 'window' property to a new and different flat JSObject for an XPCNativeWrapper for the same native identity pointer, etc. etc. Somewhere under nsWindowSH::GetProperty we are rewrapping when we should be finding a native wrapper. Or perhaps we're doing some outer=>inner=>outer dance that causes the same result. /be
Reporter | ||
Updated•19 years ago
|
Flags: blocking1.8b5+
Target Milestone: --- → mozilla1.8beta5
Assignee | ||
Comment 1•19 years ago
|
||
bz pointed me at this on IRC. We can't fall into the GetNewOrUsed case because that always returns an implicit wrapper. This patch prevents us from creating lots of new wrappers for the same object, and simply returns the one that we've already created.
Assignee | ||
Updated•19 years ago
|
Attachment #198513 -
Flags: review?(brendan)
Assignee | ||
Comment 2•19 years ago
|
||
The other patch was an old one, this one is current.
Attachment #198513 -
Attachment is obsolete: true
Attachment #198514 -
Flags: superreview?(jst)
Attachment #198514 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•19 years ago
|
Attachment #198513 -
Flags: review?(brendan)
Comment 3•19 years ago
|
||
Comment on attachment 198514 [details] [diff] [review] Move the test to where it's needed sr=jst
Attachment #198514 -
Flags: superreview?(jst) → superreview+
Comment 4•19 years ago
|
||
Comment on attachment 198514 [details] [diff] [review] Move the test to where it's needed r=bzbarsky. This should probably go in on branch too -- it's a very safe fix and makes greasemonkey a lot happier.
Attachment #198514 -
Flags: review?(bzbarsky)
Attachment #198514 -
Flags: review+
Attachment #198514 -
Flags: approval1.8b5?
Reporter | ||
Comment 5•19 years ago
|
||
Comment on attachment 198514 [details] [diff] [review] Move the test to where it's needed a=me, we have high confidence in spite of high regression rate overall. This code is better than the average, and the patch is only gonna make the bad case go away. /be
Attachment #198514 -
Flags: approval1.8b5? → approval1.8b5+
Assignee | ||
Comment 6•19 years ago
|
||
Fix checked in, MOZILLA_1_8_BRANCH and trunk.
You need to log in
before you can comment on or make changes to this bug.
Description
•