187008 2018-06-25 11:00:14 -0700 REGRESSION(r229722): WebKitLegacy clients can crash when loading alternate page 2018-06-27 16:08:20 -0700 1 1 1 Unclassified WebKit WebKit2 WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 183702 187121 1 bfulgham bfulgham bfulgham cdumez dbates ews-watchlist japhet webkit-bug-importer oldest_to_newest 1436140 0 bfulgham 2018-06-25 11:00:14 -0700 The new call to 'clearProvisionalLoadForPolicyCheck' added in r229722 broke loading behavior in WebKitLegacy. 1. We can now enter 'cancelPolicyCheckIfNeeded' without a Frame loader, in what appears to be a recursive call during the load cancellation (the 'm_waitingForContentPolicy' and 'm_waitingForNavigationPolicy' have already been nulled). It seems like we should return early here, or perhaps just move the RELEASE_ASSERT inside the case where we have an active policy check happening. 2. We also enter FrameLoader::checkContentPolicy without an active document loader. We should recognize this case and handle it, rather than trying to dereference a nullptr document loader. 1436141 1 343516 bfulgham 2018-06-25 11:04:14 -0700 Created attachment 343516 Patch 1436142 2 webkit-bug-importer 2018-06-25 11:05:10 -0700 <rdar://problem/41430690> 1436143 3 webkit-bug-importer 2018-06-25 11:05:14 -0700 <rdar://problem/41430650> 1436183 4 343516 cdumez 2018-06-25 12:32:59 -0700 Comment on attachment 343516 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343516&action=review > Source/WebCore/loader/FrameLoader.cpp:363 > void FrameLoader::checkContentPolicy(const ResourceResponse& response, ContentPolicyDecisionFunction&& function) The crash traces attached to the radar do not seem to involve FrameLoader::checkContentPolicy(), could you clarify why this change is needed? 1436206 5 343516 bfulgham 2018-06-25 13:16:17 -0700 Comment on attachment 343516 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343516&action=review >> Source/WebCore/loader/FrameLoader.cpp:363 >> void FrameLoader::checkContentPolicy(const ResourceResponse& response, ContentPolicyDecisionFunction&& function) > > The crash traces attached to the radar do not seem to involve FrameLoader::checkContentPolicy(), could you clarify why this change is needed? Yes, sorry -- this code is hit once you clear the RELEASE_ASSERT from DocumentLoader.cpp. (Historical Note: I went and spoke with Chris in person about the issue before he completed the review). 1436259 6 bfulgham 2018-06-25 14:30:32 -0700 Committed r233176: <https://trac.webkit.org/changeset/233176> 343516 2018-06-25 11:04:14 -0700 2018-06-25 12:35:18 -0700 Patch bug-187008-20180625110414.patch text/plain 3293 bfulgham U3VidmVyc2lvbiBSZXZpc2lvbjogMjMzMDgzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYzJhNjQwNDRkMTYxYmY0 MTczZDVhY2QxMjE3YTQwZjhmM2ViN2RlNi4uMWU0YmZjMjIwMmJiZDgzMDE4NThkM2RkYWEzZjg2 MGQ0OTJhNTFkNCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDMwIEBACisyMDE4LTA2LTI1ICBCcmVu dCBGdWxnaGFtICA8YmZ1bGdoYW1AYXBwbGUuY29tPgorCisgICAgICAgIFJFR1JFU1NJT04ocjIy OTcyMik6IFdlYktpdExlZ2FjeSBjbGllbnRzIGNhbiBjcmFzaCB3aGVuIGxvYWRpbmcgYWx0ZXJu YXRlIHBhZ2UKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTE4NzAwOAorICAgICAgICAKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgVGhlIG5ldyBjYWxsIHRvICdjbGVhclByb3Zpc2lvbmFsTG9hZEZvclBvbGljeUNo ZWNrJyBhZGRlZCBpbiByMjI5NzIyIGJyb2tlIGxvYWRpbmcKKyAgICAgICAgYmVoYXZpb3IgaW4g V2ViS2l0TGVnYWN5LgorCisgICAgICAgIDEuIFdlIGNhbiBub3cgZW50ZXIgJ2NhbmNlbFBvbGlj eUNoZWNrSWZOZWVkZWQnIHdpdGhvdXQgYSBGcmFtZSBsb2FkZXIsIGluIHdoYXQgYXBwZWFycwor ICAgICAgICAgICB0byBiZSBhIHJlY3Vyc2l2ZSBjYWxsIGR1cmluZyB0aGUgbG9hZCBjYW5jZWxs YXRpb24gKHRoZSAnbV93YWl0aW5nRm9yQ29udGVudFBvbGljeScKKyAgICAgICAgICAgYW5kICdt X3dhaXRpbmdGb3JOYXZpZ2F0aW9uUG9saWN5JyBoYXZlIGFscmVhZHkgYmVlbiBudWxsZWQpLiBJ dCBzZWVtcyBsaWtlIHdlIHNob3VsZAorICAgICAgICAgICByZXR1cm4gZWFybHkgaGVyZSwgb3Ig cGVyaGFwcyBqdXN0IG1vdmUgdGhlIFJFTEVBU0VfQVNTRVJUIGluc2lkZSB0aGUgY2FzZSB3aGVy ZSB3ZQorICAgICAgICAgICBoYXZlIGFuIGFjdGl2ZSBwb2xpY3kgY2hlY2sgaGFwcGVuaW5nLgor CisgICAgICAgIDIuIFdlIGFsc28gZW50ZXIgRnJhbWVMb2FkZXI6OmNoZWNrQ29udGVudFBvbGlj eSB3aXRob3V0IGFuIGFjdGl2ZSBkb2N1bWVudCBsb2FkZXIuIFdlCisgICAgICAgICAgIHNob3Vs ZCByZWNvZ25pemUgdGhpcyBjYXNlIGFuZCBoYW5kbGUgaXQsIHJhdGhlciB0aGFuIHRyeWluZyB0 byBkZXJlZmVyZW5jZSBhIG51bGxwdHIKKyAgICAgICAgICAgZG9jdW1lbnQgbG9hZGVyLgorCisg ICAgICAgICogbG9hZGVyL0RvY3VtZW50TG9hZGVyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkRv Y3VtZW50TG9hZGVyOjpjYW5jZWxQb2xpY3lDaGVja0lmTmVlZGVkKTogTW92ZSB0aGUgUkVMRUFT RV9BU1NFUlQgaW5zaWRlIHRoZQorICAgICAgICBjb25kaXRpb25hbCB3aGVyZSB0aGUgZnJhbWVM b2FkZXIgaXMgYWN0dWFsbHkgdXNlZC4KKyAgICAgICAgKiBsb2FkZXIvRnJhbWVMb2FkZXIuY3Bw OgorICAgICAgICAoV2ViQ29yZTo6RnJhbWVMb2FkZXI6OmNoZWNrQ29udGVudFBvbGljeSk6IFJl Y29nbml6ZSB0aGF0IHRoZSBhY3RpdmVEb2N1bWVudExvYWRlciBtYXkKKyAgICAgICAgYmUgbnVs bHB0ciBhdCB0aGlzIHBvaW50LCBhbmQgdGFrZSBhcHByb3ByaWF0ZSBhY3Rpb24gKHJhdGhlciB0 aGFuIGNyYXNoaW5nKS4KKwogMjAxOC0wNi0yMiAgVGhpYmF1bHQgU2F1bmllciAgPHRzYXVuaWVy QGlnYWxpYS5jb20+CiAKICAgICAgICAgW0dTdHJlYW1lcl0gQXZvaWQgc2VuZGluZyBTRUxFQ1Rf U1RSRUFNIGV2ZW50cyB3aGVuIG5vdGhpbmcgY2hhbmdlZApkaWZmIC0tZ2l0IGEvU291cmNlL1dl YkNvcmUvbG9hZGVyL0RvY3VtZW50TG9hZGVyLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2xvYWRlci9E b2N1bWVudExvYWRlci5jcHAKaW5kZXggMzA3NzViYzgzNzdlMTE1OWViMGU3OTcyMWNkMDA4N2M4 ZDUxN2YzMy4uOTU0OTYxNDk0Y2Q3MTE3NGQwZTM4ZjZmZWRjZGViMjBhNjJiZmNiOSAxMDA2NDQK LS0tIGEvU291cmNlL1dlYkNvcmUvbG9hZGVyL0RvY3VtZW50TG9hZGVyLmNwcAorKysgYi9Tb3Vy Y2UvV2ViQ29yZS9sb2FkZXIvRG9jdW1lbnRMb2FkZXIuY3BwCkBAIC0xODEzLDkgKzE4MTMsOCBA QCB2b2lkIERvY3VtZW50TG9hZGVyOjpsb2FkTWFpblJlc291cmNlKFJlc291cmNlUmVxdWVzdCYm IHJlcXVlc3QpCiAKIHZvaWQgRG9jdW1lbnRMb2FkZXI6OmNhbmNlbFBvbGljeUNoZWNrSWZOZWVk ZWQoKQogewotICAgIFJFTEVBU0VfQVNTRVJUKGZyYW1lTG9hZGVyKCkpOwotCiAgICAgaWYgKG1f d2FpdGluZ0ZvckNvbnRlbnRQb2xpY3kgfHwgbV93YWl0aW5nRm9yTmF2aWdhdGlvblBvbGljeSkg eworICAgICAgICBSRUxFQVNFX0FTU0VSVChmcmFtZUxvYWRlcigpKTsKICAgICAgICAgZnJhbWVM b2FkZXIoKS0+cG9saWN5Q2hlY2tlcigpLnN0b3BDaGVjaygpOwogICAgICAgICBtX3dhaXRpbmdG b3JDb250ZW50UG9saWN5ID0gZmFsc2U7CiAgICAgICAgIG1fd2FpdGluZ0Zvck5hdmlnYXRpb25Q b2xpY3kgPSBmYWxzZTsKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2xvYWRlci9GcmFtZUxv YWRlci5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9sb2FkZXIvRnJhbWVMb2FkZXIuY3BwCmluZGV4IDlk NzI2YWI0OGY0MGQ4ZjhjZmYxOGRmMGUyYzI4OTZiMjUyNDlmYmQuLmQzZGVhOGFmZTQ2MmU2MGIy Yzg1MTIyMjkxNWE2YzM3Njk2ZGJiYjMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2xvYWRl ci9GcmFtZUxvYWRlci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvbG9hZGVyL0ZyYW1lTG9hZGVy LmNwcApAQCAtMzYyLDYgKzM2MiwxMiBAQCB2b2lkIEZyYW1lTG9hZGVyOjpzZXREZWZlcnNMb2Fk aW5nKGJvb2wgZGVmZXJzKQogCiB2b2lkIEZyYW1lTG9hZGVyOjpjaGVja0NvbnRlbnRQb2xpY3ko Y29uc3QgUmVzb3VyY2VSZXNwb25zZSYgcmVzcG9uc2UsIENvbnRlbnRQb2xpY3lEZWNpc2lvbkZ1 bmN0aW9uJiYgZnVuY3Rpb24pCiB7CisgICAgaWYgKCFhY3RpdmVEb2N1bWVudExvYWRlcigpKSB7 CisgICAgICAgIC8vIExvYWQgd2FzIGNhbmNlbGxlZAorICAgICAgICBmdW5jdGlvbihQb2xpY3lB Y3Rpb246Oklnbm9yZSk7CisgICAgICAgIHJldHVybjsKKyAgICB9CisKICAgICBjbGllbnQoKS5k aXNwYXRjaERlY2lkZVBvbGljeUZvclJlc3BvbnNlKHJlc3BvbnNlLCBhY3RpdmVEb2N1bWVudExv YWRlcigpLT5yZXF1ZXN0KCksIFdURk1vdmUoZnVuY3Rpb24pKTsKIH0KIAo=