[webkit-reviews] review granted: [Bug 48014] Using the decidePolicyForMIMEType delegate message in an asynchronous manner does not work : [Attachment 71355] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Anders Carlsson
<andersca at apple.com>'s request for review:
Bug 48014: Using the decidePolicyForMIMEType delegate message in an
asynchronous manner does not work
https://bugs.webkit.org/show_bug.cgi?id=48014

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=71355&action=review

> WebCore/loader/MainResourceLoader.cpp:235
> +    setDefersLoading(false);

I think this needs a comment indicating where the setDefersLoading(true) is.

> WebCore/loader/MainResourceLoader.cpp:379
> +    // Defer loading while we're waiting for a response.
> +    setDefersLoading(true);

I think this needs a comment indicating where the setDefersLoading(false) is.

Is there a chance that some other code will setDefersLoading(true) and then
setDefersLoading(false) somewhere before we get the policy client response?
Does this nest properly?

> WebKitTools/DumpRenderTree/win/LayoutTestControllerWin.cpp:371
> -void LayoutTestController::setCustomPolicyDelegate(bool setDelegate, bool
permissive)
> +void LayoutTestController::setCustomPolicyDelegate(bool setDelegate, bool
permissive, , bool callIgnoreInDecidePolicyForMIMETypeAfterOneSecond)

I don’t think this will compile with that extra comma.