Bug 112593 – Merge MainResourceLoader::responseReceived into DocumentLoader

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Nate Chapin]

...as well as content policy handling, which is started within responseReceived().

[Nate Chapin]

Created attachment 193605 [details]
patch

[WebKit Review Bot]

Comment on attachment 193605 [details]
patch

Attachment 193605 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17205560

New failing tests:
platform/chromium/http/tests/security/mixedContent/insecure-iframe-in-main-frame-blocked.html
http/tests/security/contentSecurityPolicy/1.1/form-action-src-blocked.html
http/tests/security/contentSecurityPolicy/1.1/form-action-src-get-blocked.html

[Nate Chapin]

Created attachment 193634 [details]
attempted fix to test failure

[Adam Barth]

Comment on attachment 193634 [details]
attempted fix to test failure

View in context: https://bugs.webkit.org/attachment.cgi?id=193634&action=review

> Source/WebCore/loader/MainResourceLoader.cpp:-327
> -    ref(); // balanced by deref in continueAfterContentPolicy and cancel

Why don't you need these manual ref counts anymore?

[Nate Chapin]

(In reply to comment #4)
> (From update of attachment 193634 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=193634&action=review
> 
> > Source/WebCore/loader/MainResourceLoader.cpp:-327
> > -    ref(); // balanced by deref in continueAfterContentPolicy and cancel
> 
> Why don't you need these manual ref counts anymore?

I don't fully understand why they were there in the first place (they date back to the initial "make the loader platform-independent" effort). I can't find any other place we have raw ref/deref calls to protect an object during policy callbacks. Also, as far as I can tell, DocumentLoader is a little tougher to assassinate than MainResourceLoader.

My best guess is that these are already vestigial, but it's hard to prove.

[Adam Barth]

Comment on attachment 193634 [details]
attempted fix to test failure

Ok.  That's the part of this change that seems the scariest, but maybe I've got my rose colored glasses on today.  ;)

[WebKit Review Bot]

Comment on attachment 193634 [details]
attempted fix to test failure

Clearing flags on attachment: 193634

Committed r146216: <http://trac.webkit.org/changeset/146216>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.