CL for Objective-C readability

ios/web/crw_browsing_data_store.mm

Index: ios/web/crw_browsing_data_store.mm
diff --git a/ios/web/crw_browsing_data_store.mm b/ios/web/crw_browsing_data_store.mm
index 5441ea3a39de8f6a3c4ae86f2fbfc1631a6b18c3..e05eaeba966678fd59fcab4bdc0d86fc2e32bb81 100644
--- a/ios/web/crw_browsing_data_store.mm
+++ b/ios/web/crw_browsing_data_store.mm
@@ -23,53 +23,74 @@ enum OperationType {
   // the associated BrowserState's state path.
   STASH,
   // Restore operation involves restoring browsing data from the
-  // associated BrowserState's state path so that web views (UIWebViews and
-  // WKWebViews) can read from them.
+  // associated BrowserState's state path so that web views can read from them.
   RESTORE,
   // Remove operation involves removing the data that web views create.
   REMOVE,
 };
 
-// The name of the NSOperation that performs a restore operation.
+// The name of the NSOperation that performs a |RESTORE| operation.

[stuartmorgan, 2015/07/09 16:33:50]

FWIW, while the | are okay here, they are only required when it would be
ambiguous without them; given that these tokens are ALL CAPS, there's not really
any chance of confusion, so they would all be fine (and arguably a bit more
readable) without the bars.

[shreyasv1, 2015/07/09 17:18:01]

My concern is with consistency. I noticed that some comments have |STASH| while
some have stash.
I would like to be consistent throughout the file since |STASH|, |RESTORE|,
|REMOVE represent OperationTypes and have a special meaning.

Left as is -- for the sake of consistency.
Let me know if you don't agree with this and I will be happy to change it.

 NSString* const kRestoreOperationName = @"CRWBrowsingDataStore.RESTORE";
-// The name of the NSOperation that performs a stash operation.
+// The name of the NSOperation that performs a |STASH| operation.
 NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
 }  // namespace
 
+// CRWBrowsingDataStore is implemented using 2 queues.
+// 1) operationQueueForStashAndRestoreOperations:
+// - This queue is used to perform |STASH| and |RESTORE| operations.
+// - |STASH|, |RESTORE| operations are the core bits that need to be performed
+//   in order to change the mode of a CRWBrowsingDataStore and hence they are
+//   prioritized with a QoS of NSQualityOfServiceUserInteractive.
+// - |STASH|, |RESTORE| operations from 2 different CRWBrowsingDataStores are
+//    not re-entrant. Hence this is a serial queue.
+// 2) operationQueueForRemoveOperations:
+// - This queue is used to perform |REMOVE| operations.
+// - |REMOVE| operations are not as critical as |STASH|, |RESTORE|. Hence they
+//   the queue has a QoS of NSQualityOfServiceUserInitiated -- in order to not
+//   overwhelm the system.
+// - |REMOVE| operations from 2 different CRWBrowingDataStores can be run in
+//   parallel. Hence this is a concurrent queue.
+//
+// |STASH|, |RESTORE|, |REMOVE| operations of a particular CRWBrowsingDataStore
+// are not re-entrant. Hence these operations are serialized.
+
 @interface CRWBrowsingDataStore ()
+// Redefined to be read-write. Must be called from the main thread.
+@property(nonatomic, assign) web::BrowsingDataStoreMode mode;
+// The array of all browsing data managers. Must be called from the main
+// thread.
+@property(nonatomic, readonly) NSArray* allBrowsingDataManagers;
+// The number of stash or restore operations that are still pending. Must be
+// called from the main thread.
+@property(nonatomic, assign) NSUInteger numberOfPendingStashOrRestoreOperations;
+
 // Returns a serial queue on which stash and restore operations can be scheduled
-// to be run. All stash/restore operations need to be run on the same queue
+// to be run. All |STASH|/|RESTORE| operations need to be run on the same queue
 // hence it is shared with all CRWBrowsingDataStores.
 + (NSOperationQueue*)operationQueueForStashAndRestoreOperations;
 // Returns a concurrent queue on which remove operations can be scheduled to be
-// run. All remove operations need to be run on the same queue hence it is
+// run. All |REMOVE| operations need to be run on the same queue hence it is
 // shared with all CRWBrowsingDataStores.
 + (NSOperationQueue*)operationQueueForRemoveOperations;
-- (instancetype)init NS_UNAVAILABLE;
 
