New issue
Advanced search Search tips

Issue 767948 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug

Blocking:
issue 759229
issue 779106



Sign in to add a comment

Convert SystemCookieStore & NSHTTPSystemCookie store to Async

Project Member Reported by mrefaat@chromium.org, Sep 22 2017

Issue description

This is a necessary step before Adding WKHTTPSystemCookieStore that integrates with WKHTTPCookieStore as WKHTTPCookieStore uses async methods to get/set cookies.



 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8505818b354fa784d77723c2d8a7123d154770f5

commit 8505818b354fa784d77723c2d8a7123d154770f5
Author: Mohammad Refaat <mrefaat@chromium.org>
Date: Thu Sep 28 15:31:26 2017

Convert SystemCookieStore to Async.

* Converts SystemCookieStore to Async
* Add Async functions to NSHTTPSystemCookieStore and keep the synchronous
functions.
* Move creation time management to systemCookieStore.
* Update cookie_store_ios_presistent_unittest & 
cookie_store_ios_unittests to account for SystemCookieStore changes 
* (As CookieStoreIOS is using systemCookieStore internally).

Another CL is in progress to add WKSystemCookieStore & unit_tests for 
systemCookieStore (to handle async methods) with template for both 
NSHTTP & WK systemCookieStore.

Bug: 759229,  759227 ,  767948 
Change-Id: I1c24c82540f4781a0bec77ebfa4a861ca7433391
Reviewed-on: https://chromium-review.googlesource.com/675804
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505033}
[modify] https://crrev.com/8505818b354fa784d77723c2d8a7123d154770f5/ios/chrome/app/startup/BUILD.gn
[modify] https://crrev.com/8505818b354fa784d77723c2d8a7123d154770f5/ios/chrome/app/startup/client_registration.mm
[modify] https://crrev.com/8505818b354fa784d77723c2d8a7123d154770f5/ios/chrome/browser/net/chrome_cookie_store_ios_client.h
[modify] https://crrev.com/8505818b354fa784d77723c2d8a7123d154770f5/ios/chrome/browser/net/chrome_cookie_store_ios_client.mm
[modify] https://crrev.com/8505818b354fa784d77723c2d8a7123d154770f5/ios/net/cookies/cookie_store_ios.h
[modify] https://crrev.com/8505818b354fa784d77723c2d8a7123d154770f5/ios/net/cookies/cookie_store_ios.mm
[modify] https://crrev.com/8505818b354fa784d77723c2d8a7123d154770f5/ios/net/cookies/cookie_store_ios_client.h
[modify] https://crrev.com/8505818b354fa784d77723c2d8a7123d154770f5/ios/net/cookies/cookie_store_ios_client.mm
[modify] https://crrev.com/8505818b354fa784d77723c2d8a7123d154770f5/ios/net/cookies/cookie_store_ios_persistent_unittest.mm
[modify] https://crrev.com/8505818b354fa784d77723c2d8a7123d154770f5/ios/net/cookies/cookie_store_ios_test_util.h
[modify] https://crrev.com/8505818b354fa784d77723c2d8a7123d154770f5/ios/net/cookies/cookie_store_ios_test_util.mm
[modify] https://crrev.com/8505818b354fa784d77723c2d8a7123d154770f5/ios/net/cookies/cookie_store_ios_unittest.mm
[modify] https://crrev.com/8505818b354fa784d77723c2d8a7123d154770f5/ios/net/cookies/ns_http_system_cookie_store.h
[modify] https://crrev.com/8505818b354fa784d77723c2d8a7123d154770f5/ios/net/cookies/ns_http_system_cookie_store.mm
[modify] https://crrev.com/8505818b354fa784d77723c2d8a7123d154770f5/ios/net/cookies/ns_http_system_cookie_store_unittest.mm
[modify] https://crrev.com/8505818b354fa784d77723c2d8a7123d154770f5/ios/net/cookies/system_cookie_store.h
[modify] https://crrev.com/8505818b354fa784d77723c2d8a7123d154770f5/ios/net/cookies/system_cookie_store.mm

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e7d206d2687f548d544c46806e52e1ef4bb97189

commit e7d206d2687f548d544c46806e52e1ef4bb97189
Author: Yuke Liao <liaoyuke@chromium.org>
Date: Thu Sep 28 21:59:49 2017

Revert "Convert SystemCookieStore to Async."

This reverts commit 8505818b354fa784d77723c2d8a7123d154770f5.

