Deleting a Chrome profile doesn't delete its user data (such as bookmarks)
Reported by
93m4qau...@gmail.com,
Jan 18 2018
|
||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.132 Safari/537.36 Steps to reproduce the problem: 1. Open Chrome with just one profile. 2. Go to chrome://settings and add a scrap profile (be sure to create a desktop shortcut for the new profile). 3. Go to Google Maps and build up history. 4. Bookmark several pages, such as google.com, chromium.org, bugs.chromium.org, maps.google.com, and keep.google.com. 5. Right-click the desktop shortcut for the scrap profile and click Properties. 6. Select the shortcut target and press Ctrl+C to copy it to the clipboard. 7. Go to chrome://settings > Manage people and delete the scrap profile. 8. Right-click the desktop, click New, and click Shortcut. 9. Specify the target you copied to the clipboard as the new shortcut's target. 8. Double-click the new shortcut and open Chrome to find that the bookmarks and history were actually never deleted. (If you are using Chrome stable, it will actually crash when you do this, but this is fixed in Chrome canary.) What is the expected behavior? Since the scrap profile has been "deleted", its bookmarks, history, and other user data cannot be recovered simply by creating and clicking on a shortcut pointing to it. What went wrong? Simply by creating and clicking on a shortcut pointing to the "deleted" scrap profile, you can instantly pull it back up and see bookmarks, history, and other user data - perhaps even passwords and credit card numbers. Did this work before? No Chrome version: 63.0.3239.132 Channel: stable OS Version: 6.1 (Windows 7, Windows Server 2008 R2) Flash Version: When you "delete" a Chrome profile, it should be deleted, not hidden. ⛆ |
|
|
,
Jan 23 2018
+dullweber@, FYI.
,
Jan 24 2018
The Privacy label should be added to this bug.
,
Jan 24 2018
Thanks for reporting this bug. When you delete a profile, it is marked for deletion, all windows are closed and the profile directory will be deleted when you close Chrome. There are technical reasons that prevent deleting these files earlier. Of course it shouldn't be possible to load a deleted profile again. There is code that should prevent loading a deleted profile but it seems that your approach circumvents this somehow. I will investigate why this is happening. https://cs.chromium.org/chromium/src/chrome/browser/profiles/profile_manager.cc?l=549&rcl=f6d7a66d709fe816bbe0619456c01e3c867d9551
,
Jan 24 2018
When a profile is deleted we delete everything that is covered by Clear Browsing Data like history, cookies, passwords and autofill but not bookmarks. https://crbug.com/88586 seems to be the reason that we can't remove the profile earlier We should probably remove bookmarks as well. The check in CreateProfileAsnyc doesn't apply because and prevent that a windows for the profile can be opened.
,
Jan 24 2018
forgot to complete the last sentence: The check in CreateProfileAsnyc doesn't apply because the Profile object is not removed before closing Chrome. We should prevent that a windows for the profile can be opened.
,
Jan 24 2018
,
Jan 29 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/33b07d1373210013a53523516bf86cb4277861e4 commit 33b07d1373210013a53523516bf86cb4277861e4 Author: Christian Dullweber <dullweber@chromium.org> Date: Mon Jan 29 10:05:41 2018 Prevent opening a deleted profile When deleting a profile, the profile will be marked as deleted and all browsing data is removed but the profile folder will remain until shutdown and it still contains some data like bookmarks and settings. Using command line arguments it is possible to open a new window for this profile. This CL changes the profile manager to not return profiles that have been deleted. Instead of opening a window for a deleted profile, Chrome will refuse to open the window. Bug: 803643 Change-Id: Iae3caf93b2895d23b90aeb6550197a81038a1ec6 Reviewed-on: https://chromium-review.googlesource.com/883342 Reviewed-by: Stefan Kuhne <skuhne@chromium.org> Commit-Queue: Christian Dullweber <dullweber@chromium.org> Cr-Commit-Position: refs/heads/master@{#532378} [modify] https://crrev.com/33b07d1373210013a53523516bf86cb4277861e4/chrome/browser/profiles/profile_manager.cc
,
Jan 29 2018
It should not be possible to open a Chrome Window for a deleted profile anymore. We should still think about cleaning up more data when the profile is deleted as e.g. bookmarks are only delted when Chrome is closed completely. Marking as available as this doesn't need to be done immediately.
,
Jan 31 2018
Tested this issue on Windows 10 on the latest Chrome Build 66.0.3335.0 by following the steps mentioned in the original comment. Not able to open the deleted chrome profile by clicking on shortcut icon after following the steps mentioned in the original comment. Attached is the screen cast for reference. Hence adding TE verified labels as the fix is working as intended. Thanks..
,
Jan 31 2018
Tested this issue on Windows 10 on the latest Chrome Build 66.0.3335.0 by following the steps mentioned in the original comment. Unable to open the deleted Chrome profile on clicking on the shortcut icon after following the steps mentioned in the original comment. Attached is the screen cast for reference. Hence adding TE verified labels as the fix is working as intended. Thanks..
,
Feb 1 2018
Reverting this change as it causes crashes when sending feedback reports: https://crbug.com/807745
,
Feb 1 2018
,
Feb 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/94557c5bcc01a075e89ba629e4a5a20445456589 commit 94557c5bcc01a075e89ba629e4a5a20445456589 Author: Christian Dullweber <dullweber@chromium.org> Date: Thu Feb 01 09:57:37 2018 Revert "Prevent opening a deleted profile" This reverts commit 33b07d1373210013a53523516bf86cb4277861e4. Reason for revert: This change leads to crashes when sending feedback reports Original change's description: > Prevent opening a deleted profile > > When deleting a profile, the profile will be marked as deleted and all > browsing data is removed but the profile folder will remain until shutdown > and it still contains some data like bookmarks and settings. Using command > line arguments it is possible to open a new window for this profile. > This CL changes the profile manager to not return profiles that have > been deleted. Instead of opening a window for a deleted profile, > Chrome will refuse to open the window. > > Bug: 803643 > Change-Id: Iae3caf93b2895d23b90aeb6550197a81038a1ec6 > Reviewed-on: https://chromium-review.googlesource.com/883342 > Reviewed-by: Stefan Kuhne <skuhne@chromium.org> > Commit-Queue: Christian Dullweber <dullweber@chromium.org> > Cr-Commit-Position: refs/heads/master@{#532378} TBR=skuhne@chromium.org,dullweber@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 803643, 807745 Change-Id: I44ad772e3d1b7fc21ec9e414cd92c1d4b915a7af Reviewed-on: https://chromium-review.googlesource.com/897522 Reviewed-by: Christian Dullweber <dullweber@chromium.org> Commit-Queue: Christian Dullweber <dullweber@chromium.org> Cr-Commit-Position: refs/heads/master@{#533615} [modify] https://crrev.com/94557c5bcc01a075e89ba629e4a5a20445456589/chrome/browser/profiles/profile_manager.cc
,
Feb 1 2018
I found a way to reproduce the issue: - Open Chrome Stable 66.0.3335. - Delete your default profile (the one in the "default" profile directory). - Try to send feedback. This will call ProfileManager::GetActiveUserOrOffTheRecordProfileFromPath. This method tries to open the "default" profile. The default profile is deleted, so ProfileManager::GetProfile calls CreateAndInitializeProfile for the still existing profile directory, which is not a good idea.
,
Feb 1 2018
skuhne@: My recent change to prevent loading deleted profiles caused a crash when sending feedback reports after deleting the "default" profile. (see last comments)
Do you have a suggestion on how to best fix this? One way would be to move the deletion check outside profile manager to browser startup.
The issue with profile manager is that some code expects that there is a default profile or a default profile can be created. When the default profile is deleted but Chrome didn't restart yet, both aren't true.
The issue has a TODO (from 2011):
base::FilePath ProfileManager::GetInitialProfileDir() {
...
// TODO(mirandac): should not automatically be default profile.
return relative_profile_dir.AppendASCII(chrome::kInitialProfile);
}
(https://cs.chromium.org/chromium/src/chrome/browser/profiles/profile_manager.cc?l=624&rcl=3724a2a9d3e065522c18cc39757e0cf4756ddc90)
"Initial profile" doesn't really have a meaning outside ChromeOS, maybe we could return the LastUsedProfile instead?
,
Feb 1 2018
Could this also cause Issue 807857 ?
,
Feb 15 2018
,
Feb 15 2018
ping skuhne@: I created a bug for the issue that sending feedback reports is always accessing the Default profile but I think there is a general problem that ProfileManager::GetPrimaryUserProfile and ProfileManager::GetActiveUserProfile are available for Linux, Windows or Mac builds. Do you know why they are there and why they return the "Default" profile? Both functions have a todo from you that they should be moved to ash but the todo is from 2014 :)
,
Feb 15 2018
I was travelling in China and Japan - as such I assume that this fell through the cracks somehow. Yeah - that was part of the Multiprofile work. Back then there was a "getDefaultProfile" function which makes no sense in multi profile scenarios (what is the default then?) but it was called from over 250 locations. Back then we checked out each of the invocations and eliminated them one by one. The profile team was targeting a re-write of the profile manager, but their office got closed for development back in 2014 and the team members got re-assigned to new projects. As such the effort of the re-write got stopped. We could do a delayed delete like this: https://cs.chromium.org/chromium/src/chrome/browser/chromeos/app_mode/kiosk_app_manager.cc?rcl=b0e6afca6430cd3aa67504b4453ec10e15e37dd9&l=117 Xiyuan (cc'ed) was suggesting that we might want to extend this into a more generic function.
,
Feb 15 2018
Ah, this is windows. The code I suggested in #20 only works on ChromeOS. How would feedback UI pick the profile to use? GetFeedbackProfile() [1]? Does this function pick up the deleted "Default" profile? Is there any profile available when it is called in the repro? [1]: https://cs.chromium.org/chromium/src/chrome/browser/feedback/feedback_dialog_utils.cc?rcl=b0e6afca6430cd3aa67504b4453ec10e15e37dd9&l=40
,
Feb 16 2018
Thanks! The feedback code that is crashing uses GetPrimaryProfile(). Maybe I can just switch it to the GetFeedbackProfile method? https://cs.chromium.org/chromium/src/chrome/browser/feedback/system_logs/log_sources/chrome_internal_log_source.cc?type=cs&q=ChromeInternalLogSource::PopulateSyncLogs&sq=package:chromium&l=168 This bug is not caused by GetPrimaryUserProfile() or GetActiveUserprofile() but my attempt at fixing it caused crashes because some code just assumes that a Default profile exists or can be created. As you said, it doesn't make sense on Windows to assume that the Default profile is the right thing to use. One other place that looks problematic is the AutomationEventRouter. https://cs.chromium.org/chromium/src/chrome/browser/extensions/api/automation_internal/automation_event_router.cc?q=GetPrimaryUserProfile%7CGetActiveUserprofile+-f:chromeos+-f:test+-f:ash+-f:android&sq=package:chromium&l=39&dr=C It seems easier and safer to fix this bug by just checking for deleted profiles when a Browser is created and not in ProfileManager in general, I think I will create a CL to do this. (https://cs.chromium.org/chromium/src/chrome/browser/ui/startup/startup_browser_creator.cc?type=cs&sq=package:chromium&l=947)
,
Feb 16 2018
For the crash in ChromeInternalLogSource::PopulateSyncLogs, we should probably pass down the profile when creating the system log fetcher instead of going through the ProfileManager::GetPrimaryProfile to get one.
,
Mar 8 2018
,
Mar 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8d297d6d7a3ea57b49fec20a4ab0c7419ca529be commit 8d297d6d7a3ea57b49fec20a4ab0c7419ca529be Author: Christian Dullweber <dullweber@chromium.org> Date: Thu Mar 15 13:39:21 2018 Delete bookmarks on profile deletion When a profile is deleted, all browsing_data is removed and the folder is marked for deletion on shutdown. Bookmarks are currently not deleted until the folder is deleted. As bookmarks can contains sensitive data they should also be removed immediately. This also changes SigninManagerAndroidTest to use ChromeBrowsingDataRemoverDelegate. Bug: 803643, 748484 Change-Id: Icebfcec44d61d8b38939a8cee8dc4a92b6f9e258 Reviewed-on: https://chromium-review.googlesource.com/957040 Commit-Queue: Christian Dullweber <dullweber@chromium.org> Reviewed-by: Bernhard Bauer <bauerb@chromium.org> Reviewed-by: Martin Šrámek <msramek@chromium.org> Cr-Commit-Position: refs/heads/master@{#543358} [modify] https://crrev.com/8d297d6d7a3ea57b49fec20a4ab0c7419ca529be/chrome/browser/android/signin/signin_manager_android.cc [modify] https://crrev.com/8d297d6d7a3ea57b49fec20a4ab0c7419ca529be/chrome/browser/android/signin/signin_manager_android_unittest.cc [modify] https://crrev.com/8d297d6d7a3ea57b49fec20a4ab0c7419ca529be/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc [modify] https://crrev.com/8d297d6d7a3ea57b49fec20a4ab0c7419ca529be/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h [modify] https://crrev.com/8d297d6d7a3ea57b49fec20a4ab0c7419ca529be/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc |
|||||||||||
►
Sign in to add a comment |
||||||||||||||
Comment 1 by krajshree@chromium.org
, Jan 19 2018Labels: Needs-Triage-M63