-// The array of all browsing data managers. Must be accessed from the main
-// thread.
-@property(nonatomic, readonly) NSArray* allBrowsingDataManagers;
-// Returns an array of browsing data managers for the given |browsingDataTypes|.
+// Returns an array of CRWBrowsingDataManagers for the given
+// |browsingDataTypes|.
 - (NSArray*)browsingDataManagersForBrowsingDataTypes:
-        (web::BrowsingDataTypes)browsingDataTypes;
+    (web::BrowsingDataTypes)browsingDataTypes;
 // Returns the selector that needs to be performed on the
 // CRWBrowsingDataManagers for the |operationType|. |operationType| cannot be
 // |NONE|.
 - (SEL)browsingDataManagerSelectorForOperationType:(OperationType)operationType;
 // Returns the selector that needs to be performed on the
-// CRWBrowsingDataManagers for a REMOVE operation.
+// CRWBrowsingDataManagers for a |REMOVE| operation.
 - (SEL)browsingDataManagerSelectorForRemoveOperationType;
 
-// Redefined to be read-write. Must be called from the main thread.
-@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|.
 // |handler| is called immediately (in the same runloop) with a BOOL indicating
 // whether the mode change was successful or not. |handler| can be nil.
 - (void)finalizeChangeToMode:(web::BrowsingDataStoreMode)mode
     andCallCompletionHandler:(void (^)(BOOL modeChangeWasSuccessful))handler;
-
 // Changes the mode of the CRWBrowsingDataStore to |mode|. This is an
 // asynchronous operation and the mode is not changed immediately.
 // |completionHandler| can be nil.
@@ -79,23 +100,22 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
 - (void)changeMode:(web::BrowsingDataStoreMode)mode
     completionHandler:(void (^)(BOOL modeChangeWasSuccessful))completionHandler;
 
-// The number of stash or restore operations that are still pending.
-@property(nonatomic, assign) NSUInteger numberOfPendingStashOrRestoreOperations;
-
-// Performs operations of type |operationType| on each of the
+// Returns the OperationType (that needs to be performed) in order to change the
+// mode to |mode|. Consults the delegate if one is present. |mode| cannot be
+// |CHANGING|.
+- (OperationType)operationTypeToChangeMode:(web::BrowsingDataStoreMode)mode;
+// Performs an operation of type |operationType| on each of the
 // |browsingDataManagers|. |operationType| cannot be |NONE|.
 // Precondition: There must be no web views associated with the BrowserState.
 // |completionHandler| is called on the main thread and cannot be nil.
 - (void)performOperationWithType:(OperationType)operationType
             browsingDataManagers:(NSArray*)browsingDataManagers
                completionHandler:(ProceduralBlock)completionHandler;
-
-// Creates an NSOperation that calls |selector| on all the
+// Returns an NSOperation that calls |selector| on all the
 // |browsingDataManagers|. |selector| needs to be one of the methods in
 // CRWBrowsingDataManager.
-- (NSOperation*)
-    newOperationWithBrowsingDataManagers:(NSArray*)browsingDataManagers
-                                selector:(SEL)selector;
+- (NSOperation*)operationWithBrowsingDataManagers:(NSArray*)browsingDataManagers
+                                         selector:(SEL)selector;
 // Enqueues |operation| to be run on |queue|. All operations are serialized to
 // be run one after another.
 - (void)addOperation:(NSOperation*)operation toQueue:(NSOperationQueue*)queue;
@@ -110,52 +130,27 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
   // The dictionary that maps a browsing data type to its
   // CRWBrowsingDataManager.
   base::scoped_nsobject<NSDictionary> _browsingDataTypeMap;
-  // The last operation that was enqueued to be run. Can be stash, restore or a
-  // delete operation.
+  // The last operation that was enqueued to be run. Can be a |STASH|, |RESTORE|
+  // or a |REMOVE| operation.
   base::scoped_nsobject<NSOperation> _lastDispatchedOperation;
-  // 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;
-  // The number of stash or restore operations that are still pending. If this
-  // 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.
+  // The number of |STASH| or |RESTORE| operations that are still pending. If
+  // the number of stash or restore operations > 0U, the mode of the
+  // CRWBrowsingDataStore is |CHANGING|. The mode can be made ACTIVE or INACTIVE
+  // only be set when this value is 0U.
   NSUInteger _numberOfPendingStashOrRestoreOperations;
 }
 
