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

Issue 911166 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 4
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-12-05
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Session sync header deletions should cascade tab deletions

Project Member Reported by mastiz@chromium.org, Dec 3

Issue description

When it comes to SESSIONS sync (tabs from other devices and history), clients take care of garbage-collecting stale sessions, e.g. data from devices which the user doesn't use anymore, or where the browser has been uninstalled.

When a client decides a session is too old, it issues deletions (tombstones) for all corresponding sync entities, which includes one header entity and one or more tab entities.

Today, the receiving "end" (i.e. other syncing devices that haven't initiated the garbage collection) expect to receive tombstones for every single sync entity. This is not very robust and actually redundant, since the deletion of the header entity represents sufficient evidence that the whole session is being deleted.

To improve robustness and ultimately simplify the sync protocol, we should cascade deletions to all entities for a session, when the header entity is deleted.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a1074c5aac44a5751d37dcfd194881038d156693

commit a1074c5aac44a5751d37dcfd194881038d156693
Author: Mikel Astiz <mastiz@chromium.org>
Date: Tue Dec 04 09:22:49 2018

Do not rely on incoming tab deletions for session gc

When a deletion is received for a foreign session's header, it can only
mean garbage collection has kicked in (by a third device). Let's not
rely on all tab deletions being received, and instead immediately
cascade the deletion of all tabs for the session.

This should make the protocol more robust (i.e. no risk of having
sessions in a half-deleted state if a client goes offline while
committing deletions) and allow, long term, the simplification of the
sync protocol for sessions.

Bug:  911166 
Change-Id: I9b617983be37b78534d38cd4a02f7f1968e33b10
Reviewed-on: https://chromium-review.googlesource.com/c/1358463
Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613479}
[modify] https://crrev.com/a1074c5aac44a5751d37dcfd194881038d156693/components/sync/model_impl/client_tag_based_model_type_processor.cc
[modify] https://crrev.com/a1074c5aac44a5751d37dcfd194881038d156693/components/sync/model_impl/client_tag_based_model_type_processor.h
[modify] https://crrev.com/a1074c5aac44a5751d37dcfd194881038d156693/components/sync_sessions/session_store.cc
[modify] https://crrev.com/a1074c5aac44a5751d37dcfd194881038d156693/components/sync_sessions/session_store.h
[modify] https://crrev.com/a1074c5aac44a5751d37dcfd194881038d156693/components/sync_sessions/session_store_unittest.cc
[modify] https://crrev.com/a1074c5aac44a5751d37dcfd194881038d156693/components/sync_sessions/session_sync_bridge.cc
[modify] https://crrev.com/a1074c5aac44a5751d37dcfd194881038d156693/components/sync_sessions/session_sync_bridge_unittest.cc

Status: Fixed (was: Started)
NextAction: 2018-12-05
Will consider a merge request soon.
The NextAction date has arrived: 2018-12-05

Sign in to add a comment