207830 2020-02-16 19:00:49 -0800 [LFC] Remove ReplacedBox::m_layoutBox 2020-02-17 10:05:42 -0800 1 1 1 Unclassified WebKit Layout and Rendering WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 zalan zalan bfulgham commit-queue darin koivisto simon.fraser webkit-bug-importer zalan oldest_to_newest 1619553 0 zalan 2020-02-16 19:00:49 -0800 This is just a leftover. 1619554 1 webkit-bug-importer 2020-02-16 19:01:10 -0800 <rdar://problem/59498829> 1619556 2 390895 zalan 2020-02-16 19:04:04 -0800 Created attachment 390895 Patch 1619581 3 390895 darin 2020-02-16 22:19:30 -0800 Comment on attachment 390895 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=390895&action=review > Source/WebCore/layout/layouttree/LayoutReplacedBox.h:-63 > - WeakPtr<const Box> m_layoutBox; I don’t see where this was initialized to anything. Seems like it was always null. Not sure why the code wasn’t just crashing all the time. Don’t understand why the new test covers this code. 1619597 4 koivisto 2020-02-17 01:01:45 -0800 > I don’t see where this was initialized to anything. Seems like it was always > null. Not sure why the code wasn’t just crashing all the time. Don’t > understand why the new test covers this code. I think the new test covers it because it explicitly enables full-tree LFC. There was no previous test coverage. 1619638 5 zalan 2020-02-17 07:19:05 -0800 (In reply to Darin Adler from comment #3) > Comment on attachment 390895 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=390895&action=review > > > Source/WebCore/layout/layouttree/LayoutReplacedBox.h:-63 > > - WeakPtr<const Box> m_layoutBox; > > I don’t see where this was initialized to anything. Seems like it was always > null. Not sure why the code wasn’t just crashing all the time. Don’t > understand why the new test covers this code. Yes. As Antti said this code is behind a runtime flag and wasn't regression tested. 1619649 6 390895 commit-queue 2020-02-17 08:03:50 -0800 Comment on attachment 390895 Patch Clearing flags on attachment: 390895 Committed r256733: <https://trac.webkit.org/changeset/256733> 1619650 7 commit-queue 2020-02-17 08:03:51 -0800 All reviewed patches have been landed. Closing bug. 1619696 8 390895 darin 2020-02-17 10:02:14 -0800 Comment on attachment 390895 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=390895&action=review >>> Source/WebCore/layout/layouttree/LayoutReplacedBox.h:-63 >>> - WeakPtr<const Box> m_layoutBox; >> >> I don’t see where this was initialized to anything. Seems like it was always null. Not sure why the code wasn’t just crashing all the time. Don’t understand why the new test covers this code. > > Yes. As Antti said this code is behind a runtime flag and wasn't regression tested. Ah, I see, the new test says "LayoutFormattingContextEnabled=true". That’s what I missed! 1619698 9 zalan 2020-02-17 10:05:42 -0800 (In reply to Darin Adler from comment #8) > Comment on attachment 390895 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=390895&action=review > > >>> Source/WebCore/layout/layouttree/LayoutReplacedBox.h:-63 > >>> - WeakPtr<const Box> m_layoutBox; > >> > >> I don’t see where this was initialized to anything. Seems like it was always null. Not sure why the code wasn’t just crashing all the time. Don’t understand why the new test covers this code. > > > > Yes. As Antti said this code is behind a runtime flag and wasn't regression tested. > > Ah, I see, the new test says "LayoutFormattingContextEnabled=true". That’s > what I missed! And it is actually crashing here https://ews-build.webkit.org/results/iOS-13-Simulator-WK2-Tests-EWS/r390898-10935/results.html when a new test exercises this (not yet patched)code path. 390895 2020-02-16 19:04:04 -0800 2020-02-17 08:03:50 -0800 Patch bug-207830-20200216190403.patch text/plain 5921 zalan U3VidmVyc2lvbiBSZXZpc2lvbjogMjU2NzEyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYTBjYmVlNDM2ODZkODMy NjNlMGE3ZDk5ZWJlYTE2ZmRlYzhjYzU2OS4uZTM1OTFkZGNiZjZlY2M5YjM4MmQ4NDY1N2M0MmVj N2IxNThjMmU0YSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIzIEBACisyMDIwLTAyLTE2ICBaYWxh biBCdWp0YXMgIDx6YWxhbkBhcHBsZS5jb20+CisKKyAgICAgICAgW0xGQ10gUmVtb3ZlIFJlcGxh Y2VkQm94OjptX2xheW91dEJveAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MjA3ODMwCisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS81OTQ5ODgyOT4KKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUZXN0OiBmYXN0 L2xheW91dGZvcm1hdHRpbmdjb250ZXh0L3NpbXBsZS1hYnNvbHV0ZS1wb3NpdGlvbmVkLWlubGlu ZS1lbGVtZW50Lmh0bWwKKworICAgICAgICBUaGlzIGlzIG5vdCBuZWVkZWQgYWZ0ZXIgcjI1NjM4 MCAoW0xGQ10gSW50cm9kdWNlIExheW91dDo6UmVwbGFjZWRCb3gpLgorCisgICAgICAgICogbGF5 b3V0L2xheW91dHRyZWUvTGF5b3V0UmVwbGFjZWRCb3guY3BwOgorICAgICAgICAoV2ViQ29yZTo6 TGF5b3V0OjpSZXBsYWNlZEJveDo6aGFzSW50cmluc2ljV2lkdGggY29uc3QpOgorICAgICAgICAo V2ViQ29yZTo6TGF5b3V0OjpSZXBsYWNlZEJveDo6aGFzSW50cmluc2ljSGVpZ2h0IGNvbnN0KToK KyAgICAgICAgKFdlYkNvcmU6OkxheW91dDo6UmVwbGFjZWRCb3g6OmludHJpbnNpY1dpZHRoIGNv bnN0KToKKyAgICAgICAgKFdlYkNvcmU6OkxheW91dDo6UmVwbGFjZWRCb3g6OmludHJpbnNpY0hl aWdodCBjb25zdCk6CisgICAgICAgIChXZWJDb3JlOjpMYXlvdXQ6OlJlcGxhY2VkQm94OjpoYXNB c3BlY3RSYXRpbyBjb25zdCk6CisgICAgICAgICogbGF5b3V0L2xheW91dHRyZWUvTGF5b3V0UmVw bGFjZWRCb3guaDoKKwogMjAyMC0wMi0xNiAgWmFsYW4gQnVqdGFzICA8emFsYW5AYXBwbGUuY29t PgogCiAgICAgICAgIFtMRkNdW0lGQ10gTGluZUxheW91dENvbnRleHQ6Om5leHRDb250ZW50Rm9y TGluZSBzaG91bGQgYmUgYWJsZSB0byBoYW5kbGUgcmVndWxhciBpbmxpbmUgYm94ZXMKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL2xheW91dC9sYXlvdXR0cmVlL0xheW91dFJlcGxhY2VkQm94 LmNwcCBiL1NvdXJjZS9XZWJDb3JlL2xheW91dC9sYXlvdXR0cmVlL0xheW91dFJlcGxhY2VkQm94 LmNwcAppbmRleCA4YzZmYzQ4OGViMDlhZDlmZmRkN2M3OWJiZjk5ZWE4ZWJhNzhhYzVkLi41YzRk MjBmYTk5N2IzOWY4NWM1N2E4NDE5NDBlZWVhNmM5NmQ0ODVmIDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViQ29yZS9sYXlvdXQvbGF5b3V0dHJlZS9MYXlvdXRSZXBsYWNlZEJveC5jcHAKKysrIGIvU291 cmNlL1dlYkNvcmUvbGF5b3V0L2xheW91dHRyZWUvTGF5b3V0UmVwbGFjZWRCb3guY3BwCkBAIC00 MywxMiArNDMsMTIgQEAgUmVwbGFjZWRCb3g6OlJlcGxhY2VkQm94KFJlbmRlclN0eWxlJiYgc3R5 bGUpCiAKIGJvb2wgUmVwbGFjZWRCb3g6Omhhc0ludHJpbnNpY1dpZHRoKCkgY29uc3QKIHsKLSAg ICByZXR1cm4gbV9pbnRyaW5zaWNTaXplIHx8IG1fbGF5b3V0Qm94LT5zdHlsZSgpLmxvZ2ljYWxX aWR0aCgpLmlzSW50cmluc2ljKCk7CisgICAgcmV0dXJuIG1faW50cmluc2ljU2l6ZSB8fCBzdHls ZSgpLmxvZ2ljYWxXaWR0aCgpLmlzSW50cmluc2ljKCk7CiB9CiAKIGJvb2wgUmVwbGFjZWRCb3g6 Omhhc0ludHJpbnNpY0hlaWdodCgpIGNvbnN0CiB7Ci0gICAgcmV0dXJuIG1faW50cmluc2ljU2l6 ZSB8fCBtX2xheW91dEJveC0+c3R5bGUoKS5sb2dpY2FsSGVpZ2h0KCkuaXNJbnRyaW5zaWMoKTsK KyAgICByZXR1cm4gbV9pbnRyaW5zaWNTaXplIHx8IHN0eWxlKCkubG9naWNhbEhlaWdodCgpLmlz SW50cmluc2ljKCk7CiB9CiAKIGJvb2wgUmVwbGFjZWRCb3g6Omhhc0ludHJpbnNpY1JhdGlvKCkg Y29uc3QKQEAgLTYzLDcgKzYzLDcgQEAgTGF5b3V0VW5pdCBSZXBsYWNlZEJveDo6aW50cmluc2lj V2lkdGgoKSBjb25zdAogICAgIEFTU0VSVChoYXNJbnRyaW5zaWNXaWR0aCgpKTsKICAgICBpZiAo bV9pbnRyaW5zaWNTaXplKQogICAgICAgICByZXR1cm4gbV9pbnRyaW5zaWNTaXplLT53aWR0aCgp OwotICAgIHJldHVybiBMYXlvdXRVbml0KG1fbGF5b3V0Qm94LT5zdHlsZSgpLmxvZ2ljYWxXaWR0 aCgpLnZhbHVlKCkpOworICAgIHJldHVybiBMYXlvdXRVbml0IHsgc3R5bGUoKS5sb2dpY2FsV2lk dGgoKS52YWx1ZSgpIH07CiB9CiAKIExheW91dFVuaXQgUmVwbGFjZWRCb3g6OmludHJpbnNpY0hl aWdodCgpIGNvbnN0CkBAIC03MSw3ICs3MSw3IEBAIExheW91dFVuaXQgUmVwbGFjZWRCb3g6Omlu dHJpbnNpY0hlaWdodCgpIGNvbnN0CiAgICAgQVNTRVJUKGhhc0ludHJpbnNpY0hlaWdodCgpKTsK ICAgICBpZiAobV9pbnRyaW5zaWNTaXplKQogICAgICAgICByZXR1cm4gbV9pbnRyaW5zaWNTaXpl LT5oZWlnaHQoKTsKLSAgICByZXR1cm4gTGF5b3V0VW5pdChtX2xheW91dEJveC0+c3R5bGUoKS5s b2dpY2FsSGVpZ2h0KCkudmFsdWUoKSk7CisgICAgcmV0dXJuIExheW91dFVuaXQgeyBzdHlsZSgp LmxvZ2ljYWxIZWlnaHQoKS52YWx1ZSgpIH07CiB9CiAKIExheW91dFVuaXQgUmVwbGFjZWRCb3g6 OmludHJpbnNpY1JhdGlvKCkgY29uc3QKQEAgLTg2LDcgKzg2LDcgQEAgTGF5b3V0VW5pdCBSZXBs YWNlZEJveDo6aW50cmluc2ljUmF0aW8oKSBjb25zdAogCiBib29sIFJlcGxhY2VkQm94OjpoYXNB c3BlY3RSYXRpbygpIGNvbnN0CiB7Ci0gICAgcmV0dXJuIG1fbGF5b3V0Qm94LT5pc0ltYWdlKCkg fHwgbV9sYXlvdXRCb3gtPnN0eWxlKCkuYXNwZWN0UmF0aW9UeXBlKCkgPT0gQXNwZWN0UmF0aW9U eXBlOjpGcm9tSW50cmluc2ljOworICAgIHJldHVybiBpc0ltYWdlKCkgfHwgc3R5bGUoKS5hc3Bl Y3RSYXRpb1R5cGUoKSA9PSBBc3BlY3RSYXRpb1R5cGU6OkZyb21JbnRyaW5zaWM7CiB9CiAKIH0K ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2xheW91dC9sYXlvdXR0cmVlL0xheW91dFJlcGxh Y2VkQm94LmggYi9Tb3VyY2UvV2ViQ29yZS9sYXlvdXQvbGF5b3V0dHJlZS9MYXlvdXRSZXBsYWNl ZEJveC5oCmluZGV4IGZkMjlhYmUyZWFhNWJjMDZiOWZlMjc4MzE3OWM4NDVmNjgxYjBkOTMuLjY3 NDM1Njc3M2EyNzJjMTgyZmRlM2FlNzRiZjQwODA4M2Q4YTU2YWQgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL2xheW91dC9sYXlvdXR0cmVlL0xheW91dFJlcGxhY2VkQm94LmgKKysrIGIvU291 cmNlL1dlYkNvcmUvbGF5b3V0L2xheW91dHRyZWUvTGF5b3V0UmVwbGFjZWRCb3guaApAQCAtNjAs NyArNjAsNiBAQCBwdWJsaWM6CiBwcml2YXRlOgogICAgIGJvb2wgaGFzQXNwZWN0UmF0aW8oKSBj b25zdDsKIAotICAgIFdlYWtQdHI8Y29uc3QgQm94PiBtX2xheW91dEJveDsKICAgICBPcHRpb25h bDxMYXlvdXRTaXplPiBtX2ludHJpbnNpY1NpemU7CiAgICAgT3B0aW9uYWw8TGF5b3V0VW5pdD4g bV9pbnRyaW5zaWNSYXRpbzsKICAgICBDYWNoZWRJbWFnZSogbV9jYWNoZWRJbWFnZSB7IG51bGxw dHIgfTsKZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRlc3RzL0No YW5nZUxvZwppbmRleCAwYjhiMDg3OGE0OWJhNWE1ZDg5NmIxNGNmYzdkZDlmNjQ3N2E0ZmYzLi42 N2I5NDdiNjlkNTliYzgxYTc2NWM0YjhmNTE3NGIwZDBmZDgyODQwIDEwMDY0NAotLS0gYS9MYXlv dXRUZXN0cy9DaGFuZ2VMb2cKKysrIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEs MTQgQEAKKzIwMjAtMDItMTYgIFphbGFuIEJ1anRhcyAgPHphbGFuQGFwcGxlLmNvbT4KKworICAg ICAgICBbTEZDXSBSZW1vdmUgUmVwbGFjZWRCb3g6Om1fbGF5b3V0Qm94CisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMDc4MzAKKyAgICAgICAgPHJkYXI6 Ly9wcm9ibGVtLzU5NDk4ODI5PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgICogZmFzdC9sYXlvdXRmb3JtYXR0aW5nY29udGV4dC9zaW1wbGUtYWJzb2x1 dGUtcG9zaXRpb25lZC1pbmxpbmUtZWxlbWVudC1leHBlY3RlZC5odG1sOiBBZGRlZC4KKyAgICAg ICAgKiBmYXN0L2xheW91dGZvcm1hdHRpbmdjb250ZXh0L3NpbXBsZS1hYnNvbHV0ZS1wb3NpdGlv bmVkLWlubGluZS1lbGVtZW50Lmh0bWw6IEFkZGVkLgorCiAyMDIwLTAyLTE2ICBaYWxhbiBCdWp0 YXMgIDx6YWxhbkBhcHBsZS5jb20+CiAKICAgICAgICAgW0xGQ11bSUZDXSBMaW5lTGF5b3V0Q29u dGV4dDo6bmV4dENvbnRlbnRGb3JMaW5lIHNob3VsZCBiZSBhYmxlIHRvIGhhbmRsZSByZWd1bGFy IGlubGluZSBib3hlcwpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvZmFzdC9sYXlvdXRmb3JtYXR0 aW5nY29udGV4dC9zaW1wbGUtYWJzb2x1dGUtcG9zaXRpb25lZC1pbmxpbmUtZWxlbWVudC1leHBl Y3RlZC5odG1sIGIvTGF5b3V0VGVzdHMvZmFzdC9sYXlvdXRmb3JtYXR0aW5nY29udGV4dC9zaW1w bGUtYWJzb2x1dGUtcG9zaXRpb25lZC1pbmxpbmUtZWxlbWVudC1leHBlY3RlZC5odG1sCm5ldyBm aWxlIG1vZGUgMTAwNjQ0CmluZGV4IDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw MDAwMDAuLjhmNDVlNzcyOWJmOTAyMGUxMTQ1NWM5NjJlNWJlODVmNDI5OTkyYzcKLS0tIC9kZXYv bnVsbAorKysgYi9MYXlvdXRUZXN0cy9mYXN0L2xheW91dGZvcm1hdHRpbmdjb250ZXh0L3NpbXBs ZS1hYnNvbHV0ZS1wb3NpdGlvbmVkLWlubGluZS1lbGVtZW50LWV4cGVjdGVkLmh0bWwKQEAgLTAs MCArMSw5IEBACis8IS0tIHdlYmtpdC10ZXN0LXJ1bm5lciBbIGludGVybmFsOkxheW91dEZvcm1h dHRpbmdDb250ZXh0RW5hYmxlZD10cnVlIGludGVybmFsOkxheW91dEZvcm1hdHRpbmdDb250ZXh0 SW50ZWdyYXRpb25FbmFibGVkPWZhbHNlIF0gLS0+Cis8c3R5bGU+CitkaXYgeworICAgIHdpZHRo OiAxMDBweDsKKyAgICBoZWlnaHQ6IDEwMHB4OworICAgIGJhY2tncm91bmQtY29sb3I6IGdyZWVu OworfQorPC9zdHlsZT4KKzxkaXY+PC9kaXY+CmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9mYXN0 L2xheW91dGZvcm1hdHRpbmdjb250ZXh0L3NpbXBsZS1hYnNvbHV0ZS1wb3NpdGlvbmVkLWlubGlu ZS1lbGVtZW50Lmh0bWwgYi9MYXlvdXRUZXN0cy9mYXN0L2xheW91dGZvcm1hdHRpbmdjb250ZXh0 L3NpbXBsZS1hYnNvbHV0ZS1wb3NpdGlvbmVkLWlubGluZS1lbGVtZW50Lmh0bWwKbmV3IGZpbGUg bW9kZSAxMDA2NDQKaW5kZXggMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw MC4uMjEyZDFmY2VkYjViZDMzNmFkZmUwZDUxMTFjMzRlODg3MTRiMzhmNgotLS0gL2Rldi9udWxs CisrKyBiL0xheW91dFRlc3RzL2Zhc3QvbGF5b3V0Zm9ybWF0dGluZ2NvbnRleHQvc2ltcGxlLWFi c29sdXRlLXBvc2l0aW9uZWQtaW5saW5lLWVsZW1lbnQuaHRtbApAQCAtMCwwICsxLDEwIEBACis8 IS0tIHdlYmtpdC10ZXN0LXJ1bm5lciBbIGludGVybmFsOkxheW91dEZvcm1hdHRpbmdDb250ZXh0 RW5hYmxlZD10cnVlIGludGVybmFsOkxheW91dEZvcm1hdHRpbmdDb250ZXh0SW50ZWdyYXRpb25F bmFibGVkPWZhbHNlIF0gLS0+Cis8c3R5bGU+Cit2aWRlbyB7CisgICAgcG9zaXRpb246IGFic29s dXRlOworICAgIHdpZHRoOiAxMDBweDsKKyAgICBoZWlnaHQ6IDEwMHB4OworICAgIGJhY2tncm91 bmQtY29sb3I6IGdyZWVuOworfQorPC9zdHlsZT4KKzxkaXY+PHZpZGVvPjwvZGl2Pgo=