Eliminate usage of OAuth2TokenService::LoadCredentials() in OAuth2LoginManager |
||
Issue descriptionWe're aiming to have the loading of refresh tokens be totally internal to the identity codebase. https://chromium-review.googlesource.com/c/1386427 landed toward that goal. I just noticed that OAuth2LoginManager calls LoadCredentials(), seemingly just as a mechanism for having OnRefreshTokensLoaded() be fired. We should eliminate this usage. I'm first going to create a CL just removing it entirely and seeing whether the trybots are happy. If they are, then we can discuss on the CL whether that might be OK to go forward with. I see no obvious reason that any consumers would be listening for OnRefreshTokensLoaded() to be fired by OAuth2LoginManager given the way that the codebase is organized now. If there are any such consumers, we should ferret them out and they should interact with OAuth2LoginManager in some different way.
,
Jan 16
(6 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bb45be1b132a75cb1ed489035b4b603222595e1e commit bb45be1b132a75cb1ed489035b4b603222595e1e Author: Colin Blundell <blundell@chromium.org> Date: Wed Jan 16 17:44:18 2019 Remove call to LoadCredentials() from OAuth2LoginManager We're aiming to have the loading of refresh tokens be totally internal to the identity codebase. https://chromium-review.googlesource.com/c/1386427 landed toward that goal. I just noticed that OAuth2LoginManager calls LoadCredentials(), seemingly just as a mechanism for having OnRefreshTokensLoaded() be fired. I see no obvious reason that any consumers would be listening for OnRefreshTokensLoaded() to be fired by OAuth2LoginManager given the way that the codebase is organized now. This CL eliminates these calls entirely. If there are any such consumers, we should ferret them out and they should interact with OAuth2LoginManager in some different way (e.g., it can have some observer method that these consumers listen for). Bug: 922410 Change-Id: Ic8cfc1c25152dc29610fcf823ed9e98bd36ec0a3 Reviewed-on: https://chromium-review.googlesource.com/c/1412831 Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#623281} [modify] https://crrev.com/bb45be1b132a75cb1ed489035b4b603222595e1e/chrome/browser/chromeos/login/signin/oauth2_login_manager.cc [modify] https://crrev.com/bb45be1b132a75cb1ed489035b4b603222595e1e/chrome/browser/chromeos/login/signin/oauth2_login_manager.h
,
Jan 17
(5 days ago)
Fixed assuming it sticks :). |
||
►
Sign in to add a comment |
||
Comment 1 by blundell@chromium.org
, Jan 16 (6 days ago)