Eliminate account_investigator_unittest's knowledge of PO2TS, SigninManager, et al |
|||||
Issue descriptionSee blocking bug.
,
Nov 29
I think we can do 3 steps here: 1) get rid of signinmanager and po2ts direct use. 2) make it possible not to pass the |gaia_cookie_manager_service_| to AccountInvestigator. For that we would need to fix bug 903718 (I think I am volunteering, but I might need guidance with the unittest for the API) 3) make use of the simple IdentityTestEnvironment ctor, once we need not to refer to signin manager, po2ts and gaia cookie manager service directly. This depends ib bug 908810 .
,
Nov 29
,
Nov 29
This makes total sense, Antonio! Thanks for the investigation here!
,
Dec 13
,
Jan 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e950ee8af59711652eec212757d30a69d2c07685 commit e950ee8af59711652eec212757d30a69d2c07685 Author: Antonio Gomes <tonikitoo@igalia.com> Date: Fri Jan 04 16:25:28 2019 Remove unused signin_manager getter from account_investigator_unittest.cc This is an oneliner removal follow up of crrev.com/c/1351152. TBR=msarda@chromium.org BUG= 909715 Change-Id: I8bc16bfb73348c23425351761695cca2258342c6 Reviewed-on: https://chromium-review.googlesource.com/c/1396237 Reviewed-by: Antonio Gomes <tonikitoo@igalia.com> Commit-Queue: Antonio Gomes <tonikitoo@igalia.com> Cr-Commit-Position: refs/heads/master@{#619966} [modify] https://crrev.com/e950ee8af59711652eec212757d30a69d2c07685/components/signin/core/browser/account_investigator_unittest.cc
,
Jan 4
This is being implemented in 3 steps, as per comment #2: CL 1/3: https://crrev.com/c/1396238 (Make signout accounts available through identity::AccountsInCookieJarInfo). CL 2/3: https://crrev.com/c/1396240 (Remove GaiaCookieManagerService dependency from AccountsInvestigator). CL 3/3: https://crrev.com/c/1396242 (Eliminate account_investigator_unittest's knowledge of PO2TS, SigninManager, et al).
,
Jan 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2619c4f277cfa4d759fcbbe0b5ed29b5da722064 commit 2619c4f277cfa4d759fcbbe0b5ed29b5da722064 Author: Antonio Gomes <tonikitoo@igalia.com> Date: Mon Jan 07 16:49:12 2019 Make signed out accounts available through identity::AccountsInCookieJarInfo This is the first out 3 CLs that will make AccountInvestigator and its unittests not depend/use GaiaCookieManagerService anymore. For this, it was needed to extend identity::AccountsInCookieJarInfo to hold "signed out accounts" information as well. This will allow AccountInvestigator to inherit from IdentityManager::Observer instead of GaiaCookieManagerService::Observer without losing any functionality. The actual inheritance change is done in a follow up CL [2]. [2] https://crrev.com/c/1396240. BUG= 909715 Change-Id: I252ba9c24e2aa63bdc53e29a1f4cf994608abd6c Reviewed-on: https://chromium-review.googlesource.com/c/1396238 Commit-Queue: Antonio Gomes <tonikitoo@igalia.com> Reviewed-by: Colin Blundell <blundell@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#620355} [modify] https://crrev.com/2619c4f277cfa4d759fcbbe0b5ed29b5da722064/components/sync/driver/sync_auth_util.cc [modify] https://crrev.com/2619c4f277cfa4d759fcbbe0b5ed29b5da722064/components/sync/driver/sync_session_durations_metrics_recorder.cc [modify] https://crrev.com/2619c4f277cfa4d759fcbbe0b5ed29b5da722064/services/identity/public/cpp/accounts_in_cookie_jar_info.cc [modify] https://crrev.com/2619c4f277cfa4d759fcbbe0b5ed29b5da722064/services/identity/public/cpp/accounts_in_cookie_jar_info.h [modify] https://crrev.com/2619c4f277cfa4d759fcbbe0b5ed29b5da722064/services/identity/public/cpp/identity_manager.cc [modify] https://crrev.com/2619c4f277cfa4d759fcbbe0b5ed29b5da722064/services/identity/public/cpp/identity_manager.h [modify] https://crrev.com/2619c4f277cfa4d759fcbbe0b5ed29b5da722064/services/identity/public/cpp/identity_manager_unittest.cc
,
Jan 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b92aad536edcac81e0daa90deb484e668995d157 commit b92aad536edcac81e0daa90deb484e668995d157 Author: Antonio Gomes <tonikitoo@igalia.com> Date: Mon Jan 07 17:19:12 2019 Remove GaiaCookieManagerService dependency from AccountsInvestigator This a 2nd out 3 CLs that will make AccountInvestigator and its unittests not depend/use GaiaCookieManagerService anymore. Here AccountInvestigator replaces inheritance of GaiaCookieManagerService::Observer by IdentityManager::Observer. Note that because of the existing AccountInfo <-> ListedAccount conversions, a helper function was introduced with a TODO to convert the whole AccountInvestigator and its unit tests to use the former. BUG=859882, 909715 Change-Id: Iaf23e19622258ce63957c6254df3aac652558a08 Reviewed-on: https://chromium-review.googlesource.com/c/1396240 Commit-Queue: Antonio Gomes <tonikitoo@igalia.com> Reviewed-by: David Roger <droger@chromium.org> Cr-Commit-Position: refs/heads/master@{#620363} [modify] https://crrev.com/b92aad536edcac81e0daa90deb484e668995d157/chrome/browser/signin/account_investigator_factory.cc [modify] https://crrev.com/b92aad536edcac81e0daa90deb484e668995d157/components/signin/core/browser/account_investigator.cc [modify] https://crrev.com/b92aad536edcac81e0daa90deb484e668995d157/components/signin/core/browser/account_investigator.h [modify] https://crrev.com/b92aad536edcac81e0daa90deb484e668995d157/components/signin/core/browser/account_investigator_unittest.cc
,
Jan 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e67e554ecdb3bffd16141379863b6a811da42f2b commit e67e554ecdb3bffd16141379863b6a811da42f2b Author: Antonio Gomes <tonikitoo@igalia.com> Date: Tue Jan 08 14:49:18 2019 Eliminate account_investigator_unittest's knowledge of PO2TS, SigninManager, et al This cl is the last one of a series of 3 CLs that aim at removing the knowledge about SigninManager, PO2TS, SigninClient and AccountTrackerService and GaiaCookieManagerService from AccountInvestigatorTest class. For this, the CL starts to make use of the simpler IdentityTestEnvironment ctor from AccountInvestigatorTest and introduces a new method (namely SetListOfAccountsInGaiaCookieStale) to IdentityTestEnvironment. BUG= 909715 Change-Id: I82449032d06fdea795450fabce28caf3928bea88 Reviewed-on: https://chromium-review.googlesource.com/c/1396242 Commit-Queue: Antonio Gomes <tonikitoo@igalia.com> Reviewed-by: Colin Blundell <blundell@chromium.org> Reviewed-by: David Roger <droger@chromium.org> Cr-Commit-Position: refs/heads/master@{#620712} [modify] https://crrev.com/e67e554ecdb3bffd16141379863b6a811da42f2b/components/signin/core/browser/account_investigator_unittest.cc [modify] https://crrev.com/e67e554ecdb3bffd16141379863b6a811da42f2b/services/identity/public/cpp/identity_test_environment.cc [modify] https://crrev.com/e67e554ecdb3bffd16141379863b6a811da42f2b/services/identity/public/cpp/identity_test_environment.h
,
Jan 8
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by blundell@chromium.org
, Nov 28