Fix #3034, #3032, #3045 - Bookmarks performance and UX fixes. (#3043)

BraveCore/Updating the BraveRewards framework.md

@@ -0,0 +1,93 @@
+## Prerequisites
+
+1. Clone [brave-browser](https://github.com/brave/brave-browser) if you haven't already
+1. Make sure you meet [brave-browser's prerequisites](https://github.com/brave/brave-browser/wiki/macOS-Development-Environment)
+1. Run `npm install` if you haven't already
+1. Make sure its up to date:
+    ```shell
+    cd brave-browser
+    git checkout -- "*" && git pull
+    npm run init -- --target_os=ios
+    ```
+
+## Setting up an Xcode work environment
+
+You can now setup an Xcode workspace to make working on iOS code much easier.
+
+Run one of the following command while at the root folder of brave-browser:
+```shell
+# Creates a debug target
+npm run build -- Debug --target_os=ios --xcode_gen=ios
+# Creates a release target
+npm run build -- Release --target_os=ios --xcode_gen=ios
+# Creates a device build target
+npm run build -- Release --target_os=ios --target_arch=arm64 --xcode_gen=ios
+```
+
+Assuming you ran the first command, this will create a folder at `src/out/ios_Debug_Xcode` which will contain `Workspace.xcworkspace` with two projects:
+
+- `Products`: This will be what you build. You will likely only ever be choosing the `BraveRewards.framework` target scheme. You can remain on this scheme and still have proper autocomplete
+- `Sources`: This has all the sources of BraveRewards.framework found in its BUILD.gn and its dependencies. 
+
+  If a file does not appear here, it is because it either:
+  - Does not appear in the `sources` list in its accompanying BUILD.gn file, or
+  - Is a generated file (such as mojo generated models)
+
+  These files can still be navigated to by ⌘-clicking `#import`s even though they don't appear in the project after you have built the project at least once
+
+**Important Note**: Edits to this Xcode project do not affect the ninja build, so if you add/remove a file, you must update BUILD.gn to reflect that.
+
+### Debugging
+
+Assuming you have created a debug simulator Xcode environment (`npm run build -- --target_os=ios --xcode_gen=ios`), you can debug both the test app and unit tests.
+
+To debug the Brave iOS app attach to `Client` via Xcode, using `Debug` > `Attach to Process` or `Attach to Process by PID or Name` if you need the debugger attached immediately at startup
+
+To debug unit tests, attach to `brave_rewards_ios_tests` via Xcode, using `Debug` > `Attach to Process by PID or Name` before running them via command below
+
+### Unit Tests
+
+At the moment this doesn't support setting up a unit test bundle, so any changes to the tests files must still be ran using `npm run test brave_rewards_ios_tests -- --target_os=ios` as seen below
+
+## Making Changes to BraveRewards.framework
+
+When you have changes that need to be fixed in the BraveRewards.framework (such as ledger or ads API), this needs to happen in brave-core.
+
+1. Create your branch on brave-core:
+    ```shell
+    cd src/brave
+    git checkout -b my-feature-branch
+    ```
+1. Make your changes to the BraveRewards.framework files located in `src/brave/vendor/brave-ios`
+    - Any files added or removed must be reflected in `BUILD.gn` (sources)
+    - Any directories added or removed must be reflected in `BUILD.gn` (include_dirs)
+1. Build your changes either using Xcode or running one of the following commands:
+    ```shell
+    # This has to be run at brave-browser root dir
+    cd ../..
+    # Creates debug build
+    npm run build -- Debug --target_os=ios
+    # Creates release build
+    npm run build -- Release --target_os=ios
+    # Creates arm64 build
+    npm run build -- Release --target_os=ios --target_arch=arm64
+    ```
+1. Run the tests:
+    ```shell
+    npm run test brave_rewards_ios_tests -- Debug --target_os=ios
+    ```
+1. Run dependency check:
+    ```shell
+    npm run gn_check -- Debug --target_os=ios
+    ```
+1. Run linting:
+    ```shell
+    npm run lint
+    ```
+1. Copy a FAT framework to `brave-ios` by running `build_in_core.sh --skip-update ~/path/to/brave-browser`.
+    - _Note:_ You must include `--skip-update` or your local changes will be removed
+    - If your PR is likely to take some time and your branch is on the brave-core remote you can edit `package.json` in brave-browser to point brave-core to your branch. This means any `npm run init` will set brave-core to your branch and not master.
+1. When things are working correctly, open a PR in brave-core and add all recommended reviewers.
+    - Add auto-closing words to your PR description that references your original issue created in step 1 (i.e. `resolves https://github.com/brave/brave-ios/issues/9000`)
+    - If your changeset does not affect the desktop build (i.e. no changes were made to non-ios files), make sure to add the appropriate CI labels to your PR *on creation*: `CI/skip-windows`, `CI/skip-linux`, `CI/skip-macos`, and `CI/skip-android`. This will cut the time waiting for CI to complete and save CI resources. If you forget to add these you can login to Jenkins and abort the build, add the labels, then restart the build.
+1. Upon your PR being merged by you or another member and assuming your `brave-ios` changeset requires the updated brave-core code, update your library using `./build_in_core.sh ~/path/to/brave-browser` again (this time ommiting skip-upgrade) and submit a PR on `brave-ios` with the changes related to your brave-core changes.

BraveRewardsUI/Extensions/BraveLedgerExtensions.swift

@@ -55,7 +55,7 @@ extension BraveLedger {
   }
 
   var upholdWalletStatus: WalletStatus {
-    guard let userWallet = externalWallets[.uphold] else { return .notConnected }
+    guard let userWallet = upholdWallet else { return .notConnected }
     return userWallet.status
   }
 
@@ -276,17 +276,6 @@ extension BraveLedger {
   }
 }
 
-extension PublisherInfo {
-  /// Temporary conveninece method for converting the raw int now in
-  /// PublisherInfo, to the enum we've been using all along
-  var rewardsCategory: RewardsType {
-    guard let category = RewardsType(rawValue: Int(self.category)) else {
-      fatalError("Could not get a rewards category from category = \(self.category)")
-    }
-    return category
-  }
-}
-
 extension RewardsType {
   var displayString: String {
     switch self {

BraveRewardsUI/Settings/Tips/Details/TipsDetailViewController.swift

@@ -57,7 +57,6 @@ class TipsDetailViewController: UIViewController {
 
     state.ledger.listOneTimeTips { [weak self] infoList in
       guard let self = self else { return }
-      infoList.forEach({$0.category = Int32(RewardsType.oneTimeTip.rawValue)})
       self.tipsList.append(contentsOf: infoList)
       (self.view as? SettingsTableView)?.tableView.reloadData()
     }
@@ -179,7 +178,6 @@ extension TipsDetailViewController {
         return
       }
       self.tipsList.removeAll(where: {
-        $0.category == RewardsType.recurringTip.rawValue &&
         $0.id == key
       })
       self.tipsView.tableView.reloadData()

BraveRewardsUI/User Wallet/UserWalletDetailsViewController.swift

@@ -9,11 +9,11 @@ import BraveUI
 final class UserWalletDetailsViewController: UIViewController {
 
   private let state: RewardsState
-  private let wallet: ExternalWallet
+  private let wallet: UpholdWallet
 
   private var disconnectedWalletHandler: () -> Void
 
-  init(state: RewardsState, wallet: ExternalWallet, disconnectedWalletHandler: @escaping () -> Void) {
+  init(state: RewardsState, wallet: UpholdWallet, disconnectedWalletHandler: @escaping () -> Void) {
     self.state = state
     self.wallet = wallet
     self.disconnectedWalletHandler = disconnectedWalletHandler

BraveRewardsUI/Wallet/WalletViewController.swift

@@ -329,7 +329,7 @@ class WalletViewController: UIViewController, RewardsSummaryProtocol {
       attentionView.valueLabel.text = "\(publisher.percent)%"
 
       self.state.ledger.listRecurringTips { [weak self] in
-        guard let self = self, let recurringTip = $0.first(where: { $0.id == publisher.id && $0.rewardsCategory == .recurringTip }) else { return }
+        guard let self = self, let recurringTip = $0.first(where: { $0.id == publisher.id }) else { return }
 
         self.recurringTipAmount = recurringTip.weight
         self.publisherSummaryView.monthlyTipView.batValueView.amountLabel.text = "\(Int(recurringTip.weight))"
@@ -398,7 +398,7 @@ class WalletViewController: UIViewController, RewardsSummaryProtocol {
   }
 
   @objc private func tappedAddFunds() {
-    guard let wallet = state.ledger.externalWallets[.uphold], let url = URL(string: wallet.addUrl) else { return }
+    guard let wallet = state.ledger.upholdWallet, let url = URL(string: wallet.addUrl) else { return }
     state.delegate?.loadNewTabWithURL(url)
   }
 
@@ -513,7 +513,7 @@ class WalletViewController: UIViewController, RewardsSummaryProtocol {
   @objc private func tappedConnectWalletButton() {
     let verifyOnboarding = VerifyUserWalletViewController {
       self.dismiss(animated: true, completion: {
-        guard let wallet = self.state.ledger.externalWallets[.uphold],
+        guard let wallet = self.state.ledger.upholdWallet,
             let percentEncoded = wallet.verifyUrl.addingPercentEncoding(withAllowedCharacters: .urlQueryAllowed),
             let upholdURL = URL(string: percentEncoded) else { return }
           self.state.delegate?.loadNewTabWithURL(upholdURL)
@@ -523,7 +523,7 @@ class WalletViewController: UIViewController, RewardsSummaryProtocol {
   }
 
   private func showUserWalletDetails() {
-    guard let wallet = state.ledger.externalWallets[.uphold] else { return }
+    guard let wallet = state.ledger.upholdWallet else { return }
     let details = UserWalletDetailsViewController(state: state, wallet: wallet) {
       self.state.ledger.disconnectWallet(ofType: .uphold) { result in
         if result == .ledgerOk {
@@ -542,7 +542,7 @@ class WalletViewController: UIViewController, RewardsSummaryProtocol {
 
   @objc private func tappedDisconnectWalletButton() {
     dismiss(animated: true, completion: {
-      guard let wallet = self.state.ledger.externalWallets[.uphold],
+      guard let wallet = self.state.ledger.upholdWallet,
         let percentEncoded = wallet.verifyUrl.addingPercentEncoding(withAllowedCharacters: .urlQueryAllowed),
         let upholdURL = URL(string: percentEncoded) else { return }
       self.state.delegate?.loadNewTabWithURL(upholdURL)

Client/Application/ClientPreferences.swift

@@ -158,6 +158,7 @@ extension Preferences {
         static let syncV2BookmarksMigrationCompleted = Option<Bool>(key: "chromium.migration.bookmarks", default: false)
         static let syncV2BookmarksMigrationCount = Option<Int>(key: "chromium.migration.bookmarks.count", default: 0)
         static let syncEnabled = Option<Bool>(key: "chromium.sync.enabled", default: false)
+        static let lastBookmarksFolderNodeId = Option<Int?>(key: "chromium.last.bookmark.folder.node.id", default: nil)
     }
 
     final class Debug {

Client/Frontend/Browser/BrowserViewController.swift

@@ -666,16 +666,23 @@ class BrowserViewController: UIViewController {
         vpnProductInfo.load()
         BraveVPN.initialize()
 
-        self.migrateToChromiumBookmarks { success in
-            if !success {
-                DispatchQueue.main.async {
-                    let alert = UIAlertController(title: Strings.Sync.v2MigrationErrorTitle,
-                                                  message: Strings.Sync.v2MigrationErrorMessage,
-                                                  preferredStyle: .alert)
-                    alert.addAction(UIAlertAction(title: Strings.OKString, style: .default, handler: nil))
-                    self.present(alert, animated: true)
+        //We stop ever attempting migration after 3 times.
+        if Preferences.Chromium.syncV2BookmarksMigrationCount.value < 3 {
+            self.migrateToChromiumBookmarks { success in
+                if !success {
+                    DispatchQueue.main.async {
+                        let alert = UIAlertController(title: Strings.Sync.v2MigrationErrorTitle,
+                                                      message: Strings.Sync.v2MigrationErrorMessage,
+                                                      preferredStyle: .alert)
+                        alert.addAction(UIAlertAction(title: Strings.OKString, style: .default, handler: nil))
+                        self.present(alert, animated: true)
+                    }
                 }
             }
+        } else {
+            //After 3 tries, we mark Migration as successful.
+            //There is nothing more we can do for the user other than to let them export/import bookmarks.
+            Preferences.Chromium.syncV2BookmarksMigrationCompleted.value = true
         }
     }
 
@@ -1759,6 +1766,53 @@ class BrowserViewController: UIViewController {
         duckDuckGoPopup = popup
         popup.showWithType(showType: .flyUp)
     }
+
+    private func showBookmarkController() {
+        let bookmarkViewController = BookmarksViewController(
+            folder: Bookmarkv2.lastVisitedFolder(),
+            isPrivateBrowsing: PrivateBrowsingManager.shared.isPrivateBrowsing)
+
+        bookmarkViewController.toolbarUrlActionsDelegate = self
+
+        presentSettingsNavigation(with: bookmarkViewController)
+    }
+
+    func openAddBookmark() {
+        guard let selectedTab = tabManager.selectedTab,
+              let selectedUrl = selectedTab.url,
+              !(selectedUrl.isLocal || selectedUrl.isReaderModeURL) else {
+            return
+        }
+
+        let bookmarkUrl = selectedUrl.decodeReaderModeURL ?? selectedUrl
+
+        let mode = BookmarkEditMode.addBookmark(title: selectedTab.displayTitle, url: bookmarkUrl.absoluteString)
+
+        let addBookMarkController = AddEditBookmarkTableViewController(mode: mode)
+
+        presentSettingsNavigation(with: addBookMarkController, cancelEnabled: true)
+    }
+
+    private func presentSettingsNavigation(with controller: UIViewController, cancelEnabled: Bool = false) {
+        let navigationController = SettingsNavigationController(rootViewController: controller)
+        navigationController.modalPresentationStyle = .formSheet
+
+        let cancelBarbutton = UIBarButtonItem(
+            barButtonSystemItem: .cancel,
+            target: navigationController,
+            action: #selector(SettingsNavigationController.done))
+
+        let doneBarbutton = UIBarButtonItem(
+            barButtonSystemItem: .done,
+            target: navigationController,
+            action: #selector(SettingsNavigationController.done))
+
+        navigationController.navigationBar.topItem?.leftBarButtonItem = cancelEnabled ? cancelBarbutton : nil
+
+        navigationController.navigationBar.topItem?.rightBarButtonItem = doneBarbutton
+
+        present(navigationController, animated: true)
+    }
 }
 
 extension BrowserViewController: ClipboardBarDisplayHandlerDelegate {

Client/Frontend/Browser/BrowserViewController/BVC+Rewards.swift

@@ -82,7 +82,7 @@ extension BrowserViewController {
                 switch result {
                 case .ledgerOk:
                     // Fetch the wallet
-                    self.rewards.ledger.fetchExternalWallet(forType: .uphold) { _ in
+                    self.rewards.ledger.fetchUpholdWallet { _ in
                         if let redirectURL = redirectURL {
                             // Requires verification
                             let request = URLRequest(url: redirectURL)

Client/Frontend/Browser/Search/SearchLoader.swift

@@ -63,7 +63,9 @@ class _SearchLoader<UnusedA, UnusedB>: Loader<[Site], SearchViewController> {
                 }
 
                 // First, see if the query matches any URLs from the user's search history.
-                self.load(result)
+                let bookmarks = Set<Site>(Bookmarkv2.byFrequency(query: self.query).map { Site(url: $0.url ?? "", title: $0.title ?? "", bookmarked: true) })
+
+                self.load(Array(Set(result + bookmarks)))
 
                 // If the new search string is not longer than the previous
                 // we don't need to find an autocomplete suggestion.

Client/Frontend/Browser/Toolbars/BottomToolbar/Menu/Bookmarks/AddEditBookmarkTableViewController.swift

@@ -195,22 +195,14 @@ class AddEditBookmarkTableViewController: UITableViewController {
     /// Sorts folders by their older and nesting level.
     /// Indentation level starts with 0, but level 0 is designed for special folders
     /// (root level bookamrks, favorites).
-    private func sortFolders(parentID: Int? = nil,
-                             indentationLevel: Int = defaultIndentationLevel) -> [IndentedFolder] {
+    private func sortFolders() -> [IndentedFolder] {
         guard let objects = frc.fetchedObjects else { return [] }
-
-        let sortedObjects = objects.sorted(by: { $0.order < $1.order })
-
-        var result = [IndentedFolder]()
-
-        sortedObjects.filter { $0.parent?.objectID == parentID }.forEach {
-            result.append(($0, indentationLevel: indentationLevel))
-            // Append children recursively
-            result.append(contentsOf: sortFolders(parentID: $0.objectID,
-                                                  indentationLevel: indentationLevel + 1))
-        }
-
-        return result
+        return objects.map({
+            if let folder = $0 as? BraveBookmarkFolder {
+                return IndentedFolder(folder, folder.indentationLevel)
+            }
+            return IndentedFolder($0, AddEditBookmarkTableViewController.defaultIndentationLevel)
+        })
     }
 
     private func reloadData() {