fix multiple sync issues

[cezaraugusto]

This is a bundle of fixes for sync and is included together in one PR for convenience since the spec was fully updated and wouldn't make sense to have them separated.

Design spec: https://share.goabstract.com/ef5f3ea9-3aec-472e-81d0-1ad72d2a70e5

1) Update spec of Sync enabled view

Fix Clicking anywhere on the same line as 'Bookmarks' toggles Bookmark Sync on/off

Close brave/brave-browser#2574

Test plan

1. Sync two devices
2. Once Sync is enabled, clicking the bookmark text shouldn't trigger the toggle on/off.

2) Update spec of sync disabled view

Fix Text in the first screen (disabled content) feels extremely cramped and uneven

Close brave/brave-browser#2675

Test plan

1. This is set as QA/No as it is a design change and was addressed with the latest design spec (see the top of this comment for the link).

3) Update spec of Sync Devices/QR code/Sync Words modals

Fix Disable Phone/Computer buttons until the device name is queried

Close brave/brave-browser#2724

Fix Padding is too small between Word Code and QR Code in View Sync Code modal

Close brave/brave-browser#2591

Test plan

Note: This was addressed by the new design spec by removing the device name from the device type modal and the remake of the QR code and sync words.

First fix (brave/brave-browser#2724)

1. Create a new chain
2. In the Device Type modal (one with computer/mobile icons) ensure there isn't a device name in the title

Second fix (brave/brave-browser#2591)

1. With two devices connected, click "view sync code" and check that word code and QR code are different modals.

4) Add a loading state for adddin, removing and resetting Sync

Fix Sync needs a 'waiter' message when reset/confirm/remove

Close brave/brave-browser#2567

Fix Looking for device button is missing when QR code is shown

Close brave/brave-browser#2801

Fix Prevent the user from firing device removal multiple times

Close brave/brave-browser#2629

Test plan

First fix (brave/brave-browser#2567)

1. Clean profile
2. Click "start a new sync chain"
3. Click either mobile or computer
4. Check that there is a loading spinner looking for devices
5. Copy the sync code
6. Cancel the sync chain and go back to the first screen
7. Click "I have a code" and confirm your code
8. See the loading spinner running until the new screen is rendered

Second fix (brave/brave-browser#2629)

1. Have 3 devices in a sync chain
2. Once devices are loaded, remove one device by clicking the X button
   confirm removal
3. All buttons should be disabled
4. A loading spinner should show while the operation is running
   modal should self-close once the operation is completed

5) Add extra dialog to prevent the user from aborting a new sync chain

Fix when the user starts a new sync chain the modal needs to disable cancelation

Close brave/brave-browser#2564

Test plan

1. Clear profile
2. Click "start a new sync chain"
3. Click "tablet or phone"
4. Try to cancel the modal
5. A new modal appears asking if you are sure about closing it

Repeat the same process but this time clicking "computer".

6) Properly set the state for view code words and scan words

Fix Children of Sync device type modal should use the same modal

Close brave/brave-browser#2677

Test plan

1. Clear profile
2. Click "Start a New Sync Chain"
3. Click "Tablet or Phone"
4. Keep toggling between "View Sync Code" and "View QR Code"
5. Trying to close it closes them all at once

Repeat the same process with a sync chain set up by clicking "Add New Device" in the devices table screen.

7) Auto-close code modals when a new device is found

Fix Cancel and OK yield the same action in add device

Close brave/brave-browser#2550

Fix Looking for devices seems to be a dead UI on sync page question

Close brave/brave-browser#2528

Test plan

First fix (brave/brave-browser#2550)

1. Have a Sync chain enabled
2. Click "Add a New Device"
3. Should show a disabled button with a label of "looking for devices"

Second fix (brave/brave-browser#2528)

1. Clear profile
2. Create a new sync chain either by mobile or computer options
3. Sync another device with the Sync Code Words/QR Code
4. Check that the second device is synced
5. Wait a few moments and ensure the modal in the first device auto-closes once a new device is found

8) Rename resetSyncSetupError to clearSyncSetupError

Fix Sync: rename resetSyncSetupError to clearSyncSetupError

Close brave/brave-browser#2779

Test plan

n/a

[bsclifton]

Testing this out now... will leave some real-time notes:
I created a new sync group (Computer) in this branch and then had DEV channel (0.60.6) join the sync group.

1) verified Update spec of Sync enabled view 👍
2) verified Update spec of sync disabled view matches spec 👍

To test the second one, I had to leave the sync group. This did kill the group, but left this branch in a weird state. When I tried to CREATE a new sync group (computer), the UI is stuck looking like this (basically the words are missing):

Quitting and relaunching fixes the problem 👍 However, I did notice a problem also when removing a sync group. Here are the steps I did and what happens:

