Skip to content

Commit

Permalink
Change History API search to be cancellable
Browse files Browse the repository at this point in the history
  • Loading branch information
Brandon-T committed Feb 28, 2025
1 parent fdeadc3 commit 46d42fc
Show file tree
Hide file tree
Showing 6 changed files with 201 additions and 101 deletions.
140 changes: 76 additions & 64 deletions ios/brave-ios/Sources/Brave/Frontend/Browser/FrequencyQuery.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ class FrequencyQuery {
private let bookmarkManager: BookmarkManager
private let tabManager: TabManager

private let frequencyQueue = DispatchQueue(label: "frequency-query-queue")
private var task: DispatchWorkItem?
private var task: Task<Void, Error>?
private var historyCancellable: HistoryCancellable?

init(historyAPI: BraveHistoryAPI, bookmarkManager: BookmarkManager, tabManager: TabManager) {
self.historyAPI = historyAPI
Expand All @@ -27,94 +27,106 @@ class FrequencyQuery {

deinit {
task?.cancel()
historyCancellable = nil
}

public func sitesByFrequency(containing query: String, completion: @escaping (Set<Site>) -> Void)
{
task?.cancel()
@MainActor
private func fetchOpenTabs(containing query: String) async -> [Site] {
let startTime = Date()
let openTabSites = try? fetchSitesFromTabs(tabManager.tabsForCurrentMode(for: query))

print("Open Tab Sites: \(Date().timeIntervalSince(startTime)) seconds")
return openTabSites ?? []
}

task = DispatchWorkItem {
// brave-core fetch can be slow over 200ms per call,
// a cancellable serial queue is used for it.
DispatchQueue.main.async { [weak self] in
@MainActor
private func fetchBookmarks(containing query: String) async -> [Site] {
let bookmarksStartTime = Date()
return await withCheckedContinuation { continuation in
self.bookmarkManager.byFrequency(query: query) { [weak self] sites in
guard let self = self, let task = self.task, !task.isCancelled else {
continuation.resume(returning: [])
return
}

// Tab Fetch
let fetchedTabs = self.tabManager.tabsForCurrentMode(for: query)
let openTabSites = self.fetchSitesFromTabs(fetchedTabs)

let group = DispatchGroup()
group.enter()

// Bookmarks Fetch
var bookmarkSites = [Site]()
self.bookmarkManager.byFrequency(query: query) { sites in
bookmarkSites = sites.map {
continuation.resume(
returning: sites.map {
Site(url: $0.url ?? "", title: $0.title ?? "", siteType: .bookmark)
}
group.leave()
}
)

group.enter()
print("Bookmarks: \(Date().timeIntervalSince(bookmarksStartTime)) seconds")
}
}
}

@MainActor
private func fetchHistories(containing query: String) async -> [Site] {
let historysStartTime = Date()
return await withCheckedContinuation { continuation in
self.historyCancellable = self.historyAPI.byFrequency(query: query) { [weak self] sites in
guard let self = self, let task = self.task, !task.isCancelled else {
continuation.resume(returning: [])
return
}

// History Fetch
var historySites = [Site]()
self.historyAPI.byFrequency(query: query) { historyList in
historySites = historyList.map {
continuation.resume(
returning: sites.map {
Site(url: $0.url.absoluteString, title: $0.title ?? "", siteType: .history)
}
group.leave()
}
)

group.notify(queue: .main) {
self.task = nil
completion(Set<Site>(openTabSites + bookmarkSites + historySites))
}
print("History: \(Date().timeIntervalSince(historysStartTime)) seconds")
}
}
}

public func sitesByFrequency(containing query: String, completion: @escaping (Set<Site>) -> Void)
{
task?.cancel()
historyCancellable = nil

task = Task.delayed(bySeconds: 0.5) { @MainActor [weak self] in
guard let self = self else { return }

try Task.checkCancellation()

let fetches = await [
self.fetchOpenTabs(containing: query),
self.fetchBookmarks(containing: query),
self.fetchHistories(containing: query),
]

if let task = self.task {
frequencyQueue.async(execute: task)
try Task.checkCancellation()
self.task = nil
self.historyCancellable = nil

completion(Set(fetches.flatMap { $0 }))
}
}

private func fetchSitesFromTabs(_ tabs: [Tab]) -> [Site] {
private func fetchSitesFromTabs(_ tabs: [Tab]) throws -> [Site] {
var tabList = [Site]()
tabList.reserveCapacity(tabs.count)

for tab in tabs {
if tab.isPrivate {
if let url = tab.url, url.isWebPage(), !(InternalURL(url)?.isAboutHomeURL ?? false) {
if let selectedTabID = tabManager.selectedTab?.id, selectedTabID == tab.id {
continue
}
guard let selectedTab = tabManager.selectedTab else {
return []
}

tabList.append(
Site(
url: url.absoluteString,
title: tab.displayTitle,
siteType: .tab,
tabID: tab.id.uuidString
)
)
for tab in tabs {
if let url = tab.url, url.isWebPage(), !(InternalURL(url)?.isAboutHomeURL ?? false) {
if selectedTab.id == tab.id {
continue
}
} else {
let tabURL = tab.url ?? SessionTab.from(tabId: tab.id)?.url
if let url = tabURL, url.isWebPage(), !(InternalURL(url)?.isAboutHomeURL ?? false) {
if let selectedTabID = tabManager.selectedTab?.id, selectedTabID == tab.id {
continue
}

tabList.append(
Site(
url: url.absoluteString,
title: tab.displayTitle,
siteType: .tab,
tabID: tab.id.uuidString
)
tabList.append(
Site(
url: url.absoluteString,
title: tab.displayTitle,
siteType: .tab,
tabID: tab.id.uuidString
)
}
)
}
}

Expand Down
26 changes: 17 additions & 9 deletions ios/brave-ios/Sources/Brave/Frontend/Browser/TabManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -924,16 +924,24 @@ class TabManager: NSObject {
if let historyAPI = self.historyAPI {
// if we're only forgetting 1 site, we can query history by it's domain
let query = urls.count == 1 ? urls.first?.baseDomain : nil
let nodes = await historyAPI.search(
withQuery: query,
options: HistorySearchOptions(
maxCount: 0,
hostOnly: false,
duplicateHandling: .keepAll,
begin: nil,
end: nil

var historyCancellable: HistoryCancellable?
let nodes = await withCheckedContinuation { continuation in
historyCancellable = historyAPI.search(
withQuery: query,
options: HistorySearchOptions(
maxCount: 0,
hostOnly: false,
duplicateHandling: .keepAll,
begin: nil,
end: nil
),
completion: {
historyCancellable = nil
continuation.resume(returning: $0)
}
)
).filter { node in
}.filter { node in
guard let baseDomain = node.url.baseDomain else { return false }
return baseDomains.contains(baseDomain)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class HistoryModel: NSObject, ObservableObject {
private var listener: HistoryServiceListener?
private let maxFetchCount: UInt = 200
private var currentSearchQuery: String?
private var historyCancellable: HistoryCancellable?

@Published
var isHistoryServiceLoaded = false
Expand Down Expand Up @@ -130,8 +131,19 @@ class HistoryModel: NSObject, ObservableObject {
duplicateHandling: (query ?? "").isEmpty ? .removePerDay : .removeAll
)

await api?.search(withQuery: query, options: options).forEach {
[weak self] historyItem in
let historyItems = await withCheckedContinuation { continuation in
self.historyCancellable = api?.search(
withQuery: query,
options: options,
completion: {
continuation.resume(returning: $0)
}
)
}

try Task.checkCancellation()

historyItems.forEach { [weak self] historyItem in
guard let self = self else {
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,29 +25,31 @@ extension BraveHistoryAPI {
maxCount: UInt(max(20, maxLength)),
duplicateHandling: .removePerDay
)

search(
withQuery: nil,
options: options,
completion: { historyResults in
completion(historyResults.map { $0 })
}
completion: completion
)
}

func byFrequency(query: String, completion: @escaping ([HistoryNode]) -> Void) {
guard !query.isEmpty else {
return
func byFrequency(
query: String,
completion: @escaping ([HistoryNode]) -> Void
) -> HistoryCancellable? {
if query.isEmpty {
return nil
}

let options = HistorySearchOptions(
maxCount: 200,
duplicateHandling: query.isEmpty ? .removePerDay : .removeAll
duplicateHandling: .removeAll
)
search(

return search(
withQuery: query,
options: options,
completion: { historyResults in
completion(historyResults.map { $0 })
}
completion: completion
)
}

Expand Down
20 changes: 14 additions & 6 deletions ios/browser/api/history/brave_history_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ typedef NS_ENUM(NSInteger, HistoryDuplicateHandlingIOS) {
HistoryDuplicateHandlingIOSKeepAll
};

NS_SWIFT_NAME(HistoryCancellable)
OBJC_EXPORT
@interface IOSHistoryCancellable : NSObject
- (void)cancel;
@end

NS_SWIFT_NAME(HistoryNode)
OBJC_EXPORT
@interface IOSHistoryNode : NSObject
Expand Down Expand Up @@ -136,14 +142,16 @@ OBJC_EXPORT
/// @param options - Additional search options
/// @param completion - Block that notifies querying is finished with list of
/// items
- (void)searchWithQuery:(NSString* _Nullable)query
options:(IOSHistorySearchOptions*)searchOptions
completion:
(void (^)(NSArray<IOSHistoryNode*>* historyResults))completion;
- (IOSHistoryCancellable*)
searchWithQuery:(NSString* _Nullable)query
options:(IOSHistorySearchOptions*)searchOptions
completion:
(void (^)(NSArray<IOSHistoryNode*>* historyResults))completion;

/// Gets a count of unique domains visited as of now based on the `type` passed
- (void)fetchDomainDiversityForType:(DomainMetricTypeIOS)type
completion:(void (^)(NSInteger count))completion;
- (IOSHistoryCancellable*)
fetchDomainDiversityForType:(DomainMetricTypeIOS)type
completion:(void (^)(NSInteger count))completion;

@end

Expand Down
Loading

0 comments on commit 46d42fc

Please sign in to comment.