[webkit-reviews] review denied: [Bug 46579] Reproducible crash in appcache code when closing pgatour.com : [Attachment 72089] patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied	review:
Bug 46579: Reproducible crash in appcache code when closing pgatour.com
https://bugs.webkit.org/show_bug.cgi?id=46579

Attachment 72089: patch
https://bugs.webkit.org/attachment.cgi?id=72089&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=72089&action=review

> WebCore/ChangeLog:5
> +	   If we're in an unload/beforeunload/pagetransition event in any frame


+	 If we're in an unload/beforeunload/pagetransition event in any frame

Page transition events include pagehide, pageshow, and manually created events.
I don't think this changes, or should change behavior for the latter two.

> WebCore/ChangeLog:7
> +	   and we start loading a subresource, load it with PingLoader rather
than
> +	   SubresourceLoader. Otherwise, we won't cancel it and the resource
will

This new behavior sounds overly restrictive. Why shouldn't we be able to add
subresources to main frame while processing onunload in a subframe? The main
frame may not be closing at all - perhaps it wants to replace its subframe with
an image, for example!

I think that the root cause of this crash is different. Somehow, we fail to
cancel these resource loads when a frame goes away. It's not clear why the
resource load isn't canceled, and when else that can happen besides this
multi-frame setup. That needs to be investigated.

Changing PingLoader to work on pgatour.com is probably good, but it's separate
from actual crash fix.