Use GAIA avatar when user is signed in but not syncing |
|||||||||||||||||
Issue descriptionSee this slide for background: https://docs.google.com/presentation/d/1YVw8DMPLY1iK_PGReI4h0XkvoM4O5dXFh_NeelECo4s/edit?ts=5b242775#slide=id.g39bafcf35f_1_20 In short, we should show the user's GAIA avatar in the toolbar user menu chip instead of a generic dark gray avatar. Mark - can you assign to the engineer that worked on moving the chip from top Chrome to the toolbar? Added to go/identity-gm2 tracker as well.
,
Jun 15 2018
Is this not already the case? The code shows the GAIA avatar instead of the generic placeholder if we're authenticated. If this doesn't show the GAIA avatar could this maybe be because syncing is turned off before the browser has even synced it for the first time? In that case we have no GAIA avatar to show here, but'll show the placeholder until the GAIA icon is available. msarda@: This check is done using SigninManagerFactory::GetForProfile(profile_)->IsAuthenticated(). That still holds true even if you stop syncing, correct? My testing profile won't let me turn off sync without removing the profile.
,
Jun 15 2018
So I'm pretty sure that "Authenticated" in sign in codebase land means "opted into sync." With Dice, a user is signed into the browser as soon as they sign into the content area. You can test this yourself in a fresh profile by first signing into gmail.com with your account, seeing the generic avatar, then turning on sync and seeing the GAIA avatar (FYI it should be safe to delete your local profile for your account to go through these repro steps). If the profile is not authenticated (so there's no sync account), we should be checking the token service to see if there are any accounts available in the token service. We can just use the "default" account (i.e. the account at the first index) to show here to match the OGB behavior. I don't know technically what the right way to check the token service is; Mihai or David should know.
,
Jun 15 2018
I think this can easily be substituted for whatever's appropriate. AvatarToolbarButton::ShouldShowGenericIcon() needs to be modified to return false when there's a signed-in-not-syncing profile. AvatarToolbarButton::GetIconImageFromProfile() might need to be updated too, but I hope not. If the DICE folks know what should be checked I think this is a fairly simple change. Also, would we want to badge signed-in-not-syncing somehow alongside other states for issue 851530 ?
,
Jun 15 2018
In the long-run, I think the answer is yes (kind of): we'd want to badge the *syncing* state, and have the signed-in-not-syncing state just be the only state without a badge (so the set of states are: syncing, error, paused, and signed-in-not-syncing). In the short term, I think it's fine for both the signed-in-not-syncing and regular syncing cases to be treated the same (i.e. just avatar, no badge). I'll let Mihai and David comment with how to get access to the avatar in that case.
,
Jun 15 2018
Sounds like this is doable, but we need to pin down the right engineering team to do the work. pbos@ do you think this is faster on the DICE team? I know they are 100% booked, but I think that's the state we are all in.
,
Jun 15 2018
I think so, what they'd be changing from our end is probably limited to these areas: https://cs.chromium.org/chromium/src/chrome/browser/ui/views/profiles/avatar_toolbar_button.cc?l=137&rcl=5b371945d6c8c986bf7084eb31a647b382645ada https://cs.chromium.org/chromium/src/chrome/browser/ui/views/profiles/avatar_toolbar_button.cc?l=184&rcl=5b371945d6c8c986bf7084eb31a647b382645ada I could do the changes there, but they'd need to kind of tell me what APIs to use anyways, which I think uses up a similar amount of their time. If they prefer pointing me in the right direction that should be fine as well.
,
Jun 15 2018
I'll discuss with Mihai when he's back on Monday
,
Jun 18 2018
@Eli: Authenticated just means signed in, not necessarily opted into sync (for instance users that have sync disabled can still be signed-in/authenticated). Do you mean to show the first Dice promo account, i.e. the GAIA web account that is first in the cookies? signin_ui_util::GetAccountsForDicePromos(profile) gives a list of all the signed-in web accounts, the first one in the list matches the first one on the web. If this is what you mean, I can take over this bug, the change should be relatively straightforward.
,
Jun 18 2018
Yep, that's exactly what I'm looking for. Thanks Thomas! Re-assigning the bug to you.
,
Jun 18 2018
Okay, I'm on it.
,
Jun 18 2018
Thanks so much Thomas! Just to be super explicit: - If the user has not set a custom Chrome avatar via chrome://settings/manageProfile, then the button should just show the user's GAIA photo - If the user has set a custom Chrome avatar via chrome://settings/manageProfile, then we should show that, not their GAIA photo This is basically the same behavior on Canary today, except in the first case, we're showing the generic dark-grey "person" badge instead of the user's GAIA photo. Does that make sense?
,
Jun 19 2018
🙌 Perfect, thanks so much Thomas!
,
Jun 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c5604690c12b5b998c137d6e67c46b5d1d7c626b commit c5604690c12b5b998c137d6e67c46b5d1d7c626b Author: Thomas Tangl <tangltom@chromium.org> Date: Thu Jun 21 10:57:12 2018 Show promo account icon in toolbar when user is signed out When the user is signed out of Chrome and the profile icon has not been explicitly changed, AvatarToolbarButton now uses the account icon of the first sync promo account. Screenshots: https://drive.google.com/file/d/1a7kr12KtA11Wt7MQ9MLnSf-M3D9nlLdg/view?usp=sharing https://drive.google.com/file/d/1nKLnoD1sbcZOvwGQY3YtHVTUv32DQNFn/view?usp=sharing Bug: 853363 Change-Id: I11ed80e2250bc4eb9202e4a1c4cabf242954f726 Reviewed-on: https://chromium-review.googlesource.com/1105772 Commit-Queue: Thomas Tangl <tangltom@chromium.org> Reviewed-by: David Roger <droger@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Cr-Commit-Position: refs/heads/master@{#569213} [modify] https://crrev.com/c5604690c12b5b998c137d6e67c46b5d1d7c626b/chrome/browser/ui/views/profiles/avatar_toolbar_button.cc [modify] https://crrev.com/c5604690c12b5b998c137d6e67c46b5d1d7c626b/chrome/browser/ui/views/profiles/avatar_toolbar_button.h
,
Jun 21 2018
,
Jun 22 2018
Verified this issue on Win 10, Debian Reodete and Mac 10.13.5 with chrome #69.0.3469.0/3 and observed the fix is worked as intended. Hence adding TE Verified labels Attaching the screen-cast for reference
,
Jun 22 2018
kkaluri@ you also have to set chrome://flags/#top-chrome-md to "Refresh". The change only concerns the avatar in the toolbar without opening the user menu. Could you run the test again :) 1) set the flags 2) sign out of Chrome 3) sign into Gmail (if you aren't already) Expected result: Your avatar should appear in the toolbar. Thanks!
,
Jun 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c90892e44981b8f6975450943d749009fc2e950b commit c90892e44981b8f6975450943d749009fc2e950b Author: Bret Sepulveda <bsep@chromium.org> Date: Sat Jun 23 01:23:41 2018 Revert "Show promo account icon in toolbar when user is signed out" This reverts commit c5604690c12b5b998c137d6e67c46b5d1d7c626b. Reason for revert: Causes BrowserViewTest suite to fail under Refresh on Linux. See crbug.com/855797 Original change's description: > Show promo account icon in toolbar when user is signed out > > When the user is signed out of Chrome and the profile icon > has not been explicitly changed, AvatarToolbarButton now > uses the account icon of the first sync promo account. > > Screenshots: > https://drive.google.com/file/d/1a7kr12KtA11Wt7MQ9MLnSf-M3D9nlLdg/view?usp=sharing > https://drive.google.com/file/d/1nKLnoD1sbcZOvwGQY3YtHVTUv32DQNFn/view?usp=sharing > > Bug: 853363 > Change-Id: I11ed80e2250bc4eb9202e4a1c4cabf242954f726 > Reviewed-on: https://chromium-review.googlesource.com/1105772 > Commit-Queue: Thomas Tangl <tangltom@chromium.org> > Reviewed-by: David Roger <droger@chromium.org> > Reviewed-by: Trent Apted <tapted@chromium.org> > Cr-Commit-Position: refs/heads/master@{#569213} TBR=droger@chromium.org,tapted@chromium.org,tangltom@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 853363 Change-Id: Ib8d9a5350277146050677a2676577d29b315efda Reviewed-on: https://chromium-review.googlesource.com/1112599 Reviewed-by: Bret Sepulveda <bsep@chromium.org> Commit-Queue: Bret Sepulveda <bsep@chromium.org> Cr-Commit-Position: refs/heads/master@{#569870} [modify] https://crrev.com/c90892e44981b8f6975450943d749009fc2e950b/chrome/browser/ui/views/profiles/avatar_toolbar_button.cc [modify] https://crrev.com/c90892e44981b8f6975450943d749009fc2e950b/chrome/browser/ui/views/profiles/avatar_toolbar_button.h
,
Jun 25 2018
Re-opening this since it was reverted over the weekend. Thomas, PTAL?
,
Jun 25 2018
I expect --top-chrome-md=material-refresh to be default-on very soon, hence the immediate revert. Repro that we used: $ autoninja -C out/Default unit_tests && out/Default/unit_tests --gtest_filter=BrowserViewTest.BrowserView --top-chrome-md=material-refresh ninja -C out/Default unit_tests -j 160 -l 8 ninja: Entering directory `out/Default' [1/1] Regenerating ninja files [6875/6875] LINK ./unit_tests IMPORTANT DEBUGGING NOTE: batches of tests are run inside their own process. For debugging a test inside a debugger, use the --gtest_filter=<your_test_name> flag along with --single-process-tests. Using sharding settings from environment. This is shard 0/1 Using 1 parallel jobs. Note: Google Test filter = BrowserViewTest.BrowserView [==========] Running 1 test from 1 test case. [----------] Global test environment set-up. [----------] 1 test from BrowserViewTest [ RUN ] BrowserViewTest.BrowserView [117518:117518:0623/001048.077146:92318572142:FATAL:message_loop_current.cc(189)] Check failed: MessageLoop::TYPE_IO == loop->type() (3 vs. 1) #0 0x7fc89f2dec0d base::debug::StackTrace::StackTrace() #1 0x7fc89f0274bc base::debug::StackTrace::StackTrace() #2 0x7fc89f0964fa logging::LogMessage::~LogMessage() #3 0x7fc89f0c25a3 base::MessageLoopCurrentForIO::Get() #4 0x7fc8a386d85a net::SocketPosix::Connect() #5 0x7fc8a3872db9 net::TCPSocketPosix::Connect() #6 0x7fc8a34b3faa net::TCPClientSocket::DoConnect() #7 0x7fc8a34b327d net::TCPClientSocket::DoConnectLoop() #8 0x7fc8a34b2fe0 net::TCPClientSocket::Connect() #9 0x7fc8a34ba203 net::TransportConnectJob::DoTransportConnect() #10 0x7fc8a34b92cd net::TransportConnectJob::DoLoop() #11 0x7fc8a34b8fef net::TransportConnectJob::OnIOComplete() #12 0x7fc8a2f3b81f _ZN4base8internal13FunctorTraitsIMN3net18ClientSocketHandleEFviEvE6InvokeIS5_PS3_JiEEEvT_OT0_DpOT1_ #13 0x7fc8a2f3b74f _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKMN3net18ClientSocketHandleEFviEJPS5_iEEEvOT_DpOT0_ #14 0x7fc8a2f3b6e5 _ZN4base8internal7InvokerINS0_9BindStateIMN3net18ClientSocketHandleEFviEJNS0_17UnretainedWrapperIS4_EEEEEFviEE7RunImplIRKS6_RKNSt3__15tupleIJS8_EEEJLm0EEEEvOT_OT0_NSF_16integer_sequenceImJXspT1_EEEEOi #15 0x7fc8a2f3b61b _ZN4base8internal7InvokerINS0_9BindStateIMN3net18ClientSocketHandleEFviEJNS0_17UnretainedWrapperIS4_EEEEEFviEE3RunEPNS0_13BindStateBaseEi #16 0x000002d4ae0f _ZNO4base12OnceCallbackIFvN14LoginUIService30SyncConfirmationUIClosedResultEEE3RunES2_ #17 0x000006beb8a3 net::MockHostResolverBase::RequestImpl::OnResolveCompleted() #18 0x000006be7722 net::MockHostResolverBase::ResolveNow() #19 0x000004d5e242 _ZN4base8internal13FunctorTraitsIMN11google_apis22FilesListRequestRunnerEFvPNS_17RepeatingCallbackIFvvEEEEvE6InvokeIS9_RKNS_7WeakPtrIS3_EEJS7_EEEvT_OT0_DpOT1_ #20 0x000004d5e1a5 _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMN11google_apis22FilesListRequestRunnerEFvPNS_17RepeatingCallbackIFvvEEEERKNS_7WeakPtrIS5_EEJS9_EEEvOT_OT0_DpOT1_ #21 0x000005c3c91d _ZN4base8internal7InvokerINS0_9BindStateIMN10extensions24ClearCacheQuotaHeuristicEFvPNS3_19QuotaLimitHeuristic6BucketEEJNS_7WeakPtrIS4_EES7_EEEFvvEE7RunImplIRKS9_RKNSt3__15tupleIJSB_S7_EEEJLm0ELm1EEEEvOT_OT0_NSI_16integer_sequenceImJXspT1_EEEE #22 0x000005c3c88c _ZN4base8internal7InvokerINS0_9BindStateIMN10extensions24ClearCacheQuotaHeuristicEFvPNS3_19QuotaLimitHeuristic6BucketEEJNS_7WeakPtrIS4_EES7_EEEFvvEE3RunEPNS0_13BindStateBaseE #23 0x7fc89efd61ee _ZNO4base12OnceCallbackIFvvEE3RunEv #24 0x7fc89f028982 base::debug::TaskAnnotator::RunTask() #25 0x7fc89f0b5bd9 base::internal::IncomingTaskQueue::RunTask() #26 0x7fc89f0bf9c7 base::MessageLoop::RunTask() #27 0x7fc89f0bfc38 base::MessageLoop::DeferOrRunPendingTask() #28 0x7fc89f0bff69 base::MessageLoop::DoWork() #29 0x7fc89f0c3ed6 base::MessagePumpGlib::Run() #30 0x7fc89f0bf1bb base::MessageLoop::Run() #31 0x7fc89f1687cd base::RunLoop::Run() #32 0x000006f5f330 content::RunAllTasksUntilIdle() #33 0x000004adee23 TestWithBrowserView::TearDown() #34 0x00000421564e _ZN7testing8internal12InvokeHelperIN16sync_file_system18RemoteServiceStateENSt3__15tupleIJEEEE12InvokeMethodINS2_25MockRemoteFileSyncServiceEMS9_KFS3_vEEES3_PT_T0_RKS6_ #35 0x000004ec3a82 testing::internal::HandleExceptionsInMethodIfSupported<>() #36 0x000004ea71db testing::Test::Run() #37 0x000004ea7bc0 testing::TestInfo::Run() #38 0x000004ea866f testing::TestCase::Run() #39 0x000004ebaa28 testing::internal::UnitTestImpl::RunAllTests() #40 0x000004ecce1e testing::internal::HandleSehExceptionsInMethodIfSupported<>() #41 0x000004ec5152 testing::internal::HandleExceptionsInMethodIfSupported<>() #42 0x000004eba687 testing::UnitTest::Run() #43 0x000006e0d701 RUN_ALL_TESTS() #44 0x000006e09fcb base::TestSuite::Run() #45 0x000006f68c70 content::UnitTestTestSuite::Run() #46 0x000001f0745d _ZN4base8internal13FunctorTraitsIMNS_7RunLoopEFvvEvE6InvokeIS4_PS2_JEEEvT_OT0_DpOT1_ #47 0x000001f073d4 _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIMNS_7RunLoopEFvvEJPS4_EEEvOT_DpOT0_ #48 0x000001f07385 _ZN4base8internal7InvokerINS0_9BindStateIMNS_7RunLoopEFvvEJNS0_17UnretainedWrapperIS3_EEEEEFvvEE7RunImplIS5_NSt3__15tupleIJS7_EEEJLm0EEEEvOT_OT0_NSC_16integer_sequenceImJXspT1_EEEE #49 0x00000250605c _ZN4base8internal7InvokerINS0_9BindStateIM13ThreadWatcherFvvEJNS0_17UnretainedWrapperI19CustomThreadWatcherEEEEEFvvEE3RunEPNS0_13BindStateBaseE #50 0x0000056c57ce _ZNO4base12OnceCallbackIFivEE3RunEv #51 0x000006e13195 base::(anonymous namespace)::LaunchUnitTestsInternal() #52 0x000006e12fe5 base::LaunchUnitTests() #53 0x000006dee531 main #54 0x7fc87ee922b1 __libc_start_main #55 0x000001ef202a _start
,
Jun 25 2018
Thanks! I've emailed Thomas to ask him to take a look and re-land before our user studies on Thursday, if possible.
,
Jun 25 2018
Hey Eli - are you and Zach synced up? There was a question today about whether this is the right approach.
,
Jun 25 2018
,
Jun 25 2018
Yes, Zach and I chatted offline. For now, we are going to re-land this. We can always revert again if necessary.
,
Jun 25 2018
It can't reland as is (with test breakage present) as they break tests in Refresh mode. We're turning Refresh on by default ASAP and are down to single-digit test failures.
,
Jun 25 2018
To close out the loop here Refresh is default-on as of https://crrev.com/570226 so this can probably not reland as is.
,
Jun 26 2018
,
Jun 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7141db77bcc81dcb4f8a27038723027b89f3fe44 commit 7141db77bcc81dcb4f8a27038723027b89f3fe44 Author: Thomas Tangl <tangltom@chromium.org> Date: Wed Jun 27 08:46:47 2018 Show promo account icon in toolbar when user is signed out Relanding crrev.com/c/1105772 after crbug.com/855797 . Broken tests caused by calls to GetAccountsForDicePromos were fixed. When the user is signed out of Chrome and the profile icon has not been explicitly changed, AvatarToolbarButton now uses the account icon of the first sync promo account. Screenshots: https://drive.google.com/file/d/1a7kr12KtA11Wt7MQ9MLnSf-M3D9nlLdg/view?usp=sharing https://drive.google.com/file/d/1nKLnoD1sbcZOvwGQY3YtHVTUv32DQNFn/view?usp=sharing TBR=droger@chromium.org TBR=tapted@chromium.org TBR=bsep@chromium.org Bug: 853363 , 855797 Change-Id: I830aedd32c30cb1c39fc24551dd225c81d62c097 Reviewed-on: https://chromium-review.googlesource.com/1112253 Commit-Queue: Thomas Tangl <tangltom@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Reviewed-by: Thomas Tangl <tangltom@chromium.org> Reviewed-by: Mihai Sardarescu <msarda@chromium.org> Cr-Commit-Position: refs/heads/master@{#570699} [modify] https://crrev.com/7141db77bcc81dcb4f8a27038723027b89f3fe44/chrome/browser/ui/views/frame/test_with_browser_view.cc [modify] https://crrev.com/7141db77bcc81dcb4f8a27038723027b89f3fe44/chrome/browser/ui/views/frame/test_with_browser_view.h [modify] https://crrev.com/7141db77bcc81dcb4f8a27038723027b89f3fe44/chrome/browser/ui/views/profiles/avatar_toolbar_button.cc [modify] https://crrev.com/7141db77bcc81dcb4f8a27038723027b89f3fe44/chrome/browser/ui/views/profiles/avatar_toolbar_button.h
,
Jun 27 2018
markchang@, signed-in+not-syncing is not the same as signed-in+sync-paused(=auth error), so removing the duplicate status. Eli, I landed the CL today, so it should be on Canary by tomorrow! :)
,
Jun 28 2018
Hey Thomas, thanks for landing this! As we're discussing over email, I think there's one followup fix we'd like to land: we should show the user's GAIA avatar when there's only one profile as well (i.e. we should replace the default "dark gray" avatar that we usually show when there's only one profile[1]). If they sign back out, then we can revert back to the "dark gray" default avatar. But while they are signed in, let's show their GAIA avatar. [1] https://screenshot.googleplex.com/RziUN0t6siz.png
,
Jun 29 2018
Yep, just found my mistake. Thanks for clarifying! The fix is about to be landed.
,
Jul 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a333057417bcd81aa5a6f6ab082f81dd8023f452 commit a333057417bcd81aa5a6f6ab082f81dd8023f452 Author: Thomas Tangl <tangltom@chromium.org> Date: Mon Jul 02 15:42:55 2018 Fix case when showing promo account in avatar button The condition of whether a user changed their profile avatar explicitly is changed. Bug: 853363 Change-Id: I1e9bc3928b74f3dbb613521b492561455a7b104b Reviewed-on: https://chromium-review.googlesource.com/1120065 Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Commit-Queue: Thomas Tangl <tangltom@chromium.org> Cr-Commit-Position: refs/heads/master@{#571903} [modify] https://crrev.com/a333057417bcd81aa5a6f6ab082f81dd8023f452/chrome/browser/ui/views/profiles/avatar_toolbar_button.cc
,
Jul 2
,
Jul 3
Tested this issue on Debian Rodete with chrome #69.0.3480.0 Steps followed : ================ 1) Set the flags #top-chrome-md -> Refresh ; #Account-consistency -> Enable Dice 2) Launch Chrome and observed the "dark gray" icon 3) Sign into chrome with gmail account and observed "dark gray" icon has changed to user GAIA avatar icon 4) On chrome://settings, click on the Turn off button and observed the user GAIA avatar has changed back to "dark gray" icon Attaching the screen-cast for reference. tangltom@ Could you confirm this is the expected behavior of fix in comment #34
,
Jul 3
Hi kkaluri@, the shown behavior is expected, but doesn't reflect the change. Please make sure to do the following: 1. Make sure you are signed out of CHROME 2. Sign into GMAIL Expected result: You see your Google Account Icon in the toolbar. And if you click on it you see something like: https://drive.google.com/file/d/1nKLnoD1sbcZOvwGQY3YtHVTUv32DQNFn/view?usp=sharing Hope this helps.
,
Jul 3
Hey Thomas, I just tested this out myself on Canary for an existing profile by turning off sync (which now also signs me out) and then signing back in. Even after restarting Chrome, this is what I'm seeing: https://screenshot.googleplex.com/GQnR0QQTZFu.png. Is there a reason I might still be seeing the dark gray generic avatar? Could it have to do with the fact that this was an existing profile from before your CL landed?
,
Jul 3
Marking as Assigned for now to follow-up on my question in #38.
,
Jul 3
Just for posterity, can you post the chrome://version you tested on when refering to Canary?
,
Jul 3
Version 69.0.3480.0 (Official Build) canary (64-bit) I verified that it's the same version that the CL above landed in.
,
Jul 3
Thanks for the trail, that's what I was going to check as versions or channels can sometimes lag behind depending on platform.
,
Jul 3
No worries! :) Thomas - let me know when you've had a chance to look at this if I can help you debug.
,
Jul 4
Hi Eli, I just tested it on the same build on my machine and it worked fine. (With 1, and with more profiles) Which platform did you test it on? Could you share a video?
,
Jul 6
I'm testing on Mac OS X. Video is here: https://drive.google.com/open?id=1cNVuO-twaki2CJ5K43m70ruTZGILtR4W As I suspected in c#38, after I deleted my profile and created a brand new profile, it was working properly (screenshot: https://screenshot.googleplex.com/cVT0bsMinqr.png). Thomas - is there any way to get this logic to work for existing profiles, or will it only work for new profiles?
,
Jul 9
I just checked the code. It should actually work for existing profiles as well, unless they changed the default profile name at one point in the past. Newer profiles shouldn't have this problem. (I unfortunately can't say from when on). Here is the reference code line: https://cs.chromium.org/chromium/src/chrome/browser/profiles/profile_info_cache.cc?type=cs&g=0&l=96 maybe pbos@ knows more?
,
Jul 9
Ah, well I had changed my default profile name at some point in the past :) By "default profile name," you mean changing it away from e.g. "Person 1," right? Why would changing the profile name have any bearing on whether we show your GAIA avatar in the toolbar? Can we remove that dependency?
,
Jul 9
Yeah exactly, changing it from "Person 1" to something else. Unfortunately it was implemented this way and there is no other way to see if the user is using the default icon or not. I could change it for new profiles, but the problem is that existing profiles already have this pref set now.
,
Jul 9
OK, let's quickly hash it out. I set up some time tomorrow for us to chat, Thomas :)
,
Jul 10
Discussed with Thomas. This bug is Fixed. There's another bug where we should be showing the Chrome profile picture when the user has selected a custom one, even if they're not signed in on the web. I filed Issue 862192 to track that and assigned to pbos.
,
Jul 12
Thomas, quick followup question. I just started from a completely fresh Canary install, and observed the following behavior: (1) Sign in with my personal Google Account. Observe that the toolbar shows my GAIA avatar (expected). Note that this account previously had a custom Chrome profile picture set for it on another syncing profile. (2) Turn on sync using the user menu promo. Toolbar stays as my GAIA photo (expected) (3) Go to chrome://settings/people and hit "Turn off" to both turn off sync and sign out. Toolbar goes back to generic gray icon, since I'm signed out (expected). (4) Sign back in with the same account as in step 1. Toolbar stays as generic gray icon (not expected). In step (4), I would expect the toolbar to go back to my GAIA picture. Is it possible that once I enable sync in step (2), Chrome syncs the pref that says I'd previously changed my picture to be a custom one, and that when I turn off sync, that pref remains, which prevents my GAIA photo from being shown? If so, I think that's fine; it should be enough of an edge case. Just want to make sure there's not another bug somewhere else? |
|||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||
Comment 1 by markchang@chromium.org
, Jun 15 2018