Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Add sync #6751

Closed
wants to merge 71 commits into from
Closed

Add sync #6751

wants to merge 71 commits into from

Conversation

diracdeltas
Copy link
Member

@diracdeltas diracdeltas commented Jan 19, 2017

this adds basic support for browser sync, using https://github.com/brave/sync

Fix #1854

Test Plan:

setup

  • make sure automated tests related to sync are passing
  • npm install to add dependencies
  • npm run download-sync-client to download the sync client to the right subdirectory
  • add the following to package.json to create a separate browser 'device' to sync to:
 "start2": "node ./tools/start.js --user-data-dir=brave-development-2 --debug=5859 --enable-logging --v=0 --enable-extension-activity-logging --enable-sandbox-logging --enable-dcheck"

test

  • npm start
  • go to about:preferences#sync
  • enable sync
  • restart, then click 'add device...'
  • npm run start2
  • in device 2, go to about:preferences#sync > i have an existing code > enter the code from device 1
  • go to any website in either of the browsers, bookmark it and make some changes to the siteSettings. after a minute, it should show up in the other browser.

todos before public release (not necessarily before merge)

@ayumi
Copy link
Contributor

ayumi commented Jan 20, 2017

I tested this on my browser data, consisting of 241 bookmarks and 525 preferences.

It took 30s of beachball spinning to sync over to pyramid 2. That's probably okay for now, but we should improve the UX soon. brave/sync#44

Edit: This is fixed now.

@luixxiul
Copy link
Contributor

Rebase needed

@ayumi
Copy link
Contributor

ayumi commented Jan 20, 2017

Rebased to 0.13.1-branch @187fa9a

@luixxiul
Copy link
Contributor

luixxiul commented Jan 20, 2017

Please create an associated issue ;-) thanks

@ayumi
Copy link
Contributor

ayumi commented Jan 23, 2017

Rebased to 0.13.1-branch @e69d86f119018a433ebec7718e8c707999c6033e

Bookmark folders might have issues when syncing (should be resolved by brave/sync#47 and brave/sync#48 )

Regular bookmarks, history and site settings (eg Shields up/down) should sync correctly.

NejcZdovc and others added 13 commits January 24, 2017 09:34
Resolves #6402

Auditors: @bsclifton

Test Plan:
-Go to link in the issue
-Select link
-Right click on text selection
-Context menu for link should appear
Considering the context menus disappear on the mousedown event
I added hamburgerMenuWasOpen to the windowState to be able to
check it when the hamburgerMenu's onClick is executed

Closes #4921
The added tests test that the hamburgerMenu's context menu appears,
disappears, reappears on different clicks
Fixes ##5828

Auditors: @bsclifton @echosa

Test Plan:
- Build brave
- Open local file with brave (double click)
- Local file should be opened
Deletes "Report an Issue" menu item because both it and "Submit Feedback"
link to community.brave.com.

Resolves #6180
diracdeltas and others added 11 commits January 24, 2017 10:42
This adds most of the UX for Brave sync setup in the case where this device is the first device in the sync profile.

Auditors: @ayumi

Test Plan:
1. npm run clean-session-store
2. npm start
3. go to about:preferences#sync
4. click the orange button to set up sync for the first time
5. name the device, then click the create button
6. shut down brave, run npm start again
7. you should see the device name from (5) in the terminal console
8. go to about:preferences#sync, click 'add new device' button
9. you should see a QR code
Auditors: @diracdeltas

Test Plan:

Prep:

0. Update sync lib to `brave/sync #fix/resolve-delete-nonexistant-props`.
1. Prepare 2 instances (pyramids) of Brave with Sync enabled.
  - Enable Sync and close Brave.
  - Copy `{userData}/brave-development` to `{userData}/brave-development-2`.
  - Edit `brave-development-2/session-store-1` `deviceId` to `1`.
  - To `browser-laptop` `package.json` add `"start2": "node ./tools/start.js --user-data-dir=brave-development-2 --debug=5859 --enable-logging --v=0 --enable-extension-activity-logging --enable-sandbox-logging --enable-dcheck",`
2. In `appConfig.js` `sync.fetchInterval` reduce to 5 seconds.

Play:

3. Open both pyramid 1 and pyramid 2.
4. In pyramid 1 visit a webpage and open up the bravery panel.
5. In pyramid 2 visit the same page and open up the bravery panel.
6. In pyramid 2 toggle each available siteSetting. Observe it appears in pyramid 1 after 1–5s.
7. Try toggling multiple settings at once, and toggling different settings simulatenously on both pyramids.
8. In both go to Preferences #Shields. Clear siteSettings with the Clear links and the red X's. Observe they sync over.
Auditors: @diracdeltas

Test Plan:
1. Have Sync disabled.
2. Browse a site and toggle a Bravery Panel setting.
3. Enable Sync and restart your pyramid. Note that siteSettings are sent.
4. Restart your pyramid again. Note this time siteSettings are *not* sent.
Fix brave/sync#44

Auditors: @diracdeltas

Test Plan:
1. Start Pyramid 1 which has a bunch of syncable data. Sync should enabled.
2. Setup Pyramid 2 (add `"start2": "node ./tools/start.js --user-data-dir=brave-development-2 --debug=5859 --enable-logging --v=0 --enable-extension-activity-logging --enable-sandbox-logging --enable-dcheck",` to package.json) and specify the Pyramid 1 Sync credentials.
3. When Pyramid 2 restarts and begins syncing data, Brave should be somewhat usable (not completely frozen for 60s).
@ayumi ayumi force-pushed the feature/syncing-0.13.1 branch from 667aa17 to 322cac5 Compare January 24, 2017 18:49
@ayumi
Copy link
Contributor

ayumi commented Jan 24, 2017

Rebased on 0.13.1-branch @ 134ad76

@bsclifton bsclifton force-pushed the 0.13.1-branch branch 2 times, most recently from 65c7753 to 238cfd3 Compare January 25, 2017 19:19
@bsclifton bsclifton closed this Jan 26, 2017
@bsclifton bsclifton changed the base branch from 0.13.1-branch to master January 26, 2017 21:19
@bsclifton bsclifton reopened this Jan 26, 2017
@diracdeltas
Copy link
Member Author

@bsclifton is this supposed to be rebased on master now?

@bsclifton
Copy link
Member

0.13.1 is now merged 😄 Sorry for all the rebasing you've had to do to keep up with the merges. You all should be good now 👍

@diracdeltas diracdeltas mentioned this pull request Jan 27, 2017
@diracdeltas
Copy link
Member Author

closing in favor of #6882 which is against master

@luixxiul luixxiul removed this from the 0.13.1 milestone Jan 27, 2017
@cezaraugusto cezaraugusto deleted the feature/syncing-0.13.1 branch June 21, 2017 20:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.