Last Comment Bug 237566 - Remove nsIContent::ReplaceChildAt codepath
: Remove nsIContent::ReplaceChildAt codepath
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Jonas Sicking (:sicking) PTO Until July 5th
: Hixie (not reading bugmail)
Mentors:
Depends on:
Blocks: 193298 224234 350795
  Show dependency treegraph
 
Reported: 2004-03-15 13:14 PST by Jonas Sicking (:sicking) PTO Until July 5th
Modified: 2004-04-12 17:18 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch to fix (77.22 KB, patch)
2004-03-15 13:16 PST, Jonas Sicking (:sicking) PTO Until July 5th
bzbarsky: review+
jst: superreview+
Details | Diff | Review

Description Jonas Sicking (:sicking) PTO Until July 5th 2004-03-15 13:14:29 PST
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.
Comment 1 Jonas Sicking (:sicking) PTO Until July 5th 2004-03-15 13:16:03 PST
Created attachment 143977 [details] [diff] [review]
patch to fix
Comment 2 Boris Zbarsky [:bz] (Out June 25-July 6) 2004-03-15 13:29:44 PST
I very much doubt I'll be able to review this until I come back from my trip
(early April).
Comment 3 Jonas Sicking (:sicking) PTO Until July 5th 2004-03-15 13:41:58 PST
That's fine, the tree won't unfreeze until then anyway
Comment 4 Johnny Stenback (:jst, jst@mozilla.com) 2004-03-29 16:01:49 PST
Comment on attachment 143977 [details] [diff] [review]
patch to fix

sr=jst
Comment 5 Boris Zbarsky [:bz] (Out June 25-July 6) 2004-04-03 10:09:12 PST
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.
Comment 6 Jonas Sicking (:sicking) PTO Until July 5th 2004-04-12 17:18:07 PDT
checked in, thanks for reviews!

Note You need to log in before you can comment on or make changes to this bug.