144975 2015-05-13 17:26:06 -0700 REGRESSION (r179958): Crash in WebCore::DocumentLoader::detachFromFrame when -[id<WebPolicyDelegate> decidePolicyForMIMEType:request:frame:decisionListener:] fails to call -[id<WebPolicyDecisionListener> download|ignore|use] 2015-05-13 22:12:13 -0700 1 1 1 Unclassified WebKit Page Loading 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 ddkilzer ddkilzer aestes beidson cdumez commit-queue japhet webkit-bug-importer oldest_to_newest 1094662 0 ddkilzer 2015-05-13 17:26:06 -0700 If an app overrides -decidePolicyForMIMEType:request:frame:decisionListener: in their id<WebPolicyDelegate> object and doesn't call one of three methods on id<WebPolicyDecisionListener> (-download, -ignore or -use), then the app will crash due to a RELEASE_ASSERT() added in r179958. <http://trac.webkit.org/r179958> This was not the intention of these RELEASE_ASSERT() changes (they were to catch the cause of a different crash which we think we found the cause of), so we can change these back to Debug-only ASSERT() statements. Note that the app SHOULD be calling -download, -ignore or -use inside their -[id<WebPolicyDelegate> -decidePolicyForMIMEType:request:frame:decisionListener:] method, but it shouldn't cause a crash, and there's no easy way to fix this without further risk of compatibility issues (due to additional callbacks being made). 1094663 1 ddkilzer 2015-05-13 17:26:21 -0700 <rdar://problem/20905401> 1094664 2 253076 ddkilzer 2015-05-13 17:32:57 -0700 Created attachment 253076 Patch v1 1094677 3 253076 commit-queue 2015-05-13 18:22:32 -0700 Comment on attachment 253076 Patch v1 Clearing flags on attachment: 253076 Committed r184323: <http://trac.webkit.org/changeset/184323> 1094678 4 commit-queue 2015-05-13 18:22:37 -0700 All reviewed patches have been landed. Closing bug. 1094679 5 cdumez 2015-05-13 18:28:44 -0700 Thanks for taking care of this! 1094721 6 beidson 2015-05-13 22:08:28 -0700 (In reply to comment #0) > Note that the app SHOULD be calling -download, -ignore or -use inside their > -[id<WebPolicyDelegate> > -decidePolicyForMIMEType:request:frame:decisionListener:] method, but it > shouldn't cause a crash, and there's no easy way to fix this without further > risk of compatibility issues (due to additional callbacks being made). This is not actually true - It's perfectly legal for an app to hang on to the decision listener and call one of the three methods later, asynchronously. That's why it's set up as a listener object instead of requiring a synchronous return value. 1094724 7 beidson 2015-05-13 22:12:13 -0700 Yah, these asserts are simply bogus in general. If the client hangs on to the policy listener object for "awhile", say maybe to ask the user what they want to do, and the document is torn down in the meantime... DocumentLoader should handle that gracefully, release build or not. 253076 2015-05-13 17:32:57 -0700 2015-05-13 18:22:32 -0700 Patch v1 bug-144975-20150513173134.patch text/plain 2858 ddkilzer U3VidmVyc2lvbiBSZXZpc2lvbjogMTg0MzE4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNTgyZDc0YWY1OGRjMDhh ZWVkMzZjYjJlNGIwNWZmNDkwNDM1YjU2ZS4uZmIwMmU3OGJkZjdiNjBmYTM1OGU3MjUwYTU5MWUx ZTdkY2ZkMDc1NyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDE1LTA1LTEzICBEYXZp ZCBLaWx6ZXIgIDxkZGtpbHplckBhcHBsZS5jb20+CisKKyAgICAgICAgUkVHUkVTSU9OIChyMTc5 OTU4KTogQ3Jhc2ggaW4gV2ViQ29yZTo6RG9jdW1lbnRMb2FkZXI6OmRldGFjaEZyb21GcmFtZSB3 aGVuIC1baWQ8V2ViUG9saWN5RGVsZWdhdGU+IGRlY2lkZVBvbGljeUZvck1JTUVUeXBlOnJlcXVl c3Q6ZnJhbWU6ZGVjaXNpb25MaXN0ZW5lcjpdIGZhaWxzIHRvIGNhbGwgLVtpZDxXZWJQb2xpY3lE ZWNpc2lvbkxpc3RlbmVyPiBkb3dubG9hZHxpZ25vcmV8dXNlXQorICAgICAgICA8aHR0cDovL3dl YmtpdC5vcmcvYi8xNDQ5NzU+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgVGhpcyBjaGFuZ2UgcmV2ZXJ0cyByMTc5OTU4LiAgSXQgY2hhbmdlcyBSRUxF QVNFX0FTU0VSVCooKQorICAgICAgICBzdGF0ZW1lbnRzIGJhY2sgdG8gRGVidWctb25seSBBU1NF UlQqKCkgc3RhdGVtZW50cy4KKworICAgICAgICAqIGxvYWRlci9Eb2N1bWVudExvYWRlci5jcHA6 CisgICAgICAgIChXZWJDb3JlOjpEb2N1bWVudExvYWRlcjo6fkRvY3VtZW50TG9hZGVyKToKKyAg ICAgICAgKFdlYkNvcmU6OkRvY3VtZW50TG9hZGVyOjpjb250aW51ZUFmdGVyQ29udGVudFBvbGlj eSk6CisgICAgICAgIChXZWJDb3JlOjpEb2N1bWVudExvYWRlcjo6ZGV0YWNoRnJvbUZyYW1lKToK KwogMjAxNS0wNS0xMyAgQW50dGkgS29pdmlzdG8gIDxhbnR0aUBhcHBsZS5jb20+CiAKICAgICAg ICAgQ2FjaGVkIENTUyBpbWFnZSByZXNvdXJjZXMgZG9uJ3Qgc2hvdyB1cCBhZnRlciByZWxvYWRp bmcgPGh0dHA6Ly9uaWdodGx5LndlYmtpdC5vcmcvc3RhcnQvPgpkaWZmIC0tZ2l0IGEvU291cmNl L1dlYkNvcmUvbG9hZGVyL0RvY3VtZW50TG9hZGVyLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2xvYWRl ci9Eb2N1bWVudExvYWRlci5jcHAKaW5kZXggMjBkZWNmODgwNDNkMjFjM2JkOTRkY2RmZTFjOTg0 MWZjMDliNTQxZC4uYjI1MDM2ZWQzMzdhZmU1YWQyYjNiMmM0ZDUyZTA5ZTc4NWNhOTA2MiAxMDA2 NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvbG9hZGVyL0RvY3VtZW50TG9hZGVyLmNwcAorKysgYi9T b3VyY2UvV2ViQ29yZS9sb2FkZXIvRG9jdW1lbnRMb2FkZXIuY3BwCkBAIC0xNjIsNyArMTYyLDcg QEAgUmVzb3VyY2VMb2FkZXIqIERvY3VtZW50TG9hZGVyOjptYWluUmVzb3VyY2VMb2FkZXIoKSBj b25zdAogRG9jdW1lbnRMb2FkZXI6On5Eb2N1bWVudExvYWRlcigpCiB7CiAgICAgQVNTRVJUKCFt X2ZyYW1lIHx8IGZyYW1lTG9hZGVyKCktPmFjdGl2ZURvY3VtZW50TG9hZGVyKCkgIT0gdGhpcyB8 fCAhaXNMb2FkaW5nKCkpOwotICAgIFJFTEVBU0VfQVNTRVJUX1dJVEhfTUVTU0FHRSghbV93YWl0 aW5nRm9yQ29udGVudFBvbGljeSwgIlRoZSBjb250ZW50IHBvbGljeSBjYWxsYmFjayBzaG91bGQg bmV2ZXIgb3V0bGl2ZSBpdHMgRG9jdW1lbnRMb2FkZXIuIik7CisgICAgQVNTRVJUX1dJVEhfTUVT U0FHRSghbV93YWl0aW5nRm9yQ29udGVudFBvbGljeSwgIlRoZSBjb250ZW50IHBvbGljeSBjYWxs YmFjayBzaG91bGQgbmV2ZXIgb3V0bGl2ZSBpdHMgRG9jdW1lbnRMb2FkZXIuIik7CiAgICAgaWYg KG1faWNvbkxvYWREZWNpc2lvbkNhbGxiYWNrKQogICAgICAgICBtX2ljb25Mb2FkRGVjaXNpb25D YWxsYmFjay0+aW52YWxpZGF0ZSgpOwogICAgIGlmIChtX2ljb25EYXRhQ2FsbGJhY2spCkBAIC02 NjcsNyArNjY3LDcgQEAgdm9pZCBEb2N1bWVudExvYWRlcjo6cmVzcG9uc2VSZWNlaXZlZChDYWNo ZWRSZXNvdXJjZSogcmVzb3VyY2UsIGNvbnN0IFJlc291cmNlUmUKIAogdm9pZCBEb2N1bWVudExv YWRlcjo6Y29udGludWVBZnRlckNvbnRlbnRQb2xpY3koUG9saWN5QWN0aW9uIHBvbGljeSkKIHsK LSAgICBSRUxFQVNFX0FTU0VSVChtX3dhaXRpbmdGb3JDb250ZW50UG9saWN5KTsKKyAgICBBU1NF UlQobV93YWl0aW5nRm9yQ29udGVudFBvbGljeSk7CiAgICAgbV93YWl0aW5nRm9yQ29udGVudFBv bGljeSA9IGZhbHNlOwogICAgIGlmIChpc1N0b3BwaW5nKCkpCiAgICAgICAgIHJldHVybjsKQEAg LTkyNyw3ICs5MjcsNyBAQCB2b2lkIERvY3VtZW50TG9hZGVyOjpkZXRhY2hGcm9tRnJhbWUoKQog ICAgIEluc3BlY3Rvckluc3RydW1lbnRhdGlvbjo6bG9hZGVyRGV0YWNoZWRGcm9tRnJhbWUoKm1f ZnJhbWUsICp0aGlzKTsKICAgICBtX2ZyYW1lID0gbnVsbHB0cjsKICAgICAvLyBUaGUgY2FsbCB0 byBzdG9wTG9hZGluZygpIGFib3ZlIHNob3VsZCBoYXZlIGNhbmNlbGVkIGFueSBwZW5kaW5nIGNv bnRlbnQgcG9saWN5IGNoZWNrLgotICAgIFJFTEVBU0VfQVNTRVJUX1dJVEhfTUVTU0FHRSghbV93 YWl0aW5nRm9yQ29udGVudFBvbGljeSwgIlRoZSBjb250ZW50IHBvbGljeSBjYWxsYmFjayBuZWVk cyBhIHZhbGlkIGZyYW1lLiIpOworICAgIEFTU0VSVF9XSVRIX01FU1NBR0UoIW1fd2FpdGluZ0Zv ckNvbnRlbnRQb2xpY3ksICJUaGUgY29udGVudCBwb2xpY3kgY2FsbGJhY2sgbmVlZHMgYSB2YWxp ZCBmcmFtZS4iKTsKIH0KIAogdm9pZCBEb2N1bWVudExvYWRlcjo6Y2xlYXJNYWluUmVzb3VyY2VM b2FkZXIoKQo=