Commit 450b0990 authored by Chris Lu's avatar Chris Lu Committed by Commit Bot

[ios] Fix Unwinding of History and ClearBrowsingData when Clear Browsing Data dialog is open

- stops all action sheet coordinators in ClearBrowsingDataTableViewController before dismissing itself.
- sets historyClearBrowsingDataCoordinator to nil after finishing unwinding ClearBrowsingData and History to prevent race condition.

Video: https://drive.google.com/open?id=1wDymlQtfStNWBY9mI9GYqlig8qkipBDF

Bug: 867329
Change-Id: I7c0825b02ed7d3c6122f0a5ae37c99181fc6d182
Reviewed-on: https://chromium-review.googlesource.com/1159683
Commit-Queue: Chris Lu <thegreenfrog@chromium.org>
Reviewed-by: default avatarSergio Collazos <sczs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580348}
parent a47efe0e
...@@ -24,13 +24,17 @@ ...@@ -24,13 +24,17 @@
@interface HistoryClearBrowsingDataCoordinator ()< @interface HistoryClearBrowsingDataCoordinator ()<
UIViewControllerTransitioningDelegate> UIViewControllerTransitioningDelegate>
// ViewController being managed by this Coordinator. // ViewControllers being managed by this Coordinator.
@property(strong, nonatomic) @property(strong, nonatomic)
TableViewNavigationController* historyClearBrowsingDataNavigationController; TableViewNavigationController* historyClearBrowsingDataNavigationController;
@property(strong, nonatomic)
ClearBrowsingDataTableViewController* clearBrowsingDataTableViewController;
@end @end
@implementation HistoryClearBrowsingDataCoordinator @implementation HistoryClearBrowsingDataCoordinator
@synthesize clearBrowsingDataTableViewController =
_clearBrowsingDataTableViewController;
@synthesize dispatcher = _dispatcher; @synthesize dispatcher = _dispatcher;
@synthesize historyClearBrowsingDataNavigationController = @synthesize historyClearBrowsingDataNavigationController =
_historyClearBrowsingDataNavigationController; _historyClearBrowsingDataNavigationController;
...@@ -39,16 +43,17 @@ ...@@ -39,16 +43,17 @@
@synthesize presentationDelegate = _presentationDelegate; @synthesize presentationDelegate = _presentationDelegate;
- (void)start { - (void)start {
ClearBrowsingDataTableViewController* clearBrowsingDataTableViewController = self.clearBrowsingDataTableViewController =
[[ClearBrowsingDataTableViewController alloc] [[ClearBrowsingDataTableViewController alloc]
initWithBrowserState:self.browserState]; initWithBrowserState:self.browserState];
clearBrowsingDataTableViewController.extendedLayoutIncludesOpaqueBars = YES; self.clearBrowsingDataTableViewController.extendedLayoutIncludesOpaqueBars =
clearBrowsingDataTableViewController.localDispatcher = self; YES;
clearBrowsingDataTableViewController.dispatcher = self.dispatcher; self.clearBrowsingDataTableViewController.localDispatcher = self;
self.clearBrowsingDataTableViewController.dispatcher = self.dispatcher;
// Configure and present ClearBrowsingDataNavigationController. // Configure and present ClearBrowsingDataNavigationController.
self.historyClearBrowsingDataNavigationController = self.historyClearBrowsingDataNavigationController =
[[TableViewNavigationController alloc] [[TableViewNavigationController alloc]
initWithTable:clearBrowsingDataTableViewController]; initWithTable:self.clearBrowsingDataTableViewController];
self.historyClearBrowsingDataNavigationController.toolbarHidden = YES; self.historyClearBrowsingDataNavigationController.toolbarHidden = YES;
// Stacks on top of history "bubble" for non-compact devices. // Stacks on top of history "bubble" for non-compact devices.
self.historyClearBrowsingDataNavigationController.transitioningDelegate = self.historyClearBrowsingDataNavigationController.transitioningDelegate =
...@@ -65,10 +70,7 @@ ...@@ -65,10 +70,7 @@
- (void)stopWithCompletion:(ProceduralBlock)completionHandler { - (void)stopWithCompletion:(ProceduralBlock)completionHandler {
if (self.historyClearBrowsingDataNavigationController) { if (self.historyClearBrowsingDataNavigationController) {
[self.historyClearBrowsingDataNavigationController [self dismissClearBrowsingDataWithCompletion:completionHandler];
dismissViewControllerAnimated:YES
completion:completionHandler];
self.historyClearBrowsingDataNavigationController = nil;
} else if (completionHandler) { } else if (completionHandler) {
completionHandler(); completionHandler();
} }
...@@ -93,10 +95,18 @@ ...@@ -93,10 +95,18 @@
- (void)dismissClearBrowsingDataWithCompletion: - (void)dismissClearBrowsingDataWithCompletion:
(ProceduralBlock)completionHandler { (ProceduralBlock)completionHandler {
DCHECK(self.historyClearBrowsingDataNavigationController);
[self.clearBrowsingDataTableViewController prepareForDismissal];
[self.historyClearBrowsingDataNavigationController [self.historyClearBrowsingDataNavigationController
dismissViewControllerAnimated:YES dismissViewControllerAnimated:YES
completion:completionHandler]; completion:^() {
self.historyClearBrowsingDataNavigationController = nil; if (completionHandler) {
completionHandler();
}
self.clearBrowsingDataTableViewController = nil;
self.historyClearBrowsingDataNavigationController =
nil;
}];
} }
#pragma mark - UIViewControllerTransitioningDelegate #pragma mark - UIViewControllerTransitioningDelegate
......
...@@ -110,9 +110,11 @@ ...@@ -110,9 +110,11 @@
self.historyNavigationController = nil; self.historyNavigationController = nil;
}; };
if (self.historyClearBrowsingDataCoordinator) { if (self.historyClearBrowsingDataCoordinator) {
[self.historyClearBrowsingDataCoordinator [self.historyClearBrowsingDataCoordinator stopWithCompletion:^() {
stopWithCompletion:dismissHistoryNavigation]; dismissHistoryNavigation();
self.historyClearBrowsingDataCoordinator = nil; self.historyClearBrowsingDataCoordinator = nil;
}];
} else { } else {
dismissHistoryNavigation(); dismissHistoryNavigation();
} }
......
...@@ -26,6 +26,10 @@ class ChromeBrowserState; ...@@ -26,6 +26,10 @@ class ChromeBrowserState;
(ChromeTableViewControllerStyle)appBarStyle (ChromeTableViewControllerStyle)appBarStyle
NS_UNAVAILABLE; NS_UNAVAILABLE;
// Prepares view controller so that -dismissViewControllerAnimated dismisses it.
// Call this method before dismissing view controller.
- (void)prepareForDismissal;
// Local Dispatcher for this ClearBrowsingDataTableView. // Local Dispatcher for this ClearBrowsingDataTableView.
@property(nonatomic, weak) id<ClearBrowsingDataLocalCommands> localDispatcher; @property(nonatomic, weak) id<ClearBrowsingDataLocalCommands> localDispatcher;
......
...@@ -115,10 +115,23 @@ class ChromeBrowserState; ...@@ -115,10 +115,23 @@ class ChromeBrowserState;
} }
- (void)dismiss { - (void)dismiss {
[self.alertCoordinator stop]; [self prepareForDismissal];
[self.localDispatcher dismissClearBrowsingDataWithCompletion:nil]; [self.localDispatcher dismissClearBrowsingDataWithCompletion:nil];
} }
#pragma mark - Public Methods
- (void)prepareForDismissal {
if (self.actionSheetCoordinator) {
[self.actionSheetCoordinator stop];
self.actionSheetCoordinator = nil;
}
if (self.alertCoordinator) {
[self.alertCoordinator stop];
self.alertCoordinator = nil;
}
}
#pragma mark - UITableViewDataSource #pragma mark - UITableViewDataSource
- (UITableViewCell*)tableView:(UITableView*)tableView - (UITableViewCell*)tableView:(UITableView*)tableView
......
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