iOS CookieStore implementations don't pass change unit tests |
|||
Issue descriptionhttps://crrev.com/c/925694 moves cookie change dispatch tests, covering CookieStoreAddCallbackFor*(), from cookie_monster_unittests to a header that can be imported by all CookieStore implementations. The iOS implementations (net::CookieStoreIOS and net::CookieStoreIOSPersistent in //ios/net/cookies) fail all the tests, so the change dispatch tests were disabled in the traits for the iOS implementations. This TODO tracks fixing the cookie store implementations so the tests can be re-enabled.
,
Feb 21 2018
mrefaat@ could you PTAL.
,
Feb 21 2018
checking it now, thanks for the redirect sczs@
,
Feb 22 2018
FYI, I'm in the middle of a refactoring of change notifications, and I plan to land that in the next day or two. WiP CL: https://crrev.com/c/919159 I mentioned this to help plan work. Also, a fair chunk of the tests pass if a call to CoookieStoreIOS::NotifySystemCookiesChanged() is added to RunUntilIdle() in the TestTraits.
,
Feb 28 2018
Assigning back to pwnall@ as per our offline conversation
,
Mar 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1dcd6759161d5b2b897c49d42978f9e8c301eb31 commit 1dcd6759161d5b2b897c49d42978f9e8c301eb31 Author: Victor Costan <pwnall@chromium.org> Date: Fri Mar 16 12:31:36 2018 Cookie Store: Enable some change tests for CookieStoreIOS. The iOS CookieStore implementations synchronize (to some degree) a CookieMonster with the system cookie store. Change detection is implemented by storing a snapshot of the system cookie store's last seen contents, and computing diffs between the current system store contents and the snapshot. This implementation has two major differences from the CookieMonsterChangeDispatcher implementation: 1) The ordering of changes observed between two snapshots cannot be known. However, the CookieStore tests expect changes to be observed in the exact order in which the corresponding commands were issued. 2) The current diffing implementation does not distinguish between deletion causes. The tests rely on a distinction between OVERWRITE and EXPLICIT. This CL introduces CookieStoreTestTraits members that account for the differences above, making it possible to run the change tests against the CookieStoreIOS implementation. CookieStoreIOSPersistent requires significant changes to pass the tests, so it will be addressed in a separate CL. Bug: 729800, 813931 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: Ib959c4a2d05ef84ab154290ee91f975436a7ee64 Reviewed-on: https://chromium-review.googlesource.com/947844 Commit-Queue: Victor Costan <pwnall@chromium.org> Reviewed-by: Matt Menke <mmenke@chromium.org> Reviewed-by: Richard Coles <torne@chromium.org> Reviewed-by: David Roger <droger@chromium.org> Cr-Commit-Position: refs/heads/master@{#543673} [modify] https://crrev.com/1dcd6759161d5b2b897c49d42978f9e8c301eb31/android_webview/browser/net/aw_cookie_store_wrapper_unittest.cc [modify] https://crrev.com/1dcd6759161d5b2b897c49d42978f9e8c301eb31/ios/net/cookies/cookie_store_ios_persistent_unittest.mm [modify] https://crrev.com/1dcd6759161d5b2b897c49d42978f9e8c301eb31/ios/net/cookies/cookie_store_ios_unittest.mm [modify] https://crrev.com/1dcd6759161d5b2b897c49d42978f9e8c301eb31/net/BUILD.gn [add] https://crrev.com/1dcd6759161d5b2b897c49d42978f9e8c301eb31/net/cookies/cookie_change_dispatcher_test_helpers.cc [add] https://crrev.com/1dcd6759161d5b2b897c49d42978f9e8c301eb31/net/cookies/cookie_change_dispatcher_test_helpers.h [modify] https://crrev.com/1dcd6759161d5b2b897c49d42978f9e8c301eb31/net/cookies/cookie_monster_unittest.cc [modify] https://crrev.com/1dcd6759161d5b2b897c49d42978f9e8c301eb31/net/cookies/cookie_store_change_unittest.h [modify] https://crrev.com/1dcd6759161d5b2b897c49d42978f9e8c301eb31/net/cookies/cookie_store_unittest.h |
|||
►
Sign in to add a comment |
|||
Comment 1 by pwnall@chromium.org
, Feb 20 2018