Reason for revert: This CL is causing CookieStoreIOSTest.SameValueDoesNotCallHook to fail on iPhone 7, iOS 11 devices.

Original change's description:
> Convert SystemCookieStore to Async.
> 
> * Converts SystemCookieStore to Async
> * Add Async functions to NSHTTPSystemCookieStore and keep the synchronous
> functions.
> * Move creation time management to systemCookieStore.
> * Update cookie_store_ios_presistent_unittest & 
> cookie_store_ios_unittests to account for SystemCookieStore changes 
> * (As CookieStoreIOS is using systemCookieStore internally).
> 
> Another CL is in progress to add WKSystemCookieStore & unit_tests for 
> systemCookieStore (to handle async methods) with template for both 
> NSHTTP & WK systemCookieStore.
> 
> Bug: 759229,  759227 ,  767948 
> Change-Id: I1c24c82540f4781a0bec77ebfa4a861ca7433391
> Reviewed-on: https://chromium-review.googlesource.com/675804
> Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
> Reviewed-by: Eugene But <eugenebut@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#505033}

TBR=mmenke@chromium.org,eugenebut@chromium.org,mrefaat@chromium.org

Change-Id: I43c71a95910d14f4717111a66f0fb0556a91fb8f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 759229,  759227 ,  767948 
Reviewed-on: https://chromium-review.googlesource.com/691037
Reviewed-by: Yuke Liao <liaoyuke@chromium.org>
Commit-Queue: Yuke Liao <liaoyuke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505178}
[modify] https://crrev.com/e7d206d2687f548d544c46806e52e1ef4bb97189/ios/chrome/app/startup/BUILD.gn
[modify] https://crrev.com/e7d206d2687f548d544c46806e52e1ef4bb97189/ios/chrome/app/startup/client_registration.mm
[modify] https://crrev.com/e7d206d2687f548d544c46806e52e1ef4bb97189/ios/chrome/browser/net/chrome_cookie_store_ios_client.h
[modify] https://crrev.com/e7d206d2687f548d544c46806e52e1ef4bb97189/ios/chrome/browser/net/chrome_cookie_store_ios_client.mm
[modify] https://crrev.com/e7d206d2687f548d544c46806e52e1ef4bb97189/ios/net/cookies/cookie_store_ios.h
[modify] https://crrev.com/e7d206d2687f548d544c46806e52e1ef4bb97189/ios/net/cookies/cookie_store_ios.mm
[modify] https://crrev.com/e7d206d2687f548d544c46806e52e1ef4bb97189/ios/net/cookies/cookie_store_ios_client.h
[modify] https://crrev.com/e7d206d2687f548d544c46806e52e1ef4bb97189/ios/net/cookies/cookie_store_ios_client.mm
[modify] https://crrev.com/e7d206d2687f548d544c46806e52e1ef4bb97189/ios/net/cookies/cookie_store_ios_persistent_unittest.mm
[modify] https://crrev.com/e7d206d2687f548d544c46806e52e1ef4bb97189/ios/net/cookies/cookie_store_ios_test_util.h
[modify] https://crrev.com/e7d206d2687f548d544c46806e52e1ef4bb97189/ios/net/cookies/cookie_store_ios_test_util.mm
[modify] https://crrev.com/e7d206d2687f548d544c46806e52e1ef4bb97189/ios/net/cookies/cookie_store_ios_unittest.mm
[modify] https://crrev.com/e7d206d2687f548d544c46806e52e1ef4bb97189/ios/net/cookies/ns_http_system_cookie_store.h
[modify] https://crrev.com/e7d206d2687f548d544c46806e52e1ef4bb97189/ios/net/cookies/ns_http_system_cookie_store.mm
[modify] https://crrev.com/e7d206d2687f548d544c46806e52e1ef4bb97189/ios/net/cookies/ns_http_system_cookie_store_unittest.mm
[modify] https://crrev.com/e7d206d2687f548d544c46806e52e1ef4bb97189/ios/net/cookies/system_cookie_store.h
[modify] https://crrev.com/e7d206d2687f548d544c46806e52e1ef4bb97189/ios/net/cookies/system_cookie_store.mm

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/336a087175cc8a996437609fa2f632939af44a87

commit 336a087175cc8a996437609fa2f632939af44a87
Author: mrefaat <mrefaat@chromium.org>
Date: Wed Oct 04 22:46:11 2017

[Reland] Convert SystemCookieStore to Async.

