Bug 112593

Summary:

Merge MainResourceLoader::responseReceived into DocumentLoader

Product:

WebKit

Reporter:

Nate Chapin <japhet>

Component:

WebCore Misc.

Assignee:

Nate Chapin <japhet>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, dglazkov, koivisto, mkwst+watchlist, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

104969, 112745, 112765

Attachments:

Description	Flags
patch	webkit.review.bot: commit-queue-
attempted fix to test failure	none

Nate Chapin

Reported 2013-03-18 10:51:57 PDT

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

Attachments
patch (26.69 KB, patch) 2013-03-18 10:59 PDT, Nate Chapin	webkit.review.bot: commit-queue-	Details Formatted Diff Diff
attempted fix to test failure (26.53 KB, patch) 2013-03-18 12:56 PDT, Nate Chapin	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Nate Chapin

Comment 1 2013-03-18 10:59:03 PDT

Created attachment 193605 [details] patch

WebKit Review Bot

Comment 2 2013-03-18 12:40:39 PDT

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

Comment 3 2013-03-18 12:56:09 PDT

Created attachment 193634 [details] attempted fix to test failure

Adam Barth

Comment 4 2013-03-18 14:40:47 PDT

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

Comment 5 2013-03-18 14:48:19 PDT

(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 6 2013-03-18 17:44:25 PDT

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 7 2013-03-19 10:10:38 PDT

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

Comment 8 2013-03-19 10:10:42 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.