If a page contains two ThebesLayers, one vertically above the other, and we scroll down, then when the topmost ThebesLayer scrolls out of view, it will be recycled to render the display items of the second ThebesLayer (since all the display items that previously pulled it off the recycle list are gone), so we'll be forced to reallocate and rerender the remaining visible display items.

Stealing.

Here's my plan: At the moment, a ThebesLayerData item is assigned a layer on creation. I'd like to change this so that a ThebesLayerData item can be in one of two phases: Phase I without an assigned layer and phase II with a layer. Phase II would be entered as soon as the ThebesLayerData item is assigned a display item for which we know its old layer - at this point, this old layer would be assigned to the ThebesLayerData item. The result of this setup would be that a ThebesLayerData item gets the old layer of the bottommost display item assigned to it which had been assigned to a layer on the previous paint. If the ThebesLayerData item only contains new display items, create a new layer for it. We know that we are done with assigning display items to a ThebesLayerData item when this ThebesLayerData item gets popped in PopThebesLayerData - that's where the new layer would be created. This setup can create new layers even when there would have been an unused layer available for recycling. But that case can only occur when the old and the new layer share no common display items, so everything would be invalidated anyway. Implementation notes: While in phase I, the display items need to be kept somewhere, before they can be streamed to the layer on entering phase II. An array mAssignedDisplayItems on ThebesLayerData should do the trick. Though it looks like we need to store pairs (nsDisplayItem*, LayerState) if we don't want to recompute the layer state for every queued display item when entering phase II. mNewChildLayers is a bit tricky. We can't add a layer to it on ThebesLayerData creation time because we might not have one at that point. But we also can't just remember the index at which to insert it later when we have the layer, because things can already have shifted underneath us, for example because a ThebesLayerData under us was popped and caused an ImageLayer or ColorLayer to be added to mNewChildLayers. One way to address this would be to make mNewChildLayers instead an array of LayerSlots, where LayerSlot would be +class LayerSlot { + NS_INLINE_DECL_REFCOUNTING(LayerSlot) +public: + LayerSlot(Layer* aLayer) : mLayer(aLayer) {} + nsRefPtr<Layer> mLayer; +}; and each ThebesLayerData item gets a reference to its layer slot, and when it knows its layer it can set it on its slot. That would keep the layer order correct. Another problem is that FindOpaqueBackgroundColorFor needs to iterate over a ThebesLayerData item's assigned display items, and it needs to be able to do that regardless of whether each ThebesLayerData is in phase I or phase II. So ThebesLayerData should probably expose a common iterator for its display items that encapsulates the phase difference. Or we could keep assigning to ThebesLayerData::mAssignedDisplayItems even during phase II, but that might be expensive. Does this sound sensible? Did you have something similar in mind? Any other suggestions?

That all sounds good to me.

These patches don't help with bug 1128342, removing the dependency.

/r/4679 - Bug 913443 - Remove mention of the word ThebesLayer in a comment. r=roc /r/4681 - Bug 913443 - Remove some #ifdefs. r=roc /r/4683 - Bug 913443 - Break up CreateOrRecyclePaintedLayer into more parts. r=roc /r/4685 - Bug 913443 - Extract layer hint calculation. r=roc /r/4687 - Bug 913443 - Break CreateOrRecyclePaintedLayer up even more. r=roc /r/4689 - Bug 913443 - Remove unused aItemVisibleRect argument. r=roc /r/4691 - Bug 913443 - Move IsWidgetLayerManager() check out of UpdateCommonClipCount. r=roc /r/4693 - Bug 913443 - Change the order of these calls. r=roc /r/4695 - Bug 913443 - Add a display item buffer for PaintedLayerData so that we can assign items without needing to know the actual Layer. r=roc /r/4697 - Bug 913443 - Delay PaintedLayer recycling until PopPaintedLayerData(). r=roc /r/4699 - Bug 913443 - When determining the layer to recycle, only consider layers that have display items in common with the layer we need. r=roc /r/4701 - Bug 913443 - Recycle PaintedLayers as soon as possible. r=roc /r/4703 - Bug 913443 - Remove duplicated argument variables. r=roc /r/4705 - Bug 913443 - Add some tests. r=roc Pull down these commits: hg pull review -r fba9fab73e132a6ba1f39f252a8310ab085cf53e

Comment on attachment 8572836 [details] MozReview Request: bz://913443/mstange https://reviewboard.mozilla.org/r/4677/#review3849 This is really good!

Well, that was fun. https://hg.mozilla.org/integration/mozilla-inbound/rev/9a4d7e0a8e55 https://hg.mozilla.org/integration/mozilla-inbound/rev/e73ab1c0c9d1 https://hg.mozilla.org/integration/mozilla-inbound/rev/add34200e05c https://hg.mozilla.org/integration/mozilla-inbound/rev/cfed9e01504d https://hg.mozilla.org/integration/mozilla-inbound/rev/396bfdd497cd https://hg.mozilla.org/integration/mozilla-inbound/rev/66fe954cbf8c https://hg.mozilla.org/integration/mozilla-inbound/rev/eaa3b47fe9de https://hg.mozilla.org/integration/mozilla-inbound/rev/c58f78777ab1 https://hg.mozilla.org/integration/mozilla-inbound/rev/3f506b4436e5 https://hg.mozilla.org/integration/mozilla-inbound/rev/ce500c03c83e https://hg.mozilla.org/integration/mozilla-inbound/rev/a472533d75f3 https://hg.mozilla.org/integration/mozilla-inbound/rev/04ce9ea7f3d8 https://hg.mozilla.org/integration/mozilla-inbound/rev/2e30759c8069 https://hg.mozilla.org/integration/mozilla-inbound/rev/2bd95d9c12bb

https://hg.mozilla.org/mozilla-central/rev/9a4d7e0a8e55 https://hg.mozilla.org/mozilla-central/rev/e73ab1c0c9d1 https://hg.mozilla.org/mozilla-central/rev/add34200e05c https://hg.mozilla.org/mozilla-central/rev/cfed9e01504d https://hg.mozilla.org/mozilla-central/rev/396bfdd497cd https://hg.mozilla.org/mozilla-central/rev/66fe954cbf8c https://hg.mozilla.org/mozilla-central/rev/eaa3b47fe9de https://hg.mozilla.org/mozilla-central/rev/c58f78777ab1 https://hg.mozilla.org/mozilla-central/rev/3f506b4436e5 https://hg.mozilla.org/mozilla-central/rev/ce500c03c83e https://hg.mozilla.org/mozilla-central/rev/a472533d75f3 https://hg.mozilla.org/mozilla-central/rev/04ce9ea7f3d8 https://hg.mozilla.org/mozilla-central/rev/2e30759c8069 https://hg.mozilla.org/mozilla-central/rev/2bd95d9c12bb

Blocking a blocker.

So whats the plan here since you removed the nomination without any explanation? We need this to fix bug 1146137 according to https://bugzilla.mozilla.org/show_bug.cgi?id=1146137#c13

(In reply to Gregor Wagner [:gwagner] from comment #26) > So whats the plan here since you removed the nomination without any > explanation? We need this to fix bug 1146137 according to > https://bugzilla.mozilla.org/show_bug.cgi?id=1146137#c13 Gregor, I thought it just for mozilla39. I see your point. I noted as 2.2+, could you help for uplift request?