Bug 74952 – Need AssociatedURLLoader tests for redirects and CORS access control

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Bill Budge]

Add unit tests for AssociatedURLLoader to check handling of redirects and cross-origin access control.

[Bill Budge]

Created attachment 120071 [details]
Proposed Patch

This follows the addition of redirect handling in weburl_loader_mock, and adds tests of redirects, CORS, and CORS w/ redirect (currently not supported by WebKit, so disabled.)

[Bill Budge]

Created attachment 120074 [details]
Proposed Patch

[Bill Budge]

Created attachment 120079 [details]
Proposed Patch

svn troubles

[Bill Budge]

Created attachment 120083 [details]
Proposed Patch

A little rusty at this. Try again.

[Bill Budge]

Created attachment 120173 [details]
Proposed Patch

I had a bad client setup.

[Adam Barth]

Comment on attachment 120173 [details]
Proposed Patch

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

> Source/WebKit/chromium/tests/AssociatedURLLoaderTest.cpp:-126
> -        EXPECT_EQ(m_expectedRedirectResponse.mimeType(), redirectResponse.mimeType());

Can you explain in the ChangeLog why you're removing this check?

> Source/WebKit/chromium/tests/AssociatedURLLoaderTest.cpp:176
> +          m_runningMessageLoop = false;
> +          webkit_support::QuitMessageLoop();

Four-space indent pls.

[Bill Budge]

Created attachment 120221 [details]
Proposed Patch

Adam, it turns out it's easier to leave that check in than to add the comment. So I put it back in.

My thinking was that there are a lot of properties I can check on the responses, but since they're created by the test code (mocks) and passed unchanged by the loader framework, it's just testing the test code.

Thanks for reviewing this!

[WebKit Review Bot]

Attachment 120221 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
From git://git.webkit.org/WebKit
   e797230..daea471  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 103455 = e797230b5731bc67238cdaca8cd2f71e3c1b08c4
r103457 = 0764c68a648212f3475b6449e45b26f52901256a
r103458 = daea471373d620a3fbc3e7dbc138f927d151a5c9
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Applying: Inform the scrolling coordinator when scrollbar layers come and go
Using index info to reconstruct a base tree...
<stdin>:474806: trailing whitespace.
        [Chromium] DatabaseTrackerChromium: iterating DatabaseSet races with Database disposal on worker thread 
<stdin>:474827: trailing whitespace.
        Nothing to test, just removing redundant code. Correct behavior tested by 
<stdin>:475346: trailing whitespace.
    
warning: 3 lines add whitespace errors.
Falling back to patching base and 3-way merge...
warning: too many files (created: 167249 deleted: 3), skipping inexact rename detection
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/chromium/test_expectations.txt
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Auto-merging Source/WebCore/rendering/RenderLayerCompositor.cpp
CONFLICT (content): Merge conflict in Source/WebCore/rendering/RenderLayerCompositor.cpp
Auto-merging Source/WebKit2/ChangeLog
CONFLICT (content): Merge conflict in Source/WebKit2/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 Inform the scrolling coordinator when scrollbar layers come and go

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 158.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Seidel (no email)]

Seems this doesn't apply, otherwise I'd be happy to r+ it.

[Bill Budge]

Created attachment 120227 [details]
Proposed Patch

Let's try this again.

[WebKit Review Bot]

Attachment 120227 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
First, rewinding head to replay your work on top of it...
Applying: Inform the scrolling coordinator when scrollbar layers come and go
Using index info to reconstruct a base tree...
<stdin>:474806: trailing whitespace.
        [Chromium] DatabaseTrackerChromium: iterating DatabaseSet races with Database disposal on worker thread 
<stdin>:474827: trailing whitespace.
        Nothing to test, just removing redundant code. Correct behavior tested by 
<stdin>:475346: trailing whitespace.
    
