Commit 44e20f48 authored by sczs's avatar sczs Committed by Commit Bot

[ios] Fixes Infobar presentation for contained BVC.

On M83 the BVC will be contained by the TabGrid rather than being
presented. This requires some updating to the Messages presentation
logic, which is done via this CL.


- BVC now informs the InfobarContainer that it will disappear, when this
happens no Infobar should be presented, until BVC "appears" again.

- Fixes a bug where presenting a Banner while BVC is being dismissed
would cause the Banner to remain on screen since now its presenting
ViewController is not being dismissed. shouldDismissBanner was added
to the InfobarBannerContainer protocol so a banner can ask its container
if it should dismiss itself after being presented.

- On InfobarCoordinator we shouldn't always rely on the baseVC (BVC)
presenting Banners, that's why when dismissing a banner we check
its presentingVC instead of BVC presentedVC, this approach would've been
safer even when BVC wasn't contained.

Bug: 1062245
Change-Id: I1d0641cd6e047a72eea9bec79997da4885d2953c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2160808Reviewed-by: default avatarChris Lu <thegreenfrog@chromium.org>
Reviewed-by: default avatarKurt Horimoto <kkhorimoto@chromium.org>
Commit-Queue: Sergio Collazos <sczs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762129}
parent 1c7aa4e5
...@@ -1509,11 +1509,13 @@ NSString* const kBrowserViewControllerSnackbarCategory = ...@@ -1509,11 +1509,13 @@ NSString* const kBrowserViewControllerSnackbarCategory =
// TODO(crbug.com/976411):This should probably move to the BannerVC once/if // TODO(crbug.com/976411):This should probably move to the BannerVC once/if
// the dismiss event from BVC is observable. // the dismiss event from BVC is observable.
if (IsInfobarUIRebootEnabled() && if (IsInfobarUIRebootEnabled() &&
!base::FeatureList::IsEnabled(kInfobarOverlayUI) && !base::FeatureList::IsEnabled(kInfobarOverlayUI)) {
(self.infobarContainerCoordinator.infobarBannerState != [self.infobarContainerCoordinator baseViewWillDisappear];
InfobarBannerPresentationState::NotPresented)) { if (self.infobarContainerCoordinator.infobarBannerState !=
[self.infobarContainerCoordinator dismissInfobarBannerAnimated:NO InfobarBannerPresentationState::NotPresented) {
completion:nil]; [self.infobarContainerCoordinator dismissInfobarBannerAnimated:NO
completion:nil];
}
} }
[_bookmarkInteractionController dismissSnackbar]; [_bookmarkInteractionController dismissSnackbar];
[super viewWillDisappear:animated]; [super viewWillDisappear:animated];
......
...@@ -12,6 +12,10 @@ ...@@ -12,6 +12,10 @@
// Called whener an InfobarBannerContained banner has been dismissed. // Called whener an InfobarBannerContained banner has been dismissed.
- (void)infobarBannerFinishedPresenting; - (void)infobarBannerFinishedPresenting;
// YES if the InfobarBannerContainer shouldn't be presenting any InfobarBanners,
// meaning that the InfobarBanner needs to be dismissed.
- (BOOL)shouldDismissBanner;
@end @end
// Infobar banners presented by an InfobarCoordinatorContainer. // Infobar banners presented by an InfobarCoordinatorContainer.
......
...@@ -309,6 +309,12 @@ const CGFloat kLongPressTimeDurationInSeconds = 0.4; ...@@ -309,6 +309,12 @@ const CGFloat kLongPressTimeDurationInSeconds = 0.4;
[super viewDidAppear:animated]; [super viewDidAppear:animated];
[self.metricsRecorder recordBannerEvent:MobileMessagesBannerEvent::Presented]; [self.metricsRecorder recordBannerEvent:MobileMessagesBannerEvent::Presented];
self.bannerAppearedTime = [NSDate timeIntervalSinceReferenceDate]; self.bannerAppearedTime = [NSDate timeIntervalSinceReferenceDate];
// Once the Banner animation has completed check if the banner container
// should still present banners.
if ([self.infobarBannerContainer shouldDismissBanner]) {
[self.presentingViewController dismissViewControllerAnimated:NO
completion:nil];
}
} }
- (void)viewWillDisappear:(BOOL)animated { - (void)viewWillDisappear:(BOOL)animated {
......
...@@ -461,13 +461,12 @@ ...@@ -461,13 +461,12 @@
// InfobarDelelgate being destroyed. Trying to dismiss it twice might cause a // InfobarDelelgate being destroyed. Trying to dismiss it twice might cause a
// UIKit crash on iOS12. // UIKit crash on iOS12.
if (!self.bannerIsBeingDismissed && if (!self.bannerIsBeingDismissed &&
self.baseViewController.presentedViewController && self.bannerViewController.presentingViewController) {
self.baseViewController.presentedViewController ==
self.bannerViewController) {
self.bannerIsBeingDismissed = YES; self.bannerIsBeingDismissed = YES;
[self infobarBannerWillBeDismissed:userInitiated]; [self infobarBannerWillBeDismissed:userInitiated];
[self.baseViewController dismissViewControllerAnimated:animated [self.bannerViewController.presentingViewController
completion:completion]; dismissViewControllerAnimated:animated
completion:completion];
} else if (completion) { } else if (completion) {
completion(); completion();
} }
......
...@@ -32,6 +32,11 @@ class WebState; ...@@ -32,6 +32,11 @@ class WebState;
// means the view is now visible and part of the main window hierarchy. // means the view is now visible and part of the main window hierarchy.
- (void)baseViewDidAppear; - (void)baseViewDidAppear;
// Notifies the coordinator that its baseViewController's ViewWillDisappear.
// This means the view is not visible and no longer part of the main window
// hierarchy.
- (void)baseViewWillDisappear;
// YES if an Infobar is being presented for |webState|. // YES if an Infobar is being presented for |webState|.
- (BOOL)isInfobarPresentingForWebState:(web::WebState*)webState; - (BOOL)isInfobarPresentingForWebState:(web::WebState*)webState;
......
...@@ -55,6 +55,10 @@ ...@@ -55,6 +55,10 @@
NSMutableArray<InfobarCoordinator*>* infobarCoordinatorsToPresent; NSMutableArray<InfobarCoordinator*>* infobarCoordinatorsToPresent;
// If YES, the banner is not shown, but the badge and subsequent modals will be. // If YES, the banner is not shown, but the badge and subsequent modals will be.
@property(nonatomic, assign) BOOL skipBanner; @property(nonatomic, assign) BOOL skipBanner;
// YES if this container baseViewController is currently visible and part of
// the view hierarchy.
@property(nonatomic, assign, getter=isBaseViewControllerVisible)
BOOL baseViewControllerVisible;
@end @end
...@@ -174,12 +178,17 @@ ...@@ -174,12 +178,17 @@
} }
- (void)baseViewDidAppear { - (void)baseViewDidAppear {
self.baseViewControllerVisible = YES;
InfobarCoordinator* coordinator = InfobarCoordinator* coordinator =
[self.infobarCoordinatorsToPresent firstObject]; [self.infobarCoordinatorsToPresent firstObject];
if (coordinator) if (coordinator)
[self presentBannerForInfobarCoordinator:coordinator]; [self presentBannerForInfobarCoordinator:coordinator];
} }
- (void)baseViewWillDisappear {
self.baseViewControllerVisible = NO;
}
#pragma mark - ChromeCoordinator #pragma mark - ChromeCoordinator
- (MutableCoordinatorArray*)childCoordinators { - (MutableCoordinatorArray*)childCoordinators {
...@@ -268,6 +277,10 @@ ...@@ -268,6 +277,10 @@
[self presentNextBannerInQueue]; [self presentNextBannerInQueue];
} }
- (BOOL)shouldDismissBanner {
return !self.baseViewControllerVisible;
}
#pragma mark InfobarCommands #pragma mark InfobarCommands
- (void)displayModalInfobar:(InfobarType)infobarType { - (void)displayModalInfobar:(InfobarType)infobarType {
...@@ -301,7 +314,8 @@ ...@@ -301,7 +314,8 @@
// return. // return.
if (!(self.infobarBannerState == if (!(self.infobarBannerState ==
InfobarBannerPresentationState::NotPresented) || InfobarBannerPresentationState::NotPresented) ||
(!self.baseViewController.view.window)) { (!self.baseViewController.view.window) ||
(!self.baseViewControllerVisible)) {
[self queueInfobarCoordinatorForPresentation:infobarCoordinator]; [self queueInfobarCoordinatorForPresentation:infobarCoordinator];
return; return;
} }
......
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