208162 2020-02-24 15:51:25 -0800 [WTF] Attach WARN_UNUSED_RETURN to makeScopeExit and fix existing wrong usage 2020-02-24 18:58:48 -0800 1 1 1 Unclassified WebKit Web Template Framework WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 208171 206343 1 ysuzuki ysuzuki achristensen benjamin cdumez cmarcelo darin dbates emilio esprehn+autocc ews-watchlist gyuyoung.kim keith_miller mark.lam msaboff rmorisset rniwa saam tzagallo webkit-bug-importer oldest_to_newest 1622372 0 ysuzuki 2020-02-24 15:51:25 -0800 Fix r254739. 1622380 1 391592 ysuzuki 2020-02-24 16:03:00 -0800 Created attachment 391592 Patch 1622381 2 391592 rmorisset 2020-02-24 16:06:40 -0800 Comment on attachment 391592 Patch r=me 1622382 3 ysuzuki 2020-02-24 16:07:31 -0800 Land it after ensuring all the EWS bots get green. 1622406 4 391592 saam 2020-02-24 16:39:56 -0800 Comment on attachment 391592 Patch oops. r=me too 1622419 5 ysuzuki 2020-02-24 16:55:13 -0800 OK, building worked. Landing. 1622430 6 ysuzuki 2020-02-24 16:57:48 -0800 Committed r257285: <https://trac.webkit.org/changeset/257285> 1622431 7 webkit-bug-importer 2020-02-24 16:58:18 -0800 <rdar://problem/59747287> 1622433 8 391592 darin 2020-02-24 16:59:59 -0800 Comment on attachment 391592 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391592&action=review > Source/WebCore/html/HTMLLinkElement.cpp:305 > + auto scopeExit = makeScopeExit([&] { m_isHandlingBeforeLoad = previous; }); Seems like for this we should figure out what behavior was broken by not setting m_isHandlingBeforeLoad to true, and make a test that would detect that mistake! 1622453 9 391592 ysuzuki 2020-02-24 17:38:42 -0800 Comment on attachment 391592 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391592&action=review >> Source/WebCore/html/HTMLLinkElement.cpp:305 >> + auto scopeExit = makeScopeExit([&] { m_isHandlingBeforeLoad = previous; }); > > Seems like for this we should figure out what behavior was broken by not setting m_isHandlingBeforeLoad to true, and make a test that would detect that mistake! Yeah, look like we are lacking the test for this case. To me, this looks like HTMLLinkElement is detached and reinserted during emitting before-load event is attached, but need some investigation. Filed https://bugs.webkit.org/show_bug.cgi?id=208171 1622458 10 391592 darin 2020-02-24 17:46:59 -0800 Comment on attachment 391592 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391592&action=review >>> Source/WebCore/html/HTMLLinkElement.cpp:305 >>> + auto scopeExit = makeScopeExit([&] { m_isHandlingBeforeLoad = previous; }); >> >> Seems like for this we should figure out what behavior was broken by not setting m_isHandlingBeforeLoad to true, and make a test that would detect that mistake! > > Yeah, look like we are lacking the test for this case. To me, this looks like HTMLLinkElement is detached and reinserted during emitting before-load event is attached, but need some investigation. > Filed https://bugs.webkit.org/show_bug.cgi?id=208171 Oh, I forgot that this was the one that inspired this entire exercise! 1622473 11 391592 ysuzuki 2020-02-24 18:58:48 -0800 Comment on attachment 391592 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391592&action=review >>>> Source/WebCore/html/HTMLLinkElement.cpp:305 >>>> + auto scopeExit = makeScopeExit([&] { m_isHandlingBeforeLoad = previous; }); >>> >>> Seems like for this we should figure out what behavior was broken by not setting m_isHandlingBeforeLoad to true, and make a test that would detect that mistake! >> >> Yeah, look like we are lacking the test for this case. To me, this looks like HTMLLinkElement is detached and reinserted during emitting before-load event is attached, but need some investigation. >> Filed https://bugs.webkit.org/show_bug.cgi?id=208171 > > Oh, I forgot that this was the one that inspired this entire exercise! The good thing is that, we also found this case in JSC Parser.cpp :). While it is only used for debug assertions, anyway, having this annotation is super useful! Thanks Emilio! 391592 2020-02-24 16:03:00 -0800 2020-02-24 16:06:40 -0800 Patch bug-208162-20200224160259.patch text/plain 4026 ysuzuki U3VidmVyc2lvbiBSZXZpc2lvbjogMjU3MjY1CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCA4 MjZkMjI0OTFkZTk3ZjU3ZTU4MGNjNDYzZDgyNGVjMmQ0MGY3NWY2Li4yODljYjA3NmM2MzczMjY1 MzI2NmYzNDM2ZTExYmNiMDQxMjE5ODdhIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxMyBAQAorMjAyMC0wMi0yNCAgWXVzdWtlIFN1enVraSAgPHlzdXp1a2lAYXBwbGUuY29t PgorCisgICAgICAgIFtXVEZdIEF0dGFjaCBXQVJOX1VOVVNFRF9SRVRVUk4gdG8gbWFrZVNjb3Bl RXhpdCBhbmQgZml4IGV4aXN0aW5nIHdyb25nIHVzYWdlCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMDgxNjIKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIHBhcnNlci9QYXJzZXIuY3BwOgorICAgICAgICAo SlNDOjpQYXJzZXI8TGV4ZXJUeXBlPjo6cGFyc2VVbmFyeUV4cHJlc3Npb24pOgorCiAyMDIwLTAy LTI0ICBDYWlvIExpbWEgIDx0aWNhaW9saW1hQGdtYWlsLmNvbT4KIAogICAgICAgICBbSlNDXSAz Mi1iaXRzIGRlYnVnIGJ1aWxkIGJyb2tlbiBhZnRlciByMjU3MjEyCmRpZmYgLS1naXQgYS9Tb3Vy Y2UvV1RGL0NoYW5nZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IDQwN2EzOGFiMDA3 YTUzNmRlYTA5NGE0NTNiZjgwMzY0MDZlMTRmOWYuLmFhNDYwYTEwY2JkNzAzMDQxOTNjZGIwNWEw OTRhMDRjNDZiNTgwMmMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XVEYvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMjAtMDItMjQgIFl1c3VrZSBT dXp1a2kgIDx5c3V6dWtpQGFwcGxlLmNvbT4KKworICAgICAgICBbV1RGXSBBdHRhY2ggV0FSTl9V TlVTRURfUkVUVVJOIHRvIG1ha2VTY29wZUV4aXQgYW5kIGZpeCBleGlzdGluZyB3cm9uZyB1c2Fn ZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjA4MTYy CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgV2Ugc2hv dWxkIGhvbGQgU2NvcGVFeGl0IHRvIGNhbGwgZGVzdHJ1Y3RvciB3aGVuIHdlIGV4aXQgZnJvbSB0 aGUgc2NvcGUgYWN0dWFsbHkuCisgICAgICAgIFB1dHRpbmcgV0FSTl9VTlVTRURfUkVUVVJOIHRv IGZpeCBleGlzdGluZyBtaXN1c2UuCisKKyAgICAgICAgKiB3dGYvU2NvcGUuaDoKKwogMjAyMC0w Mi0yMyAgWXVzdWtlIFN1enVraSAgPHlzdXp1a2lAYXBwbGUuY29tPgogCiAgICAgICAgIFtKU0Nd IFNocmluayBTdHJ1Y3R1cmUKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBi L1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCA4ZTViODZlY2U1ZWYxYzYzOWM5NGQ3YTBm NzgxODJhYTExNDBkY2Y5Li44YmU4YTQ3ZjY0YjY3MjZlYTk0ZDYxNjM1OTBjMGZiNTkzMGVmOTE0 IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNv cmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTMgQEAKKzIwMjAtMDItMjQgIFl1c3VrZSBTdXp1a2kg IDx5c3V6dWtpQGFwcGxlLmNvbT4KKworICAgICAgICBbV1RGXSBBdHRhY2ggV0FSTl9VTlVTRURf UkVUVVJOIHRvIG1ha2VTY29wZUV4aXQgYW5kIGZpeCBleGlzdGluZyB3cm9uZyB1c2FnZQorICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjA4MTYyCisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBodG1sL0hUTUxM aW5rRWxlbWVudC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpIVE1MTGlua0VsZW1lbnQ6OnByb2Nl c3MpOgorCiAyMDIwLTAyLTI0ICBBbmRyZXMgR29uemFsZXogIDxhbmRyZXNnXzIyQGFwcGxlLmNv bT4KIAogICAgICAgICBbV2ViQWNjZXNzaWJpbGl0eU9iamVjdFdyYXBwZXIgdXBkYXRlT2JqZWN0 QmFja2luZ1N0b3JlXSBzaG91bGQgcmV0dXJuIHRoZSBiYWNraW5nIG9iamVjdC4KZGlmZiAtLWdp dCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9wYXJzZXIvUGFyc2VyLmNwcCBiL1NvdXJjZS9KYXZh U2NyaXB0Q29yZS9wYXJzZXIvUGFyc2VyLmNwcAppbmRleCAyYTE5MWM4NTYwYzRlODczODAxM2Vm MzVhZWZmMmJjYzY3ZWZhZDBmLi5jNGFhYzhlNTVlZDczNjliMDlmOTc1NGU1NGIzMmRhZWNmNzMy YjdiIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcGFyc2VyL1BhcnNlci5jcHAK KysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL3BhcnNlci9QYXJzZXIuY3BwCkBAIC01MDcyLDcg KzUwNzIsNyBAQCB0ZW1wbGF0ZSA8Y2xhc3MgVHJlZUJ1aWxkZXI+IFRyZWVFeHByZXNzaW9uIFBh cnNlcjxMZXhlclR5cGU+OjpwYXJzZVVuYXJ5RXhwcmVzcwogICAgIEpTVG9rZW5Mb2NhdGlvbiBs b2NhdGlvbih0b2tlbkxvY2F0aW9uKCkpOwogCiAgICAgaW50IG9sZFRva2VuU3RhY2tEZXB0aCA9 IGNvbnRleHQudW5hcnlUb2tlblN0YWNrRGVwdGgoKTsKLSAgICBtYWtlU2NvcGVFeGl0KFsmXSB7 CisgICAgYXV0byBzY29wZUV4aXQgPSBtYWtlU2NvcGVFeGl0KFsmXSB7CiAgICAgICAgIEFTU0VS VF9VTlVTRUQob2xkVG9rZW5TdGFja0RlcHRoLCBvbGRUb2tlblN0YWNrRGVwdGggPD0gY29udGV4 dC51bmFyeVRva2VuU3RhY2tEZXB0aCgpKTsKICAgICB9KTsKIApkaWZmIC0tZ2l0IGEvU291cmNl L1dURi93dGYvU2NvcGUuaCBiL1NvdXJjZS9XVEYvd3RmL1Njb3BlLmgKaW5kZXggN2JkNzYzNzAw NDBlZTlkOTg4MWNkOTkzMjM5OTczMDIyM2YwMmVlZC4uYjc0ZDc1YWE1N2Y1YjBlZTE5ZTk3MjE3 NDhiZDIxYmRkNWM3MWQwZiAxMDA2NDQKLS0tIGEvU291cmNlL1dURi93dGYvU2NvcGUuaAorKysg Yi9Tb3VyY2UvV1RGL3d0Zi9TY29wZS5oCkBAIC02Nyw2ICs2Nyw4IEBAIGNsYXNzIFNjb3BlRXhp dCBmaW5hbCB7CiAgICAgYm9vbCBtX2V4ZWN1dGVPbkRlc3RydWN0aW9uIHsgdHJ1ZSB9OwogfTsK IAorCit0ZW1wbGF0ZTx0eXBlbmFtZSBFeGl0RnVuY3Rpb24+IFNjb3BlRXhpdDxFeGl0RnVuY3Rp b24+IG1ha2VTY29wZUV4aXQoRXhpdEZ1bmN0aW9uJiYpIFdBUk5fVU5VU0VEX1JFVFVSTjsKIHRl bXBsYXRlPHR5cGVuYW1lIEV4aXRGdW5jdGlvbj4KIFNjb3BlRXhpdDxFeGl0RnVuY3Rpb24+IG1h a2VTY29wZUV4aXQoRXhpdEZ1bmN0aW9uJiYgZXhpdEZ1bmN0aW9uKQogewpkaWZmIC0tZ2l0IGEv U291cmNlL1dlYkNvcmUvaHRtbC9IVE1MTGlua0VsZW1lbnQuY3BwIGIvU291cmNlL1dlYkNvcmUv aHRtbC9IVE1MTGlua0VsZW1lbnQuY3BwCmluZGV4IGM3N2VlOTE2MzIyZmRmOThlZmY1ODc3MjJk YWEzMzhhOWNlNmIyMjguLjAyZDM4N2NiY2MzZjgzNWE5MjZiMmFlMDQ4N2VmM2VjYWNkYWY0YWQg MTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2h0bWwvSFRNTExpbmtFbGVtZW50LmNwcAorKysg Yi9Tb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxMaW5rRWxlbWVudC5jcHAKQEAgLTMwMiw3ICszMDIs NyBAQCB2b2lkIEhUTUxMaW5rRWxlbWVudDo6cHJvY2VzcygpCiAgICAgICAgIHsKICAgICAgICAg ICAgIGJvb2wgcHJldmlvdXMgPSBtX2lzSGFuZGxpbmdCZWZvcmVMb2FkOwogICAgICAgICAgICAg bV9pc0hhbmRsaW5nQmVmb3JlTG9hZCA9IHRydWU7Ci0gICAgICAgICAgICBtYWtlU2NvcGVFeGl0 KFsmXSB7IG1faXNIYW5kbGluZ0JlZm9yZUxvYWQgPSBwcmV2aW91czsgfSk7CisgICAgICAgICAg ICBhdXRvIHNjb3BlRXhpdCA9IG1ha2VTY29wZUV4aXQoWyZdIHsgbV9pc0hhbmRsaW5nQmVmb3Jl TG9hZCA9IHByZXZpb3VzOyB9KTsKICAgICAgICAgICAgIGlmICghc2hvdWxkTG9hZExpbmsoKSkK ICAgICAgICAgICAgICAgICByZXR1cm47CiAgICAgICAgIH0K