Commit dfafd36d authored by sczs's avatar sczs Committed by Commit Bot

[ios] Creates InfobarContainerCoordinator unittests.

This CL also makes some improvements to InfobarContainerCoordinator so
the test can pass:
- isPresentingInfobarBanner is now directly based on the ChildCoordinator
Banner presentation and not just the existence of a VC.
- A disconnect method is added to make sure the mediator stops observing
WebStateList before being dealloced (This is a common pattern throughout
mediators since the memory de-alloaction order in unittest can cause this)
- Adds a legacyContainerFullscrenSupportDisabled to disable FullScreen
support in the legacyContainer. This is needed to avoid a crash after
cleaning up each test case, in order to properly fix this we'd need to
know when the legacyContainer has been dismissed, that would require
some refactoring and the effort/risk is not worth it at this point.
- Removes a DCHECK in the legacyContainer since now the count of total
Infobars doesn't necessarily match the count of total Infobars in the
legacy Container. This because one of them can be a message. The iOS
implementation of the container was merely sending infobar.count() for
position, meaning we were never using position anyways.

Bug: 911864
Change-Id: Ie46c9dc88d2f5277401eb1d08aa54a09d4dbaf82
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1566756
Commit-Queue: Sergio Collazos <sczs@chromium.org>
Reviewed-by: default avatarPeter Lee <pkl@chromium.org>
Reviewed-by: default avatarChris Lu <thegreenfrog@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652604}
parent c7f7bcc3
......@@ -94,10 +94,27 @@ source_set("unit_tests") {
testonly = true
sources = [
"confirm_infobar_view_unittest.mm",
"infobar_container_coordinator_unittest.mm",
]
deps = [
":feature_flags",
":infobars",
":infobars_ui",
":public",
":test_support",
"//base/test:test_support",
"//ios/chrome/browser/browser_state:test_support",
"//ios/chrome/browser/infobars",
"//ios/chrome/browser/infobars:badge",
"//ios/chrome/browser/ui/commands",
"//ios/chrome/browser/ui/infobars/coordinators",
"//ios/chrome/browser/web_state_list",
"//ios/chrome/browser/web_state_list:test_support",
"//ios/chrome/test:test_support",
"//ios/web/public/test/fakes",
"//testing/gtest",
"//testing/gtest",
"//third_party/ocmock",
]
}
......
......@@ -80,6 +80,10 @@ class InfoBarDelegate;
// baseViewController will be set on init.
@property(nonatomic, weak) UIViewController* baseViewController;
// YES if an InfobarBanner is being presented.
@property(nonatomic, assign, getter=isPresentingInfobarBanner)
BOOL presentingInfobarBanner;
@end
#endif // IOS_CHROME_BROWSER_UI_INFOBARS_COORDINATORS_INFOBAR_COORDINATOR_H_
......@@ -84,7 +84,11 @@ const CGFloat kBannerOverlapWithOmnibox = 5.0;
self.bannerViewController.transitioningDelegate = self.bannerTransitionDriver;
[self.baseViewController presentViewController:self.bannerViewController
animated:animated
completion:completion];
completion:^{
self.presentingInfobarBanner = YES;
if (completion)
completion();
}];
}
- (void)presentInfobarModal {
......@@ -154,6 +158,7 @@ const CGFloat kBannerOverlapWithOmnibox = 5.0;
[self.baseViewController
dismissViewControllerAnimated:animated
completion:^{
weakSelf.presentingInfobarBanner = NO;
[weakSelf.badgeDelegate infobarBannerWasDismissed];
weakSelf.bannerTransitionDriver = nil;
animatedFullscreenDisabler_ = nullptr;
......
......@@ -13,4 +13,8 @@ extern const int kInfobarBackgroundColor;
extern NSString* const kConfirmInfobarButton1AccessibilityIdentifier;
extern NSString* const kConfirmInfobarButton2AccessibilityIdentifier;
// The duration in seconds that the InfobarCoordinator banner will be presented
// for.
extern const NSTimeInterval kInfobarBannerPresentationDurationInSeconds;
#endif // IOS_CHROME_BROWSER_UI_INFOBARS_INFOBAR_CONSTANTS_H_
......@@ -15,3 +15,5 @@ NSString* const kConfirmInfobarButton1AccessibilityIdentifier =
@"confirmInfobarButton1AXID";
NSString* const kConfirmInfobarButton2AccessibilityIdentifier =
@"confirmInfobarButton2AXID";
const NSTimeInterval kInfobarBannerPresentationDurationInSeconds = 6.0;
......@@ -12,6 +12,7 @@
#import "ios/chrome/browser/ui/commands/command_dispatcher.h"
#import "ios/chrome/browser/ui/fullscreen/fullscreen_controller_factory.h"
#import "ios/chrome/browser/ui/infobars/coordinators/infobar_coordinator.h"
#import "ios/chrome/browser/ui/infobars/infobar_constants.h"
#import "ios/chrome/browser/ui/infobars/infobar_container_consumer.h"
#include "ios/chrome/browser/ui/infobars/infobar_container_mediator.h"
#import "ios/chrome/browser/ui/infobars/infobar_feature.h"
......@@ -23,12 +24,6 @@
#error "This file requires ARC support."
#endif
namespace {
// The duration in seconds that the InfobarCoordinator banner will be presented
// for.
const double kBannerPresentationDurationInSeconds = 6.0;
} // namespace
@interface InfobarContainerCoordinator () <InfobarContainerConsumer>
@property(nonatomic, assign) WebStateList* webStateList;
......@@ -42,6 +37,10 @@ const double kBannerPresentationDurationInSeconds = 6.0;
@property(nonatomic, strong) InfobarContainerMediator* mediator;
// The dispatcher for this Coordinator.
@property(nonatomic, weak) id<ApplicationCommands> dispatcher;
// If YES the legacyContainer Fullscreen support will be disabled.
// TODO(crbug.com/927064): Remove this once the legacy container is no longer
// needed.
@property(nonatomic, assign) BOOL legacyContainerFullscrenSupportDisabled;
@end
......@@ -76,6 +75,8 @@ const double kBannerPresentationDurationInSeconds = 6.0;
aboveSubview:self.positioner.parentView];
[legacyContainer didMoveToParentViewController:self.baseViewController];
legacyContainer.positioner = self.positioner;
legacyContainer.disableFullscreenSupport =
self.legacyContainerFullscrenSupportDisabled;
self.legacyContainerViewController = legacyContainer;
// Creates the mediator using both consumers.
......@@ -92,7 +93,14 @@ const double kBannerPresentationDurationInSeconds = 6.0;
- (void)stop {
[[UpgradeCenter sharedInstance] unregisterClient:self.mediator];
self.mediator = nil;
[self.mediator disconnect];
if (!self.legacyContainerViewController)
return;
[self.legacyContainerViewController willMoveToParentViewController:nil];
[self.legacyContainerViewController.view removeFromSuperview];
[self.legacyContainerViewController removeFromParentViewController];
self.legacyContainerViewController = nil;
}
#pragma mark - Public Interface
......@@ -149,7 +157,9 @@ const double kBannerPresentationDurationInSeconds = 6.0;
- (BOOL)isPresentingInfobarBanner {
DCHECK(IsInfobarUIRebootEnabled());
_presentingInfobarBanner = self.infobarViewController ? YES : NO;
InfobarCoordinator* infobarCoordinator =
static_cast<InfobarCoordinator*>(self.activeChildCoordinator);
_presentingInfobarBanner = [infobarCoordinator isPresentingInfobarBanner];
return _presentingInfobarBanner;
}
......@@ -169,10 +179,11 @@ const double kBannerPresentationDurationInSeconds = 6.0;
self.infobarViewController = infobarCoordinator.bannerViewController;
[self.childCoordinators addObject:infobarCoordinator];
// Dismissed the presented InfobarCoordinator banner after
// kBannerPresentationDuration seconds.
dispatch_time_t popTime = dispatch_time(
DISPATCH_TIME_NOW, kBannerPresentationDurationInSeconds * NSEC_PER_SEC);
// Dismisses the presented InfobarCoordinator banner after
// kInfobarBannerPresentationDurationInSeconds seconds.
dispatch_time_t popTime =
dispatch_time(DISPATCH_TIME_NOW,
kInfobarBannerPresentationDurationInSeconds * NSEC_PER_SEC);
dispatch_after(popTime, dispatch_get_main_queue(), ^(void) {
[infobarCoordinator dismissInfobarBannerAfterInteraction];
});
......
......@@ -35,6 +35,9 @@ class WebStateList;
// The SigninPresenter delegate for this Mediator.
@property(nonatomic, weak) id<SigninPresenter> signinPresenter;
// Stops observing all objects.
- (void)disconnect;
@end
#endif // IOS_CHROME_BROWSER_UI_INFOBARS_INFOBAR_CONTAINER_MEDIATOR_H_
......@@ -57,8 +57,15 @@
}
- (void)dealloc {
_webStateList->RemoveObserver(_webStateListObserver.get());
_webStateListObserver.reset();
[self disconnect];
}
- (void)disconnect {
if (_webStateList) {
_webStateList->RemoveObserver(_webStateListObserver.get());
_webStateListObserver.reset();
_webStateList = nullptr;
}
}
#pragma mark - WebStateListObserver
......
......@@ -32,6 +32,9 @@ class FullscreenController;
// The delegate used to position the InfoBarContainer in the view.
@property(nonatomic, weak) id<InfobarPositioner> positioner;
// If YES the Container Fullscreen support will be disabled.
@property(nonatomic, assign) BOOL disableFullscreenSupport;
@end
#endif // IOS_CHROME_BROWSER_UI_INFOBARS_LEGACY_INFOBAR_CONTAINER_VIEW_CONTROLLER_H_
......@@ -61,14 +61,14 @@ const CGFloat kAlphaChangeAnimationDuration = 0.35;
[super viewDidAppear:animated];
self.visible = YES;
if (!_fullscreenObserver) {
if (!_fullscreenObserver && !self.disableFullscreenSupport) {
_fullscreenObserver = std::make_unique<FullscreenUIUpdater>(self);
self.fullscreenController->AddObserver(_fullscreenObserver.get());
}
}
- (void)viewDidDisappear:(BOOL)animated {
if (_fullscreenObserver) {
if (_fullscreenObserver && !self.disableFullscreenSupport) {
self.fullscreenController->RemoveObserver(_fullscreenObserver.get());
_fullscreenObserver = nullptr;
}
......
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