171643 2017-05-03 20:17:39 -0700 Should never hit layout while updating the render tree. 2017-05-04 23:48:37 -0700 1 1 1 Unclassified WebKit Layout and Rendering WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=171717 P2 Normal --- 1 zalan zalan bfulgham commit-queue koivisto simon.fraser zalan oldest_to_newest 1304483 0 zalan 2017-05-03 20:17:39 -0700 Running layout on a half-backed tree is not the greatest idea. We'd better ASSERT on it. 1304486 1 309007 zalan 2017-05-03 20:24:31 -0700 Created attachment 309007 Patch 1304494 2 309007 bfulgham 2017-05-03 21:03:47 -0700 Comment on attachment 309007 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=309007&action=review > Source/WebCore/page/FrameView.cpp:1291 > + ASSERT(!frame().document()->inRenderTreeUpdate()); Should this be a release assert so we can't continue if this happens? 1304498 3 309007 simon.fraser 2017-05-03 21:12:18 -0700 Comment on attachment 309007 Patch What about asserting about layouts in render tree update, and about style recall in render tree update? Seems like we really want to have more stateful asserting. 1304500 4 309007 zalan 2017-05-03 21:16:05 -0700 Comment on attachment 309007 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=309007&action=review >> Source/WebCore/page/FrameView.cpp:1291 >> + ASSERT(!frame().document()->inRenderTreeUpdate()); > > Should this be a release assert so we can't continue if this happens? That might work too (though we could actually finish the layout just fine), but it should be at least assertion with security implication. 1304503 5 309009 zalan 2017-05-03 21:18:16 -0700 Created attachment 309009 Patch 1304621 6 zalan 2017-05-04 08:03:08 -0700 (In reply to zalan from comment #4) > Comment on attachment 309007 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=309007&action=review > > >> Source/WebCore/page/FrameView.cpp:1291 > >> + ASSERT(!frame().document()->inRenderTreeUpdate()); > > > > Should this be a release assert so we can't continue if this happens? > > That might work too (though we could actually finish the layout just fine), > but it should be at least assertion with security implication. Actually I think this should be part of the "why don't we turn all assert_with_security_implication into release asserts" conversation. 1304712 7 309009 commit-queue 2017-05-04 11:27:12 -0700 Comment on attachment 309009 Patch Clearing flags on attachment: 309009 Committed r216196: <http://trac.webkit.org/changeset/216196> 1304713 8 commit-queue 2017-05-04 11:27:14 -0700 All reviewed patches have been landed. Closing bug. 309007 2017-05-03 20:24:31 -0700 2017-05-03 21:18:14 -0700 Patch bug-171643-20170503202430.patch text/plain 1587 zalan U3VidmVyc2lvbiBSZXZpc2lvbjogMjE2MTU5CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMTAzNjZmYzFmMzg0MzY5 YThjNmMzYTMwODRkNzFkZWY1Mzk3NDNmZC4uZDBiNDg0NjU3NTQ1ZTU0N2YxNWQ1OWFkMmEyOTEx NmY2MDkzZjY4YSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDE4IEBACiAyMDE3LTA1LTAzICBaYWxh biBCdWp0YXMgIDx6YWxhbkBhcHBsZS5jb20+CiAKKyAgICAgICAgU2hvdWxkIG5ldmVyIGhpdCBs YXlvdXQgd2hpbGUgdXBkYXRpbmcgdGhlIHJlbmRlciB0cmVlLgorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTcxNjQzCisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTGF5aW5nIG91dCBhIGhhbGYtYmFrZWQgcmVu ZGVyIHRyZWUgaXMgbm90IGEgZ3JlYXQgaWRlYS4gRXNwZWNpYWxseSBjb25zaWRlcmluZworICAg ICAgICB0aGF0IGxheW91dCAoc2FkbHkpIGNhbiBtdXRhdGUgdGhlIHJlbmRlciB0cmVlLiAgCisK KyAgICAgICAgKiBwYWdlL0ZyYW1lVmlldy5jcHA6CisgICAgICAgIChXZWJDb3JlOjpGcmFtZVZp ZXc6OmxheW91dCk6CisKKzIwMTctMDUtMDMgIFphbGFuIEJ1anRhcyAgPHphbGFuQGFwcGxlLmNv bT4KKwogICAgICAgICBTZWFyY2hJbnB1dFR5cGUgY291bGQgZW5kIHVwIHdpdGggYSBtaXNtYXRj aGVkIHJlbmRlcmVyLgogICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9MTcxNTQ3CiAgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8zMTkzNTA0Nz4KZGlmZiAtLWdp dCBhL1NvdXJjZS9XZWJDb3JlL3BhZ2UvRnJhbWVWaWV3LmNwcCBiL1NvdXJjZS9XZWJDb3JlL3Bh Z2UvRnJhbWVWaWV3LmNwcAppbmRleCBjZTg3YzYyMzlmMDAxYzI0YzY5OWI0ZTA0MWJlYTVjMTIw NDEzOWYwLi44YTc3M2RiNjc1NDM3MzM2MjhiYjk4YTRmZjFlZWNhOTFhODEyY2ZkIDEwMDY0NAot LS0gYS9Tb3VyY2UvV2ViQ29yZS9wYWdlL0ZyYW1lVmlldy5jcHAKKysrIGIvU291cmNlL1dlYkNv cmUvcGFnZS9GcmFtZVZpZXcuY3BwCkBAIC0xMjg4LDYgKzEyODgsOCBAQCBpbmxpbmUgdm9pZCBG cmFtZVZpZXc6OmZvcmNlTGF5b3V0UGFyZW50Vmlld0lmTmVlZGVkKCkKIAogdm9pZCBGcmFtZVZp ZXc6OmxheW91dChib29sIGFsbG93U3VidHJlZSkKIHsKKyAgICBBU1NFUlQoIWZyYW1lKCkuZG9j dW1lbnQoKS0+aW5SZW5kZXJUcmVlVXBkYXRlKCkpOworCiAgICAgTE9HKExheW91dCwgIkZyYW1l VmlldyAlcCAoJWR4JWQpIGxheW91dCwgbWFpbiBmcmFtZXZpZXcgJWQsIGFsbG93U3VidHJlZT0l ZCIsIHRoaXMsIHNpemUoKS53aWR0aCgpLCBzaXplKCkuaGVpZ2h0KCksIGZyYW1lKCkuaXNNYWlu RnJhbWUoKSwgYWxsb3dTdWJ0cmVlKTsKICAgICBpZiAoaXNJblJlbmRlclRyZWVMYXlvdXQoKSkg ewogICAgICAgICBMT0coTGF5b3V0LCAiICBpbiBsYXlvdXQsIGJhaWxpbmciKTsK 309009 2017-05-03 21:18:16 -0700 2017-05-04 11:27:12 -0700 Patch bug-171643-20170503211815.patch text/plain 1613 zalan U3VidmVyc2lvbiBSZXZpc2lvbjogMjE2MTU5CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMTAzNjZmYzFmMzg0MzY5 YThjNmMzYTMwODRkNzFkZWY1Mzk3NDNmZC4uZDBiNDg0NjU3NTQ1ZTU0N2YxNWQ1OWFkMmEyOTEx NmY2MDkzZjY4YSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDE4IEBACiAyMDE3LTA1LTAzICBaYWxh biBCdWp0YXMgIDx6YWxhbkBhcHBsZS5jb20+CiAKKyAgICAgICAgU2hvdWxkIG5ldmVyIGhpdCBs YXlvdXQgd2hpbGUgdXBkYXRpbmcgdGhlIHJlbmRlciB0cmVlLgorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTcxNjQzCisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTGF5aW5nIG91dCBhIGhhbGYtYmFrZWQgcmVu ZGVyIHRyZWUgaXMgbm90IGEgZ3JlYXQgaWRlYS4gRXNwZWNpYWxseSBjb25zaWRlcmluZworICAg ICAgICB0aGF0IGxheW91dCAoc2FkbHkpIGNhbiBtdXRhdGUgdGhlIHJlbmRlciB0cmVlLiAgCisK KyAgICAgICAgKiBwYWdlL0ZyYW1lVmlldy5jcHA6CisgICAgICAgIChXZWJDb3JlOjpGcmFtZVZp ZXc6OmxheW91dCk6CisKKzIwMTctMDUtMDMgIFphbGFuIEJ1anRhcyAgPHphbGFuQGFwcGxlLmNv bT4KKwogICAgICAgICBTZWFyY2hJbnB1dFR5cGUgY291bGQgZW5kIHVwIHdpdGggYSBtaXNtYXRj aGVkIHJlbmRlcmVyLgogICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9MTcxNTQ3CiAgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8zMTkzNTA0Nz4KZGlmZiAtLWdp dCBhL1NvdXJjZS9XZWJDb3JlL3BhZ2UvRnJhbWVWaWV3LmNwcCBiL1NvdXJjZS9XZWJDb3JlL3Bh Z2UvRnJhbWVWaWV3LmNwcAppbmRleCBjZTg3YzYyMzlmMDAxYzI0YzY5OWI0ZTA0MWJlYTVjMTIw NDEzOWYwLi42NzE5ZTkwNjEwNTRiYjVjOTU2YjMxZTRmOTZkY2I5OTdiNmVlMmQ3IDEwMDY0NAot LS0gYS9Tb3VyY2UvV2ViQ29yZS9wYWdlL0ZyYW1lVmlldy5jcHAKKysrIGIvU291cmNlL1dlYkNv cmUvcGFnZS9GcmFtZVZpZXcuY3BwCkBAIC0xMjg4LDYgKzEyODgsOCBAQCBpbmxpbmUgdm9pZCBG cmFtZVZpZXc6OmZvcmNlTGF5b3V0UGFyZW50Vmlld0lmTmVlZGVkKCkKIAogdm9pZCBGcmFtZVZp ZXc6OmxheW91dChib29sIGFsbG93U3VidHJlZSkKIHsKKyAgICBBU1NFUlRfV0lUSF9TRUNVUklU WV9JTVBMSUNBVElPTighZnJhbWUoKS5kb2N1bWVudCgpLT5pblJlbmRlclRyZWVVcGRhdGUoKSk7 CisKICAgICBMT0coTGF5b3V0LCAiRnJhbWVWaWV3ICVwICglZHglZCkgbGF5b3V0LCBtYWluIGZy YW1ldmlldyAlZCwgYWxsb3dTdWJ0cmVlPSVkIiwgdGhpcywgc2l6ZSgpLndpZHRoKCksIHNpemUo KS5oZWlnaHQoKSwgZnJhbWUoKS5pc01haW5GcmFtZSgpLCBhbGxvd1N1YnRyZWUpOwogICAgIGlm IChpc0luUmVuZGVyVHJlZUxheW91dCgpKSB7CiAgICAgICAgIExPRyhMYXlvdXQsICIgIGluIGxh eW91dCwgYmFpbGluZyIpOwo=