New issue
Advanced search Search tips

Issue 900374 link

Starred by 12 users

Taskbar/Desktop icons refreshing on every launch (again)

Reported by superhac...@bk.ru, Oct 30

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.77 Safari/537.36

Steps to reproduce the problem:
1. Start Chrome

What is the expected behavior?

What went wrong?
Desktop and taskbar icons start refreshing.

Did this work before? Yes Not sure, probably 69.x

Chrome version: 70.0.3538.77  Channel: stable
OS Version: 10.0
Flash Version: 

Probably related to  https://crbug.com/449569 
I made a log using Process Monitor as explained in this comment: https://bugs.chromium.org/p/chromium/issues/detail?id=449569#c90 and shared it with grt at chromium dot org.
 
Cc: brucedaw...@chromium.org chengx@chromium.org grt@chromium.org
Thank you for the report. Other users, please star this bug if you are also seeing this.

If you are seeing it on Windows 8.1 or earlier then please let us know.

What version of Windows 10 is this happening on?

Version 1803 Build 17134.345
Same has been happening to me since the previous Chrome stable update, and the last one didn't fix it. It's especially problematic in a slower computer like mine. Since the desktop icons need to recover from that. Chrome also takes longer to be usable.

Windows 7 Professional.
Cc: phanindra.mandapaka@chromium.org
Components: UI>Browser
Labels: -Type-Bug-Regression Triaged-ET Target-72 FoundIn-72 M-72 FoundIn-71 FoundIn-70 Needs-Triage-M70 Type-Bug
Status: Untriaged (was: Unconfirmed)
Able to reproduce the issue on reported chrome version 70.0.3538.77 also on latest chrome 72.0.3596.0 using Windows 7 and Windows 10.  

Same behavior is seen on M60(60.0.3112.113) hence considering it as non-regression and marking it as Untriaged. Issue not seen on Mac and Ubuntu.
 
Thanks! 
900374.mp4
1.0 MB View Download
Labels: Hotlist-DesktopUIConsider
Hi Superhacker. Thanks for the procmon log. While it shows the registry reads by explorer.exe, it's the writes by chrome.exe that we need to see in order to get a glimpse of why this is happening. Could you send a new log that includes those writes?
Labels: Needs-Feedback
That's strange. I see registry writes by chrome.exe in the log.
I made the new log using latest Procmon (version 3.50; previous log was from Procmon 3.20) and shared in with you.
ProcMon.png
44.5 KB View Download
 Issue 898148  has been merged into this issue.
Cc: susan.boorgula@chromium.org
 Issue 899533  has been merged into this issue.
