Rework composited scrolling updates.

There is a plan afoot to call RenderLayer::updateCompositingLayers less frequently and in a well defined phase. This patch reworks the updating of composited scrolling to be tied to updateCompositingLayers. Rather than updating m_needsCompositedScrolling whenever it may change, we simply dirty state and clean it in updateCompositingLayers.


BUG=

[hartmanng, 2013-04-10 17:34:05]

https://codereview.chromium.org/13859006/diff/2001/Source/WebCore/rendering/R...
File Source/WebCore/rendering/RenderLayer.cpp (right):

https://codereview.chromium.org/13859006/diff/2001/Source/WebCore/rendering/R...
Source/WebCore/rendering/RenderLayer.cpp:151: ,
m_descendantsAreContiguousInStackingOrder(false)
should be true by default

https://codereview.chromium.org/13859006/diff/2001/Source/WebCore/rendering/R...
Source/WebCore/rendering/RenderLayer.cpp:1973:
HistogramSupport::histogramEnumeration("Renderer.NeedsCompositedScrolling",
needsCompositedScrolling, 2);
this should only be set for scrollable divs, not all divs

https://codereview.chromium.org/13859006/diff/2001/Source/WebCore/rendering/R...
Source/WebCore/rendering/RenderLayer.cpp:5521: void
RenderLayer::dirtyStackingContainerZOrderLists()
rename to dirtyStackingContextZOrderLists. Also add comment to explain why we're
looping now.

https://codereview.chromium.org/13859006/diff/2001/Source/WebCore/rendering/R...
Source/WebCore/rendering/RenderLayer.cpp:6088: if (hasOverflow &&
isVisibleToHitTest)
frameView->{add,remove}ScrollableArea() no longer need to return values.

[Ian Vollick, 2013-04-11 16:36:03]

James, Julien, can you take a look at this?

With this patch, I now just set dirty bits, clean the state at a well defined
point, and ASSERT that we don't access the state when we shouldn't.

I've managed to get the layout tests to succeed in debug. I've also run chrome
in debug and loaded gmail, and things seem to be working fine.

But to get to this point, I had to reorder/remove some updates in RenderLayer,
and I'm not convinced that these changes are safe. There are some FIXME's
pointing these spots out.

Also, we issue a lot of repaints as part of style recalc. These repaints need to
know about stacking containers in order to find repaint containers. I've
kludgily hacked the code so that we don't assert in this case -- I figured that
it's ok to use the old opt-in values from the last from when we're doing these
repaints, but I could be wrong. Please let me know.

https://chromiumcodereview.appspot.com/13859006/diff/2001/Source/WebCore/rend...
File Source/WebCore/rendering/RenderLayer.cpp (right):

https://chromiumcodereview.appspot.com/13859006/diff/2001/Source/WebCore/rend...
Source/WebCore/rendering/RenderLayer.cpp:151: ,
m_descendantsAreContiguousInStackingOrder(false)
On 2013/04/10 17:34:05, hartmanng wrote:
> should be true by default

Done.

https://chromiumcodereview.appspot.com/13859006/diff/2001/Source/WebCore/rend...
Source/WebCore/rendering/RenderLayer.cpp:1973:
HistogramSupport::histogramEnumeration("Renderer.NeedsCompositedScrolling",
needsCompositedScrolling, 2);
On 2013/04/10 17:34:05, hartmanng wrote:
> this should only be set for scrollable divs, not all divs

Done.

https://chromiumcodereview.appspot.com/13859006/diff/2001/Source/WebCore/rend...
Source/WebCore/rendering/RenderLayer.cpp:5521: void
RenderLayer::dirtyStackingContainerZOrderLists()
On 2013/04/10 17:34:05, hartmanng wrote:
> rename to dirtyStackingContextZOrderLists. Also add comment to explain why
we're
> looping now.
I think the function name still works, but I explain in the comment why this
affects the stacking context as well.

[hartmanng, 2013-04-12 03:51:42]

Apart from the comment, lgtm

https://chromiumcodereview.appspot.com/13859006/diff/2001/Source/WebCore/rend...
File Source/WebCore/rendering/RenderLayer.cpp (right):

https://chromiumcodereview.appspot.com/13859006/diff/2001/Source/WebCore/rend...
Source/WebCore/rendering/RenderLayer.cpp:6088: if (hasOverflow &&
isVisibleToHitTest)
On 2013/04/10 17:34:05, hartmanng wrote:
> frameView->{add,remove}ScrollableArea() no longer need to return values.

I think you missed this comment

[hartmanng, 2013-04-12 03:51:43]

Apart from the comment, lgtm