Commit 159d0ea6 authored by edchin's avatar edchin Committed by Commit Bot

[ios] Properly dismiss history in tab grid

History coordinator can now be started on top of the tab grid.
This history coordinator was not properly being dismissed
inside main_controller's |-dismissModalDialogsWithCompletion:|.

As part of fixing dismissal, this CL also fixes a bookkeeping flaw
in main_controller. Previously, main_controller conflated the
existence of |self.currentBVC| with the BVC being presented and active.
This CL changes that logic to use |-isTabSwitcherActive| instead.

Bug : 852243

Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I93d5efb33a2de8eaed7b0472fa2994c14603d11a
Reviewed-on: https://chromium-review.googlesource.com/1098555
Commit-Queue: edchin <edchin@chromium.org>
Reviewed-by: default avatarRohit Rao <rohitrao@chromium.org>
Reviewed-by: default avatarMark Cogan <marq@chromium.org>
Reviewed-by: default avataredchin <edchin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567172}
parent c281785e
...@@ -108,6 +108,12 @@ typedef Tab* (^mock_gurl_nsuinteger_pagetransition)(const GURL&, ...@@ -108,6 +108,12 @@ typedef Tab* (^mock_gurl_nsuinteger_pagetransition)(const GURL&,
return nil; return nil;
} }
- (void)clearPresentedStateWithCompletion:(ProceduralBlock)completion
dismissOmnibox:(BOOL)dismissOmnibox {
if (completion)
completion();
}
- (void)expectNewForegroundTab { - (void)expectNewForegroundTab {
// no-op. // no-op.
} }
...@@ -242,6 +248,8 @@ TEST_F(URLOpenerTest, HandleOpenURLWithOpenTabs) { ...@@ -242,6 +248,8 @@ TEST_F(URLOpenerTest, HandleOpenURLWithOpenTabs) {
static_cast<BrowserViewController*>(bvc_mock); static_cast<BrowserViewController*>(bvc_mock);
controller.browserViewInformation.otrBVC = controller.browserViewInformation.otrBVC =
static_cast<BrowserViewController*>(otr_bvc_mock); static_cast<BrowserViewController*>(otr_bvc_mock);
controller.browserViewInformation.currentBVC =
static_cast<BrowserViewController*>(bvc_mock);
NSDictionary<NSString*, id>* options = nil; NSDictionary<NSString*, id>* options = nil;
[URLOpener openURL:url [URLOpener openURL:url
......
...@@ -2345,21 +2345,34 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData( ...@@ -2345,21 +2345,34 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
// Then, depending on what the SSO view controller is presented on, dismiss // Then, depending on what the SSO view controller is presented on, dismiss
// it. // it.
ProceduralBlock completionWithBVC = ^{ ProceduralBlock completionWithBVC = ^{
DCHECK(self.currentBVC);
DCHECK(![self isTabSwitcherActive]);
// This will dismiss the SSO view controller. // This will dismiss the SSO view controller.
[self.currentBVC clearPresentedStateWithCompletion:completion [self.currentBVC clearPresentedStateWithCompletion:completion
dismissOmnibox:dismissOmnibox]; dismissOmnibox:dismissOmnibox];
}; };
ProceduralBlock completionWithoutBVC = ^{ ProceduralBlock completionWithoutBVC = ^{
// |self.currentBVC| may exist but tab switcher should be active.
DCHECK([self isTabSwitcherActive]);
// This will dismiss the SSO view controller. // This will dismiss the SSO view controller.
[self dismissSigninInteractionCoordinator]; [self dismissSigninInteractionCoordinator];
if (completion) if (GetTabSwitcherMode() == TabSwitcherMode::GRID) {
// History coordinator can be started on top of the tab grid. This is not
// true of the other tab switchers.
DCHECK(self.mainCoordinator);
TabGridCoordinator* tabGridCoordinator =
base::mac::ObjCCastStrict<TabGridCoordinator>(self.mainCoordinator);
[tabGridCoordinator stopChildCoordinatorsWithCompletion:completion];
} else if (completion) {
// Run completion separately if not in tab grid.
completion(); completion();
}
}; };
// As a top level rule, if the settings are showing, they need to be // As a top level rule, if the settings are showing, they need to be
// dismissed. Then, based on whether the BVC is present or not, a different // dismissed. Then, based on whether the BVC is present or not, a different
// completion callback is called. // completion callback is called.
if (self.currentBVC && _settingsNavigationController) { if (![self isTabSwitcherActive] && _settingsNavigationController) {
// In this case, the settings are up and the BVC is showing. Close the // In this case, the settings are up and the BVC is showing. Close the
// settings then call the BVC completion. // settings then call the BVC completion.
[self closeSettingsAnimated:NO completion:completionWithBVC]; [self closeSettingsAnimated:NO completion:completionWithBVC];
...@@ -2367,7 +2380,7 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData( ...@@ -2367,7 +2380,7 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
// In this case, the settings are up but the BVC is not showing. Close the // In this case, the settings are up but the BVC is not showing. Close the
// settings then call the no-BVC completion. // settings then call the no-BVC completion.
[self closeSettingsAnimated:NO completion:completionWithoutBVC]; [self closeSettingsAnimated:NO completion:completionWithoutBVC];
} else if (self.currentBVC) { } else if (![self isTabSwitcherActive]) {
// In this case, the settings are not shown but the BVC is showing. Call the // In this case, the settings are not shown but the BVC is showing. Call the
// BVC completion. // BVC completion.
completionWithBVC(); completionWithBVC();
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#import <UIKit/UIKit.h> #import <UIKit/UIKit.h>
#import "base/ios/block_types.h"
#import "ios/chrome/browser/ui/coordinators/chrome_coordinator.h" #import "ios/chrome/browser/ui/coordinators/chrome_coordinator.h"
@protocol ApplicationCommands; @protocol ApplicationCommands;
...@@ -22,6 +23,8 @@ ...@@ -22,6 +23,8 @@
@property(nonatomic, weak) id<UrlLoader> loader; @property(nonatomic, weak) id<UrlLoader> loader;
// Delegate used to make the Tab UI visible. // Delegate used to make the Tab UI visible.
@property(nonatomic, weak) id<HistoryPresentationDelegate> presentationDelegate; @property(nonatomic, weak) id<HistoryPresentationDelegate> presentationDelegate;
// Stops this Coordinator then calls |completionHandler|.
- (void)stopWithCompletion:(ProceduralBlock)completionHandler;
@end @end
#endif // IOS_CHROME_BROWSER_UI_HISTORY_HISTORY_COORDINATOR_H_ #endif // IOS_CHROME_BROWSER_UI_HISTORY_HISTORY_COORDINATOR_H_
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#import <Foundation/Foundation.h> #import <Foundation/Foundation.h>
#import "base/ios/block_types.h"
#import "ios/chrome/browser/ui/main/main_coordinator.h" #import "ios/chrome/browser/ui/main/main_coordinator.h"
#import "ios/chrome/browser/ui/main/view_controller_swapping.h" #import "ios/chrome/browser/ui/main/view_controller_swapping.h"
...@@ -33,6 +34,10 @@ ...@@ -33,6 +34,10 @@
// without animation. This should only be used by unittests. // without animation. This should only be used by unittests.
@property(nonatomic, readwrite, assign) BOOL animationsDisabledForTesting; @property(nonatomic, readwrite, assign) BOOL animationsDisabledForTesting;
// Stops all child coordinators then calls |completion|. |completion| is called
// whether or not child coordinators exist.
- (void)stopChildCoordinatorsWithCompletion:(ProceduralBlock)completion;
@end @end
#endif // IOS_CHROME_BROWSER_UI_TAB_GRID_TAB_GRID_COORDINATOR_H_ #endif // IOS_CHROME_BROWSER_UI_TAB_GRID_TAB_GRID_COORDINATOR_H_
...@@ -91,7 +91,7 @@ ...@@ -91,7 +91,7 @@
return self; return self;
} }
#pragma mark - Public properties #pragma mark - Public
- (id<TabSwitcher>)tabSwitcher { - (id<TabSwitcher>)tabSwitcher {
return self.adaptor; return self.adaptor;
...@@ -127,6 +127,15 @@ ...@@ -127,6 +127,15 @@
} }
} }
- (void)stopChildCoordinatorsWithCompletion:(ProceduralBlock)completion {
// History may be presented on top of the tab grid.
if (self.historyCoordinator) {
[self.historyCoordinator stopWithCompletion:completion];
} else if (completion) {
completion();
}
}
#pragma mark - MainCoordinator properties #pragma mark - MainCoordinator properties
- (id<ViewControllerSwapping>)viewControllerSwapper { - (id<ViewControllerSwapping>)viewControllerSwapper {
......
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