Skip to content

/ brave-core

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

Brave syncer #2010

Merged
merged 73 commits into from Aug 20, 2019
Merged

Brave syncer #2010

merged 73 commits into from Aug 20, 2019
+4,684 −4,769

Conversation

@darkdh
Copy link
Member

darkdh commented Mar 19, 2019 •
edited

brave browser branch: brave-syncer
chromium: 76.0.3809.100

fix brave/brave-browser#2754

Overview:
This PR intercepts syncer's nudge/poll jobs to add brave sync records from brave/sync client.
Applying remote changes and committing local changes leverages chromium BookmarkChangeProcessor with minimal changes

Future work:
kSyncUSSBookmarks (https://www.chromium.org/developers/design-documents/sync/unified-sync-and-storage-overview)

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Verified that these changes build without errors on
    • Windows
    • macOS
    • Linux
  • Verified that these changes pass automated tests (npm test brave_unit_tests && npm test brave_browser_tests) on
    • Windows
    • macOS
    • Linux
  • Verified that all lint errors/warnings are resolved (npm run lint)
  • Ran git rebase master (if needed).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed.
  • Add appropriate QA labels (QA/Yes or QA/No) to include the closed issue in milestone

Test Plan:

  1. brave/brave-browser#2754 (comment)
  2. Unit tests

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source
@darkdh darkdh self-assigned this Mar 19, 2019
@darkdh darkdh force-pushed the brave-syncer branch 5 times, most recently from 00e1d5a to 300aaa2 Mar 29, 2019
@darkdh darkdh force-pushed the brave-syncer branch 6 times, most recently from 0b34a8b to 47e2612 Apr 8, 2019
@darkdh darkdh force-pushed the brave-syncer branch 3 times, most recently from 29b7ad2 to 3dc3465 Apr 16, 2019
@darkdh darkdh mentioned this pull request Apr 18, 2019
Merged
@darkdh darkdh force-pushed the brave-syncer branch 5 times, most recently from 48d583b to 8c4cb39 Apr 18, 2019
@darkdh darkdh force-pushed the brave-syncer branch from c484549 to 9035e6c May 7, 2019
@AlexeyBarabash AlexeyBarabash force-pushed the brave-syncer branch from c62cd58 to 9a9b5f7 May 13, 2019
@darkdh darkdh force-pushed the brave-syncer branch 5 times, most recently from f80b0d2 to a8a5f2b May 17, 2019
@darkdh darkdh force-pushed the brave-syncer branch from bdaf863 to 5fc65a6 May 24, 2019
@darkdh darkdh marked this pull request as ready for review May 24, 2019
darkdh added 6 commits Aug 15, 2019
@darkdh
Maintain SyncEntityInfo in bookmark node's meta info for legacy sync
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
ce62ff9 
compatibility
@darkdh
Review Feedback (1)
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
99b97f5 
1. Patch standardization
2. Apply clang-format
@darkdh
Review Feedback (2)
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
220d604 
  1. Remvoe weak_ptr for syncer and make sure get_record_cb_ won't get
  called when sync_thread is not running
  2. Remove unnecessary forward declaration
@darkdh
Use WaitableEvent::TimedWait as fail-safe
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
99f29b6
@darkdh
Review feedback (3)
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
8b24310 
1. Generate fake NIGORI entity when apply update
2. Set dummy encryption key for every update_response
@darkdh
Do not send meta info until all platforms support necessary chromium
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
Loading status checks…
42c5c31 
sync protocol like "version" and "UniquePosition", ...etc
@darkdh darkdh force-pushed the brave-syncer branch from 40bba95 to 42c5c31 Aug 19, 2019
@darkdh darkdh mentioned this pull request Aug 19, 2019
Open
@bridiver
bridiver approved these changes Aug 20, 2019
View changes
@darkdh
Copy link
Member Author

darkdh commented Aug 20, 2019

CI shows all green but reported aborted
https://staging.ci.brave.com/job/brave-browser-build-pr/job/brave-syncer/101/
@darkdh darkdh merged commit 05c7b61 into master Aug 20, 2019
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/jenkins/pr-head The build of this commit was aborted
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@darkdh darkdh added this to the 0.71.x - Nightly milestone Aug 20, 2019
mkarolin added a commit that referenced this pull request Aug 20, 2019
@mkarolin
TEMPORARY: bookmarks workaround. #2010 Pending.
55fd350
@darkdh
Copy link
Member Author

darkdh commented Aug 20, 2019

it was actually get terminated (https://staging.ci.brave.com/job/brave-browser-build-pr/job/brave-syncer/101/execution/node/425/log/)
but I did a local release build to make sure it passed and future CI run also pass
https://staging.ci.brave.com/job/brave-browser-build-pr/job/ca-5346/7/execution/node/383/log/?consoleFull
mkarolin added a commit that referenced this pull request Aug 20, 2019
@mkarolin
TEMPORARY: bookmarks workaround. #2010 Pending.
3a17b6a
darkdh added a commit that referenced this pull request Aug 20, 2019
@darkdh
Merge pull request #2010 from brave/brave-syncer
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
Loading status checks…
05086d8 
Brave syncer
@darkdh darkdh mentioned this pull request Aug 20, 2019
Merged
darkdh added a commit that referenced this pull request Aug 20, 2019
@darkdh
Merge pull request #2010 from brave/brave-syncer
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
Loading status checks…
9209cc0 
Brave syncer
@darkdh darkdh mentioned this pull request Aug 20, 2019
Merged
darkdh added a commit that referenced this pull request Aug 21, 2019
@darkdh
Merge pull request #2010 from brave/brave-syncer
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
Loading status checks…
d0a91db 
Brave syncer
darkdh added a commit that referenced this pull request Aug 23, 2019
@darkdh
Merge pull request #2010 from brave/brave-syncer
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
fa9fc38 
Brave syncer
darkdh added a commit that referenced this pull request Aug 23, 2019
@darkdh
Merge pull request #2010 from brave/brave-syncer
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
56c2a47 
Brave syncer
@darkdh darkdh mentioned this pull request Oct 3, 2019
Closed
@bsclifton bsclifton deleted the brave-syncer branch Oct 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexeyBarabash AlexeyBarabash

@bridiver bridiver

@bbondy bbondy

Assignees

@darkdh darkdh

Labels
feature/sync
Projects
None yet
Milestone
  0.70.x - Release
Linked issues

Successfully merging this pull request may close these issues.

Migrate sync code to Syncable Service
4 participants
@darkdh @AlexeyBarabash @bridiver @bsclifton
You can’t perform that action at this time.