Convert sync's WebUI from consuming //components/signin to using IdentityManager |
|||
Issue descriptionchrome://sync-internals uses //components/signin to get the information of the signed-in user. As part of moving the signin code to a service, we need to convert this usage to instead be usage of the Identity Service. The usage occurs via //components/sync/driver/about_sync_util.cc, which internally calls SigninManagerBase::GetAuthenticatedAccountInfo(). My plan is to do the following: - Change sync_ui_util::ConstructAboutInfo() to take in the info of the signed-in user as a parameter rather than getting it internally. - Move the production callsites of this function to supply this information via the Identity Service: * //chrome's sync WebUI * //ios/chrome's sync WebUI * //chrome's feedback flow All of the above are already asynchronous flows, so having them get the primary account information asynchronously from the Identity Service won't be problematic. - Move the test callsites of this function to either use the Identity Service or just supply test values for the signed-in user account information as appropriate depending on the test. Making these changes will also entail bringing up the Identity Service on Android and iOS, where it is not yet used in production.
,
Nov 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7c71692850862ed6b2e77c0ee4e23cfc24848bf7 commit 7c71692850862ed6b2e77c0ee4e23cfc24848bf7 Author: Colin Blundell <blundell@chromium.org> Date: Fri Nov 10 08:32:31 2017 [Android] Remove unused call of sync_ui_util::ConstructAboutInfo() This CL is a precursor to converting the callers of sync_ui_util::ConstructAboutInfo() to supply the information of the signed-in user via the Identity Service. Analysis of this caller shows that it's actually dead code. Bug: 783144 Change-Id: Ia7060fe0e0af6ec54858bab800f8481281871cab Reviewed-on: https://chromium-review.googlesource.com/758586 Reviewed-by: Nicolas Zea <zea@chromium.org> Reviewed-by: Ted Choc <tedchoc@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#515506} [modify] https://crrev.com/7c71692850862ed6b2e77c0ee4e23cfc24848bf7/chrome/android/java/src/org/chromium/chrome/browser/sync/ProfileSyncService.java [modify] https://crrev.com/7c71692850862ed6b2e77c0ee4e23cfc24848bf7/chrome/browser/sync/profile_sync_service_android.cc [modify] https://crrev.com/7c71692850862ed6b2e77c0ee4e23cfc24848bf7/chrome/browser/sync/profile_sync_service_android.h
,
Nov 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/08b8e66cea8b533cbdbf6ec4649a9b58837d0f31 commit 08b8e66cea8b533cbdbf6ec4649a9b58837d0f31 Author: Colin Blundell <blundell@chromium.org> Date: Mon Nov 13 16:52:46 2017 [Sync] Prepare to remove //components/signin dep from about_sync_util We will shortly be converting sync WebUI to obtain information about the signed-in user via the Identity Service rather than the SigninManager. To prepare for doing that conversion incrementally, this CL adds a variant of sync_ui_util::ConstructAboutInfo() that takes in the AccountInfo of the primary account directly. In upcoming CLs, we will port over callers to obtain this AccountInfo asynchronously via the Identity Service and pass it in to ConstructAboutInfo() once obtained. All callers are already part of an asynchronous flow, so these conversions will not be difficult. TBR=rkc@chromium.org Bug: 783144 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: Icf30d739a01106feb454a67eb2b83a79dd2fc5bd Reviewed-on: https://chromium-review.googlesource.com/758771 Reviewed-by: Colin Blundell <blundell@chromium.org> Reviewed-by: Nicolas Zea <zea@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#515963} [modify] https://crrev.com/08b8e66cea8b533cbdbf6ec4649a9b58837d0f31/chrome/browser/feedback/system_logs/log_sources/chrome_internal_log_source.cc [modify] https://crrev.com/08b8e66cea8b533cbdbf6ec4649a9b58837d0f31/chrome/browser/sync/test/integration/profile_sync_service_harness.cc [modify] https://crrev.com/08b8e66cea8b533cbdbf6ec4649a9b58837d0f31/chrome/browser/ui/webui/sync_internals_message_handler.cc [modify] https://crrev.com/08b8e66cea8b533cbdbf6ec4649a9b58837d0f31/components/sync/driver/about_sync_util.cc [modify] https://crrev.com/08b8e66cea8b533cbdbf6ec4649a9b58837d0f31/components/sync/driver/about_sync_util.h [modify] https://crrev.com/08b8e66cea8b533cbdbf6ec4649a9b58837d0f31/components/sync/driver/about_sync_util_unittest.cc [modify] https://crrev.com/08b8e66cea8b533cbdbf6ec4649a9b58837d0f31/ios/chrome/browser/ui/webui/sync_internals/sync_internals_message_handler.cc
,
Nov 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0a8256755909cffdef106c42ddff0de8ac303e60 commit 0a8256755909cffdef106c42ddff0de8ac303e60 Author: Colin Blundell <blundell@chromium.org> Date: Mon Nov 20 14:25:18 2017 Move away from deprecated ConstructAboutInfo() in PSSHarness sync_ui_util::ConstructAboutInfo_DEPRECATED() is ... deprecated. This CL ports ProfileSyncServiceHarness away from it, having PSSHarness directly supply its cached account info to ConstructAboutInfo(). Bug: 783144 Change-Id: I6bff37d2e39244dfdcb853980783d1e9cada36bd Reviewed-on: https://chromium-review.googlesource.com/768875 Reviewed-by: Nicolas Zea <zea@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#517816} [modify] https://crrev.com/0a8256755909cffdef106c42ddff0de8ac303e60/chrome/browser/sync/test/integration/profile_sync_service_harness.cc
,
Nov 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9aca76ca55d5588cde3d1baac82df92000318510 commit 9aca76ca55d5588cde3d1baac82df92000318510 Author: Colin Blundell <blundell@chromium.org> Date: Mon Nov 20 14:28:38 2017 ConstructAboutInformation unittest: Move away from deprecated version ConstructAboutInformation_DEPRECATED() is ... deprecated. This CL moves the ConstructAboutInformation unittest away from using it, which is trivial. Bug: 783144 Change-Id: Ie492afe35dba907e556870b799bcb58ac094a7b7 Reviewed-on: https://chromium-review.googlesource.com/769008 Reviewed-by: Nicolas Zea <zea@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#517817} [modify] https://crrev.com/9aca76ca55d5588cde3d1baac82df92000318510/components/sync/driver/about_sync_util_unittest.cc
,
Jan 31 2018
,
Feb 5 2018
Note: now that we have brought up the Identity Service client library (//services/identity/public/cpp), this should just be a straightforward conversion to talk to IdentityManager rather than SigninManager.
,
Apr 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ab6ff6e59b098430a494b8b535b69cb94cf33287 commit ab6ff6e59b098430a494b8b535b69cb94cf33287 Author: Marc Treib <treib@chromium.org> Date: Fri Apr 20 09:09:03 2018 about_sync_util: un-deprecate ConstructAboutInformation_DEPRECATED Now that SyncService directly exposes the relevant AccountInfo, there's no need to pass in that information explicitly. TBRing trivial call site update in chrome_internal_log_source.cc TBR=afakhry@chromium.org Bug: 783144 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: I4570c4c81de2fe1566b4cfb70085c51f0256239b Reviewed-on: https://chromium-review.googlesource.com/1016801 Commit-Queue: Marc Treib <treib@chromium.org> Reviewed-by: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#552289} [modify] https://crrev.com/ab6ff6e59b098430a494b8b535b69cb94cf33287/chrome/browser/feedback/system_logs/log_sources/chrome_internal_log_source.cc [modify] https://crrev.com/ab6ff6e59b098430a494b8b535b69cb94cf33287/chrome/browser/sync/test/integration/profile_sync_service_harness.cc [modify] https://crrev.com/ab6ff6e59b098430a494b8b535b69cb94cf33287/chrome/browser/ui/webui/sync_internals_message_handler.cc [modify] https://crrev.com/ab6ff6e59b098430a494b8b535b69cb94cf33287/components/sync/driver/about_sync_util.cc [modify] https://crrev.com/ab6ff6e59b098430a494b8b535b69cb94cf33287/components/sync/driver/about_sync_util.h [modify] https://crrev.com/ab6ff6e59b098430a494b8b535b69cb94cf33287/components/sync/driver/about_sync_util_unittest.cc [modify] https://crrev.com/ab6ff6e59b098430a494b8b535b69cb94cf33287/ios/chrome/browser/ui/webui/sync_internals/sync_internals_message_handler.cc
,
Apr 25 2018
I believe this is done; none of Sync's WebUI uses the old signin classes anymore. Leaving open for now because I don't know what "//chrome's feedback flow" in #0 refers to.
,
May 11 2018
Thanks, Marc! Yes, as c#7 indicates, the presence of IdentityManager means that this change could be localized in the sync WebUI code itself, as you did. For posterity, "//chrome's feedback flow" refers to chrome/browser/feedback/system_logs/log_sources/chrome_internal_log_source.cc, which calls ConstructAboutInformation. |
|||
►
Sign in to add a comment |
|||
Comment 1 by bugdroid1@chromium.org
, Nov 10 2017