Fix #3227: Implement History Sync

[soner-yuksel]

Summary of Changes

This Pull Request contains

• History Migration
• Sync Profile
• History Fetch/Add/Delete with Core and Sync
• Brave-Core Bump 1.26.x
• Delete All functionality on History
• Empty State for History

History Migration:

We are adding history brave-core migration to our existing migrator used for bookmarks and making both of the sync types work together and handle error cases related for both of the types. So in future the new sync types can also use same migrator.

History Fetch/Add/Delete/Remove with Core and Sync

A new HistoryFetcher is created where we perform the fetch from brave-core and various utility methods are created over the data. This fetcher is using Ordered Dictionary so in order to use this we added Swift Collections package. Believe the utility brought with Swift Collections will also be useful for the rest of the application. https://swift.org/blog/swift-collections/

In addition old add delete methods are replaced with brave-core functionality.

Sync Profile

Enable/Disable different Sync Types is added. Sync Settings will contain a new section which will allow user to toggle different Sync types according to users preference. This is using BraveSyncProfileService and it will also help when new sync types are added to give user ability to turn them on/off.

This pull request fixes #3227
This pull request fixes #3723
This pull request fixes #3721
This pull request fixes #3869
This pull request fixes #3591

Brave Core PR is merged and uplifted to 1.26.x and in addition this PR also handles the changes made in core to align history and bookmarks code.

Core PR For History Sync: brave/brave-core#8869

Core PR For alignment: brave/brave-core#9145

SecReview: https://github.com/brave/security/issues/501

Submitter Checklist:

• Unit Tests are updated to cover new or changed functionality
• User-facing strings use NSLocalizableString()

Test Plan:

Test Migration: The test cases and conditions are same with bookmark. Try migration 3 times but for both history and bookmarks together in this case.

• Update application with existing History base
• Check migration is happened and data is same

Normal Add / Delete / Delete All

• Fresh install or updated app
• Try Add history by entering urls or redirecting from other places
• Delete individual history
• Delete All history in History Screen

Test Sync: Same testing cases with bookmarks release but this time after joining to sync chain can actually turn on/off
which type of sync should be enabled

• Can test turning on different sync types and check If their functionality can be turned on/off
• History Sync should be tested but keep in mind only typed urls is going to be synced from desktop to mobile also mobile to desktop
• Migrated urls will be marked as types since we dont have informationa bout them so they should be synced

Screenshots:

Reviewer Checklist:

• Issues include necessary QA labels:
  • QA/(Yes|No)
  • release-notes/(include|exclude)
  • bug / enhancement
• Necessary security reviews have taken place.
• Adequate unit test coverage exists to prevent regressions.
• Adequate test plan exists for QA to validate (if applicable).
• Issue is assigned to a milestone (should happen at merge time).

[jumde]

@soner-yuksel - iOS supports special URLs and external URLs as well. about: | mailto: and tel: URLs. Do you know if these URLs are filtered before adding to the sync chain?

[soner-yuksel]

So the condition of the where the add is used is not changed. Only how we add is changed in this context.
History add is used inside func navigateInTab(tab: Tab, to navigation: WKNavigation? = nil)

And the conditions are
if !url.isErrorPageURL, !url.isAboutHomeURL, !url.isFileURL

brave-ios/Client/Frontend/Browser/BrowserViewController.swift

Line 1837 in 7391ecf

if !url.isErrorPageURL, !url.isAboutHomeURL, !url.isFileURL {

[jumde]

This looks good. I think we should add externalURL to this list. I can file a follow-up issue for that? Let me know if you disagree.

[soner-yuksel]

The security review is ongoing. Merging this to unblock feature core requirements. If any issue arose in security review there will be upcoming Pull Request.

[jumde]

@soner-yuksel - do you know if the device-name is sanitized before presenting in UI? I've come across cases where <script> tags in device name causes script execution.

[soner-yuksel]

This was an existing method which was moved in the file from previous position.
However the device_name is not a user entry field, it is fetched using BraveSyncAPI and underneath this information is fetched from DeviceInfoSyncServiceFactory

[jumde]

I verified that the device-name is not sanitized. I wasn't able to cause an injection, but as a defense in depth strategy what do you think about encoding the string before printing out the device name.