Closed
Bug 350030
Opened 18 years ago
Closed 18 years ago
Change insertion point lists from nsVoidArray to something which is not manually refcounted
Categories
(Core :: XBL, defect)
Core
XBL
Tracking
()
RESOLVED
FIXED
People
(Reporter: smaug, Assigned: smaug)
References
Details
Attachments
(2 files, 1 obsolete file)
28.62 KB,
patch
|
Details | Diff | Splinter Review | |
28.74 KB,
patch
|
Details | Diff | Splinter Review |
Assignee | ||
Comment 1•18 years ago
|
||
Anyone willing to review ;) (I won't check in this before Bug 205735 is in branches.)
Assignee | ||
Updated•18 years ago
|
Attachment #235255 -
Flags: review?(jst)
Comment 2•18 years ago
|
||
Comment on attachment 235255 [details] [diff] [review] Using nsTArray<nsRefPtr<nsXBLInsertionPoint> > - In BuildContentLists(): ContentListData* data = (ContentListData*)aClosure; [...] + nsInsertionPointList* contentList = new nsInsertionPointList; + if (!contentList) { + // XXX How to handle OOM properly in this case? + return PL_DHASH_NEXT; + } Maybe add a nsresult member to ContentListData and set that to NS_ERROR_OUT_OF_MEMORY and stop enumerating? r=jst.
Attachment #235255 -
Flags: review?(jst) → review+
Assignee | ||
Comment 3•18 years ago
|
||
mRv in ContentListData. nsXBLBinding::GenerateAnonymousContent() should not be a void method, but there are also other methods which should be changed to return nsresult, so different bug IMO.
Attachment #235255 -
Attachment is obsolete: true
Attachment #238988 -
Flags: superreview?(bzbarsky)
Comment 4•18 years ago
|
||
Comment on attachment 238988 [details] [diff] [review] v2 >Index: content/xbl/src/nsBindingManager.cpp > class nsAnonymousContentList : public nsGenericDOMNodeList > nsXBLInsertionPoint* GetInsertionPointAt(PRInt32 i) { return NS_STATIC_CAST(nsXBLInsertionPoint*, mElements->ElementAt(i)); } That cast can go away, right? >Index: content/xbl/src/nsXBLBinding.cpp >+BuildContentLists(nsISupports* aKey, > // Add the currPoint to the supports array. >- NS_IF_ADDREF(currPoint); > contentList->AppendElement(currPoint); Fix comment to reflect reality? > // Add in all the remaining insertion points. > for ( ; j < count; j++) { >- currPoint = NS_STATIC_CAST(nsXBLInsertionPoint*, arr->ElementAt(j)); >- NS_IF_ADDREF(currPoint); >+ currPoint = aData->ElementAt(j); > contentList->AppendElement(currPoint); > } Why not just: contentList->AppendElements(aData->Elements() + j; count - j); ? That seems like it should do the same thing.... sr=bzbarsky with those nits.
Attachment #238988 -
Flags: superreview?(bzbarsky)
Assignee | ||
Comment 5•18 years ago
|
||
Assignee | ||
Updated•18 years ago
|
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•