New issue
Advanced search Search tips

Issue 914783 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 15
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug


Sign in to add a comment

Provide IdentityManager API to replace AccountTrackerService::GetAccountImage

Project Member Reported by sdefresne@chromium.org, Dec 13

Issue description

Umbrella bug.
 
Labels: -Pri-1 Pri-2
Moving to P2 to reflect that current milestone is for completing conversion of PO2TS/SigninManager.
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/97ad3187869d4025a7ea5f94f634f95cc58b98be

commit 97ad3187869d4025a7ea5f94f634f95cc58b98be
Author: Sylvain Defresne <sdefresne@chromium.org>
Date: Thu Dec 13 17:35:24 2018

Store account image in AccountInfo

In order to simplify the API of Identity Service, store
the account image into AccountInfo. The class gfx::Image
is a smart refcounted wrapper around the image data so it
is quite cheap to copy (though it is not thread-safe but
the class enforece this via DCHECK so incorrect use will
be caught by build with assertion checks enabled).

Bug:  914783 
Change-Id: I75798e5ff95b2f300467391a4b354ba7256f39d0
Reviewed-on: https://chromium-review.googlesource.com/c/1375729
Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616354}
[modify] https://crrev.com/97ad3187869d4025a7ea5f94f634f95cc58b98be/components/signin/core/browser/BUILD.gn
[modify] https://crrev.com/97ad3187869d4025a7ea5f94f634f95cc58b98be/components/signin/core/browser/account_info.h
[modify] https://crrev.com/97ad3187869d4025a7ea5f94f634f95cc58b98be/components/signin/core/browser/account_tracker_service.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 8

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cfb41cfbc1a49990cdb64d2407c0fd107ac9a276

commit cfb41cfbc1a49990cdb64d2407c0fd107ac9a276
Author: Sylvain Defresne <sdefresne@chromium.org>
Date: Fri Jan 11 10:23:03 2019

Remove AccountTrackerService::Observer::OnAccountImageUpdated

All observers of that method were UI components that refreshed
themselves when notified. Since they then fetch the data from
the AccountTrackerService, and does the same refresh when the
account is updated, it is simpler to just use OnAccountUpdated
for both events.

Bug:  914783 
Change-Id: I4d085baceb72373eda1c11e169d04ce55a2ac8e2
Reviewed-on: https://chromium-review.googlesource.com/c/1402568
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Thomas Tangl <tangltom@chromium.org>
Reviewed-by: Dan Beam <dbeam@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621962}
[modify] https://crrev.com/cfb41cfbc1a49990cdb64d2407c0fd107ac9a276/chrome/browser/ui/views/profiles/avatar_toolbar_button.cc
[modify] https://crrev.com/cfb41cfbc1a49990cdb64d2407c0fd107ac9a276/chrome/browser/ui/views/profiles/avatar_toolbar_button.h
[modify] https://crrev.com/cfb41cfbc1a49990cdb64d2407c0fd107ac9a276/chrome/browser/ui/webui/settings/chromeos/account_manager_handler.cc
[modify] https://crrev.com/cfb41cfbc1a49990cdb64d2407c0fd107ac9a276/chrome/browser/ui/webui/settings/chromeos/account_manager_handler.h
[modify] https://crrev.com/cfb41cfbc1a49990cdb64d2407c0fd107ac9a276/chrome/browser/ui/webui/settings/people_handler.cc
[modify] https://crrev.com/cfb41cfbc1a49990cdb64d2407c0fd107ac9a276/chrome/browser/ui/webui/settings/people_handler.h
[modify] https://crrev.com/cfb41cfbc1a49990cdb64d2407c0fd107ac9a276/components/signin/core/browser/account_tracker_service.cc
[modify] https://crrev.com/cfb41cfbc1a49990cdb64d2407c0fd107ac9a276/components/signin/core/browser/account_tracker_service.h
[modify] https://crrev.com/cfb41cfbc1a49990cdb64d2407c0fd107ac9a276/components/signin/core/browser/account_tracker_service_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 14

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3e3c634b7774c99cd6d4375e096a6dfe57a68af5

commit 3e3c634b7774c99cd6d4375e096a6dfe57a68af5
Author: Sylvain Defresne <sdefresne@chromium.org>
Date: Mon Jan 14 17:39:29 2019

Remove unnecessary AccountTrackerService::GetAccountImage

As the AccountInfo already stores the account image, remove
the now unnecessary GetAccountImage and convert client code
to get the image from AccountInfo's account_image field.

