Remove nsIContent::ReplaceChildAt codepath

RESOLVED FIXED

Status

()

Core
DOM
RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: sicking, Assigned: sicking)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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.
Created attachment 143977 [details] [diff] [review]
patch to fix
Attachment #143977 - Flags: superreview?(jst)
Attachment #143977 - Flags: review?(bzbarsky)
I very much doubt I'll be able to review this until I come back from my trip
(early April).
That's fine, the tree won't unfreeze until then anyway
Blocks: 193298
Blocks: 224234
Comment on attachment 143977 [details] [diff] [review]
patch to fix

sr=jst
Attachment #143977 - Flags: superreview?(jst) → superreview+
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+
Blocks: 76994
No longer blocks: 76994
checked in, thanks for reviews!
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED

Updated

11 years ago
Blocks: 350795
You need to log in before you can comment on or make changes to this bug.