Closed Bug 237566 Opened 21 years ago Closed 21 years ago

Remove nsIContent::ReplaceChildAt codepath

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: sicking, Assigned: sicking)

References

Details

Attachments

(1 file)

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.
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
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
Closed: 21 years ago
Resolution: --- → FIXED
Blocks: 350795
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: