AssociatedURLLoader should handle CORS requests that redirect. Specifically, it should cancel the load when the redirect fails the CORS check.
Created attachment 134096 [details] Proposed patch
Comment on attachment 134096 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=134096&action=review > Source/WebKit/chromium/tests/AssociatedURLLoaderTest.cpp:471 > - EXPECT_TRUE(m_willSendRequest); > + // CORS requires that the redirect be transparent. > + EXPECT_FALSE(m_willSendRequest); I'm not sure I understand this comment. This means that willSendRequest isn't called, right?
Yes, that's right. I'll make it clearer. (In reply to comment #2) > (From update of attachment 134096 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=134096&action=review > > > Source/WebKit/chromium/tests/AssociatedURLLoaderTest.cpp:471 > > - EXPECT_TRUE(m_willSendRequest); > > + // CORS requires that the redirect be transparent. > > + EXPECT_FALSE(m_willSendRequest); > > I'm not sure I understand this comment. This means that willSendRequest isn't called, right?
Created attachment 134165 [details] Proposed Patch I worked around the test crash on Mac by not clearing the mocked responses in the test class's TearDown method.
Comment on attachment 134165 [details] Proposed Patch Thanks Bill.
Comment on attachment 134165 [details] Proposed Patch Clearing flags on attachment: 134165 Committed r112339: <http://trac.webkit.org/changeset/112339>
All reviewed patches have been landed. Closing bug.
This change seems to have caused crashes in webkit_unit_tests on snow leopard and lion (chromium) http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Mac10.7/builds/2934/steps/webkit_unit_tests/logs/stdio
Reverted r112339 for reason: webkit_unit_tests crashing on chromium mac bots Committed r112346: <http://trac.webkit.org/changeset/112346>
also linux, it appears ... http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Linux%2032/builds/15415/steps/webkit_unit_tests/logs/stdio
I'll like to re-land this after the fix lands in webkit_support. http://codereview.chromium.org/9874008/
After that patch lands, would you be willing to update http://trac.webkit.org/browser/trunk/Source/WebKit/chromium/DEPS past that revision so that webkit.org builds see the fix as well?
I will update DEPS before resubmitting.
Created attachment 134382 [details] Proposed Patch Fixed WebURLLoaderMockFactory to handle canceled loaders. Bumped DEPS to get fix.
Comment on attachment 134382 [details] Proposed Patch Rejecting attachment 134382 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: rce/WebKit/chromium/src/AssociatedURLLoader.cpp patching file Source/WebKit/chromium/tests/AssociatedURLLoaderTest.cpp Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Adam Barth']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue/ --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://queues.webkit.org/results/12156375
Created attachment 134440 [details] Proposed Patch Refreshed patch. DEPS got rolled by someone else.
Comment on attachment 134440 [details] Proposed Patch Clearing flags on attachment: 134440 Committed r112485: <http://trac.webkit.org/changeset/112485>