Commit b0fe1358 authored by Huanzhong Huang's avatar Huanzhong Huang Committed by Commit Bot

[iOS] Let CBDManager handle counter/remover events

- Move counter/remover observer related logic from CBDCollectionViewController to CBDManager, so it can be reused by CBDTableViewController.
- Let CBDManager maintains browsing data counters upon construction.
- Let CBDManager be the browsing data remover observer.

Notes:
- A counter gathers info about its corresponding browsing data (history, cache, etc). A counter is constructed with a callback (typically to update UI).
- Two occasions when we want to restarts the counters so to keep UI updated: 1) When a CBD page is shown. 2) When a browsing data is cleared by a remover.

Bug: 935350
Change-Id: Idb060bb8af67f549534df7ad24721ba2a45977f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1545407
Commit-Queue: Huanzhong Huang <huanzhong@chromium.org>
Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#646279}
parent 2440e872
...@@ -7,7 +7,6 @@ ...@@ -7,7 +7,6 @@
#import "ios/chrome/browser/ui/settings/settings_root_collection_view_controller.h" #import "ios/chrome/browser/ui/settings/settings_root_collection_view_controller.h"
#import "ios/chrome/browser/browsing_data/browsing_data_remover_observer_bridge.h"
#import "ios/chrome/browser/ui/settings/clear_browsing_data/clear_browsing_data_consumer.h" #import "ios/chrome/browser/ui/settings/clear_browsing_data/clear_browsing_data_consumer.h"
namespace ios { namespace ios {
...@@ -17,8 +16,7 @@ class ChromeBrowserState; ...@@ -17,8 +16,7 @@ class ChromeBrowserState;
// CollectionView for clearing browsing data (including history, // CollectionView for clearing browsing data (including history,
// cookies, caches, passwords, and autofill). // cookies, caches, passwords, and autofill).
@interface ClearBrowsingDataCollectionViewController @interface ClearBrowsingDataCollectionViewController
: SettingsRootCollectionViewController <ClearBrowsingDataConsumer, : SettingsRootCollectionViewController <ClearBrowsingDataConsumer>
BrowsingDataRemoverObserving>
- (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState - (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState
NS_DESIGNATED_INITIALIZER; NS_DESIGNATED_INITIALIZER;
......
...@@ -49,14 +49,6 @@ ...@@ -49,14 +49,6 @@
@interface ClearBrowsingDataCollectionViewController () { @interface ClearBrowsingDataCollectionViewController () {
ios::ChromeBrowserState* _browserState; // weak ios::ChromeBrowserState* _browserState; // weak
// Observer for browsing data removal events and associated ScopedObserver
// used to track registration with BrowsingDataRemover. They both may be
// null if the new Clear Browser Data UI is disabled.
std::unique_ptr<BrowsingDataRemoverObserver> observer_;
std::unique_ptr<
ScopedObserver<BrowsingDataRemover, BrowsingDataRemoverObserver>>
scoped_observer_;
} }
// TODO(crbug.com/850699): remove direct dependency and replace with // TODO(crbug.com/850699): remove direct dependency and replace with
...@@ -71,9 +63,6 @@ ...@@ -71,9 +63,6 @@
@property(nonatomic, strong) @property(nonatomic, strong)
ChromeActivityOverlayCoordinator* chromeActivityOverlayCoordinator; ChromeActivityOverlayCoordinator* chromeActivityOverlayCoordinator;
// Restarts the counters for data types specified in the mask.
- (void)restartCounters:(BrowsingDataRemoveMask)mask;
@end @end
@implementation ClearBrowsingDataCollectionViewController @implementation ClearBrowsingDataCollectionViewController
...@@ -101,15 +90,6 @@ ...@@ -101,15 +90,6 @@
self.title = l10n_util::GetNSString(IDS_IOS_CLEAR_BROWSING_DATA_TITLE); self.title = l10n_util::GetNSString(IDS_IOS_CLEAR_BROWSING_DATA_TITLE);
self.collectionViewAccessibilityIdentifier = self.collectionViewAccessibilityIdentifier =
kClearBrowsingDataCollectionViewAccessibilityIdentifier; kClearBrowsingDataCollectionViewAccessibilityIdentifier;
if (IsNewClearBrowsingDataUIEnabled()) {
observer_ = std::make_unique<BrowsingDataRemoverObserverBridge>(self);
scoped_observer_ = std::make_unique<
ScopedObserver<BrowsingDataRemover, BrowsingDataRemoverObserver>>(
observer_.get());
scoped_observer_->Add(
BrowsingDataRemoverFactory::GetForBrowserState(browserState));
}
} }
return self; return self;
} }
...@@ -122,7 +102,7 @@ ...@@ -122,7 +102,7 @@
- (void)viewWillAppear:(BOOL)animated { - (void)viewWillAppear:(BOOL)animated {
[super viewWillAppear:animated]; [super viewWillAppear:animated];
[self restartCounters:BrowsingDataRemoveMask::REMOVE_ALL]; [self.dataManager restartCounters:BrowsingDataRemoveMask::REMOVE_ALL];
} }
#pragma mark CollectionViewController #pragma mark CollectionViewController
...@@ -141,28 +121,6 @@ ...@@ -141,28 +121,6 @@
[self.collectionView.collectionViewLayout invalidateLayout]; [self.collectionView.collectionViewLayout invalidateLayout];
} }
- (void)updateCounter:(NSInteger)itemType detailText:(NSString*)detailText {
CollectionViewModel* model = self.collectionViewModel;
if (!model)
return;
NSIndexPath* indexPath =
[model indexPathForItemType:itemType
sectionIdentifier:SectionIdentifierDataTypes];
ClearBrowsingDataItem* clearDataItem =
base::mac::ObjCCastStrict<ClearBrowsingDataItem>(
[model itemAtIndexPath:indexPath]);
// Do nothing if the text has not changed.
if ([detailText isEqualToString:clearDataItem.detailText])
return;
clearDataItem.detailText = detailText;
[self reconfigureCellsForItems:@[ clearDataItem ]];
[self.collectionView.collectionViewLayout invalidateLayout];
}
- (void)showBrowsingHistoryRemovedDialog { - (void)showBrowsingHistoryRemovedDialog {
NSString* title = NSString* title =
l10n_util::GetNSString(IDS_IOS_CLEAR_BROWSING_DATA_HISTORY_NOTICE_TITLE); l10n_util::GetNSString(IDS_IOS_CLEAR_BROWSING_DATA_HISTORY_NOTICE_TITLE);
...@@ -314,59 +272,6 @@ ...@@ -314,59 +272,6 @@
[self.dispatcher closeSettingsUIAndOpenURL:openMyActivityCommand]; [self.dispatcher closeSettingsUIAndOpenURL:openMyActivityCommand];
} }
- (void)restartCounters:(BrowsingDataRemoveMask)mask {
CollectionViewModel* model = self.collectionViewModel;
if (!self.collectionViewModel)
return;
if (IsRemoveDataMaskSet(mask, BrowsingDataRemoveMask::REMOVE_HISTORY)) {
NSIndexPath* indexPath = [self.collectionViewModel
indexPathForItemType:ItemTypeDataTypeBrowsingHistory
sectionIdentifier:SectionIdentifierDataTypes];
ClearBrowsingDataItem* historyItem =
base::mac::ObjCCastStrict<ClearBrowsingDataItem>(
[model itemAtIndexPath:indexPath]);
[historyItem restartCounter];
}
if (IsRemoveDataMaskSet(mask, BrowsingDataRemoveMask::REMOVE_CACHE)) {
NSIndexPath* indexPath = [self.collectionViewModel
indexPathForItemType:ItemTypeDataTypeCache
sectionIdentifier:SectionIdentifierDataTypes];
ClearBrowsingDataItem* cacheItem =
base::mac::ObjCCastStrict<ClearBrowsingDataItem>(
[model itemAtIndexPath:indexPath]);
[cacheItem restartCounter];
}
if (IsRemoveDataMaskSet(mask, BrowsingDataRemoveMask::REMOVE_PASSWORDS)) {
NSIndexPath* indexPath = [self.collectionViewModel
indexPathForItemType:ItemTypeDataTypeSavedPasswords
sectionIdentifier:SectionIdentifierDataTypes];
ClearBrowsingDataItem* passwordsItem =
base::mac::ObjCCastStrict<ClearBrowsingDataItem>(
[model itemAtIndexPath:indexPath]);
[passwordsItem restartCounter];
}
if (IsRemoveDataMaskSet(mask, BrowsingDataRemoveMask::REMOVE_FORM_DATA)) {
NSIndexPath* indexPath = [self.collectionViewModel
indexPathForItemType:ItemTypeDataTypeAutofill
sectionIdentifier:SectionIdentifierDataTypes];
ClearBrowsingDataItem* autofillItem =
base::mac::ObjCCastStrict<ClearBrowsingDataItem>(
[model itemAtIndexPath:indexPath]);
[autofillItem restartCounter];
}
}
#pragma mark BrowsingDataRemoverObserving
- (void)browsingDataRemover:(BrowsingDataRemover*)remover
didRemoveBrowsingDataWithMask:(BrowsingDataRemoveMask)mask {
[self restartCounters:mask];
}
#pragma mark MDCCollectionViewStylingDelegate #pragma mark MDCCollectionViewStylingDelegate
- (BOOL)collectionView:(UICollectionView*)collectionView - (BOOL)collectionView:(UICollectionView*)collectionView
......
...@@ -33,11 +33,6 @@ enum class BrowsingDataRemoveMask; ...@@ -33,11 +33,6 @@ enum class BrowsingDataRemoveMask;
// Indicate to user that data has been cleared. // Indicate to user that data has been cleared.
- (void)showBrowsingHistoryRemovedDialog; - (void)showBrowsingHistoryRemovedDialog;
// Only necessary for ClearBrowsingDataCollectionView to implement
// IsNewClearBrowsingDataUIEnabled experiment.
@optional
// Updates item of |itemType| with |detailText|.
- (void)updateCounter:(NSInteger)itemType detailText:(NSString*)detailText;
@end @end
#endif // IOS_CHROME_BROWSER_UI_SETTINGS_CLEAR_BROWSING_DATA_CLEAR_BROWSING_DATA_CONSUMER_H_ #endif // IOS_CHROME_BROWSER_UI_SETTINGS_CLEAR_BROWSING_DATA_CLEAR_BROWSING_DATA_CONSUMER_H_
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#import <Foundation/Foundation.h> #import <Foundation/Foundation.h>
#import "ios/chrome/browser/browsing_data/browsing_data_remover_observer_bridge.h"
#import "ios/chrome/browser/ui/settings/clear_browsing_data/clear_browsing_data_consumer.h" #import "ios/chrome/browser/ui/settings/clear_browsing_data/clear_browsing_data_consumer.h"
#import "ios/chrome/browser/ui/settings/clear_browsing_data/time_range_selector_table_view_controller.h" #import "ios/chrome/browser/ui/settings/clear_browsing_data/time_range_selector_table_view_controller.h"
...@@ -69,7 +70,8 @@ enum class ClearBrowsingDataListType { ...@@ -69,7 +70,8 @@ enum class ClearBrowsingDataListType {
// Manager that serves as the bulk of the logic for // Manager that serves as the bulk of the logic for
// ClearBrowsingDataConsumer. // ClearBrowsingDataConsumer.
@interface ClearBrowsingDataManager @interface ClearBrowsingDataManager
: NSObject <TimeRangeSelectorTableViewControllerDelegate> : NSObject <BrowsingDataRemoverObserving,
TimeRangeSelectorTableViewControllerDelegate>
// The manager's consumer. // The manager's consumer.
@property(nonatomic, assign) id<ClearBrowsingDataConsumer> consumer; @property(nonatomic, assign) id<ClearBrowsingDataConsumer> consumer;
...@@ -86,6 +88,10 @@ enum class ClearBrowsingDataListType { ...@@ -86,6 +88,10 @@ enum class ClearBrowsingDataListType {
// Fills |model| with appropriate sections and items. // Fills |model| with appropriate sections and items.
- (void)loadModel:(ListModel*)model; - (void)loadModel:(ListModel*)model;
// Restarts browsing data counters, which in turn updates UI, with those data
// types specified by |mask|.
- (void)restartCounters:(BrowsingDataRemoveMask)mask;
// Returns a ActionSheetCoordinator that has action block to clear data of type // Returns a ActionSheetCoordinator that has action block to clear data of type
// |dataTypeMaskToRemove|. // |dataTypeMaskToRemove|.
// When action triggered by a UIButton. // When action triggered by a UIButton.
......
...@@ -22,6 +22,8 @@ ...@@ -22,6 +22,8 @@
#include "ios/chrome/browser/browsing_data/browsing_data_counter_wrapper.h" #include "ios/chrome/browser/browsing_data/browsing_data_counter_wrapper.h"
#include "ios/chrome/browser/browsing_data/browsing_data_features.h" #include "ios/chrome/browser/browsing_data/browsing_data_features.h"
#include "ios/chrome/browser/browsing_data/browsing_data_remove_mask.h" #include "ios/chrome/browser/browsing_data/browsing_data_remove_mask.h"
#include "ios/chrome/browser/browsing_data/browsing_data_remover.h"
#include "ios/chrome/browser/browsing_data/browsing_data_remover_factory.h"
#include "ios/chrome/browser/chrome_url_constants.h" #include "ios/chrome/browser/chrome_url_constants.h"
#include "ios/chrome/browser/feature_engagement/tracker_factory.h" #include "ios/chrome/browser/feature_engagement/tracker_factory.h"
#include "ios/chrome/browser/history/web_history_service_factory.h" #include "ios/chrome/browser/history/web_history_service_factory.h"
...@@ -63,11 +65,35 @@ const int kMaxTimesHistoryNoticeShown = 1; ...@@ -63,11 +65,35 @@ const int kMaxTimesHistoryNoticeShown = 1;
// The tableView button red background color. // The tableView button red background color.
const CGFloat kTableViewButtonBackgroundColor = 0xE94235; const CGFloat kTableViewButtonBackgroundColor = 0xE94235;
// List of flags that have corresponding counters.
const std::vector<BrowsingDataRemoveMask> _browsingDataRemoveFlags = {
// BrowsingDataRemoveMask::REMOVE_COOKIES not included; we don't have cookie
// counters yet.
BrowsingDataRemoveMask::REMOVE_HISTORY,
BrowsingDataRemoveMask::REMOVE_CACHE,
BrowsingDataRemoveMask::REMOVE_PASSWORDS,
BrowsingDataRemoveMask::REMOVE_FORM_DATA,
};
} // namespace } // namespace
@interface ClearBrowsingDataManager () { @interface ClearBrowsingDataManager () {
// Access to the kDeleteTimePeriod preference. // Access to the kDeleteTimePeriod preference.
IntegerPrefMember _timeRangePref; IntegerPrefMember _timeRangePref;
// Observer for browsing data removal events and associated ScopedObserver
// used to track registration with BrowsingDataRemover. They both may be
// null if the new Clear Browser Data UI is disabled.
std::unique_ptr<BrowsingDataRemoverObserver> observer_;
std::unique_ptr<
ScopedObserver<BrowsingDataRemover, BrowsingDataRemoverObserver>>
scoped_observer_;
// Corresponds browsing data counters to their masks/flags. Items are inserted
// as clear data items are constructed. Remains empty if the new Clear Browser
// Data UI is disabled.
std::map<BrowsingDataRemoveMask, std::unique_ptr<BrowsingDataCounterWrapper>>
_countersByMasks;
} }
@property(nonatomic, assign) ios::ChromeBrowserState* browserState; @property(nonatomic, assign) ios::ChromeBrowserState* browserState;
...@@ -109,6 +135,15 @@ const CGFloat kTableViewButtonBackgroundColor = 0xE94235; ...@@ -109,6 +135,15 @@ const CGFloat kTableViewButtonBackgroundColor = 0xE94235;
_timeRangePref.Init(browsing_data::prefs::kDeleteTimePeriod, _timeRangePref.Init(browsing_data::prefs::kDeleteTimePeriod,
_browserState->GetPrefs()); _browserState->GetPrefs());
if (IsNewClearBrowsingDataUIEnabled()) {
observer_ = std::make_unique<BrowsingDataRemoverObserverBridge>(self);
scoped_observer_ = std::make_unique<
ScopedObserver<BrowsingDataRemover, BrowsingDataRemoverObserver>>(
observer_.get());
scoped_observer_->Add(
BrowsingDataRemoverFactory::GetForBrowserState(self.browserState));
}
} }
return self; return self;
} }
...@@ -323,6 +358,17 @@ const CGFloat kTableViewButtonBackgroundColor = 0xE94235; ...@@ -323,6 +358,17 @@ const CGFloat kTableViewButtonBackgroundColor = 0xE94235;
})); }));
} }
- (void)restartCounters:(BrowsingDataRemoveMask)mask {
for (auto flag : _browsingDataRemoveFlags) {
if (IsRemoveDataMaskSet(mask, flag)) {
const auto it = _countersByMasks.find(flag);
if (it != _countersByMasks.end()) {
it->second->RestartCounter();
}
}
}
}
#pragma mark Items #pragma mark Items
- (ListItem*)clearButtonItem { - (ListItem*)clearButtonItem {
...@@ -361,25 +407,12 @@ const CGFloat kTableViewButtonBackgroundColor = 0xE94235; ...@@ -361,25 +407,12 @@ const CGFloat kTableViewButtonBackgroundColor = 0xE94235;
mask:(BrowsingDataRemoveMask)mask mask:(BrowsingDataRemoveMask)mask
prefName:(const char*)prefName { prefName:(const char*)prefName {
PrefService* prefs = self.browserState->GetPrefs(); PrefService* prefs = self.browserState->GetPrefs();
std::unique_ptr<BrowsingDataCounterWrapper> counter;
if (IsNewClearBrowsingDataUIEnabled()) {
__weak ClearBrowsingDataManager* weakSelf = self;
counter = BrowsingDataCounterWrapper::CreateCounterWrapper(
prefName, self.browserState, prefs,
base::BindRepeating(
^(const browsing_data::BrowsingDataCounter::Result& result) {
[weakSelf.consumer
updateCounter:itemType
detailText:[weakSelf counterTextFromResult:result]];
}));
}
ListItem* clearDataItem; ListItem* clearDataItem;
// Create a ClearBrowsingDataItem for a CollectionView model and a // Create a ClearBrowsingDataItem for a CollectionView model and a
// TableViewClearBrowsingDataItem for a TableView model. // TableViewClearBrowsingDataItem for a TableView model.
if (self.listType == ClearBrowsingDataListType::kListTypeCollectionView) { if (self.listType == ClearBrowsingDataListType::kListTypeCollectionView) {
ClearBrowsingDataItem* collectionClearDataItem = ClearBrowsingDataItem* collectionClearDataItem =
[[ClearBrowsingDataItem alloc] initWithType:itemType [[ClearBrowsingDataItem alloc] initWithType:itemType counter:nullptr];
counter:std::move(counter)];
collectionClearDataItem.text = l10n_util::GetNSString(titleMessageID); collectionClearDataItem.text = l10n_util::GetNSString(titleMessageID);
if (prefs->GetBoolean(prefName)) { if (prefs->GetBoolean(prefName)) {
collectionClearDataItem.accessoryType = collectionClearDataItem.accessoryType =
...@@ -389,13 +422,28 @@ const CGFloat kTableViewButtonBackgroundColor = 0xE94235; ...@@ -389,13 +422,28 @@ const CGFloat kTableViewButtonBackgroundColor = 0xE94235;
collectionClearDataItem.prefName = prefName; collectionClearDataItem.prefName = prefName;
collectionClearDataItem.accessibilityIdentifier = collectionClearDataItem.accessibilityIdentifier =
[self accessibilityIdentifierFromItemType:itemType]; [self accessibilityIdentifierFromItemType:itemType];
if (IsNewClearBrowsingDataUIEnabled()) {
// Because there is no counter for cookies, an explanatory text is if (itemType == ItemTypeDataTypeCookiesSiteData) {
// displayed. // Because there is no counter for cookies, an explanatory text is
if (itemType == ItemTypeDataTypeCookiesSiteData && // displayed.
IsNewClearBrowsingDataUIEnabled()) { collectionClearDataItem.detailText =
collectionClearDataItem.detailText = l10n_util::GetNSString(IDS_DEL_COOKIES_COUNTER);
l10n_util::GetNSString(IDS_DEL_COOKIES_COUNTER); } else {
__weak ClearBrowsingDataManager* weakSelf = self;
__weak ClearBrowsingDataItem* weakCollectionClearDataItem =
collectionClearDataItem;
std::unique_ptr<BrowsingDataCounterWrapper> counter =
BrowsingDataCounterWrapper::CreateCounterWrapper(
prefName, self.browserState, prefs,
base::BindRepeating(^(
const browsing_data::BrowsingDataCounter::Result& result) {
weakCollectionClearDataItem.detailText =
[weakSelf counterTextFromResult:result];
[weakSelf.consumer
updateCellsForItem:weakCollectionClearDataItem];
}));
_countersByMasks.emplace(mask, std::move(counter));
}
} }
clearDataItem = collectionClearDataItem; clearDataItem = collectionClearDataItem;
} else { } else {
...@@ -713,4 +761,11 @@ const CGFloat kTableViewButtonBackgroundColor = 0xE94235; ...@@ -713,4 +761,11 @@ const CGFloat kTableViewButtonBackgroundColor = 0xE94235;
} }
} }
#pragma mark BrowsingDataRemoverObserving
- (void)browsingDataRemover:(BrowsingDataRemover*)remover
didRemoveBrowsingDataWithMask:(BrowsingDataRemoveMask)mask {
[self restartCounters:mask];
}
@end @end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment