Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Sync v2] device list update #10198

Closed
darkdh opened this issue Jun 10, 2020 · 2 comments · Fixed by brave/brave-core#5871
Closed

[Sync v2] device list update #10198

darkdh opened this issue Jun 10, 2020 · 2 comments · Fixed by brave/brave-core#5871

Comments

@darkdh
Copy link
Member

darkdh commented Jun 10, 2020

When a device leaves sync chain, other devices on the sync chain will still see the leaving device listed.

In order to solve this issue,
Server needs to:

  1. Listen to SyncDisabledEvent on /event endpoint (components/sync/protocol/sync.proto)
    1.1. Client sends this event in syncer::SyncStoppedReporter(components/sync/driver/sync_stopped_reporter.cc)

  2. Prepare SyncEntity with same id_string or client_defined_unique_tag as the client who sends SyncDisabledEvent for other clients on sync chain to fetch
    2.1. leave DeviceInfoSpecifics empty because client doesn't access it when applying deleted update (see ProcessUpdate in components/sync/engine_impl/syncer_util.cc). Also when client creates deleted SyncEntity commit, it sends empty specific

  3. Keep track device number of sync chain, when there is no devices on sync chain, delete all the data

Client needs to:

  1. Send server_id instead of cache_guid in SyncDisabledEvent so that server can update db
  2. check if itself is the last client who is about to leave the sync chain, if yes, warning user this action will clear all the data on server and ask whether to continue.

OR

Client makes sure it commits delete self device info record. This method can also allow us to delete other devices on device list

cc @bridiver @jsecretan @yrliou

@btlechowski
Copy link

btlechowski commented Jul 28, 2020

Verification passed on

Brave 1.12.102 Chromium: 84.0.4147.89 (Official Build) dev (64-bit)
Revision 19abfe7bcba9318a0b2a6bc6634a67fc834aa592-refs/branch-heads/4147@{#852}
OS Ubuntu 18.04 LTS

Verified test plan from brave/brave-core#5871
Online
image

Offline
image


Verification passed on

Brave | 1.12.107 Chromium: 84.0.4147.105 (Official Build) (64-bit)
-- | --
Revision | a6b12dfad6663f13a7e16e9a42a6a4975374096b-refs/branch-heads/4147@{#943}
OS | Windows 10 OS Version 1903 (Build 18362.959)

Online
image

Device A - Sync disabled:
image

Offline
Device A: Desktop
Device B: Android

  • Ensured that sync is disabled in Device A when the sync is reset in Device A and device A is offline and again back to Online
    image
  • Ensured that Device B is having only itself in device list when Device A is back to online
    Screenshot_2020-08-05-23-42-14

Verification PASSED on macOS 10.15.6 x64 using the following build:

Brave | 1.12.108 Chromium: 84.0.4147.105 (Official Build) (64-bit)
-- | --
Revision | a6b12dfad6663f13a7e16e9a42a6a4975374096b-refs/branch-heads/4147@{#943}
OS | macOS Version 10.15.6 (Build 19G73)

Went through the cases outlined via brave/brave-core#5871

Online Cases

Removed from Chain (macOS) Removed from Chain (macOS)
Screen Shot 2020-08-10 at 9 02 43 PM Screen Shot 2020-08-10 at 9 03 00 PM
Still on Sync Chain (Win 10 x64) Still on Sync Chain (Win 10 x64)
Annotation 2020-08-10 204208 Annotation 2020-08-10 210715

Offline Cases

Examples of enabling the network on Device A (macOS) once Leave sync chain was clicked:

macOS macOS
Screen Shot 2020-08-10 at 9 12 21 PM Screen Shot 2020-08-10 at 9 12 33 PM

Examples of Device B (Win 10 x64) after removing Device A (macOS) from the sync chain once the network was enabled:

Win 10 x64 Win 10 x64
Annotation 2020-08-10 211527 Annotation 2020-08-10 211554

@srirambv
Copy link
Contributor

Verification passed on OnePlus 6T with Android 10 running 1.12.111 x64 build


Verification passed on Samsung Tab A with Android 10 running 1.12.111 x64 build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment