sync site settings applied on every start

[ayumi]

• Did you search for similar issues before submitting this one?
  yes

• Describe the issue you encountered:
  Sync site settings are applied every time Brave starts.

• Platform (Win7, 8, 10? macOS? Linux distro?): MacOS

• Brave Version (revision SHA):
  Since at least 0.14.2-preview3 (f07c142)

• Steps to reproduce:
  0. Add a console.log to syncUtil applySiteSettingRecord line 172, around applySetting(key, record.siteSetting[key]).

  1. Enable Sync.
  2. Visit wikipedia.org and turn shields down.
  3. Visit archive.org and turn on fingerprinting protection.
  4. Restart browser.
  5. Observe terminal for applied site settings.
  6. Restart browser.
  7. Observe terminal for applied site settings.

• Actual result:
  site settings are applied every start

• Expected result:
  site settings applied once only

• Will the steps above reproduce in a fresh profile? If not what other info can be added?
  yes

• Is this an issue in the currently released version?
  yes

• Can this issue be consistently reproduced?
  y

• Extra QA steps:
  1.
  2.
  3.

• Screenshot if needed:

• Any related issues:
  Startup Performance decrease when sync is turned on #8354

Might have been exacerbated by #8242

[ayumi]

Was an issue with the sync library (brave/sync#75), fixed in the latest release (https://github.com/brave/sync/releases/tag/v1.2.1). Future npm install will download latest sync lib which should fix this issue.

[bsclifton]

@ayumi how can we check the version?

[ayumi]

@bsclifton hard to do once packaged– we should probably make it log some version info.

if you're in dev / browser-laptop, you could:
ls -la app/extensions/brave/content/scripts/sync.js
and compare with brave/sync releases' file sizes:
curl https://api.github.com/repos/brave/sync/releases | less

[bsclifton]

@ayumi awesome- thanks for the heads up 😄 that should be enough for me to manually test using dev / browser-laptop. Anything else left in order to close this issue?

[ayumi]

that should be it

since this should work correctly for new npm installs i will close