warning: 3 lines add whitespace errors.
Falling back to patching base and 3-way merge...
warning: too many files (created: 167249 deleted: 3), skipping inexact rename detection
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/chromium/test_expectations.txt
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Auto-merging Source/WebCore/page/ScrollingCoordinator.h
CONFLICT (content): Merge conflict in Source/WebCore/page/ScrollingCoordinator.h
Auto-merging Source/WebCore/page/mac/ScrollingCoordinatorMac.mm
CONFLICT (content): Merge conflict in Source/WebCore/page/mac/ScrollingCoordinatorMac.mm
Auto-merging Source/WebCore/platform/ScrollView.cpp
Auto-merging Source/WebCore/rendering/RenderLayerCompositor.cpp
CONFLICT (content): Merge conflict in Source/WebCore/rendering/RenderLayerCompositor.cpp
Auto-merging Source/WebKit2/ChangeLog
CONFLICT (content): Merge conflict in Source/WebKit2/ChangeLog
Auto-merging Tools/ChangeLog
CONFLICT (content): Merge conflict in Tools/ChangeLog
Auto-merging Tools/Scripts/build-webkit
Auto-merging Tools/Scripts/webkitdirs.pm
CONFLICT (content): Merge conflict in Tools/Scripts/webkitdirs.pm
Failed to merge in the changes.
Patch failed at 0001 Inform the scrolling coordinator when scrollbar layers come and go

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 158.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Bill Budge]

Created attachment 120231 [details]
Proposed Patch

Freshened up my client. Hope it fixes this.

[WebKit Review Bot]

Attachment 120231 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
First, rewinding head to replay your work on top of it...
Applying: Inform the scrolling coordinator when scrollbar layers come and go
Using index info to reconstruct a base tree...
<stdin>:474806: trailing whitespace.
        [Chromium] DatabaseTrackerChromium: iterating DatabaseSet races with Database disposal on worker thread 
<stdin>:474827: trailing whitespace.
        Nothing to test, just removing redundant code. Correct behavior tested by 
<stdin>:475346: trailing whitespace.
    
warning: 3 lines add whitespace errors.
Falling back to patching base and 3-way merge...
warning: too many files (created: 167254 deleted: 3), skipping inexact rename detection
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/chromium/test_expectations.txt
CONFLICT (content): Merge conflict in LayoutTests/platform/chromium/test_expectations.txt
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Auto-merging Source/WebCore/page/ScrollingCoordinator.h
CONFLICT (content): Merge conflict in Source/WebCore/page/ScrollingCoordinator.h
Auto-merging Source/WebCore/page/mac/ScrollingCoordinatorMac.mm
CONFLICT (content): Merge conflict in Source/WebCore/page/mac/ScrollingCoordinatorMac.mm
Auto-merging Source/WebCore/platform/ScrollView.cpp
Auto-merging Source/WebCore/rendering/RenderLayerCompositor.cpp
CONFLICT (content): Merge conflict in Source/WebCore/rendering/RenderLayerCompositor.cpp
Auto-merging Source/WebKit2/ChangeLog
CONFLICT (content): Merge conflict in Source/WebKit2/ChangeLog
Auto-merging Tools/ChangeLog
CONFLICT (content): Merge conflict in Tools/ChangeLog
Auto-merging Tools/Scripts/build-webkit
Auto-merging Tools/Scripts/webkitdirs.pm
CONFLICT (content): Merge conflict in Tools/Scripts/webkitdirs.pm
Failed to merge in the changes.
Patch failed at 0001 Inform the scrolling coordinator when scrollbar layers come and go

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 158.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Adam Barth]

Sorry, the style bot was having some issues today.

[Adam Barth]

Comment on attachment 120231 [details]
Proposed Patch

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

> Source/WebKit/chromium/tests/AssociatedURLLoaderTest.cpp:-126
> -        EXPECT_EQ(m_expectedRedirectResponse.mimeType(), redirectResponse.mimeType());

I thought you were going to leave this in?

> Source/WebKit/chromium/tests/AssociatedURLLoaderTest.cpp:177
> +        if (m_runningMessageLoop) {
> +          m_runningMessageLoop = false;
> +          webkit_support::QuitMessageLoop();
> +        }

Bad indent.

[Bill Budge]

Created attachment 120338 [details]
Proposed Patch

Sorry, in all the confusion, I didn't save the file with your changes.

[WebKit Review Bot]

Comment on attachment 120338 [details]
Proposed Patch

Clearing flags on attachment: 120338

Committed r103612: <http://trac.webkit.org/changeset/103612>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.