Components: -UI>Browser Services>SignIn UI>Browser>Profiles
Labels: -Type-Bug -Pri-2 Pri-1 Type-Bug-Regression
Owner: msarda@chromium.org
Status: Assigned (was: Untriaged)
I believe r595076 is the culprit. It looks to me like GAIAInfoUpdateService::GAIAInfoUpdateService is calling OnUsernameChanged(std::string()); on every launch (perhaps when the profile isn't signed in).

The change landed at 71.0.3565.0 and was merged back into 70.0.3538.43.

Bumping to P1 since this may impact all non-signed in users.
BTW: thanks for the procmon logs!
grt@chromium.org: It looks like you know a lot more about this bug than I do. We need the change in the CL https://chromium-review.googlesource.com/c/chromium/src/+/1251442 to clear the profile information related to the account if the users turns off browser sign in and restarts Chrome, so I cannot just blindly revert it.

As a fix, I could change the code to return early on line https://cs.chromium.org/chromium/src/chrome/browser/profiles/gaia_info_update_service.cc?rcl=b60074a6e4d55c4cebce38b845cf7adb015b8029&l=168 if entry->GetGAIAName() is empty. Do you think that would fix this issue?

I am not sure how to repro this, so it will be hard for me to make sure if this would fix it.

grt@: I looked more at the code and here are my findings:
1. entry->SetGAIAName(base::string16()) and entry->SetGAIAGivenName(base::string16()) are no-ops if the GAIA name and given name are already empty strings
2. entry->SetGAIAPIcture(nullptr) is a no-op if the picture is already null (see https://cs.chromium.org/chromium/src/chrome/browser/profiles/profile_info_cache.cc?rcl=07a1e235baae77255e03416489ee8e46f599b056&l=533)
3. profile_->GetPrefs()->ClearPref(prefs::kProfileGAIAInfoPictureURL) just clears a preference which was already cleared, so this should also be an no-op
4. entry->SetAvatarIconIndex may be the only thing that that may be a real operation as it triggers NotifyOnProfileAvatarChanged (per https://cs.chromium.org/chromium/src/chrome/browser/profiles/profile_attributes_entry.cc?rcl=07a1e235baae77255e03416489ee8e46f599b056&l=315), but I am not 100% certain this would happen in the video in this bug as it look ike this occurs in a new profile.

grt@: Do you have the call stack that leads to this behavior? Would it be possible to add it to this bug? Thank you.

Comment 15 Deleted

I definitely have no knowledge to understand the more technical stuff you're not discussing. But if it helps with anything, I first thought the bug could have to do with me choosing the new option where if you un-click it, you don't have to be signed in to your account on the browser to access Google sites. Just because it was the only thing I'd changed recently.

So I marked that again, and I've been permanently signed in to my primary Google account since then. My new tab and configuration show the account and avatar, etc. The only thing I don't have on is sync. My point is, the bug happened the same to me whether I was signed in or not and whether that new option was on or not.

Just in case, repeating: this is on Window 7 Professional 32 bits.
Hi. I have a stable repro for this (all I have to do is launch chrome), though I'm not at all sure what I may have done in the past to get my user data/profiles in whatever state is triggering the problem. Here's what I see:

- when GAIAInfoUpdateService::GAIAInfoUpdateService is hit, identity_manager->HasPrimaryAccount() returns false
- in OnUsernameChanged, a ProfileAttributesEntry is found
- SetGAIAPicture(nullptr) is called
- in ProfileInfoCache::SetGAIAPictureOfProfileAtIndex, old_file_name is empty and gaia_picture_file_name appears to have already been empty
- OnProfileAvatarChanged is called for all observers
- ProfileShortcutManagerWin::OnProfileAvatarChanged handles this by calling CreateOrUpdateProfileIcon, which calls CreateOrUpdateShortcutsForProfileAtPath
- in ProfileShortcutManagerWin::CreateOrUpdateShortcutsForProfileAtPath, remove_badging is true and the early exit is not taken (create_mode is CREATE_OR_UPDATE_ICON_ONLY, though params.old_profile_name is empty).
- CreateOrUpdateDesktopShortcutsAndIconForProfile is posted to a blocking task runner, which ultimately calls SHChangeNotify, causing the flash.

So it looks to me like something in that processing maybe should realize that the shortcuts don't need to be updated after all.

Here's the callstack:

 # Child-SP          RetAddr           Call Site
00 000000e2`bd9fd4c8 00007ffa`201da1a3 chrome_7ffa1ea90000!ProfileShortcutManagerWin::CreateOrUpdateShortcutsForProfileAtPath [C:\b\c\b\win64_clang\src\chrome\browser\profiles\profile_shortcut_manager_win.cc @ 962]
01 000000e2`bd9fd4d0 00007ffa`20ad57dc chrome_7ffa1ea90000!ProfileInfoCache::SetGAIAPictureOfProfileAtIndex+0x39d [C:\b\c\b\win64_clang\src\chrome\browser\profiles\profile_info_cache.cc @ 566]
02 000000e2`bd9fd690 00007ffa`1ec7363a chrome_7ffa1ea90000!GAIAInfoUpdateService::OnUsernameChanged+0xec [C:\b\c\b\win64_clang\src\chrome\browser\profiles\gaia_info_update_service.cc @ 172]
03 000000e2`bd9fd720 00007ffa`1ec73579 chrome_7ffa1ea90000!GAIAInfoUpdateService::GAIAInfoUpdateService+0xb2 [C:\b\c\b\win64_clang\src\chrome\browser\profiles\gaia_info_update_service.cc @ 49]
04 000000e2`bd9fd790 00007ffa`1ebf2b85 chrome_7ffa1ea90000!GAIAInfoUpdateServiceFactory::BuildServiceInstanceFor+0x2d [C:\b\c\b\win64_clang\src\chrome\browser\profiles\gaia_info_update_service_factory.cc @ 40]
05 000000e2`bd9fd7d0 00007ffa`1ebf29d5 chrome_7ffa1ea90000!BrowserContextKeyedServiceFactory::BuildServiceInstanceFor+0x11 [C:\b\c\b\win64_clang\src\components\keyed_service\content\browser_context_keyed_service_factory.cc @ 97]
06 000000e2`bd9fd800 00007ffa`1ec715b6 chrome_7ffa1ea90000!KeyedServiceFactory::GetServiceForContext+0x1b3 [C:\b\c\b\win64_clang\src\components\keyed_service\core\keyed_service_factory.cc @ 0]
07 000000e2`bd9fd900 00007ffa`1ec04f06 chrome_7ffa1ea90000!ProfileImpl::DoFinalInit+0x684 [C:\b\c\b\win64_clang\src\chrome\browser\profiles\profile_impl.cc @ 662]
08 000000e2`bd9fdad0 00007ffa`1ec04b27 chrome_7ffa1ea90000!ProfileImpl::OnLocaleReady+0x2f0 [C:\b\c\b\win64_clang\src\chrome\browser\profiles\profile_impl.cc @ 941]
09 000000e2`bd9fdc40 00007ffa`1ebdddbb chrome_7ffa1ea90000!ProfileImpl::OnPrefsLoaded+0x83 [C:\b\c\b\win64_clang\src\chrome\browser\profiles\profile_impl.cc @ 973]
0a 000000e2`bd9fdd30 00007ffa`1ebdd578 chrome_7ffa1ea90000!ProfileImpl::ProfileImpl+0x6b1 [C:\b\c\b\win64_clang\src\chrome\browser\profiles\profile_impl.cc @ 583]
0b 000000e2`bd9fdf30 00007ffa`1ebdd359 chrome_7ffa1ea90000!Profile::CreateProfile+0xe0 [C:\b\c\b\win64_clang\src\chrome\browser\profiles\profile_impl.cc @ 352]
0c 000000e2`bd9fe070 00007ffa`1ebdd06f chrome_7ffa1ea90000!ProfileManager::CreateProfileHelper+0x63 [C:\b\c\b\win64_clang\src\chrome\browser\profiles\profile_manager.cc @ 1395]
0d 000000e2`bd9fe160 00007ffa`1ebdcdd2 chrome_7ffa1ea90000!ProfileManager::CreateAndInitializeProfile+0x75 [C:\b\c\b\win64_clang\src\chrome\browser\profiles\profile_manager.cc @ 1472]
0e 000000e2`bd9fe260 00007ffa`1ebdca91 chrome_7ffa1ea90000!ProfileManager::GetProfile+0x60 [C:\b\c\b\win64_clang\src\chrome\browser\profiles\profile_manager.cc @ 523]
0f 000000e2`bd9fe350 00007ffa`1ebb8bfb chrome_7ffa1ea90000!GetStartupProfile+0x52 [C:\b\c\b\win64_clang\src\chrome\browser\ui\startup\startup_browser_creator.cc @ 1000]
10 000000e2`bd9fe410 00007ffa`1ebb8202 chrome_7ffa1ea90000!ChromeBrowserMainParts::PreMainMessageLoopRunImpl+0x8f3 [C:\b\c\b\win64_clang\src\chrome\browser\chrome_browser_main.cc @ 1719]
11 000000e2`bd9fe730 00007ffa`1ebb807f chrome_7ffa1ea90000!ChromeBrowserMainParts::PreMainMessageLoopRun+0xae [C:\b\c\b\win64_clang\src\chrome\browser\chrome_browser_main.cc @ 1397]
12 000000e2`bd9fe810 00007ffa`1eb0a853 chrome_7ffa1ea90000!content::BrowserMainLoop::PreMainMessageLoopRun+0x55 [C:\b\c\b\win64_clang\src\content\browser\browser_main_loop.cc @ 1022]
13 000000e2`bd9fe8f0 00007ffa`1eb0a4fa chrome_7ffa1ea90000!content::StartupTaskRunner::RunAllTasksNow+0x2b [C:\b\c\b\win64_clang\src\content\browser\startup_task_runner.cc @ 43]
14 000000e2`bd9fe940 00007ffa`1eaae4ce chrome_7ffa1ea90000!content::BrowserMainLoop::CreateStartupTasks+0x268 [C:\b\c\b\win64_clang\src\content\browser\browser_main_loop.cc @ 935]
15 000000e2`bd9fea80 00007ffa`1eaae23a chrome_7ffa1ea90000!content::BrowserMainRunnerImpl::Initialize+0x7e [C:\b\c\b\win64_clang\src\content\browser\browser_main_runner_impl.cc @ 141]
16 000000e2`bd9febc0 00007ffa`1eaae114 chrome_7ffa1ea90000!content::BrowserMain+0xb0 [C:\b\c\b\win64_clang\src\content\browser\browser_main.cc @ 44]
17 000000e2`bd9feca0 00007ffa`1eaa85ff chrome_7ffa1ea90000!content::RunBrowserProcessMain+0x6f [C:\b\c\b\win64_clang\src\content\app\content_main_runner_impl.cc @ 537]
18 000000e2`bd9fed10 00007ffa`1ea95048 chrome_7ffa1ea90000!content::ContentMainRunnerImpl::Run+0x26d [C:\b\c\b\win64_clang\src\content\app\content_main_runner_impl.cc @ 0]
19 000000e2`bd9feec0 00007ffa`1ea94c48 chrome_7ffa1ea90000!service_manager::Main+0x333 [C:\b\c\b\win64_clang\src\services\service_manager\embedder\main.cc @ 472]
1a 000000e2`bd9ff230 00007ffa`1ea919ca chrome_7ffa1ea90000!content::ContentMain+0x41 [C:\b\c\b\win64_clang\src\content\app\content_main.cc @ 19]
1b 000000e2`bd9ff2c0 00007ff6`625a376c chrome_7ffa1ea90000!ChromeMain+0x118 [C:\b\c\b\win64_clang\src\chrome\app\chrome_main.cc @ 0]
1c 000000e2`bd9ff3b0 00007ff6`625a1697 chrome!MainDllLoader::Launch+0x26c [C:\b\c\b\win64_clang\src\chrome\app\main_dll_loader_win.cc @ 201]
1d 000000e2`bd9ff4a0 00007ff6`62676542 chrome!wWinMain+0x697 [C:\b\c\b\win64_clang\src\chrome\app\chrome_exe_main_win.cc @ 230]
1e 000000e2`bd9ff880 00007ffa`61c13034 chrome!__scrt_common_main_seh+0x106 [f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl @ 283]
1f 000000e2`bd9ff8c0 00007ffa`61d61461 KERNEL32!BaseThreadInitThunk+0x14
20 000000e2`bd9ff8f0 00000000`00000000 ntdll!RtlUserThreadStart+0x21

I hope this helps.
Labels: -M-72 ReleaseBlock-Stable M-71
grt@chromium.org: Thank you so much for the detailed explination. I'll see how I can avoid making these calls (I think the GAIAInfoUpdateService::OnUsernameChanged() should be a no-op in this case). Marking this bug as RBS for M72 so that it gets proper tracking.
Status: Started (was: Assigned)
M71 Stable promotion is coming VERY soon. Your bug is labelled as Stable  ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. Thank you.
Labels: Group-Windows_Integration
Labels: -Hotlist-DesktopUIConsider Hotlist-DesktopUITriaged
Project Member

Comment 23 by bugdroid1@chromium.org, Nov 12

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

commit 8b49dd022282696d499bd6cdf2ebee7a2547f5cd
Author: Mihai Sardarescu <msarda@chromium.org>
Date: Mon Nov 12 09:34:52 2018

Revert "Restore avatar after user logs out from sync."

This reverts commit ec686dc0ec5b8706a01dad1663f80f771da8a269.

Reason for revert:
The fix does not work in all cases and it adds a lot of complexity in
the way avatar images are now used. There are now 3 sources of truth for
the avatar icon index and it is not clear what the relationship between
them is:
* kAccountIdKey in the profile entry cache
* kProfileAvatarIndex which is a synced pref.
* kLocalProfileAvatarIndex which is non-syncable pref.

Note that kProfileAvatarIndex and kLocalProfileAvatarIndex are both set
when the user updates the image icon in chrome://settings/manageProfile.

I've just tested and if I start sync, set a local avatar icon, then turn
off sync, I still have the same image. So this CL does not fix the bug
863008 for this scenario.

The core issue comes from the fact that we try to fix a problem that
cannot be fixed in the current architecture of sync - sync preferences
do not have a local variant and do not get reverted to a local variant when
the user turns sync off. I think doing a custom implementation for the profile
avatar (and not for the profile name for example or settings) just adds confusion.

The fact that the relationship between these preferences is not very clear,
fixing the  issue 900374  becomes hard and is risky.

Bug:  900374 

Original change's description:
> Restore avatar after user logs out from sync.
>
> BUG=863008
>
> Change-Id: If083bc0703e251fb6e9c597fe5b7f842e37875c9
>
> R=​bauerb@chromium.org,sky@chromium.org
>
> Change-Id: If083bc0703e251fb6e9c597fe5b7f842e37875c9
> Reviewed-on: https://chromium-review.googlesource.com/1135137
> Commit-Queue: Thomas Tangl <tangltom@chromium.org>
> Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
> Reviewed-by: Thomas Tangl <tangltom@chromium.org>
> Reviewed-by: David Roger <droger@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#582541}

TBR=sky@chromium.org,droger@chromium.org,bauerb@chromium.org,tangltom@chromium.org,boocmp@yandex-team.ru

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 863008
Change-Id: I46aed023494d94e197828339f6870bf9ba0c9da8
Reviewed-on: https://chromium-review.googlesource.com/c/1329153
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Thomas Tangl <tangltom@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: anthonyvd <anthonyvd@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607176}
[modify] https://crrev.com/8b49dd022282696d499bd6cdf2ebee7a2547f5cd/chrome/browser/profiles/gaia_info_update_service.cc
[modify] https://crrev.com/8b49dd022282696d499bd6cdf2ebee7a2547f5cd/chrome/browser/profiles/gaia_info_update_service_unittest.cc
[modify] https://crrev.com/8b49dd022282696d499bd6cdf2ebee7a2547f5cd/chrome/browser/profiles/profile_impl.cc
[modify] https://crrev.com/8b49dd022282696d499bd6cdf2ebee7a2547f5cd/chrome/browser/profiles/profile_manager.cc
[modify] https://crrev.com/8b49dd022282696d499bd6cdf2ebee7a2547f5cd/chrome/browser/ui/webui/settings/settings_manage_profile_handler.cc
[modify] https://crrev.com/8b49dd022282696d499bd6cdf2ebee7a2547f5cd/chrome/common/pref_names.cc
[modify] https://crrev.com/8b49dd022282696d499bd6cdf2ebee7a2547f5cd/chrome/common/pref_names.h

Project Member

Comment 24 by bugdroid1@chromium.org, Nov 12

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

commit 1090ae4b0f89b3573580b6317e7fb3c4c648bea2
Author: Mihai Sardarescu <msarda@chromium.org>
Date: Mon Nov 12 11:50:28 2018

Avoid sending OnProfileAvatarChanged when the avatar image does not change

On Windows, Taskbar and Desktop icons are refreshed every time |OnProfileAvatarChanged|
notification is fired.

This CL only fires the notification |OnProfileAvatarChanged| when the profile
avatar image actually changes and avoids firing it otherwise.

Bug:  900374 

Change-Id: I9c930a3eab9d59aa91c785715336d520dcc813c5
Reviewed-on: https://chromium-review.googlesource.com/c/1323711
Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607196}
[modify] https://crrev.com/1090ae4b0f89b3573580b6317e7fb3c4c648bea2/chrome/browser/profiles/gaia_info_update_service_unittest.cc
[modify] https://crrev.com/1090ae4b0f89b3573580b6317e7fb3c4c648bea2/chrome/browser/profiles/profile_attributes_entry.cc
[modify] https://crrev.com/1090ae4b0f89b3573580b6317e7fb3c4c648bea2/chrome/browser/profiles/profile_info_cache.cc

Reminder M71 Stable is approaching VERY soon. Please review this bug and assess if this is indeed a RBS. If not, please remove the RBS label. If so, please make sure any planned work will be tested in Beta and verified before the Stable date. Thank you.

Requesting to take a look at M71 blockers ASAP due to upcoming Thanksgiving holidays next week.

Labels: Merge-Request-71
Status: Fixed (was: Started)
Requesting merge to M71 (note that I could not reproduce the original bug on Canary, so it is hard for me to verify the fix on Canary). However the fix does avoid sending the notification that lead to this bug, it is low risk and we should definitely merge it back to stable.

grt@: Would you be able to check whether this fixes the issue on Canary.
Project Member

Comment 27 by sheriffbot@chromium.org, Nov 14

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
I'll try installing a recent canary build on the machine where I observed the flashing at startup to see if the problem is resolved. It might take me a day or two.
Cc: vamshi.kommuri@chromium.org
 Issue 904807  has been merged into this issue.
Cc: viswa.karala@chromium.org
 Issue 904444  has been merged into this issue.
Canary 72.0.3610.0 doesn't do this anymore
Apparently this was verified by skypresetwarehouse@gmail.com on Canary (see comment #31).
Which CL you're requesting a  merge for? Is it fully safe to merge this late in release cycle? Also this is regressed in M70, is this critical to merge to M71?
We need to merge back both CLs. Merging them back to M71 should be fine. I think merging back to M70 is also fine. I cannot speak for how critical the issue is though and I also do not know how far we are for M71 stable.
Labels: -Merge-Review-71 Merge-Approved-71
Approving merge to M71 branch 3578 based on comment #26, #32, #34 and per offline chat with msarda@, these changes are fully safe to merge. 
If it helps - what I have also noticed is that if any of my desktop items have a red cross beside them (network drives) - when the icons refresh, the red cross is removed! That is a positive result so I wonder if it is a windows bug that is annoying in one sense and good in another. Just a thought..
Project Member

Comment 37 by bugdroid1@chromium.org, Nov 15

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/750491e4e361bcc8bb66b2285a5e90a2a1ce6c11

commit 750491e4e361bcc8bb66b2285a5e90a2a1ce6c11
Author: Mihai Sardarescu <msarda@chromium.org>
Date: Thu Nov 15 16:31:39 2018

Revert "Restore avatar after user logs out from sync."

This reverts commit ec686dc0ec5b8706a01dad1663f80f771da8a269.

Reason for revert:
The fix does not work in all cases and it adds a lot of complexity in
the way avatar images are now used. There are now 3 sources of truth for
the avatar icon index and it is not clear what the relationship between
them is:
* kAccountIdKey in the profile entry cache
* kProfileAvatarIndex which is a synced pref.
* kLocalProfileAvatarIndex which is non-syncable pref.

Note that kProfileAvatarIndex and kLocalProfileAvatarIndex are both set
when the user updates the image icon in chrome://settings/manageProfile.

I've just tested and if I start sync, set a local avatar icon, then turn
off sync, I still have the same image. So this CL does not fix the bug
863008 for this scenario.

The core issue comes from the fact that we try to fix a problem that
cannot be fixed in the current architecture of sync - sync preferences
do not have a local variant and do not get reverted to a local variant when
the user turns sync off. I think doing a custom implementation for the profile
avatar (and not for the profile name for example or settings) just adds confusion.

The fact that the relationship between these preferences is not very clear,
fixing the  issue 900374  becomes hard and is risky.

Bug:  900374 

Original change's description:
> Restore avatar after user logs out from sync.
>
> BUG=863008
>
> Change-Id: If083bc0703e251fb6e9c597fe5b7f842e37875c9
>
> R=​bauerb@chromium.org,sky@chromium.org
>
> Change-Id: If083bc0703e251fb6e9c597fe5b7f842e37875c9
> Reviewed-on: https://chromium-review.googlesource.com/1135137
> Commit-Queue: Thomas Tangl <tangltom@chromium.org>
> Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
> Reviewed-by: Thomas Tangl <tangltom@chromium.org>
> Reviewed-by: David Roger <droger@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#582541}

TBR=sky@chromium.org,droger@chromium.org,bauerb@chromium.org,tangltom@chromium.org,boocmp@yandex-team.ru

(cherry picked from commit 8b49dd022282696d499bd6cdf2ebee7a2547f5cd)

Bug: 863008
Change-Id: I46aed023494d94e197828339f6870bf9ba0c9da8
Reviewed-on: https://chromium-review.googlesource.com/c/1329153
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Thomas Tangl <tangltom@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: anthonyvd <anthonyvd@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#607176}
Reviewed-on: https://chromium-review.googlesource.com/c/1338102
Cr-Commit-Position: refs/branch-heads/3578@{#691}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/750491e4e361bcc8bb66b2285a5e90a2a1ce6c11/chrome/browser/profiles/gaia_info_update_service.cc
[modify] https://crrev.com/750491e4e361bcc8bb66b2285a5e90a2a1ce6c11/chrome/browser/profiles/gaia_info_update_service_unittest.cc
[modify] https://crrev.com/750491e4e361bcc8bb66b2285a5e90a2a1ce6c11/chrome/browser/profiles/profile_impl.cc
[modify] https://crrev.com/750491e4e361bcc8bb66b2285a5e90a2a1ce6c11/chrome/browser/profiles/profile_manager.cc
[modify] https://crrev.com/750491e4e361bcc8bb66b2285a5e90a2a1ce6c11/chrome/browser/ui/webui/settings/settings_manage_profile_handler.cc
[modify] https://crrev.com/750491e4e361bcc8bb66b2285a5e90a2a1ce6c11/chrome/common/pref_names.cc
[modify] https://crrev.com/750491e4e361bcc8bb66b2285a5e90a2a1ce6c11/chrome/common/pref_names.h

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/750491e4e361bcc8bb66b2285a5e90a2a1ce6c11

Commit: 750491e4e361bcc8bb66b2285a5e90a2a1ce6c11
Author: msarda@chromium.org
Commiter: msarda@chromium.org
Date: 2018-11-15 16:31:39 +0000 UTC

Revert "Restore avatar after user logs out from sync."

This reverts commit ec686dc0ec5b8706a01dad1663f80f771da8a269.

Reason for revert:
The fix does not work in all cases and it adds a lot of complexity in
the way avatar images are now used. There are now 3 sources of truth for
the avatar icon index and it is not clear what the relationship between
them is:
* kAccountIdKey in the profile entry cache
* kProfileAvatarIndex which is a synced pref.
* kLocalProfileAvatarIndex which is non-syncable pref.

Note that kProfileAvatarIndex and kLocalProfileAvatarIndex are both set
when the user updates the image icon in chrome://settings/manageProfile.

I've just tested and if I start sync, set a local avatar icon, then turn
off sync, I still have the same image. So this CL does not fix the bug
863008 for this scenario.

The core issue comes from the fact that we try to fix a problem that
cannot be fixed in the current architecture of sync - sync preferences
do not have a local variant and do not get reverted to a local variant when
the user turns sync off. I think doing a custom implementation for the profile
avatar (and not for the profile name for example or settings) just adds confusion.

The fact that the relationship between these preferences is not very clear,
fixing the  issue 900374  becomes hard and is risky.

Bug:  900374 

Original change's description:
> Restore avatar after user logs out from sync.
>
> BUG=863008
>
> Change-Id: If083bc0703e251fb6e9c597fe5b7f842e37875c9
>
> R=​bauerb@chromium.org,sky@chromium.org
>
> Change-Id: If083bc0703e251fb6e9c597fe5b7f842e37875c9
> Reviewed-on: https://chromium-review.googlesource.com/1135137
> Commit-Queue: Thomas Tangl <tangltom@chromium.org>
> Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
> Reviewed-by: Thomas Tangl <tangltom@chromium.org>
> Reviewed-by: David Roger <droger@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#582541}

TBR=sky@chromium.org,droger@chromium.org,bauerb@chromium.org,tangltom@chromium.org,boocmp@yandex-team.ru

(cherry picked from commit 8b49dd022282696d499bd6cdf2ebee7a2547f5cd)

Bug: 863008
Change-Id: I46aed023494d94e197828339f6870bf9ba0c9da8
Reviewed-on: https://chromium-review.googlesource.com/c/1329153
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Thomas Tangl <tangltom@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: anthonyvd <anthonyvd@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#607176}
Reviewed-on: https://chromium-review.googlesource.com/c/1338102
Cr-Commit-Position: refs/branch-heads/3578@{#691}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Project Member

Comment 39 by bugdroid1@chromium.org, Nov 15

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

commit 64e64e3bc4d97892f185100355ea35cb72709fb2
Author: Mihai Sardarescu <msarda@chromium.org>
Date: Thu Nov 15 16:45:10 2018

Avoid sending OnProfileAvatarChanged when the avatar image does not change

On Windows, Taskbar and Desktop icons are refreshed every time |OnProfileAvatarChanged|
notification is fired.

This CL only fires the notification |OnProfileAvatarChanged| when the profile
avatar image actually changes and avoids firing it otherwise.

Bug:  900374 

(cherry picked from commit 1090ae4b0f89b3573580b6317e7fb3c4c648bea2)

Change-Id: I9c930a3eab9d59aa91c785715336d520dcc813c5
Reviewed-on: https://chromium-review.googlesource.com/c/1323711
Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#607196}
Reviewed-on: https://chromium-review.googlesource.com/c/1338086
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#692}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/64e64e3bc4d97892f185100355ea35cb72709fb2/chrome/browser/profiles/gaia_info_update_service_unittest.cc
[modify] https://crrev.com/64e64e3bc4d97892f185100355ea35cb72709fb2/chrome/browser/profiles/profile_attributes_entry.cc
[modify] https://crrev.com/64e64e3bc4d97892f185100355ea35cb72709fb2/chrome/browser/profiles/profile_info_cache.cc

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

Commit: 64e64e3bc4d97892f185100355ea35cb72709fb2
Author: msarda@chromium.org
Commiter: msarda@chromium.org
Date: 2018-11-15 16:45:10 +0000 UTC

Avoid sending OnProfileAvatarChanged when the avatar image does not change

On Windows, Taskbar and Desktop icons are refreshed every time |OnProfileAvatarChanged|
notification is fired.

This CL only fires the notification |OnProfileAvatarChanged| when the profile
avatar image actually changes and avoids firing it otherwise.

Bug:  900374 

(cherry picked from commit 1090ae4b0f89b3573580b6317e7fb3c4c648bea2)

Change-Id: I9c930a3eab9d59aa91c785715336d520dcc813c5
Reviewed-on: https://chromium-review.googlesource.com/c/1323711
Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#607196}
Reviewed-on: https://chromium-review.googlesource.com/c/1338086
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#692}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
btw. in 70.0.3538.102 behaviour is the same
 Issue 906348  has been merged into this issue.
70.0.3538.110 official version is still doing this :(
I can confirm comment 43... still doing it.
Please hold tight until Chrome 71 reaches stable channel (approximately Dec 4, according to https://www.chromium.org/developers/calendar). Thanks.
Desktop icon refresh issue has been fixed today with Version 71.0.3578.80 (Official Build) (64-bit)! Finally!
ye,fixed ,。
One additional comment. By updating to version 71 of chrome, while fixing the icon refresh issue, you will now notice an internet speed reduction, oh my! Run a few different speed test sites on the old chrome (version 70 or older), the new chrome, firefox, and IE, and on a 300 MB high-speed connection you will see a reduction in speed of 10-20 MB! I guess you just can't have your cake and eat it too these days!
xiannekyl@gmail.com: These are 2 different issues (refresh icon issue vs internet speed reduction). Could you please open a separate bug for the internet speed reduction and CC grt@ on it (I saw he already replied to one of your comments on a separate bug)?

If you have repro steps on Chrome M70 vs Chrome M71 that would be really great as it may allow us to track down the root cause of the slowness.

Sign in to add a comment