Remove crossOriginRequestPolicy from ThreadableLoaderOptions

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[youenn fablet]

This is redundant with FetchOptions::Mode.

[youenn fablet]

Created attachment 282779 [details]
Patch

[Alex Christensen]

Comment on attachment 282779 [details]
Patch

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

FetchOptions::Mode has 4 values and CrossOriginRequestPolicy has 3.  Is FetchOptions::Mode::Navigate not used?

> Source/WebCore/ChangeLog:3
> +        Fix 159417

This isn't necessary.  The bug link is already in the changelog.

> Source/WebCore/loader/ThreadableLoader.cpp:46
> +    mode = FetchOptions::Mode::SameOrigin;

Things like this and the growing list of things in the ResourceLoaderOptions constructor make me think we should be calling superclass constructors instead of manually setting member variables in constructors.  That might be best done in another patch, though.

[youenn fablet]

(In reply to comment #2)
> Comment on attachment 282779 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=282779&action=review
> 
> FetchOptions::Mode has 4 values and CrossOriginRequestPolicy has 3.  Is
> FetchOptions::Mode::Navigate not used?

Navigate is triggered by user interaction like page loads and so on.
Fetch API does not expose it, setting it will trigger a TypeError when constructing q Request. So it is not used in WebKit yet.

I think it becomes useful with service worker, although we could try to use it if doing some additional refactoring to better align with fetch algorith.

> > Source/WebCore/ChangeLog:3
> > +        Fix 159417
> 
> This isn't necessary.  The bug link is already in the changelog.

Sure, that is a left over, I'll remove it.

> > Source/WebCore/loader/ThreadableLoader.cpp:46
> > +    mode = FetchOptions::Mode::SameOrigin;
> 
> Things like this and the growing list of things in the ResourceLoaderOptions
> constructor make me think we should be calling superclass constructors
> instead of manually setting member variables in constructors.  That might be
> best done in another patch, though.

Constructing a ResourceLoaderOptions with all options is not very readable.
It would be easier to have some meaningful options profile, overriding 1 or 2 values locally when needed.

[youenn fablet]

Created attachment 284098 [details]
Updating change log

[WebKit Commit Bot]

Comment on attachment 284098 [details]
Updating change log

Clearing flags on attachment: 284098

Committed r203490: <http://trac.webkit.org/changeset/203490>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.