Convert components/signin/core/browser uses of cookies to Mojo CookieManager interface |
||||||||||||||||
Issue descriptioncomponents/signin/core/browser currently uses the CookieStore C++ ABC to access cookies; it should be converted to use the Mojo CookieManager interface. This is probably not too hard, but it's believed large enough to get its own bug rather than be lumped in to issue 792660 (which has a pointer to example transformation code). Specific locations to start untangling from: components/signin/core/browser signin_client.h: ABC, AddCookieChangedCallback(n::CS::CCC) signing_cookie_changed_subscription.h: Implemention of RV from above Only one caller: GaiaCookieManagerService::Init
,
Dec 6 2017
,
Dec 21 2017
--Chrome Identity automated triaging-- This bug is Untriaged and has gone for two weeks without any activity, so it is being moved to Available. Please see https://goo.gl/78kbny for more details. Please remove the Services>SignIn or UI>Browser>Profiles components if this bug isn't related to Chrome Identity. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 22 2017
Assigning to David.
,
Jan 22 2018
--Chrome Identity automated triaging-- This bug is Assigned and has gone one month without any activity, so it is being moved to Available to indicate that it is not actively being worked on. If you are working on this bug, please mark yourself as the owner and move back to Assigned. Please see https://goo.gl/78kbny for more details. Please remove the Services>SignIn or UI>Browser>Profiles components if this bug isn't related to Chrome Identity. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 17 2018
,
May 18 2018
,
May 22 2018
Assigning to David (per comment #4).
,
May 29 2018
david, can you take a look at this bug? We need this fixed by end of June since it will block our launch of network service to canary.
,
May 31 2018
Looking at the code it seems that the mojo API is identical to the C++ one. It's only a matter of writing the mojo boilerplate (which I am completely unfamiliar with), but I can try copying what is done in content::CookieStoreManager.
,
Jun 25 2018
droger@, let me know if you're running into any blockers on this issue. I can take a look at finishing this up if you don't have time.
,
Jun 26 2018
I can do it, but probably not very soon. If you need it by the end of June, it's probably better if you can take a look.
,
Jun 26 2018
Note that the "Pri-1" is used on network service bugs to mean they block Canary. Don't ask me why it's used in combination with Proj-Servicification-Canary, which would seem to imply the same thing.
,
Jul 18
reily, can you help take a look?
,
Jul 18
Yes, I'll start this once I've finished up the other work I have in progress.
,
Aug 10
Change is out for review: https://chromium-review.googlesource.com/c/chromium/src/+/1170224
,
Aug 13
Issue 873748 has been merged into this issue.
,
Aug 14
,
Aug 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d055e32c44b577a2559bc61c9ce126b18ec2cf41 commit d055e32c44b577a2559bc61c9ce126b18ec2cf41 Author: Reilly Grant <reillyg@chromium.org> Date: Wed Aug 15 00:01:18 2018 Convert GaiaCookieManagerService to the Mojo CookieManager interface This change updates the GaiaCookieManagerService to use the CookieManager Mojo interface provided by the Network Service rather than the net::CookieManager. The setup for listening to cookie changes can be drastically simplified because the CookieChangeListener pipe can be bound directly on the main thread and so all the thread hopping provided by SigninCookieChangeSubscription is no longer needed. Bug: 792663 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: Ic84f66cf780a7f6ca9f0c6f1ff8da3ea1df65875 Reviewed-on: https://chromium-review.googlesource.com/1170224 Reviewed-by: Eugene But <eugenebut@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Reviewed-by: Roger Tawa <rogerta@chromium.org> Commit-Queue: Reilly Grant <reillyg@chromium.org> Cr-Commit-Position: refs/heads/master@{#583089} [modify] https://crrev.com/d055e32c44b577a2559bc61c9ce126b18ec2cf41/chrome/browser/extensions/api/identity/identity_apitest.cc [modify] https://crrev.com/d055e32c44b577a2559bc61c9ce126b18ec2cf41/chrome/browser/signin/chrome_signin_client.cc [modify] https://crrev.com/d055e32c44b577a2559bc61c9ce126b18ec2cf41/chrome/browser/signin/chrome_signin_client.h [modify] https://crrev.com/d055e32c44b577a2559bc61c9ce126b18ec2cf41/components/signin/core/browser/BUILD.gn [modify] https://crrev.com/d055e32c44b577a2559bc61c9ce126b18ec2cf41/components/signin/core/browser/DEPS [modify] https://crrev.com/d055e32c44b577a2559bc61c9ce126b18ec2cf41/components/signin/core/browser/gaia_cookie_manager_service.cc [modify] https://crrev.com/d055e32c44b577a2559bc61c9ce126b18ec2cf41/components/signin/core/browser/gaia_cookie_manager_service.h [modify] https://crrev.com/d055e32c44b577a2559bc61c9ce126b18ec2cf41/components/signin/core/browser/signin_client.h [delete] https://crrev.com/32cd8fcf95ca03bbc1d39938a2c7c927fe88974e/components/signin/core/browser/signin_cookie_change_subscription.cc [delete] https://crrev.com/32cd8fcf95ca03bbc1d39938a2c7c927fe88974e/components/signin/core/browser/signin_cookie_change_subscription.h [modify] https://crrev.com/d055e32c44b577a2559bc61c9ce126b18ec2cf41/components/signin/core/browser/test_signin_client.cc [modify] https://crrev.com/d055e32c44b577a2559bc61c9ce126b18ec2cf41/components/signin/core/browser/test_signin_client.h [modify] https://crrev.com/d055e32c44b577a2559bc61c9ce126b18ec2cf41/ios/chrome/browser/signin/ios_chrome_signin_client.h [modify] https://crrev.com/d055e32c44b577a2559bc61c9ce126b18ec2cf41/ios/chrome/browser/signin/ios_chrome_signin_client.mm [modify] https://crrev.com/d055e32c44b577a2559bc61c9ce126b18ec2cf41/ios/web/browser_state.mm [modify] https://crrev.com/d055e32c44b577a2559bc61c9ce126b18ec2cf41/ios/web/public/browser_state.h [modify] https://crrev.com/d055e32c44b577a2559bc61c9ce126b18ec2cf41/ios/web_view/internal/signin/ios_web_view_signin_client.h [modify] https://crrev.com/d055e32c44b577a2559bc61c9ce126b18ec2cf41/ios/web_view/internal/signin/ios_web_view_signin_client.mm [modify] https://crrev.com/d055e32c44b577a2559bc61c9ce126b18ec2cf41/ios/web_view/internal/signin/web_view_signin_client_factory.mm
,
Aug 15
,
Aug 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6760a5514ed70f2712d246c96bc6f86327d1645c commit 6760a5514ed70f2712d246c96bc6f86327d1645c Author: Reilly Grant <reillyg@chromium.org> Date: Tue Aug 28 17:33:41 2018 Attempt to reinitialize GaiaCookieManagerService on connection error This is a follow-up to r583089 as it was pointed out that the original patch did not handle connection errors caused by a network service process restart. In this case we can simply try to reinstall the cookie change listener. Bug: 792663 Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs Change-Id: I5ef4b038b942a47c8aa1f8cc485327ba14f2fa28 Reviewed-on: https://chromium-review.googlesource.com/1192022 Commit-Queue: Reilly Grant <reillyg@chromium.org> Reviewed-by: David Roger <droger@chromium.org> Cr-Commit-Position: refs/heads/master@{#586770} [modify] https://crrev.com/6760a5514ed70f2712d246c96bc6f86327d1645c/chrome/browser/profiles/profile_manager.cc [modify] https://crrev.com/6760a5514ed70f2712d246c96bc6f86327d1645c/components/signin/core/browser/gaia_cookie_manager_service.cc [modify] https://crrev.com/6760a5514ed70f2712d246c96bc6f86327d1645c/components/signin/core/browser/gaia_cookie_manager_service.h [modify] https://crrev.com/6760a5514ed70f2712d246c96bc6f86327d1645c/ios/chrome/browser/browser_state/chrome_browser_state_manager_impl.cc |
||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||
Comment 1 by mmenke@chromium.org
, Dec 6 2017