-+ (NSOperationQueue*)operationQueueForStashAndRestoreOperations {
-  static dispatch_once_t onceToken = 0;
-  static NSOperationQueue* operationQueueForStashAndRestoreOperations = nil;
-  dispatch_once(&onceToken, ^{
-    operationQueueForStashAndRestoreOperations =
-        [[NSOperationQueue alloc] init];
-    [operationQueueForStashAndRestoreOperations
-        setMaxConcurrentOperationCount:1U];
-    if (base::ios::IsRunningOnIOS8OrLater()) {
-      [operationQueueForStashAndRestoreOperations
-          setQualityOfService:NSQualityOfServiceUserInteractive];
-    }
-  });
-  return operationQueueForStashAndRestoreOperations;
-}
-
-+ (NSOperationQueue*)operationQueueForRemoveOperations {
-  static dispatch_once_t onceToken = 0;
-  static NSOperationQueue* operationQueueForRemoveOperations = nil;
-  dispatch_once(&onceToken, ^{
-    operationQueueForRemoveOperations = [[NSOperationQueue alloc] init];
-    [operationQueueForRemoveOperations
-        setMaxConcurrentOperationCount:NSUIntegerMax];
-    if (base::ios::IsRunningOnIOS8OrLater()) {
-      [operationQueueForRemoveOperations
-          setQualityOfService:NSQualityOfServiceUserInitiated];
-    }
-  });
-  return operationQueueForRemoveOperations;
-}
+#pragma mark -
+#pragma mark NSObject Methods
 
 - (instancetype)initWithBrowserState:(web::BrowserState*)browserState {
   self = [super init];
   if (self) {
     DCHECK([NSThread isMainThread]);
+    DCHECK(browserState);
     // TODO(shreyasv): Instantiate the necessary CRWBrowsingDataManagers that
     // are encapsulated within this class. crbug.com/480654.
     _browserState = browserState;
@@ -181,23 +176,169 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
 }
 
 - (NSString*)description {
-  NSString* format = @"<%@: %p; hasPendingOperations = { %@ }>";
+  NSString* format = @"<%@: %p; hasPendingOperations = { %@ }; mode = { %@ }>";
   NSString* hasPendingOperationsString =
       [self hasPendingOperations] ? @"YES" : @"NO";
-  NSString* result =
-      [NSString stringWithFormat:format, NSStringFromClass(self.class), self,
-                                 hasPendingOperationsString];
+  NSString* modeString = nil;
+  switch (self.mode) {
+    case web::ACTIVE:
+      modeString = @"ACTIVE";
+      break;
+    case web::CHANGING:
+      modeString = @"CHANGING";
+      break;
+    case web::INACTIVE:
+      modeString = @"INACTIVE";
+      break;
+  }
+  NSString* result = [NSString stringWithFormat:format,
+      NSStringFromClass(self.class), self, hasPendingOperationsString,
+          modeString];

[stuartmorgan, 2015/07/09 16:33:50]

This shouldn't be indented any more than the line above it.

   return result;
 }
 
++ (BOOL)automaticallyNotifiesObserversForKey:(NSString*)key {
+  // It is necessary to override this for |mode| because the default KVO
+  // behavior in NSObject is to fire a notification irrespective of if an actual
+  // change was made to the ivar or not. The |mode| property needs fine grained
+  // control over the actual notifications being fired since observers need to
+  // be notified iff the |mode| actually changed.
+  if ([key isEqual:@"mode"]) {
+    return NO;
+  }
+  return [super automaticallyNotifiesObserversForKey:(NSString*)key];

[stuartmorgan, 2015/07/09 16:33:50]

Remove (NSString*)

[shreyasv1, 2015/07/09 17:18:01]

Done.

+}
+
+#pragma mark -
+#pragma mark Public Properties
+
+- (id<CRWBrowsingDataStoreDelegate>)delegate {
+  return _delegate;
+}
+
+- (void)setDelegate:(id<CRWBrowsingDataStoreDelegate>)delegate {
+  _delegate.reset(delegate);
+}
+
+- (web::BrowsingDataStoreMode)mode {
+  DCHECK([NSThread isMainThread]);
+  return _mode;
+}
+
+- (BOOL)hasPendingOperations {
+  if (!_lastDispatchedOperation) {
+    return NO;
+  }
+  return ![_lastDispatchedOperation isFinished];
+}
+
+#pragma mark -
+#pragma mark Public Methods
+
+- (void)makeActiveWithCompletionHandler:
+    (void (^)(BOOL success))completionHandler {
+  DCHECK([NSThread isMainThread]);
+  // TODO(shreyasv): Verify the preconditions for this method when
+  // web::WebViewCounter class is implemented. crbug.com/480507.
+
+  [self changeMode:web::ACTIVE completionHandler:completionHandler];
+}
+
+- (void)makeInactiveWithCompletionHandler:
+    (void (^)(BOOL success))completionHandler {
+  DCHECK([NSThread isMainThread]);
+  // TODO(shreyasv): Verify the preconditions for this method when
+  // web::WebViewCounter class is implemented. crbug.com/480507.
+
+  [self changeMode:web::INACTIVE completionHandler:completionHandler];
+}
+
+- (void)removeDataOfTypes:(web::BrowsingDataTypes)browsingDataTypes
+        completionHandler:(ProceduralBlock)completionHandler {
+  DCHECK([NSThread isMainThread]);
+  // TODO(shreyasv): Verify the preconditions for this method when
+  // web::WebViewCounter class is implemented. crbug.com/480507.
+
+  NSArray* browsingDataManagers =
+      [self browsingDataManagersForBrowsingDataTypes:browsingDataTypes];
+  [self performOperationWithType:REMOVE
+            browsingDataManagers:browsingDataManagers
+               completionHandler:completionHandler];
+}
+
+#pragma mark -
+#pragma mark Private Properties
+
+- (void)setMode:(web::BrowsingDataStoreMode)mode {
+  DCHECK([NSThread isMainThread]);
+
+  if (_mode == mode) {
+    return;
+  }
+  if (mode == web::ACTIVE || mode == web::INACTIVE) {
+    DCHECK(!self.numberOfPendingStashOrRestoreOperations);
+  }
+  [self willChangeValueForKey:@"mode"];
+  _mode = mode;
+  [self didChangeValueForKey:@"mode"];
+}
 
 - (NSArray*)allBrowsingDataManagers {
   DCHECK([NSThread isMainThread]);
   return [_browsingDataTypeMap allValues];
 }
 
+- (NSUInteger)numberOfPendingStashOrRestoreOperations {
+  DCHECK([NSThread isMainThread]);
+  return _numberOfPendingStashOrRestoreOperations;
+}
+
+- (void)setNumberOfPendingStashOrRestoreOperations:
+    (NSUInteger)numberOfPendingStashOrRestoreOperations {
+  DCHECK([NSThread isMainThread]);
+  _numberOfPendingStashOrRestoreOperations =
+      numberOfPendingStashOrRestoreOperations;
+}
+
+#pragma mark -
+#pragma mark Private Class Methods
+
++ (NSOperationQueue*)operationQueueForStashAndRestoreOperations {
+  static dispatch_once_t onceToken = 0;
+  static NSOperationQueue* operationQueueForStashAndRestoreOperations = nil;
+  dispatch_once(&onceToken, ^{
+    operationQueueForStashAndRestoreOperations =
+        [[NSOperationQueue alloc] init];
+    [operationQueueForStashAndRestoreOperations
+        setMaxConcurrentOperationCount:1U];
+    if (base::ios::IsRunningOnIOS8OrLater()) {
+      [operationQueueForStashAndRestoreOperations
+          setQualityOfService:NSQualityOfServiceUserInteractive];
+    }
+  });
+  return operationQueueForStashAndRestoreOperations;
+}
+
++ (NSOperationQueue*)operationQueueForRemoveOperations {
+  static dispatch_once_t onceToken = 0;
+  static NSOperationQueue* operationQueueForRemoveOperations = nil;
+  dispatch_once(&onceToken, ^{
+    operationQueueForRemoveOperations = [[NSOperationQueue alloc] init];
+    [operationQueueForRemoveOperations
+        setMaxConcurrentOperationCount:NSUIntegerMax];
+    if (base::ios::IsRunningOnIOS8OrLater()) {
+      [operationQueueForRemoveOperations
+          setQualityOfService:NSQualityOfServiceUserInitiated];
+    }
+  });
+  return operationQueueForRemoveOperations;
+}
+
+#pragma mark -
+#pragma mark Private Methods
+
 - (NSArray*)browsingDataManagersForBrowsingDataTypes:
-        (web::BrowsingDataTypes)browsingDataTypes {
+    (web::BrowsingDataTypes)browsingDataTypes {
   __block NSMutableArray* result = [NSMutableArray array];
   [_browsingDataTypeMap
       enumerateKeysAndObjectsUsingBlock:^(NSNumber* dataType,
@@ -210,16 +351,8 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
   return result;
 }
 
-- (id<CRWBrowsingDataStoreDelegate>)delegate {
-  return _delegate;
-}
-
-- (void)setDelegate:(id<CRWBrowsingDataStoreDelegate>)delegate {
-  _delegate.reset(delegate);
-}
-
 - (SEL)browsingDataManagerSelectorForOperationType:
-        (OperationType)operationType {
+    (OperationType)operationType {
   switch (operationType) {
     case NONE:
       NOTREACHED();
@@ -231,8 +364,6 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
     case REMOVE:
       return [self browsingDataManagerSelectorForRemoveOperationType];
   };
-  NOTREACHED();
-  return nullptr;
 }
 
 - (SEL)browsingDataManagerSelectorForRemoveOperationType {
@@ -242,6 +373,9 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
   if (self.mode == web::INACTIVE) {
     return @selector(removeDataAtStashPath);
   }
+  // Since the mode is |CHANGING|, find the last |STASH| or |RESTORE| operation
+  // that was enqueued in order to find out the eventual mode that the
+  // CRWBrowsingDataStore will be in when this |REMOVE| operation is run.
   DCHECK(_lastDispatchedStashOrRestoreOperation);
   NSString* lastDispatchedStashOrRestoreOperationName =
       [_lastDispatchedStashOrRestoreOperation name];
@@ -256,36 +390,6 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
   return nullptr;
 }
 
-+ (BOOL)automaticallyNotifiesObserversForKey:(NSString*)key {
-  // It is necessary to override this for |mode| because the default KVO
-  // behavior in NSObject is to fire a notification irrespective of if an actual
-  // change was made to the ivar or not. The |mode| property needs fine grained
-  // control over the actual notifications being fired since observers need to
-  // be notified iff the |mode| actually changed.
-  if ([key isEqual:@"mode"]) {
-    return NO;
-  }
-  return [super automaticallyNotifiesObserversForKey:(NSString*)key];
-}
-
-- (web::BrowsingDataStoreMode)mode {
-  DCHECK([NSThread isMainThread]);
-  return _mode;
-}
-
-- (void)setMode:(web::BrowsingDataStoreMode)mode {
-  DCHECK([NSThread isMainThread]);
-  if (_mode == mode) {
-    return;
-  }
-  if (mode == web::ACTIVE || mode == web::INACTIVE) {
-    DCHECK(!self.numberOfPendingStashOrRestoreOperations);
-  }
-  [self willChangeValueForKey:@"mode"];
-  _mode = mode;
-  [self didChangeValueForKey:@"mode"];
-}
-
 - (void)finalizeChangeToMode:(web::BrowsingDataStoreMode)mode
     andCallCompletionHandler:(void (^)(BOOL modeChangeWasSuccessful))handler {
   DCHECK([NSThread isMainThread]);
@@ -301,32 +405,6 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
   }
 }
 
-- (NSUInteger)numberOfPendingStashOrRestoreOperations {
-  DCHECK([NSThread isMainThread]);
-  return _numberOfPendingStashOrRestoreOperations;
-}
-
-- (void)setNumberOfPendingStashOrRestoreOperations:
-        (NSUInteger)numberOfPendingStashOrRestoreOperations {
-  DCHECK([NSThread isMainThread]);
-  _numberOfPendingStashOrRestoreOperations =
-      numberOfPendingStashOrRestoreOperations;
-}
-
-- (void)makeActiveWithCompletionHandler:
-        (void (^)(BOOL success))completionHandler {
-  DCHECK([NSThread isMainThread]);
-
-  [self changeMode:web::ACTIVE completionHandler:completionHandler];
-}
-
-- (void)makeInactiveWithCompletionHandler:
-        (void (^)(BOOL success))completionHandler {
-  DCHECK([NSThread isMainThread]);
-
-  [self changeMode:web::INACTIVE completionHandler:completionHandler];
-}
-
 - (void)changeMode:(web::BrowsingDataStoreMode)mode
     completionHandler:
         (void (^)(BOOL modeChangeWasSuccessful))completionHandler {
@@ -336,18 +414,27 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
     [self finalizeChangeToMode:mode andCallCompletionHandler:completionHandler];
   };
 
-  // Already in the desired mode.
-  if (self.mode == mode) {
+  OperationType operationType = [self operationTypeToChangeMode:mode];
+  if (operationType == NONE) {
     // As a caller of this API, it is awkward to get the callback before the
     // method call has completed, hence defer it.
-    dispatch_async(dispatch_get_main_queue(), ^{
-      completionHandlerAfterPerformingOperation();
-    });
-    return;
+    dispatch_async(dispatch_get_main_queue(),
+                   completionHandlerAfterPerformingOperation);
+  } else {
+    [self performOperationWithType:operationType
+              browsingDataManagers:[self allBrowsingDataManagers]
+                 completionHandler:completionHandlerAfterPerformingOperation];
   }
+}
+
+- (OperationType)operationTypeToChangeMode:(web::BrowsingDataStoreMode)mode {
+  DCHECK_NE(web::CHANGING, mode);
 
   OperationType operationType = NONE;
-  if (mode == web::ACTIVE) {
+  if (mode == self.mode) {
+    // Already in the desired mode.
+    operationType = NONE;
+  } else if (mode == web::ACTIVE) {
     // By default a |RESTORE| operation is performed when the mode is changed
     // to |ACTIVE|.
     operationType = RESTORE;
@@ -362,32 +449,7 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
         [_delegate decideMakeInactiveOperationPolicyForBrowsingDataStore:self];
     operationType = (makeInactivePolicy == web::DELETE) ? REMOVE : STASH;
   }
-  DCHECK_NE(NONE, operationType);
-  [self performOperationWithType:operationType
-            browsingDataManagers:[self allBrowsingDataManagers]
-               completionHandler:completionHandlerAfterPerformingOperation];
-}
-
-- (void)removeDataOfTypes:(web::BrowsingDataTypes)browsingDataTypes
-        completionHandler:(ProceduralBlock)completionHandler {
-  DCHECK([NSThread isMainThread]);
-
-  NSArray* browsingDataManagers =
-      [self browsingDataManagersForBrowsingDataTypes:browsingDataTypes];
-  [self performOperationWithType:REMOVE
-            browsingDataManagers:browsingDataManagers
-               completionHandler:^{
-                 // Since this may be called on a background thread, bounce to
-                 // the main thread.
-                 dispatch_async(dispatch_get_main_queue(), completionHandler);
-               }];
-}
-
-- (BOOL)hasPendingOperations {
-  if (!_lastDispatchedOperation) {
-    return NO;
-  }
-  return ![_lastDispatchedOperation isFinished];
+  return operationType;
 }
 
 - (void)performOperationWithType:(OperationType)operationType
@@ -415,13 +477,11 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
   ProceduralBlock callCompletionHandlerOnMainThread = ^{
     // This is called on a background thread, hence the need to bounce to the
     // main thread.
-    dispatch_async(dispatch_get_main_queue(), ^{
-      completionHandler();
-    });
+    dispatch_async(dispatch_get_main_queue(), completionHandler);
   };
-  base::scoped_nsobject<NSOperation> operation(
-      [self newOperationWithBrowsingDataManagers:browsingDataManagers
-                                        selector:selector]);
+  NSOperation* operation =
+      [self operationWithBrowsingDataManagers:browsingDataManagers
+                                     selector:selector];
 
   if (operationType == RESTORE || operationType == STASH) {
     [operation setName:(RESTORE ? kRestoreOperationName : kStashOperationName)];
@@ -430,6 +490,9 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
 
   NSOperationQueue* queue = nil;
   switch (operationType) {
+    case NONE:
+      NOTREACHED();
+      break;
     case STASH:
     case RESTORE:
       queue = [CRWBrowsingDataStore operationQueueForStashAndRestoreOperations];
@@ -437,9 +500,6 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
     case REMOVE:
       queue = [CRWBrowsingDataStore operationQueueForRemoveOperations];
       break;
-    default:
-      NOTREACHED();
-      break;
   }
 
   NSOperation* completionHandlerOperation = [NSBlockOperation
@@ -449,10 +509,9 @@ NSString* const kStashOperationName = @"CRWBrowsingDataStore.STASH";
   [self addOperation:completionHandlerOperation toQueue:queue];
 }
 
-- (NSOperation*)
-    newOperationWithBrowsingDataManagers:(NSArray*)browsingDataManagers
-                                selector:(SEL)selector {
-  NSBlockOperation* operation = [[NSBlockOperation alloc] init];
+- (NSOperation*)operationWithBrowsingDataManagers:(NSArray*)browsingDataManagers
+                                         selector:(SEL)selector {
+  NSBlockOperation* operation = [[[NSBlockOperation alloc] init] autorelease];
   for (id<CRWBrowsingDataManager> manager : browsingDataManagers) {
     // |addExecutionBlock| farms out the different blocks added to it. hence the
     // operations are implicitly parallelized.