New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 695122 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Last visit > 30 days ago
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Allow deletion of sync records through sync-internals

Project Member Reported by s...@chromium.org, Feb 22 2017

Issue description

Would be very nice if we could target a specific entry in the Sync Node Browser to delete. Might be useful to give control of if this should be a commit or just a local change.

I've had multiple cases where users had bad sync data as a result of various bugs. After we determined the root cause behind a bug, it still takes a non trivial amount of time for the fix to be implemented and rolled out. In the meantime, we'd like an easy way to remove specific problematic pieces of data.
 

Comment 1 by zea@chromium.org, Feb 22 2017

Interesting idea. One thing we'd need to be careful of here is that not all datatypes support deletion of data (e.g. preferences). But, if we treat this as a developer tool and "use at your own risk", perhaps it would be good to support arbitrarily editing sync data via sync-internals? Would be useful to testing.

Comment 2 by s...@chromium.org, Feb 22 2017

Editing would be nice at times as well, but super risky. Editing the wrong value would very easily cause things to break. Adding new values would be near impossible.

By saying preferences doesn't support deletion, they do respond to deletes, it just means something slightly different, more like reset to default for them, right? Or am I misunderstanding?
Status: Available (was: Untriaged)

Comment 4 by zea@chromium.org, Apr 19 2017

Owner: rishiag@chromium.org
Status: Assigned (was: Available)
Status: Started (was: Assigned)

Comment 6 by s...@chromium.org, Apr 25 2017

Cc: pav...@chromium.org
I think the best way to do this is to create a deletion that looks like it is coming from the server. This will allow it to go through the sync machinery and all the way to the model type, regardless of if this ModelType is migrated to USS or still on the Directory.

Most hops are going to require access to private fields, so you're likely adding a lot of new methods to interfaces and implementations.

The signal will come from chrome://sync-internals [1], go through the SyncInternalsMessageHandler [2] to the ProfileSyncService [3]. This is all happening on the UI thread, but now we need to get to the Sync thread. The PSS has an |engine_| field which is actually a SyncBackendHostImpl [4], which knows how to post a task to the Sync thread, being executed by the SyncBackendHostCore [5]. The core holds a SyncManagerImpl [6], which holds a ModelTypeRegistry [7], which holds a map of UpdateHandler [7][8] objects. The key to this map is ModelType, which needs to have been passed all the way from sync-internals. UpdateHandler is where the Directory/USS split happens, the two possible implementations being DirectoryUpdateHandler [9] and ModelTypeWorker [10]. I think what you need to do is call ProcessGetUpdatesResponse [11] and then ApplyUpdates[12]. You'll need to give the ProcessGetUpdatesResponse call a SyncEntity object, which is where you set the necessary id fields (should have been passed from sync-internals) and set deleted [13] to true.

Both implementations of UpdateHandler should take care of jumping to the model thread and actually doing the work when ApplyUpdates is called. They don't expose a convenient way to understand if it was successful or not. I believe DirectoryUpdateHandler will block for the model type to be updated, while ModelTypeWorker will not block, but post and return immediately. This shouldn't really affect you. It is very likely that the implementations here are making some assumptions that you're breaking, so make sure to test.

You can force USS on for given model type by running Chrome with flags to force on features. These are defined in sync_driver_switches.cc
[14]. Typed URLs isn't fully implemented yet, I'd stick with device info and autocomplete. To actually use one of these, it'd look like:

> out/Default/chrome --enable-features=SyncUSSAutocomplete

[1] https://cs.chromium.org/chromium/src/components/sync/driver/resources/?q=chrome_sync.js
[2] https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/sync_internals_message_handler.h
[3] https://cs.chromium.org/chromium/src/components/browser_sync/profile_sync_service.h
[4] https://cs.chromium.org/chromium/src/components/sync/driver/glue/sync_backend_host_impl.h
[5] https://cs.chromium.org/chromium/src/components/sync/driver/glue/sync_backend_host_core.h
[6] https://cs.chromium.org/chromium/src/components/sync/engine_impl/sync_manager_impl.h
[7] https://cs.chromium.org/chromium/src/components/sync/engine_impl/model_type_registry.h?l=134
[8] https://cs.chromium.org/chromium/src/components/sync/engine_impl/update_handler.h
[9] https://cs.chromium.org/chromium/src/components/sync/engine_impl/directory_update_handler.h
[10] https://cs.chromium.org/chromium/src/components/sync/engine_impl/model_type_worker.h
[11] https://cs.chromium.org/chromium/src/components/sync/engine_impl/get_updates_processor.h?q=ProcessGetUpdatesResponse&sq=package:chromium&dr=CSs&l=71
[12] https://cs.chromium.org/chromium/src/components/sync/engine_impl/get_updates_processor.h?q=ProcessGetUpdatesResponse&sq=package:chromium&dr=CSs&l=51
[13] https://cs.chromium.org/chromium/src/components/sync/protocol/sync.proto?q=sync.proto&sq=package:chromium&dr=C&l=302
[14] https://cs.chromium.org/chromium/src/components/sync/driver/sync_driver_switches.cc

Comment 7 by s...@chromium.org, Apr 25 2017

Pavel pointed out this doesn't delete on the server. Which I believe we really do want. The three options I see are:
1. Send a one off commit that can somehow defeat reflection blocking.
2. Same path as in comment #6, but also inject change into Directory/ModelTypeWorker to get rolled into next commit.
3. Instead of going to the Sync thread, go PSS -> DTC -> processor, and write Directory/USS/Bookmarks specific code to apply the changes to both sync and model type.

Comment 8 by gangwu@chromium.org, Jan 17 2018

Status: WontFix (was: Started)

Sign in to add a comment