| Summary: | [Curl] Fix issue that extra cookie is added when redirect happens. | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Basuke Suzuki <Basuke.Suzuki> | ||||||
| Component: | Platform | Assignee: | Basuke Suzuki <Basuke.Suzuki> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | achristensen, Basuke.Suzuki, commit-queue, ews-watchlist, galpeter, Hironori.Fujii, webkit-bug-importer, youennf | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Basuke Suzuki
2018-07-20 16:44:00 PDT
Created attachment 348185 [details]
PATCH
Comment on attachment 348185 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=348185&action=review > LayoutTests/http/tests/cookies/multiple-redirect-and-set-cookie.php:3 > + * This is test for https://bugs.webkit.org/show_bug.cgi?id=187874 . Probably not necessary. version control can be used for anyone who wants to find this out > Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp:136 > -Ref<CurlRequest> ResourceHandle::createCurlRequest(ResourceRequest& request, RequestStatus status) > +Ref<CurlRequest> ResourceHandle::createCurlRequest(ResourceRequest request, RequestStatus status) This could probably be a ResourceRequest&& Created attachment 348313 [details]
PATCH
Comment on attachment 348313 [details]
PATCH
Alex, right. Simple copying was a little bit uncomfortable for me. Thanks for that advice.
Comment on attachment 348313 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=348313&action=review > Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp:136 > + return CurlRequest::create(request, *this, CurlRequest::ShouldSuspend::Yes); This could also probably take a ResourceRequest&& Comment on attachment 348313 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=348313&action=review thanks for r+. >> Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp:136 >> + return CurlRequest::create(request, *this, CurlRequest::ShouldSuspend::Yes); > > This could also probably take a ResourceRequest&& I'll open other bug for this. I'm taking a look into CurlRequest in detail and researching who gonna deref-ing the instance. If I understand correctly, if the object was deleted in main thread, there's no need to isolateCopy for the argument's ResourceRequest even if they referred from worker thread. So by calling deref() from main thread, the need of isolatedCopy() can be removed and we can store moved request directly. Please correct me if something wrong, Alex. Comment on attachment 348313 [details] PATCH Clearing flags on attachment: 348313 Committed r235437: <https://trac.webkit.org/changeset/235437> All reviewed patches have been landed. Closing bug. |