500124 - Slow processing of innerHTML in a table body

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

The chrome test page is very slow to load. This is a regression from bug 495385. There are two problems (thanks to Boris for tracking them down):
-- AddTextItemIfNeeded doesn't initialize the item properly. I'm fixing this by having it just call AddFrameConstructionItems instead of trying to be clever and create an item directly.
-- Setting innerHTML removes child nodes one at a time. When we delete the first node in a block, we reframe the following whitespace text node even though it can still be suppressed, since it is now adjacent to the block start. In a table body, the reframe of the text node triggers reframing of the entire table body, which really hurts. I'm fixing this by having ContentRemoved avoid reframing text nodes that are now the first or last child.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 384808 [details] [diff] [review]
fix

>+nsCSSFrameConstructor::AddTextItemIfNeeded(nsFrameConstructorState& aState,
>+  AddFrameConstructionItems(aState, content, aContentIndex, aParentFrame, aItems);

This is probably fine.  It does a tiny bit more work than calling the Internal method directly, but that should be ok, I think.

>@@ -7361,21 +7354,29 @@ nsCSSFrameConstructor::ContentRemoved(ns
>+      PRInt32 prevSibling = aIndexInContainer - 1;

prevSiblingIndex, please.

I wonder whether we can add crashtests here somehow...  Performance bugs are hard to test.  :(

r+sr=bzbarskt with the nit about prevSibling fixed.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/mozilla-central/rev/fde224dd1e4d