brave / brave-browser

Brave browser for Android, iOS, Linux, macOS, Windows.
https://brave.com
Mozilla Public License 2.0
17.63k stars 2.3k forks source link

Deleting a browser profile should not delete that data from Synced profiles #39503

Closed Brave-Matt closed 1 month ago

Brave-Matt commented 3 months ago

Description

In Brave, deleting a browser profile deletes all associated data with that profile. It also deletes all that same data from any devices that are on a Sync chain with that device — the same way it would if you were only deleting the data not the profile.

As @AlexeyBarabash points out, Chromium has code to prevent this propagation of data:

// Disables sync in order to prevent that browsing data deletions propagate
// across devices via sync.
void DisableSyncForProfileDeletion(Profile* profile) {
  signin::IdentityManager* identity_manager =
      IdentityManagerFactory::GetForProfileIfExists(profile);
  if (!identity_manager ||
      !identity_manager->HasPrimaryAccount(signin::ConsentLevel::kSignin)) {
    // Nothing to do as the user is signed out (hence sync is guaranteed to be
    // disabled).
    return;
  }

Brave also needs this same protection to ensure users do not unintentionally delete their browsing data across devices when trying to delete a profile.

Steps to reproduce

  1. Have 2+ browser profiles on the same chain
  2. Delete a browser profile
  3. Observe any shared data also gets deleted from Synced devices

Actual result

Data deleted from Synced devices

Expected result

Data on other devices should be preserved unless the data is explicitly deleted by the user.

Reproduces how often

Easily reproduced

Brave version (brave://version info)

v1.67.123

Channel information

Reproducibility

Miscellaneous information

User reports: https://community.brave.com/t/lost-sync-data-i-still-have-the-sync-code/551647/2

jagadeshjai commented 3 months ago

@AlexeyBarabash Can I work on this?

Brave-Matt commented 2 months ago

Another user reporting that this happened to them: https://community.brave.com/t/brave-deleted-all-passwords-and-bookmarks/559382/3

I think this issue should be prioritized higher. I'd also like to add that, if the behavior is not to be changed or requires too much overhead at this time, at minimum, users should be warned that the profile they're deleting is part of a Sync chain and that it should be removed from the chain first before they follow through with profile deletion.

cc @AlexeyBarabash

kjozwiak commented 1 month ago

The above requires 1.68.141 or higher for 1.68.x verification 👍

GeetaSarvadnya commented 1 month ago

Verification PASSED on

Brave | 1.68.141 Chromium: 127.0.6533.120 (Official Build) (64-bit)
-- | --
Revision | b3f23500b575d50584510ea1814ef440d30741a8
OS | Windows 10 Version 22H2 (Build 19045.4651)

Reproduced the issue on 1.68.137 Chromium: 127.0.6533.100

Sync chain between Machine 1, Profile 1 on machine 1 and Machine 2

1. Synced data shown across all the devices in a sync chain before deleting the Profile 1 on machine 1

Machine 1

Example Example Example
image image image

Profile 1 on machine 1

Example Example Example
image image image

Machine 2

Example Example Example
image image image

2. Delete Profile 1 from machine 1

Example Example Example Example
image image image image

3. Synced data is NOT shown across all the devices in a sync chain after deleting the Profile 1 on machine 1

Machine 1

Example Example Example
image image image

Machine 2

Example Example Example
image image image

Verifying Fix

Using the same STR/Cases mentioned/outlined above, created the same scenario using 1.68.141 Chromium: 127.0.6533.120 as per the following:

1. Synced data shown across all the devices in a sync chain before deleting the Profile 1 on machine 1

Machine 1

Example Example Example
image image image

Profile 1 on machine 1

Example Example Example
image image image

Machine 2

Example Example Example
image image image

2. Delete Profile 1 from machine 1

Example Example Example Example
image image image image

3. Synced data is shown across all the devices in a sync chain after deleting the Profile 1 on machine 1

Machine 1

Example Example Example
image image image

Machine 2

Example Example Example
image image image
GeetaSarvadnya commented 1 month ago

@AlexeyBarabash: When Profile 1 is deleted (via profile manager) on machine 1, the sync data is NOT removed from Machine 1 and Machine 2 as expected. But Profile 1 is not removed from the sync chain even after 15 mins of deletion of profile 1, this is behaving same on 1.68.137 and 1.68.141