[iOS][QD] Add tab closure when deleting browsing data [chromium/src : main]
Filipa Senra (Gerrit)
Filipa Senra added 1 comment![]( "Open in Gerrit")
Hey Sylvain! Can you take a look? Ty.
Related details
- Sylvain Defresne
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sylvain Defresne (Gerrit)
Sylvain Defresne added 4 comments![]( "Open in Gerrit")
if (IsRemoveDataMaskSet(mask, BrowsingDataRemoveMask::CLOSE_TABS)) {
SessionRestorationService* service =
SessionRestorationServiceFactory::GetForBrowserState(browser_state_);
BrowserList* browser_list =
BrowserListFactory::GetForBrowserState(browser_state_);
for (Browser* browser : browser_list->AllRegularBrowsers()) {
service->LoadDataFromStorage(
browser,
base::BindRepeating(
&tabs_closure_util::GetLastCommittedTimestampFromStorage),
base::BindOnce(&OnTabsInformationLoaded, browser, delete_begin,
delete_end, CreatePendingTaskCompletionClosure()));
}
}
You already retrieve that info in `TabsCloser`.
Maybe we should avoid duplicating this query. Having a `TabsWithNoRecentNavigationCloser` which does the counting, keeping the list, and closing, and we pass it to `BrowsingDataRemoverImpl`.
And if `BrowsingDataRemoveMask::CLOSE_TABS` is set, it uses its `TabsWithNoRecentNavigationCloser` to close the tabs. This will avoid duplicating the logic to get the tabs, fetch their data, decide/count which should be closed and closing them.
void CloseTabs(Browser* browser, std::set<web::WebStateID> tabs) {Pass by const reference, and probably renamed to `tabs_to_close`.
void CloseTabs(
Browser* browser,
const std::set<web::WebStateID>& tabs_to_close) {
web_state_list->CloseWebStateAt(index, WebStateList::CLOSE_NO_FLAGS);Don't close tabs one by one as this will cause the WebStateList to perform many redundant work. Instead use `CloseWebStatesAtIndices()`
void CloseWebStatesAtIndices(int close_flags,
RemovingIndexes removing_indexes);
You would use it like this:
std::vector<int> indices_to_close;
for (int index = 0; index < web_state_list->count(); ++index) {
web::WebSate* web_state = web_state_list->GetWebStateAt(index);
web::WebStateID web_state_id = web_state->GetUniqueIdentifier();
if (base::Contains(tabs, web_state_id)) {
indices_to_close.push_back(index);
if (indices_to_close.size() == tabs.size()) {
// All tabs to close have been found, break early.
break;
}
}
}
WebStateList::ScopedBatchOperation lock =
web_state_list->StartBatchOperation();
web_state_list-> CloseWebStatesAtIndices(
WebStateList::CLOSE_NO_FLAGS,
RemovingIndexes(std::move(indices_to_close));
This avoid some catastrophic O(n**2) algorithm when closing many tabs as WebStateList can do all the bookkeeping at once instead of having to perform incremental bookkeeping for each closed tabs.
Related details
- Filipa Senra
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |