1443380 - We don't need to mark frames modified when we recurse into InvalidateFrame

Status: RESOLVED FIXED

(Core :: Web Painting, enhancement, P2)

Description

[Matt Woodrow (:mattwoodrow)]

Attachment: dont-modify-all-descendants

There are two cases where this happens:

Table invalidation - Whenever we get an invalidation on a border-collapse table, we bubble the InvalidateFrame call up to the table root so that the nsTableBorderCollapse (that paints the backgrounds/borders for the table sub parts) receives the appropriate invalidation.
We only need the initial frame in the display list modified list, since building items for that frame will descend through the table root and build the nsTableBorderCollapse.

InvalidateFrameSubtree - This calls InvalidateFrame recursively on all descendants. We only need the initial frame modified, as we build all display items within the overflow rect, which includes all descendants.

Both of these cases occur on gmail, the former causes us to rebuild the entire mail list for every change within it (it's all one table), the latter causes us to hit our modified frame limit and rebuild the whole page.

Comment 1

[Miko Mynttinen]

Comment on attachment 8956312 [details] [diff] [review]
dont-modify-all-descendants

Review of attachment 8956312 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM.

::: layout/tables/nsTableCellFrame.cpp
@@ +437,5 @@
>    // don't overflow the cell
>    return nsDisplayItem::GetBounds(aBuilder, aSnap);
>  }
>  
> +void nsTableCellFrame::InvalidateFrame(uint32_t aDisplayItemKey, bool aRebuildDisplayItems)

InvalidateFrame() and InvalidateFrameWithRect() functions for nsTableCellFrame, nsTableColFrame, nsTableColGroupFrame, nsTableRowFrame, and nsTableRowGroupFrame seem to be same.

If it's not too much work, it would be nice to get rid of this duplicated code, or at least consolidate it in one place.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/36e0be4f9c14
Don't mark all descendant frames modified for display list building when invalidating a frame subtree since marking just the root is sufficient. r=miko

Comment 3

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/36e0be4f9c14