When signing out from managed account, sync data should not be kept on local device |
||||||||
Issue descriptionSigning out from manage account should wipe all sync data. At the moment, this is done by every model. This may lead to leaks when adding a new data type. Find a way to enforce that local data is not kept on signing out.
,
Mar 10 2017
,
Mar 10 2017
So I believe what we've talked about in the past is that the ModelTypeChangeProcessor should always be given the metadata on startup by each model type/bridge. And then eventually sync will tell the proc what's going on, and it will make the appropriate choices. This means if a data type is disabled, sync should tell is as much, causing the proc to tell the bridge to ApplySyncChanges with a MCL that basically deletes all metadata. We knew we needed this because setting user prefs to disable sync, and deleting metadata data, is clearly not atomic, and we can easily get into an inconsistency. However, looking at the code, it doesn't look like we ever got around to this. Not only for steady state disabled particular types, but if the user has completely disabled sync or signed out, a significant portion of sync machinery is never initialized. Hopefully the controllers will still be registered, and this process can be driven from the UI thread in this case. Regardless though, I'm not sure how the above steps really satisfy the original request of this issue, and I'm not sure how satisfy-able they are. We have to, at the end of the day, trust the implementation of MetadataChangeList and ApplySyncChanges. The idea behind USS is that we're storing model type data and metadata in the same place, and taking away control from sync. There are some half measures we could do to make things slightly better against model type bugs. We could write something special/one off for ModelTypeStore to look for metadata when we don't think there should be any. We could also look into emitting a histogram from the processor, maybe the count of metadata when the proc is told the model type and/or sync is disabled.
,
Mar 11 2017
Clearing sync metadata on sign out (or disabling particular datatype) is sync engine's responsibility. Change processor should get the signal that syncing of datatype is being disabled, prepare MetadataChangeList with deletions of data type state and all entities' metadata and pass it to ModelTypeSyncBridge::ApplySyncChanges. Bridge is responsibile to apply these changes to storage. I'll look at this scenario see which parts don't work and fix them. Deleting data records though should be model's responsibility for the following reasons: - In some situations sync engine is not running and not aware of data existence when clearing data is performed. This happens when user signs-in into managed account or when signed out user invokes "Clear browsing data" from settings page. - It is possible that local model maintains records that are not synced with server (I think "Typed URLs" behave this way). In this case sync is not awre of the records. I think IOSChromeBrowsingDataRemover::RemoveImpl is the right place for deleting data records because both "Clear browsing data" and clearing data on sign-in/sign-out eventually land here.
,
Mar 15 2017
I verified in code that sync metadata is cleared when datatype is disabled or user signed out. I also ran a few tests to verify that autocomplete sync metadata gets deleted from database. We still have a race condition, browser cna crash right after preferences holding datatype enabled state get persisted, but before sync metadata is cleared. In this case sync metadata will not be cleared after restart. I'll address this issue separately.
,
Mar 20 2017
,
Jan 17 2018
,
Jan 19 2018
,
Apr 30 2018
mastiz/jkrcal, I believe you've been looking into similar things?
,
Jun 7 2018
Ping mastiz/jkrcal: Any updates? Do we have another bug that we can dupe this one into?
,
Jun 12 2018
Well, I think we need to retriage this bug. The issue (race condition) mentioned in #5 is tracked by bug 820049 so we can forget about it. The rest of the issue still seems unaddressed. - Does it only apply to iOS? Do all other platform remove the data in ChromeBrowsingDataRemoverDelegate? - Is there a plan to fix this issue? Do we agree with the plan in #4?
,
Aug 7
sync-triage ping: any updates?
,
Sep 9
sync-triage-ping, any updates?
,
Nov 13
Is this bug about sync data only? I don't know how iOS works, but on other platforms (at least desktop), if you enable sync for an enterprise account, you cannot disable it without deleting the local profile. Should we transform this bug into such feature request for iOS?
,
Nov 14
This concerns both iOS and Android as Chrome is single profile on these platforms.
,
Dec 17
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by stkhapugin@chromium.org
, Mar 10 2017Owner: pav...@chromium.org
Status: Assigned (was: Untriaged)