* Converts SystemCookieStore to Async
* Add Async functions to NSHTTPSystemCookieStore and keep the synchronous
functions.
* Move creation time management to systemCookieStore.
* Update cookie_store_ios_presistent_unittest &
cookie_store_ios_unittests to account for SystemCookieStore changes
* (As CookieStoreIOS is using systemCookieStore internally).

Another CL is in progress to add WKSystemCookieStore & unit_tests for
systemCookieStore (to handle async methods) with template for both
NSHTTP & WK systemCookieStore.

Bug: 759229,  759227 ,  767948 
Change-Id: If5e1fdb8d6424085d5212a3ea867968642db554f
Reviewed-on: https://chromium-review.googlesource.com/700818
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506549}
[modify] https://crrev.com/336a087175cc8a996437609fa2f632939af44a87/ios/chrome/app/startup/BUILD.gn
[modify] https://crrev.com/336a087175cc8a996437609fa2f632939af44a87/ios/chrome/app/startup/client_registration.mm
[modify] https://crrev.com/336a087175cc8a996437609fa2f632939af44a87/ios/chrome/browser/net/chrome_cookie_store_ios_client.h
[modify] https://crrev.com/336a087175cc8a996437609fa2f632939af44a87/ios/chrome/browser/net/chrome_cookie_store_ios_client.mm
[modify] https://crrev.com/336a087175cc8a996437609fa2f632939af44a87/ios/net/cookies/cookie_store_ios.h
[modify] https://crrev.com/336a087175cc8a996437609fa2f632939af44a87/ios/net/cookies/cookie_store_ios.mm
[modify] https://crrev.com/336a087175cc8a996437609fa2f632939af44a87/ios/net/cookies/cookie_store_ios_client.h
[modify] https://crrev.com/336a087175cc8a996437609fa2f632939af44a87/ios/net/cookies/cookie_store_ios_client.mm
[modify] https://crrev.com/336a087175cc8a996437609fa2f632939af44a87/ios/net/cookies/cookie_store_ios_persistent_unittest.mm
[modify] https://crrev.com/336a087175cc8a996437609fa2f632939af44a87/ios/net/cookies/cookie_store_ios_test_util.h
[modify] https://crrev.com/336a087175cc8a996437609fa2f632939af44a87/ios/net/cookies/cookie_store_ios_test_util.mm
[modify] https://crrev.com/336a087175cc8a996437609fa2f632939af44a87/ios/net/cookies/cookie_store_ios_unittest.mm
[modify] https://crrev.com/336a087175cc8a996437609fa2f632939af44a87/ios/net/cookies/ns_http_system_cookie_store.h
[modify] https://crrev.com/336a087175cc8a996437609fa2f632939af44a87/ios/net/cookies/ns_http_system_cookie_store.mm
[modify] https://crrev.com/336a087175cc8a996437609fa2f632939af44a87/ios/net/cookies/ns_http_system_cookie_store_unittest.mm
[modify] https://crrev.com/336a087175cc8a996437609fa2f632939af44a87/ios/net/cookies/system_cookie_store.h
[modify] https://crrev.com/336a087175cc8a996437609fa2f632939af44a87/ios/net/cookies/system_cookie_store.mm

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f34616fb87c3d63e56a0268ce60fab1eb1d94ab2

commit f34616fb87c3d63e56a0268ce60fab1eb1d94ab2
Author: Matt Menke <mmenke@chromium.org>
Date: Thu Oct 05 18:31:03 2017

Revert "[Reland] Convert SystemCookieStore to Async."

This reverts commit 336a087175cc8a996437609fa2f632939af44a87.

Reason for revert: This is breaking Cronet iOS tests.

Sample trace:

