Commit e5e2170f authored by shreyasv's avatar shreyasv Committed by Commit bot

Cleaning up CRWBrowsingDataMode

2 notable changes
1) Moved this inside an NS_ENUM inside the web namespace for
consistency with BrowsingDataTypes

2) Changed SYNCHRONIZING to CHANGING for 2 reasons
- It better reflects the actual intent of this enum value.
SYNCHRONIZING does not mean much in the context of the
CRWBrowsingDataStore since it's unclear
which object it's trying to synchronize with.
- To avoid confusion with |BrowsingDataParition::IsSynchronized|

BUG=480654

Review URL: https://codereview.chromium.org/1146223012

Cr-Commit-Position: refs/heads/master@{#333135}
parent ed250d34
...@@ -23,12 +23,12 @@ ...@@ -23,12 +23,12 @@
// being observed for KVO changes in the |mode| value. |browserState| cannot be // being observed for KVO changes in the |mode| value. |browserState| cannot be
// null and the |browserState|'s associated CRWBrowsingDataStore must be // null and the |browserState|'s associated CRWBrowsingDataStore must be
// |browsingDataStore|. // |browsingDataStore|.
// The |browsingDataStore|'s mode must not already be SYNCHRONIZING. // The |browsingDataStore|'s mode must not already be |CHANGING|.
- (void)startObservingBrowsingDataStore:(CRWBrowsingDataStore*)browsingDataStore - (void)startObservingBrowsingDataStore:(CRWBrowsingDataStore*)browsingDataStore
browserState:(web::BrowserState*)browserState; browserState:(web::BrowserState*)browserState;
// Stops observing |browsingDataStore| for its |mode| change. // Stops observing |browsingDataStore| for its |mode| change.
// The |browsingDataStore|'s mode must not be SYNCHRONIZING. // The |browsingDataStore|'s mode must not be |CHANGING|.
- (void)stopObservingBrowsingDataStore:(CRWBrowsingDataStore*)browsingDataStore; - (void)stopObservingBrowsingDataStore:(CRWBrowsingDataStore*)browsingDataStore;
@end @end
...@@ -42,7 +42,7 @@ ...@@ -42,7 +42,7 @@
web::BrowserState::GetBrowsingDataPartition(browserState); web::BrowserState::GetBrowsingDataPartition(browserState);
DCHECK(browsing_data_partition); DCHECK(browsing_data_partition);
DCHECK_EQ(browsing_data_partition->GetBrowsingDataStore(), browsingDataStore); DCHECK_EQ(browsing_data_partition->GetBrowsingDataStore(), browsingDataStore);
DCHECK_NE(SYNCHRONIZING, browsingDataStore.mode); DCHECK_NE(web::CHANGING, browsingDataStore.mode);
[browsingDataStore addObserver:self [browsingDataStore addObserver:self
forKeyPath:@"mode" forKeyPath:@"mode"
...@@ -52,7 +52,7 @@ ...@@ -52,7 +52,7 @@
- (void)stopObservingBrowsingDataStore: - (void)stopObservingBrowsingDataStore:
(CRWBrowsingDataStore*)browsingDataStore { (CRWBrowsingDataStore*)browsingDataStore {
DCHECK_NE(SYNCHRONIZING, browsingDataStore.mode); DCHECK_NE(web::CHANGING, browsingDataStore.mode);
[browsingDataStore removeObserver:self forKeyPath:@"mode"]; [browsingDataStore removeObserver:self forKeyPath:@"mode"];
} }
...@@ -65,7 +65,7 @@ ...@@ -65,7 +65,7 @@
DCHECK([browsingDataStore isKindOfClass:[CRWBrowsingDataStore class]]); DCHECK([browsingDataStore isKindOfClass:[CRWBrowsingDataStore class]]);
DCHECK(context); DCHECK(context);
if (browsingDataStore.mode == SYNCHRONIZING) { if (browsingDataStore.mode == web::CHANGING) {
++self.outOfSyncStoreCount; ++self.outOfSyncStoreCount;
return; return;
} }
...@@ -76,9 +76,9 @@ ...@@ -76,9 +76,9 @@
bool activeState = activeStateManager->IsActive(); bool activeState = activeStateManager->IsActive();
// Check if the |browsingDataStore|'s associated ActiveStateManager's active // Check if the |browsingDataStore|'s associated ActiveStateManager's active
// state is still out of sync. // state is still out of sync.
if (activeState && browsingDataStore.mode == INACTIVE) { if (activeState && browsingDataStore.mode == web::INACTIVE) {
[browsingDataStore makeActiveWithCompletionHandler:nil]; [browsingDataStore makeActiveWithCompletionHandler:nil];
} else if (!activeState && browsingDataStore.mode == ACTIVE) { } else if (!activeState && browsingDataStore.mode == web::ACTIVE) {
[browsingDataStore makeInactiveWithCompletionHandler:nil]; [browsingDataStore makeInactiveWithCompletionHandler:nil];
} }
...@@ -115,7 +115,7 @@ BrowsingDataPartitionImpl::~BrowsingDataPartitionImpl() { ...@@ -115,7 +115,7 @@ BrowsingDataPartitionImpl::~BrowsingDataPartitionImpl() {
if (active_state_manager_) { if (active_state_manager_) {
active_state_manager_->RemoveObserver(this); active_state_manager_->RemoveObserver(this);
} }
DCHECK_NE(SYNCHRONIZING, [browsing_data_store_ mode]); DCHECK_NE(CHANGING, [browsing_data_store_ mode]);
[g_browsing_data_store_mode_observer [g_browsing_data_store_mode_observer
stopObservingBrowsingDataStore:browsing_data_store_]; stopObservingBrowsingDataStore:browsing_data_store_];
} }
......
...@@ -61,12 +61,12 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH"; ...@@ -61,12 +61,12 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
- (SEL)browsingDataManagerSelectorForRemoveOperationType; - (SEL)browsingDataManagerSelectorForRemoveOperationType;
// Redefined to be read-write. Must be called from the main thread. // Redefined to be read-write. Must be called from the main thread.
@property(nonatomic, assign) CRWBrowsingDataStoreMode mode; @property(nonatomic, assign) web::BrowsingDataStoreMode mode;
// Sets the mode iff there are no more stash or restore operations that are // Sets the mode iff there are no more stash or restore operations that are
// still pending. |mode| can only be either |ACTIVE| or |INACTIVE|. // still pending. |mode| can only be either |ACTIVE| or |INACTIVE|.
// |handler| is called immediately (in the same runloop) with a BOOL indicating // |handler| is called immediately (in the same runloop) with a BOOL indicating
// whether the mode change was successful or not. |handler| can be nil. // whether the mode change was successful or not. |handler| can be nil.
- (void)finalizeChangeToMode:(CRWBrowsingDataStoreMode)mode - (void)finalizeChangeToMode:(web::BrowsingDataStoreMode)mode
andCallCompletionHandler:(void (^)(BOOL modeChangeWasSuccessful))handler; andCallCompletionHandler:(void (^)(BOOL modeChangeWasSuccessful))handler;
// Changes the mode of the CRWBrowsingDataStore to |mode|. This is an // Changes the mode of the CRWBrowsingDataStore to |mode|. This is an
...@@ -75,7 +75,7 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH"; ...@@ -75,7 +75,7 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
// |completionHandler| is called on the main thread. This block has no return // |completionHandler| is called on the main thread. This block has no return
// value and takes a single BOOL argument that indicates whether or not the // value and takes a single BOOL argument that indicates whether or not the
// mode change was successfully changed to |mode|. // mode change was successfully changed to |mode|.
- (void)changeMode:(CRWBrowsingDataStoreMode)mode - (void)changeMode:(web::BrowsingDataStoreMode)mode
completionHandler:(void (^)(BOOL modeChangeWasSuccessful))completionHandler; completionHandler:(void (^)(BOOL modeChangeWasSuccessful))completionHandler;
// The number of stash or restore operations that are still pending. // The number of stash or restore operations that are still pending.
...@@ -107,7 +107,7 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH"; ...@@ -107,7 +107,7 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
// The delegate. // The delegate.
base::WeakNSProtocol<id<CRWBrowsingDataStoreDelegate>> _delegate; base::WeakNSProtocol<id<CRWBrowsingDataStoreDelegate>> _delegate;
// The mode of the CRWBrowsingDataStore. // The mode of the CRWBrowsingDataStore.
CRWBrowsingDataStoreMode _mode; web::BrowsingDataStoreMode _mode;
// The dictionary that maps a browsing data type to its // The dictionary that maps a browsing data type to its
// CRWBrowsingDataManager. // CRWBrowsingDataManager.
base::scoped_nsobject<NSDictionary> _browsingDataTypeMap; base::scoped_nsobject<NSDictionary> _browsingDataTypeMap;
...@@ -117,7 +117,7 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH"; ...@@ -117,7 +117,7 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
// The last dispatched stash or restore operation that was enqueued to be run. // The last dispatched stash or restore operation that was enqueued to be run.
base::scoped_nsobject<NSOperation> _lastDispatchedStashOrRestoreOperation; base::scoped_nsobject<NSOperation> _lastDispatchedStashOrRestoreOperation;
// The number of stash or restore operations that are still pending. If this // The number of stash or restore operations that are still pending. If this
// value > 0 the mode of the CRWBrowsingDataStore is SYNCHRONIZING. The mode // value > 0 the mode of the CRWBrowsingDataStore is |CHANGING|. The mode
// can be made ACTIVE or INACTIVE only be set when this value is 0. // can be made ACTIVE or INACTIVE only be set when this value is 0.
NSUInteger _numberOfPendingStashOrRestoreOperations; NSUInteger _numberOfPendingStashOrRestoreOperations;
} }
...@@ -163,7 +163,7 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH"; ...@@ -163,7 +163,7 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
web::ActiveStateManager* activeStateManager = web::ActiveStateManager* activeStateManager =
web::BrowserState::GetActiveStateManager(browserState); web::BrowserState::GetActiveStateManager(browserState);
DCHECK(activeStateManager); DCHECK(activeStateManager);
_mode = activeStateManager->IsActive() ? ACTIVE : INACTIVE; _mode = activeStateManager->IsActive() ? web::ACTIVE : web::INACTIVE;
// TODO(shreyasv): If the creation of CRWBrowsingDataManagers turns out to // TODO(shreyasv): If the creation of CRWBrowsingDataManagers turns out to
// be an expensive operations re-visit this with a lazy-evaluation approach. // be an expensive operations re-visit this with a lazy-evaluation approach.
base::scoped_nsobject<CRWCookieBrowsingDataManager> base::scoped_nsobject<CRWCookieBrowsingDataManager>
...@@ -232,10 +232,10 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH"; ...@@ -232,10 +232,10 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
} }
- (SEL)browsingDataManagerSelectorForRemoveOperationType { - (SEL)browsingDataManagerSelectorForRemoveOperationType {
if (self.mode == ACTIVE) { if (self.mode == web::ACTIVE) {
return @selector(removeDataAtCanonicalPath); return @selector(removeDataAtCanonicalPath);
} }
if (self.mode == INACTIVE) { if (self.mode == web::INACTIVE) {
return @selector(removeDataAtStashPath); return @selector(removeDataAtStashPath);
} }
DCHECK(_lastDispatchedStashOrRestoreOperation); DCHECK(_lastDispatchedStashOrRestoreOperation);
...@@ -264,17 +264,17 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH"; ...@@ -264,17 +264,17 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
return [super automaticallyNotifiesObserversForKey:(NSString*)key]; return [super automaticallyNotifiesObserversForKey:(NSString*)key];
} }
- (CRWBrowsingDataStoreMode)mode { - (web::BrowsingDataStoreMode)mode {
DCHECK([NSThread isMainThread]); DCHECK([NSThread isMainThread]);
return _mode; return _mode;
} }
- (void)setMode:(CRWBrowsingDataStoreMode)mode { - (void)setMode:(web::BrowsingDataStoreMode)mode {
DCHECK([NSThread isMainThread]); DCHECK([NSThread isMainThread]);
if (_mode == mode) { if (_mode == mode) {
return; return;
} }
if (mode == ACTIVE || mode == INACTIVE) { if (mode == web::ACTIVE || mode == web::INACTIVE) {
DCHECK(!self.numberOfPendingStashOrRestoreOperations); DCHECK(!self.numberOfPendingStashOrRestoreOperations);
} }
[self willChangeValueForKey:@"mode"]; [self willChangeValueForKey:@"mode"];
...@@ -282,10 +282,10 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH"; ...@@ -282,10 +282,10 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
[self didChangeValueForKey:@"mode"]; [self didChangeValueForKey:@"mode"];
} }
- (void)finalizeChangeToMode:(CRWBrowsingDataStoreMode)mode - (void)finalizeChangeToMode:(web::BrowsingDataStoreMode)mode
andCallCompletionHandler:(void (^)(BOOL modeChangeWasSuccessful))handler { andCallCompletionHandler:(void (^)(BOOL modeChangeWasSuccessful))handler {
DCHECK([NSThread isMainThread]); DCHECK([NSThread isMainThread]);
DCHECK_NE(SYNCHRONIZING, mode); DCHECK_NE(web::CHANGING, mode);
BOOL modeChangeWasSuccessful = NO; BOOL modeChangeWasSuccessful = NO;
if (!self.numberOfPendingStashOrRestoreOperations) { if (!self.numberOfPendingStashOrRestoreOperations) {
...@@ -313,17 +313,17 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH"; ...@@ -313,17 +313,17 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
(void (^)(BOOL success))completionHandler { (void (^)(BOOL success))completionHandler {
DCHECK([NSThread isMainThread]); DCHECK([NSThread isMainThread]);
[self changeMode:ACTIVE completionHandler:completionHandler]; [self changeMode:web::ACTIVE completionHandler:completionHandler];
} }
- (void)makeInactiveWithCompletionHandler: - (void)makeInactiveWithCompletionHandler:
(void (^)(BOOL success))completionHandler { (void (^)(BOOL success))completionHandler {
DCHECK([NSThread isMainThread]); DCHECK([NSThread isMainThread]);
[self changeMode:INACTIVE completionHandler:completionHandler]; [self changeMode:web::INACTIVE completionHandler:completionHandler];
} }
- (void)changeMode:(CRWBrowsingDataStoreMode)mode - (void)changeMode:(web::BrowsingDataStoreMode)mode
completionHandler: completionHandler:
(void (^)(BOOL modeChangeWasSuccessful))completionHandler { (void (^)(BOOL modeChangeWasSuccessful))completionHandler {
DCHECK([NSThread isMainThread]); DCHECK([NSThread isMainThread]);
...@@ -343,7 +343,7 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH"; ...@@ -343,7 +343,7 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
} }
OperationType operationType = NONE; OperationType operationType = NONE;
if (mode == ACTIVE) { if (mode == web::ACTIVE) {
// By default a |RESTORE| operation is performed when the mode is changed // By default a |RESTORE| operation is performed when the mode is changed
// to |ACTIVE|. // to |ACTIVE|.
operationType = RESTORE; operationType = RESTORE;
...@@ -398,7 +398,7 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH"; ...@@ -398,7 +398,7 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
DCHECK(selector); DCHECK(selector);
if (operationType == RESTORE || operationType == STASH) { if (operationType == RESTORE || operationType == STASH) {
[self setMode:SYNCHRONIZING]; [self setMode:web::CHANGING];
++self.numberOfPendingStashOrRestoreOperations; ++self.numberOfPendingStashOrRestoreOperations;
completionHandler = ^{ completionHandler = ^{
--self.numberOfPendingStashOrRestoreOperations; --self.numberOfPendingStashOrRestoreOperations;
......
...@@ -158,29 +158,29 @@ TEST_F(BrowsingDataStoreTest, MakeActiveAndInactiveOperations) { ...@@ -158,29 +158,29 @@ TEST_F(BrowsingDataStoreTest, MakeActiveAndInactiveOperations) {
id unsucessfullCallback = ^(BOOL success) { id unsucessfullCallback = ^(BOOL success) {
ASSERT_TRUE([NSThread isMainThread]); ASSERT_TRUE([NSThread isMainThread]);
CRWBrowsingDataStoreMode mode = [browsing_data_store_ mode]; BrowsingDataStoreMode mode = [browsing_data_store_ mode];
EXPECT_FALSE(success); EXPECT_FALSE(success);
EXPECT_EQ(SYNCHRONIZING, mode); EXPECT_EQ(CHANGING, mode);
}; };
[browsing_data_store_ makeActiveWithCompletionHandler:unsucessfullCallback]; [browsing_data_store_ makeActiveWithCompletionHandler:unsucessfullCallback];
EXPECT_EQ(SYNCHRONIZING, [browsing_data_store_ mode]); EXPECT_EQ(CHANGING, [browsing_data_store_ mode]);
EXPECT_EQ(1U, [observer modeChangeCount]); EXPECT_EQ(1U, [observer modeChangeCount]);
[browsing_data_store_ makeInactiveWithCompletionHandler:unsucessfullCallback]; [browsing_data_store_ makeInactiveWithCompletionHandler:unsucessfullCallback];
EXPECT_EQ(SYNCHRONIZING, [browsing_data_store_ mode]); EXPECT_EQ(CHANGING, [browsing_data_store_ mode]);
[browsing_data_store_ makeActiveWithCompletionHandler:unsucessfullCallback]; [browsing_data_store_ makeActiveWithCompletionHandler:unsucessfullCallback];
EXPECT_EQ(SYNCHRONIZING, [browsing_data_store_ mode]); EXPECT_EQ(CHANGING, [browsing_data_store_ mode]);
__block BOOL block_was_called = NO; __block BOOL block_was_called = NO;
[browsing_data_store_ makeInactiveWithCompletionHandler:^(BOOL success) { [browsing_data_store_ makeInactiveWithCompletionHandler:^(BOOL success) {
ASSERT_TRUE([NSThread isMainThread]); ASSERT_TRUE([NSThread isMainThread]);
CRWBrowsingDataStoreMode mode = [browsing_data_store_ mode]; BrowsingDataStoreMode mode = [browsing_data_store_ mode];
EXPECT_TRUE(success); EXPECT_TRUE(success);
EXPECT_EQ(INACTIVE, mode); EXPECT_EQ(INACTIVE, mode);
block_was_called = YES; block_was_called = YES;
}]; }];
EXPECT_EQ(SYNCHRONIZING, [browsing_data_store_ mode]); EXPECT_EQ(CHANGING, [browsing_data_store_ mode]);
base::test::ios::WaitUntilCondition(^bool{ base::test::ios::WaitUntilCondition(^bool{
return block_was_called; return block_was_called;
......
...@@ -22,21 +22,21 @@ typedef NS_OPTIONS(NSUInteger, BrowsingDataTypes) { ...@@ -22,21 +22,21 @@ typedef NS_OPTIONS(NSUInteger, BrowsingDataTypes) {
BROWSING_DATA_TYPE_ALL = BROWSING_DATA_TYPE_COOKIES, BROWSING_DATA_TYPE_ALL = BROWSING_DATA_TYPE_COOKIES,
}; };
} // namespace web // Represents the modes that a CRWBrowsingDataStore can be in.
typedef NS_ENUM(NSUInteger, BrowsingDataStoreMode) {
// Represents the modes that a CRWBrowsingDataStore is currently at.
enum CRWBrowsingDataStoreMode {
// Web views (associated transitively through the BrowseState) are // Web views (associated transitively through the BrowseState) are
// flushing/reading their data from disk. // flushing/reading their data from disk.
ACTIVE, ACTIVE = 1,
// The CRWBrowsingDataStore's mode is in the process of becoming either ACTIVE // The CRWBrowsingDataStore's mode is in the process of becoming either ACTIVE
// or INACTIVE. // or INACTIVE.
SYNCHRONIZING, CHANGING,
// Browsing data is stored in a path unique to the BrowserState and is // Browsing data is stored in a path unique to the BrowserState and is
// currently not being read or written to by web views. // currently not being read or written to by web views.
INACTIVE INACTIVE,
}; };
} // namespace web
// A CRWBrowsingDataStore represents various types of data that a web view // A CRWBrowsingDataStore represents various types of data that a web view
// (UIWebView and WKWebView) uses. // (UIWebView and WKWebView) uses.
// All methods must be called on the UI thread. // All methods must be called on the UI thread.
...@@ -52,7 +52,7 @@ enum CRWBrowsingDataStoreMode { ...@@ -52,7 +52,7 @@ enum CRWBrowsingDataStoreMode {
@property(nonatomic, weak) id<CRWBrowsingDataStoreDelegate> delegate; @property(nonatomic, weak) id<CRWBrowsingDataStoreDelegate> delegate;
// The mode that the CRWBrowsingDataStore is in. KVO compliant. // The mode that the CRWBrowsingDataStore is in. KVO compliant.
@property(nonatomic, assign, readonly) CRWBrowsingDataStoreMode mode; @property(nonatomic, assign, readonly) web::BrowsingDataStoreMode mode;
// TODO(shreyasv): Verify the preconditions for the following 3 methods when // TODO(shreyasv): Verify the preconditions for the following 3 methods when
// web::WebViewCounter class is implemented. crbug.com/480507 // web::WebViewCounter class is implemented. crbug.com/480507
......
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