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 missing bookmarks after chain creation #7251

Closed
AlexeyBarabash opened this issue Dec 6, 2019 · 4 comments · Fixed by brave/brave-core#4168
Closed

Sync missing bookmarks after chain creation #7251

AlexeyBarabash opened this issue Dec 6, 2019 · 4 comments · Fixed by brave/brave-core#4168

Comments

@AlexeyBarabash
Copy link
Contributor

Description

I made a regression in PR https://github.com/brave/brave-core/pull/3988/files#diff-ab344835b09fb6ac1c601af787b116bbL223

there is a discussion of that changes and I missed point 3 which was addressed by pending_send_records_ : device can switch to SQS , but lambda didn't yet discover that records, so such records could be missed. This happens after sync initialization, if there are some records created right after device connected to chain.

pending_send_records_ - fixed that when there are two devices in chain, so 1st device didn't send records until be sure 2nd device created SQS queue and the queue is discovered by lambda. But with 3 devices this approach could not work, though with 3rd device it is extremly hard to reproduce such case.

Steps to Reproduce

  1. Get deviceA with bookmarks A1.com, A2.com and deviceB with bookmarks B1.com, B2.com
  2. Enable sync on deviceA, copy sync code to deviceB to establish the chain
  3. Don't wait until chain is fully created and on deviceA create in bookmarks folder folderA and the bookmark AA.com as soon as you can after step2

Actual result:

  1. AA.com and folderA will never be created on deviceB. Any bookmarks created later on deviceA will be synced well.

Expected result:

  1. AA.com and folderA will be created on deviceB on the next sync cycle

Reproduces how often:

easily

Brave version (brave://version info)

Brave 1.4.3 Chromium: 79.0.3945.56 (Developer Build) (64-bit)
Revision 73cc6bf591f792b99f8fc7cdfb8addedbd084bf8-refs/branch-heads/3945@{#788}
OS Linux

Version/Channel Information:

  • Can you reproduce this issue with the current release? No
  • Can you reproduce this issue with the beta channel? No
  • Can you reproduce this issue with the dev channel? Probably yes, because of migrated Sync prefs redo issue 6504 brave-core#3988
  • Can you reproduce this issue with the nightly channel? Yes

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields?
  • Does the issue resolve itself when disabling Brave Rewards?
  • Is the issue reproducible on the latest version of Chrome?

Miscellaneous Information:

@AlexeyBarabash
Copy link
Contributor Author

As uplift brave/brave-core#4203 was merged to 1.3.x, modified the milestone.

@btlechowski
Copy link
Collaborator

btlechowski commented Jan 8, 2020

Added QA/Blocked as testing is blocked due to #6941

@kjozwiak
Copy link
Member

Removing QA/Blocked as both brave/brave-core#4407 & brave/brave-core#4408 have been merged. Once we get a new 1.3.x build that's > 1.3.100, QA should be able to verify this issue.

@btlechowski
Copy link
Collaborator

btlechowski commented Feb 5, 2020

Verification passed on

Brave 1.3.111 Chromium: 80.0.3987.85 (Official Build) (64-bit)
Revision 583b05dfa4ffc657f0f7c5cc13f53aa17c9a5bcf-refs/branch-heads/3987@{#791}
OS Ubuntu 18.04 LTS

Verified test plan from the description. First A1.com, A2.com, B1.com, B2.com were synced, then folder a and bookmark aaa. Verified 5 times. Also switched devices.

image

Verification passed using

Brave 1.3.111 Chromium: 80.0.3987.85 (Official Build) (64-bit)
Revision 583b05dfa4ffc657f0f7c5cc13f53aa17c9a5bcf-refs/branch-heads/3987@{#791}
OS macOS Version 10.14.6 (Build 18G103)
  • Verified STR from description 5x. Device A was always macOS laptop, device B was Ubuntu VM

Verification passed on

Brave 1.3.112 Chromium: 80.0.3987.87 (Official Build) (64-bit)
Revision 449cb163497b70dbf98d389f54e38e85d4c59b43-refs/branch-heads/3987@{#801}
OS Windows 10 OS Version 1803 (Build 17134.1006)
  • Verified test plan from the description
    image

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