#0	0x0000000102c2e6b0 in net::RunCookieCallback(base::OnceCallback<void ()>) at /Users/kapishnikov/chrome/repo/ios/src/out/Debug-iphonesimulator/../../ios/net/cookies/ns_http_system_cookie_store.mm:25
#1	0x0000000102c2eb20 in net::NSHTTPSystemCookieStore::GetCookiesForURLAsync(GURL const&, base::OnceCallback<void (NSArray<NSHTTPCookie*>*)>) at /Users/kapishnikov/chrome/repo/ios/src/out/Debug-iphonesimulator/../../ios/net/cookies/ns_http_system_cookie_store.mm:45
#2	0x0000000102c0f5b0 in net::CookieStoreIOS::GetCookieListWithOptionsAsync(GURL const&, net::CookieOptions const&, base::OnceCallback<void (std::__1::vector<net::CanonicalCookie, std::__1::allocator<net::CanonicalCookie> > const&)>) at /Users/kapishnikov/chrome/repo/ios/src/out/Debug-iphonesimulator/../../ios/net/cookies/cookie_store_ios.mm:459
#3	0x000000010380caec in net::URLRequestHttpJob::AddCookieHeaderAndStart() at /Users/kapishnikov/chrome/repo/ios/src/out/Debug-iphonesimulator/../../net/url_request/url_request_http_job.cc:597
#4	0x000000010380c598 in net::URLRequestHttpJob::Start() at /Users/kapishnikov/chrome/repo/ios/src/out/Debug-iphonesimulator/../../net/url_request/url_request_http_job.cc:321
#5	0x00000001037e87d8 in net::URLRequest::StartJob(net::URLRequestJob*) at /Users/kapishnikov/chrome/repo/ios/src/out/Debug-iphonesimulator/../../net/url_request/url_request.cc:668
#6	0x00000001037e7ec4 in net::URLRequest::BeforeRequestComplete(int) at /Users/kapishnikov/chrome/repo/ios/src/out/Debug-iphonesimulator/../../net/url_request/url_request.cc:613
#7	0x00000001037e7738 in net::URLRequest::Start() at /Users/kapishnikov/chrome/repo/ios/src/out/Debug-iphonesimulator/../../net/url_request/url_request.cc:538
#8	0x0000000102c3a620 in net::HttpProtocolHandlerCore::Start(id<CRNNetworkClientProtocol>) at /Users/kapishnikov/chrome/repo/ios/src/out/Debug-iphonesimulator/../../ios/net/crn_http_protocol_handler.mm:708
#9	0x0000000102c41130 in void base::internal::FunctorTraits<void (net::HttpProtocolHandlerCore::*)(id<CRNNetworkClientProtocol>), void>::Invoke<scoped_refptr<net::HttpProtocolHandlerCore> const&, base::scoped_nsprotocol<id<CRNHTTPProtocolHandlerProxy> __strong> const&>(void (id<CRNNetworkClientProtocol>), scoped_refptr<net::HttpProtocolHandlerCore> const&&&, base::scoped_nsprotocol<id<CRNHTTPProtocolHandlerProxy> __strong> const&&&) at /Users/kapishnikov/chrome/repo/ios/src/out/Debug-iphonesimulator/../../base/bind_internal.h:194
#10	0x0000000102c41034 in void base::internal::InvokeHelper<false, void>::MakeItSo<void (net::HttpProtocolHandlerCore::* const&)(id<CRNNetworkClientProtocol>), scoped_refptr<net::HttpProtocolHandlerCore> const&, base::scoped_nsprotocol<id<CRNHTTPProtocolHandlerProxy> __strong> const&>(void (net::HttpProtocolHandlerCore::* const&&&)(id<CRNNetworkClientProtocol>), scoped_refptr<net::HttpProtocolHandlerCore> const&&&, base::scoped_nsprotocol<id<CRNHTTPProtocolHandlerProxy> __strong> const&&&) at /Users/kapishnikov/chrome/repo/ios/src/out/Debug-iphonesimulator/../../base/bind_internal.h:277
#11	0x0000000102c40fc0 in void base::internal::Invoker<base::internal::BindState<void (net::HttpProtocolHandlerCore::*)(id<CRNNetworkClientProtocol>), scoped_refptr<net::HttpProtocolHandlerCore>, base::scoped_nsprotocol<id<CRNHTTPProtocolHandlerProxy> __strong> >, void ()>::RunImpl<void  const(&)(id<CRNNetworkClientProtocol>), std::__1::tuple<scoped_refptr, id<CRNHTTPProtocolHandlerProxy> > const&, 0ul, 1ul>(void  const(&&&)(id<CRNNetworkClientProtocol>), std::__1::tuple<scoped_refptr, id<CRNHTTPProtocolHandlerProxy> > const&&&, void  const(id<CRNNetworkClientProtocol>)::integer_sequence<unsigned long, 0ul, 1ul>) at /Users/kapishnikov/chrome/repo/ios/src/out/Debug-iphonesimulator/../../base/bind_internal.h:349
#12	0x0000000102c40ebc in base::internal::Invoker<base::internal::BindState<void (net::HttpProtocolHandlerCore::*)(id<CRNNetworkClientProtocol>), scoped_refptr<net::HttpProtocolHandlerCore>, base::scoped_nsprotocol<id<CRNHTTPProtocolHandlerProxy> __strong> >, void ()>::Run(base::internal::BindStateBase*) at /Users/kapishnikov/chrome/repo/ios/src/out/Debug-iphonesimulator/../../base/bind_internal.h:331

Original change's description:
> [Reland] Convert SystemCookieStore to Async.
> 
> * Converts SystemCookieStore to Async
> * Add Async functions to NSHTTPSystemCookieStore and keep the synchronous
> functions.
> * Move creation time management to systemCookieStore.
> * Update cookie_store_ios_presistent_unittest &
> cookie_store_ios_unittests to account for SystemCookieStore changes
> * (As CookieStoreIOS is using systemCookieStore internally).
> 
> Another CL is in progress to add WKSystemCookieStore & unit_tests for
> systemCookieStore (to handle async methods) with template for both
> NSHTTP & WK systemCookieStore.
> 
> Bug: 759229,  759227 ,  767948 
> Change-Id: If5e1fdb8d6424085d5212a3ea867968642db554f
> Reviewed-on: https://chromium-review.googlesource.com/700818
> Reviewed-by: Eugene But <eugenebut@chromium.org>
> Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#506549}

TBR=eugenebut@chromium.org,mrefaat@chromium.org

Change-Id: I7c653c57501745359ecb35ff422996545b7587f2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 759229,  759227 ,  767948 
Reviewed-on: https://chromium-review.googlesource.com/702974
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506798}
[modify] https://crrev.com/f34616fb87c3d63e56a0268ce60fab1eb1d94ab2/ios/chrome/app/startup/BUILD.gn
[modify] https://crrev.com/f34616fb87c3d63e56a0268ce60fab1eb1d94ab2/ios/chrome/app/startup/client_registration.mm
[modify] https://crrev.com/f34616fb87c3d63e56a0268ce60fab1eb1d94ab2/ios/chrome/browser/net/chrome_cookie_store_ios_client.h
[modify] https://crrev.com/f34616fb87c3d63e56a0268ce60fab1eb1d94ab2/ios/chrome/browser/net/chrome_cookie_store_ios_client.mm
[modify] https://crrev.com/f34616fb87c3d63e56a0268ce60fab1eb1d94ab2/ios/net/cookies/cookie_store_ios.h
[modify] https://crrev.com/f34616fb87c3d63e56a0268ce60fab1eb1d94ab2/ios/net/cookies/cookie_store_ios.mm
[modify] https://crrev.com/f34616fb87c3d63e56a0268ce60fab1eb1d94ab2/ios/net/cookies/cookie_store_ios_client.h
[modify] https://crrev.com/f34616fb87c3d63e56a0268ce60fab1eb1d94ab2/ios/net/cookies/cookie_store_ios_client.mm
[modify] https://crrev.com/f34616fb87c3d63e56a0268ce60fab1eb1d94ab2/ios/net/cookies/cookie_store_ios_persistent_unittest.mm
[modify] https://crrev.com/f34616fb87c3d63e56a0268ce60fab1eb1d94ab2/ios/net/cookies/cookie_store_ios_test_util.h
[modify] https://crrev.com/f34616fb87c3d63e56a0268ce60fab1eb1d94ab2/ios/net/cookies/cookie_store_ios_test_util.mm
[modify] https://crrev.com/f34616fb87c3d63e56a0268ce60fab1eb1d94ab2/ios/net/cookies/cookie_store_ios_unittest.mm
[modify] https://crrev.com/f34616fb87c3d63e56a0268ce60fab1eb1d94ab2/ios/net/cookies/ns_http_system_cookie_store.h
[modify] https://crrev.com/f34616fb87c3d63e56a0268ce60fab1eb1d94ab2/ios/net/cookies/ns_http_system_cookie_store.mm
[modify] https://crrev.com/f34616fb87c3d63e56a0268ce60fab1eb1d94ab2/ios/net/cookies/ns_http_system_cookie_store_unittest.mm
[modify] https://crrev.com/f34616fb87c3d63e56a0268ce60fab1eb1d94ab2/ios/net/cookies/system_cookie_store.h
[modify] https://crrev.com/f34616fb87c3d63e56a0268ce60fab1eb1d94ab2/ios/net/cookies/system_cookie_store.mm

Blocking: 779106
Status: Fixed (was: Assigned)

Sign in to add a comment