It was used when both redirect and authentication happens, but by adding const reference getter to CurlRequest, it is no longer needed.
This is a successor bug for https://bugs.webkit.org/show_bug.cgi?id=181704.
Created attachment 334406 [details] patch
Comment on attachment 334406 [details] patch Some questions below. View in context: https://bugs.webkit.org/attachment.cgi?id=334406&action=review > Source/WebCore/platform/network/ResourceHandle.h:300 > + enum class PreflightRequest { I am not clear what PreflightRequest means in that context. In the context of HTTP, preflight requests are used for CORS, but this does not seem related. Would there be a better name to explicit the purpose. Is it a mechanism to do conditional requests? > Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp:136 > +Ref<CurlRequest> ResourceHandle::createCurlRequest(ResourceRequest& request, PreflightRequest preflight) Would it make sense to take a ResourceRequest&& in case CurlRequest is probably creating its own copy of the request? > Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp:364 > + auto previousRequest = d->m_curlRequest->resourceRequest().isolatedCopy(); Is the isolatedCopy needed here?
Comment on attachment 334406 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=334406&action=review Thanks for reviewing! >> Source/WebCore/platform/network/ResourceHandle.h:300 >> + enum class PreflightRequest { > > I am not clear what PreflightRequest means in that context. > In the context of HTTP, preflight requests are used for CORS, but this does not seem related. > Would there be a better name to explicit the purpose. Is it a mechanism to do conditional requests? Ah, right. I use preflight not meaning about CORS, but general meaning of preparation by parsing the headers or such info. Is `Preprocess` the better name? >> Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp:136 >> +Ref<CurlRequest> ResourceHandle::createCurlRequest(ResourceRequest& request, PreflightRequest preflight) > > Would it make sense to take a ResourceRequest&& in case CurlRequest is probably creating its own copy of the request? Okay, i'll think about it. >> Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp:364 >> + auto previousRequest = d->m_curlRequest->resourceRequest().isolatedCopy(); > > Is the isolatedCopy needed here? Ditto.
> > I am not clear what PreflightRequest means in that context. > > In the context of HTTP, preflight requests are used for CORS, but this does not seem related. > > Would there be a better name to explicit the purpose. Is it a mechanism to do conditional requests? > > Ah, right. I use preflight not meaning about CORS, but general meaning of > preparation by parsing the headers or such info. Is `Preprocess` the better > name? Preprocess is very generic as well and it seems that this is related to an existing cache so I would try to come up with a name inducing the notion of cache. If this is related to HTTP conditional request/revalidation, I would try to make the name as specific as possible.
Created attachment 335136 [details] Patch Rename Preflight to RequestStatus which indicates it's new or reuse. Also stop isolateCopy, but simple copy. Move is not used here because original request need to be valid in m_firstRequest.
Comment on attachment 335136 [details] Patch Rejecting attachment 335136 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 335136, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: =335136 Found no modified ChangeLogs, cannot create a commit message. All changes require a ChangeLog. See: http://webkit.org/coding/contributing.html Found no modified ChangeLogs, cannot create a commit message. All changes require a ChangeLog. See: http://webkit.org/coding/contributing.html Found no modified ChangeLogs, cannot create a commit message. All changes require a ChangeLog. See: http://webkit.org/coding/contributing.html Updating OpenSource Current branch master is up to date. Full output: http://webkit-queues.webkit.org/results/6831883
Created attachment 335142 [details] changelog
Comment on attachment 335142 [details] changelog Clearing flags on attachment: 335142 Committed r229347: <https://trac.webkit.org/changeset/229347>
All reviewed patches have been landed. Closing bug.
<rdar://problem/38200401>