• Leave established sync group on DEV channel (0.60.6)
• Refresh brave://sync page in this branch (several times if needed)
• Sync group is not removed
• On this branch, click the X next to the device that has already left the group
• Confirm - click yes. Watching confirmation spinner and being impatient... Reload page before it finishes to force modal to go away
• Click Leave Sync Group
• App quits with the following DCHECK error:

[83053:775:0103/160933.609992:FATAL:sync_devices.cc(175)] Check failed: existing_it != std::end(devices_).
0   libbase.dylib                       0x00000001056c3cf3 base::debug::StackTrace::StackTrace(unsigned long) + 83
1   libbase.dylib                       0x00000001056c3dad base::debug::StackTrace::StackTrace(unsigned long) + 29
2   libbase.dylib                       0x00000001053b10fa base::debug::StackTrace::StackTrace() + 26
3   libbase.dylib                       0x00000001053f8aae logging::LogMessage::~LogMessage() + 142
4   libbase.dylib                       0x00000001053f78a5 logging::LogMessage::~LogMessage() + 21
5   libchrome_dll.dylib                 0x000000010f621847 brave_sync::SyncDevices::Merge(brave_sync::SyncDevice const&, int, bool*) + 807
6   libchrome_dll.dylib                 0x000000010f62a789 brave_sync::BraveSyncServiceImpl::OnResolvedPreferences(std::__1::vector<std::__1::unique_ptr<brave_sync::jslib::SyncRecord, std::__1::default_delete<brave_sync::jslib::SyncRecord> >, std::__1::allocator<std::__1::unique_ptr<brave_sync::jslib::SyncRecord, std::__1::default_delete<brave_sync::jslib::SyncRecord> > > > const&) + 649
7   libchrome_dll.dylib                 0x000000010f62a359 brave_sync::BraveSyncServiceImpl::OnResolvedSyncRecords(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::unique_ptr<std::__1::vector<std::__1::unique_ptr<brave_sync::jslib::SyncRecord, std::__1::default_delete<brave_sync::jslib::SyncRecord> >, std::__1::allocator<std::__1::unique_ptr<brave_sync::jslib::SyncRecord, std::__1::default_delete<brave_sync::jslib::SyncRecord> > > >, std::__1::default_delete<std::__1::vector<std::__1::unique_ptr<brave_sync::jslib::SyncRecord, std::__1::default_delete<brave_sync::jslib::SyncRecord> >, std::__1::allocator<std::__1::unique_ptr<brave_sync::jslib::SyncRecord, std::__1::default_delete<brave_sync::jslib::SyncRecord> > > > > >) + 345
8   libchrome_dll.dylib                 0x000000010fea48cf extensions::api::BraveSyncResolvedSyncRecordsFunction::Run() + 559
9   libchrome_dll.dylib                 0x000000010e44dd56 ExtensionFunction::RunWithValidation() + 454
.....

At this point, loading up brave://sync crashes every time. "Fixed" by deleting my profile

[bsclifton]

3) looking at Update spec of Sync Devices/QR code/Sync Words modals

• verified first fix; looks good! 👍
• verified second fix; There are two different modals 🤔

Even though the above (3) works, it does show a weird loading spinner in the modal (is this expected?):

4) Verifying Add a loading state for adddin, removing and resetting Sync

• Test plan for first fix works good... however, is that expected? Basically, I was able to join my PC to itself (in the same Brave instance; not another instance). Just to confirm:
  • Click new sync group
  • Copy words
  • Cancel + confirm cancellation
  • Join using keywords
  • Computer is now syncing with itself?
• Second fix verified 👍

[bsclifton]

5. Verified Add extra dialog to prevent the user from aborting a new sync chain 👍
6. Verified Properly set the state for view code words and scan words 👍
7. Verifying Auto-close code modals when a new device is found:

• First fix verified 👍
• Second fix verified 👍

8. test plan says n/a 😄

[bsclifton]

Feedback left - noticed some odd behavior. I doubt the crash I found is related to this PR (seems we may need to harden the sync library, so that bad users like me don't cause a crash 😄)

[AlexeyBarabash]

I cannot review tsx files, so built and launched.
UI changes looks good. The crash mentioned by @bsclifton is failed DCHECK in the case when device was already removed from the chain and is not result of this PR.

[bsclifton]

@AlexeyBarabash you are correct! Just for reference, here's the issue capturing that:
brave/brave-browser#2817

[bsclifton]

++! (after discussion on Slack; link below)

[cezaraugusto]

for those interested: the issues mentioned by @bsclifton were discussed at https://bravesoftware.slack.com/archives/C2HJYB45N/p1546637100018400 and brave/brave-browser#2604 was re-opened