72284 2011-11-14 09:53:38 -0800 ASSERT in RenderLayerCompositor on /fast/multicol/pagination-v-horizontal-bt.html 2013-03-05 10:56:58 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified NEW P2 Normal --- 1 menard webkit-unassigned hausmann hyatt kbalazs kenneth simon.fraser oldest_to_newest 502025 0 menard 2011-11-14 09:53:38 -0800 ASSERT in RenderLayerCompositor with Qt port. 502029 1 114972 menard 2011-11-14 09:56:19 -0800 Created attachment 114972 Patch 502037 2 menard 2011-11-14 10:01:45 -0800 ./WebKitTestRunner --verbose ../../../webkit/LayoutTests/fast/multicol/pagination-v-horizontal-bt.html assert with the Qt port on WK2. We use AC by default now. The assert is 0x00007f35eb3ba376 in WebCore::RenderLayerCompositor::computeCompositingRequirements (this=0x7f359403f7f0, layer=0x7f359403f6b8, overlapMap=0x7fff43cb51d0, compositingState=..., layersChanged=@0x7fff43cb523f) at ../../../../webkit/Source/WebCore/rendering/RenderLayerCompositor.cpp:758 758 ASSERT(willBeComposited == needsToBeComposited(layer)); The test triggers a style change in the root layer moving from a normal flow only to a different one. I'm not very familiar with the code here but it seems that if a layer is not normal flow only it can't be composited but computeCompositingRequirements assume that if we are in composition mode (the state we are before) and AC is turned on (which is always true) then the root layer will be composited but according to canBeComposited the layer can be composited only if layer->isSelfPaintingLayer(); is true which is not true anymore as the latter check !isNormalFlowOnly(). Any suggestion on how I can improve the patch is welcome, or if I spot the right problem in the first place. Thanks. 502040 3 simon.fraser 2011-11-14 10:03:46 -0800 canBeComposited() can return false for the root layer? That would break all kinds of stuff. 502042 4 menard 2011-11-14 10:05:49 -0800 (In reply to comment #3) > canBeComposited() can return false for the root layer? That would break all kinds of stuff. It doesn't seem that there is specific check for that. The root layer should be always composited not matter the style? 502288 5 simon.fraser 2011-11-14 14:06:08 -0800 (In reply to comment #4) > (In reply to comment #3) > > canBeComposited() can return false for the root layer? That would break all kinds of stuff. > > It doesn't seem that there is specific check for that. The root layer should be always composited not matter the style? If there are any other compositing layers, the root layer is composited, yes. If that's no longer true for multicol, then we'll have to fix it somehow. 525484 6 simon.fraser 2011-12-21 12:44:51 -0800 *** Bug 74999 has been marked as a duplicate of this bug. *** 525608 7 kbalazs 2011-12-21 14:33:40 -0800 (In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > canBeComposited() can return false for the root layer? That would break all kinds of stuff. > > > > It doesn't seem that there is specific check for that. The root layer should be always composited not matter the style? > > If there are any other compositing layers, the root layer is composited, yes. If that's no longer true for multicol, then we'll have to fix it somehow. But we are dealing with the root layer here, rigth? So, there is no other compositing layers yet, and we are deciding whether to composite the root layer. But ts is normalFlowOnly() => !isSelfPainting(), and canBeComposited say that such layers cannot be composited. But I guess that rather means such layers _should_ not be composited, because they are too primitive to be worth to composite them. Note that canBeComposited is called from needToBeComposited. Furthermore, there was no check for whether it is a root layer before http://trac.webkit.org/changeset/63149. I think the preassumption that the root layer must be composited if we have entered into compositing mode is wrong. I don't know exactly what it means "entering into compositing mode" but it seems to me that it doesn't mean to force compositing to all kinds of content. Well, I'm not familiar with the AC machinery, I just try to interpret what I see :) What do you think? 526102 8 kbalazs 2011-12-22 01:41:23 -0800 Test results (locally, debug) with the patch: no unexpected regression, no crash on pagination-* tests. 114972 2011-11-14 09:56:19 -0800 2011-12-21 12:45:09 -0800 Patch bug-72284-20111114145609.patch text/plain 1810 menard U3VidmVyc2lvbiBSZXZpc2lvbjogMTAwMTIzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZjA1YTM1YjgzZDFmYTlj ZGEwYWJkOWFlZTkwZmU4ZTFlZjA3ZmViOS4uMWI5MjU0OWFmOTIxYmMzY2RmZDlmYTcyYTA1MDg1 M2MwNGE4MWNhNCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDExLTExLTE0ICBBbGV4 aXMgTWVuYXJkICA8YWxleGlzLm1lbmFyZEBvcGVuYm9zc2Eub3JnPgorCisgICAgICAgIEFTU0VS VCBpbiBSZW5kZXJMYXllckNvbXBvc2l0b3Igd2l0aCBRdCBwb3J0LgorICAgICAgICBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NzIyODQKKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBXaGVuIHRoZSBzdHlsZSBvZiB0aGUgbGF5 ZXIgY2hhbmdlZCBmcm9tIGEgbm9ybWFsIGZsb3cgb25seQorICAgICAgICB0byBzb21ldGhpbmcg ZWxzZSB3ZSBuZWVkIHRvIG1ha2Ugc3VyZSB0aGF0IHRoZSByb290IGxheWVyCisgICAgICAgIGNh biBiZSBjb21wb3NpdGVkICh3aGljaCBpcyBub3QgdGhlIGNhc2Ugd2hlbiB0aGUgZmxvdyBpcyBu b3QKKyAgICAgICAgb25seSBub3JtYWwpLgorCisgICAgICAgIE5vIG5ldyB0ZXN0cyA6IGV4aXN0 aW5nIG9uZXMgY292ZXIgaXQuCisKKyAgICAgICAgKiByZW5kZXJpbmcvUmVuZGVyTGF5ZXJDb21w b3NpdG9yLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlJlbmRlckxheWVyQ29tcG9zaXRvcjo6Y29t cHV0ZUNvbXBvc2l0aW5nUmVxdWlyZW1lbnRzKToKKwogMjAxMS0xMS0xMSAgUGllcnJlIFJvc3Np ICA8cGllcnJlLnJvc3NpQGdtYWlsLmNvbT4KIAogICAgICAgICBbUXRdIFJlbW92ZSB0aGUgUVN0 eWxlIGRlcGVuZGVuY3kgaW4gUXQncyBtb2JpbGUgdGhlbWUKZGlmZiAtLWdpdCBhL1NvdXJjZS9X ZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJMYXllckNvbXBvc2l0b3IuY3BwIGIvU291cmNlL1dlYkNv cmUvcmVuZGVyaW5nL1JlbmRlckxheWVyQ29tcG9zaXRvci5jcHAKaW5kZXggZjkzYTM5OWJlMTEz YTU3YzVlZDVmYzJlY2RlY2Y5ZjFjY2JhYjA3NC4uNTQwMTZkM2EzMDIwZjIzZmE4NWZlYzI2MTgz ODM5NmRiMWJlMjNhZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRl ckxheWVyQ29tcG9zaXRvci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRl ckxheWVyQ29tcG9zaXRvci5jcHAKQEAgLTc0OSw3ICs3NDksNyBAQCB2b2lkIFJlbmRlckxheWVy Q29tcG9zaXRvcjo6Y29tcHV0ZUNvbXBvc2l0aW5nUmVxdWlyZW1lbnRzKFJlbmRlckxheWVyKiBs YXllciwgTwogICAgIAogICAgIC8vIElmIHdlIGp1c3QgZW50ZXJlZCBjb21wb3NpdGluZyBtb2Rl LCB0aGUgcm9vdCB3aWxsIGhhdmUgYmVjb21lIGNvbXBvc2l0ZWQgKGFzIGxvbmcgYXMgYWNjZWxl cmF0ZWQgY29tcG9zaXRpbmcgaXMgZW5hYmxlZCkuCiAgICAgaWYgKGxheWVyLT5pc1Jvb3RMYXll cigpKSB7Ci0gICAgICAgIGlmIChpbkNvbXBvc2l0aW5nTW9kZSgpICYmIG1faGFzQWNjZWxlcmF0 ZWRDb21wb3NpdGluZykKKyAgICAgICAgaWYgKGluQ29tcG9zaXRpbmdNb2RlKCkgJiYgbV9oYXNB Y2NlbGVyYXRlZENvbXBvc2l0aW5nICYmIGNhbkJlQ29tcG9zaXRlZChsYXllcikpCiAgICAgICAg ICAgICB3aWxsQmVDb21wb3NpdGVkID0gdHJ1ZTsKICAgICB9CiAgICAgCg==