Closed
Bug 237566
Opened 21 years ago
Closed 21 years ago
Remove nsIContent::ReplaceChildAt codepath
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: sicking, Assigned: sicking)
References
Details
Attachments
(1 file)
(deleted),
patch
|
bzbarsky
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
The only place nsIContent::ReplaceChildAt is used is when someone calls
nsIDOMNode::ReplaceChild. However that function is called very rarly, both on
the web and in our chrome.
And while we in theory can save cycles by having a separate replace-path, the
place where we can save most cycles, the nsCSSFrameConstructor, doesn't really
take advantage of this. And it's very debatable if it should seing how rarly
it's used.
Also, since the codepath is so rarly used there's a big risk for bugs. Bug
193298 is one example.
Patch removing nsIContent::ReplaceChildAt and
nsIDocumentObserver::ContentReplaced comming up.
Assignee | ||
Comment 1•21 years ago
|
||
Assignee | ||
Updated•21 years ago
|
Attachment #143977 -
Flags: superreview?(jst)
Attachment #143977 -
Flags: review?(bzbarsky)
Comment 2•21 years ago
|
||
I very much doubt I'll be able to review this until I come back from my trip
(early April).
Assignee | ||
Comment 3•21 years ago
|
||
That's fine, the tree won't unfreeze until then anyway
Comment 4•21 years ago
|
||
Comment on attachment 143977 [details] [diff] [review]
patch to fix
sr=jst
Attachment #143977 -
Flags: superreview?(jst) → superreview+
Comment 5•21 years ago
|
||
Comment on attachment 143977 [details] [diff] [review]
patch to fix
>Index: content/base/src/nsGenericElement.cpp
>@@ -2883,22 +2845,15 @@ nsGenericElement::doReplaceChild(nsICont
> nsCOMPtr<nsIContent> oldContent(do_QueryInterface(aOldChild, &res));
>
>- if (NS_FAILED(res)) {
Don't bother writing to res if you won't check it, ok?
r=bzbarsky with that change. Looks good.
Attachment #143977 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 6•21 years ago
|
||
checked in, thanks for reviews!
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•