Issue metadata
Sign in to add a comment
|
Taskbar/Desktop icons refreshing on every launch (again)
Reported by
superhac...@bk.ru,
Oct 30
|
||||||||||||||||||||||
Issue descriptionUserAgent: 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.
,
Oct 30
Version 1803 Build 17134.345
,
Oct 31
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.
,
Oct 31
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!
,
Nov 1
,
Nov 1
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?
,
Nov 1
,
Nov 1
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.
,
Nov 2
Issue 898148 has been merged into this issue.
,
Nov 2
,
Nov 5
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.
,
Nov 5
BTW: thanks for the procmon logs!
,
Nov 5
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.
,
Nov 5
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.
,
Nov 6
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.
,
Nov 6
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.
,
Nov 6
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.
,
Nov 7
,
Nov 8
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.
,
Nov 8
,
Nov 8
,
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
,
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
,
Nov 13
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.
,
Nov 14
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.
,
Nov 14
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
,
Nov 14
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.
,
Nov 14
,
Nov 14
,
Nov 14
Canary 72.0.3610.0 doesn't do this anymore
,
Nov 14
Apparently this was verified by skypresetwarehouse@gmail.com on Canary (see comment #31).
,
Nov 14
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?
,
Nov 14
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.
,
Nov 14
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.
,
Nov 15
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..
,
Nov 15
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
,
Nov 15
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}
,
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
,
Nov 15
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}
,
Nov 16
btw. in 70.0.3538.102 behaviour is the same
,
Nov 19
Issue 906348 has been merged into this issue.
,
Nov 22
70.0.3538.110 official version is still doing this :(
,
Nov 22
I can confirm comment 43... still doing it.
,
Nov 23
Please hold tight until Chrome 71 reaches stable channel (approximately Dec 4, according to https://www.chromium.org/developers/calendar). Thanks.
,
Dec 8
Desktop icon refresh issue has been fixed today with Version 71.0.3578.80 (Official Build) (64-bit)! Finally!
,
Dec 8
ye,fixed ,。
,
Dec 9
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!
,
Dec 10
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 |
|||||||||||||||||||||||
Comment 1 by brucedaw...@chromium.org
, Oct 30