Created attachment 282779 [details] Patch

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.

(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.

Created attachment 284098 [details] Updating change log

Comment on attachment 284098 [details] Updating change log Clearing flags on attachment: 284098 Committed r203490: <http://trac.webkit.org/changeset/203490>

All reviewed patches have been landed. Closing bug.