Remove now unnecessary array of image from DiceAccountsMenu.

Bug:  914783 
Change-Id: I690d7f5f9be2c9d143ecfabf1355070329139122
Reviewed-on: https://chromium-review.googlesource.com/c/1402759
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Thomas Tangl <tangltom@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622499}
[modify] https://crrev.com/3e3c634b7774c99cd6d4375e096a6dfe57a68af5/chrome/browser/ui/views/profiles/avatar_toolbar_button.cc
[modify] https://crrev.com/3e3c634b7774c99cd6d4375e096a6dfe57a68af5/chrome/browser/ui/views/profiles/dice_accounts_menu.cc
[modify] https://crrev.com/3e3c634b7774c99cd6d4375e096a6dfe57a68af5/chrome/browser/ui/views/profiles/dice_accounts_menu.h
[modify] https://crrev.com/3e3c634b7774c99cd6d4375e096a6dfe57a68af5/chrome/browser/ui/views/profiles/profile_chooser_view.cc
[modify] https://crrev.com/3e3c634b7774c99cd6d4375e096a6dfe57a68af5/chrome/browser/ui/views/sync/dice_bubble_sync_promo_view.cc
[modify] https://crrev.com/3e3c634b7774c99cd6d4375e096a6dfe57a68af5/chrome/browser/ui/views/sync/dice_bubble_sync_promo_view.h
[modify] https://crrev.com/3e3c634b7774c99cd6d4375e096a6dfe57a68af5/chrome/browser/ui/webui/settings/chromeos/account_manager_handler.cc
[modify] https://crrev.com/3e3c634b7774c99cd6d4375e096a6dfe57a68af5/chrome/browser/ui/webui/settings/people_handler.cc
[modify] https://crrev.com/3e3c634b7774c99cd6d4375e096a6dfe57a68af5/chrome/browser/ui/webui/settings/people_handler.h
[modify] https://crrev.com/3e3c634b7774c99cd6d4375e096a6dfe57a68af5/chrome/browser/ui/webui/settings/people_handler_unittest.cc
[modify] https://crrev.com/3e3c634b7774c99cd6d4375e096a6dfe57a68af5/components/signin/core/browser/account_tracker_service.cc
[modify] https://crrev.com/3e3c634b7774c99cd6d4375e096a6dfe57a68af5/components/signin/core/browser/account_tracker_service.h
[modify] https://crrev.com/3e3c634b7774c99cd6d4375e096a6dfe57a68af5/components/signin/core/browser/account_tracker_service_unittest.cc

Owner: sdefresne@chromium.org
Status: Fixed (was: Available)

Comment 7 by dxie@google.com, Jan 16 (6 days ago)

Blockedon: 922744

Comment 8 by dxie@google.com, Jan 16 (6 days ago)

Blockedon: 922749

Comment 9 by dxie@google.com, Jan 16 (6 days ago)

Blockedon: 922757

Comment 10 by dxie@google.com, Jan 16 (6 days ago)

Blockedon: 922760

Comment 11 by dxie@google.com, Jan 16 (6 days ago)

Blockedon: 922765

Comment 12 by dxie@google.com, Jan 16 (6 days ago)

Blockedon: 922766

Comment 13 by dxie@google.com, Jan 16 (6 days ago)

Blockedon: 922767

Comment 14 by dxie@google.com, Jan 17 (6 days ago)

Blockedon: 922771

Comment 15 by dxie@google.com, Jan 17 (6 days ago)

Blockedon: 922772

Comment 16 by dxie@google.com, Jan 17 (6 days ago)

Blockedon: 922774

Comment 17 by dxie@google.com, Jan 17 (6 days ago)

Blockedon: 922775

Comment 18 by dxie@google.com, Jan 17 (6 days ago)

Blockedon: 922776

Comment 19 by dxie@google.com, Jan 17 (6 days ago)

Blockedon: 922787

Comment 20 by dxie@google.com, Jan 17 (6 days ago)

Blockedon: 922789

Comment 21 by dxie@google.com, Jan 17 (6 days ago)

Blockedon: 922792

Comment 22 by dxie@google.com, Jan 17 (6 days ago)

Blockedon: 922794

Comment 23 by dxie@google.com, Jan 17 (6 days ago)

Blockedon: 922795

Comment 24 by dxie@google.com, Jan 17 (6 days ago)

Blockedon: 922801

Sign in to add a comment