Closed
Bug 216721
Opened 21 years ago
Closed 21 years ago
Make mAttachedQueue a Stack
Categories
(Core :: XBL, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: timeless, Assigned: timeless)
References
Details
Attachments
(1 file, 1 obsolete file)
881 bytes,
patch
|
bryner
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
bz asked about this. i wanted to avoid doing it because i was afraid things like bug 215325 would happen. well they happened anyway. and amusingly enough for the case in that bug, doing this would 'fix' the problem. i'm not suggesting this change as a replacement for the change in that bug, but i'd like to make this change, and we can deal with fallout during 1.6a.
Attachment #130101 -
Flags: superreview?(bz-vacation)
Attachment #130101 -
Flags: review?(bryner)
Comment 2•21 years ago
|
||
Comment on attachment 130101 [details] [diff] [review] make the queue a stack >- nsCOMPtr<nsISupports> supp; >- mAttachedQueue->GetElementAt(0, getter_AddRefs(supp)); >- mAttachedQueue->RemoveElementAt(0); >+ while (NS_SUCCEEDED(mAttachedQueue->Count(&count)) && count--) { >+ nsCOMPtr<nsISupports> supp = do_QueryElementAt(mAttachedQueue, count); >+ mAttachedQueue->RemoveElementAt(count); > > nsCOMPtr<nsIXBLBinding> binding(do_QueryInterface(supp)); > if (binding) When I said you should use do_QueryElementAt I meant nsCOMPtr<nsIXBLBinding> binding(do_QueryElementAt(mAttachedQueue, count));
Comment 3•21 years ago
|
||
Anyway, the patch does appear to resolve bug 215325.
Attachment #130101 -
Attachment is obsolete: true
Comment 5•21 years ago
|
||
Comment on attachment 130129 [details] [diff] [review] make the queue a stack and really save a QI Transferring timeless' review requests to the patch I actually tested.
Attachment #130129 -
Flags: superreview?(bz-vacation)
Attachment #130129 -
Flags: review?(bryner)
Updated•21 years ago
|
Attachment #130101 -
Flags: review?(bryner)
Comment 6•21 years ago
|
||
timeless, how about a little more explanation here? For example, what problem does your patch solve?
Updated•21 years ago
|
Attachment #130101 -
Flags: superreview?(bz-vacation)
ok, here's the way this xbl stuff works (I think): mozilla encounters a <foo> and tries to construct it. <foo> contains a <bar>, so mozilla considers constructing <bar>. in the original code it would put <bar> on the stack, call the cleanup function (which causes recursion/crashes) which cleaned out <bar>. in the current code it puts <bar> in the queue and finishes off <foo>, albeit unhappily because it doesn't construct <bar> in time, resulting in bugs like bug 215325. with the patch we get the old behavior where <foo> pauses, adds <bar> to the list, and processes <bar>.
Comment 8•21 years ago
|
||
Comment on attachment 130129 [details] [diff] [review] make the queue a stack and really save a QI Looks good, assuming we're all in agreement that this restores the original firing order of the constructors.
Attachment #130129 -
Flags: review?(bryner) → review+
Comment 9•21 years ago
|
||
Comment on attachment 130129 [details] [diff] [review] make the queue a stack and really save a QI sr=bzbarsky if you also rename mAttachedQueue and mProcessningAttachedQueue to reflect the fact that it's a stack, not a queue.
Attachment #130129 -
Flags: superreview?(bz-vacation) → superreview+
Assignee | ||
Comment 10•21 years ago
|
||
checked in
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•