Remove PostAttachCallbacks and replace with something more specialized

Remove PostAttachCallbacks and replace with something more specialized

The last consumer of PostAttachCallbacks is layer updating, this patch
switches away from the generic "call me after recalc style" system that
was PostAttachCallbacks to a very specialized system that only marks
elements as needing a style recalc for the purposes of creating and
destroying layers.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169891

[abarth-chromium, 2014-03-24 17:47:06]

lgtm

https://codereview.chromium.org/143983007/diff/250001/Source/core/dom/Documen...
File Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/143983007/diff/250001/Source/core/dom/Documen...
Source/core/dom/Document.cpp:1803: documentElement->recalcStyle(NoChange);
Can we really spin an arbitrary number of times here?

[ojan, 2014-03-24 19:39:15]

https://codereview.chromium.org/143983007/diff/250001/Source/core/dom/Document.h
File Source/core/dom/Document.h (right):

https://codereview.chromium.org/143983007/diff/250001/Source/core/dom/Documen...
Source/core/dom/Document.h:446: void unscheduleLayerUpdate(Element&);
The overloading of layer to mean RenderLayer and GraphicsLayer is super
confused. I'm not sure which one this is. I guess you're not creating that
problem here since scheduleLayerUpdate already exists on Element.

[esprehn, 2014-03-24 22:28:59]

The CQ bit was checked by esprehn@chromium.org

[commit-bot: I haz the power, 2014-03-24 22:29:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/esprehn@chromium.org/143983007/250001

[esprehn, 2014-03-24 22:29:12]

On 2014/03/24 17:47:06, abarth wrote:
> lgtm
> 
>
https://codereview.chromium.org/143983007/diff/250001/Source/core/dom/Documen...
> File Source/core/dom/Document.cpp (right):
> 
>
https://codereview.chromium.org/143983007/diff/250001/Source/core/dom/Documen...
> Source/core/dom/Document.cpp:1803: documentElement->recalcStyle(NoChange);
> Can we really spin an arbitrary number of times here?

Probably not, but just to be safe I added the loop.

[esprehn, 2014-03-24 22:31:50]

On 2014/03/24 19:39:15, ojan wrote:
>
https://codereview.chromium.org/143983007/diff/250001/Source/core/dom/Document.h
> File Source/core/dom/Document.h (right):
> 
>
https://codereview.chromium.org/143983007/diff/250001/Source/core/dom/Documen...
> Source/core/dom/Document.h:446: void unscheduleLayerUpdate(Element&);
> The overloading of layer to mean RenderLayer and GraphicsLayer is super
> confused. I'm not sure which one this is. I guess you're not creating that
> problem here since scheduleLayerUpdate already exists on Element.

It means both, scheduleLayerUpdate() means "create/destroy my RenderLayer if
needed now that the reasons for having a layer might have changed" and the
reason that keeps changing is "needs compositing".

It's silly though, we should always create a RenderLayer for those elements
(<canvas>, plugins, <video>, etc.) instead and let the compositing update
decided if they need to be composited or not. Having reasons outside recalcStyle
for creating a RenderLayer is a mess.

[ojan, 2014-03-24 22:52:00]

On 2014/03/24 22:29:12, esprehn wrote:
> On 2014/03/24 17:47:06, abarth wrote:
> > lgtm
> > 
> >
>
https://codereview.chromium.org/143983007/diff/250001/Source/core/dom/Documen...
> > File Source/core/dom/Document.cpp (right):
> > 
> >
>
https://codereview.chromium.org/143983007/diff/250001/Source/core/dom/Documen...
> > Source/core/dom/Document.cpp:1803: documentElement->recalcStyle(NoChange);
> > Can we really spin an arbitrary number of times here?
> 
> Probably not, but just to be safe I added the loop.

That's kinda scary. If possible, it'd be better to have this ASSERT that we do a
bounded number of loops. I can easily imagine someone writing some code that's
wrong, but that happens to end up working because we loop through this code a
bunch of times.

[commit-bot: I haz the power, 2014-03-24 23:59:34]

Change committed as 169891