Commit 559fe24a authored by sczs's avatar sczs Committed by Commit Bot

[ios] Stores presented InfobarsCoordinators in a Dictionary.

- Adds infobarManagerWillChange consumer method to clean up
this dictionary when the manager/webstate has changed.
- Changes childCoordinatorStopped so it sends the InfobarType.
- Updates unittests.

Bug: 961343
Change-Id: I275ba5a5f3091202b7eff3d40777ac782c438946
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1731141Reviewed-by: default avatarPeter Lee <pkl@chromium.org>
Commit-Queue: Sergio Collazos <sczs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#684238}
parent d73a1f67
...@@ -86,7 +86,8 @@ ...@@ -86,7 +86,8 @@
self.bannerIsPresenting = YES; self.bannerIsPresenting = YES;
[self.infobarCoordinator presentInfobarBannerAnimated:NO completion:nil]; [self.infobarCoordinator presentInfobarBannerAnimated:NO completion:nil];
} }
- (void)infobarManagerWillChange {
}
- (void)setUserInteractionEnabled:(BOOL)enabled { - (void)setUserInteractionEnabled:(BOOL)enabled {
} }
- (void)updateLayoutAnimated:(BOOL)animated { - (void)updateLayoutAnimated:(BOOL)animated {
......
...@@ -21,6 +21,13 @@ class InfoBarContainerIOS : public infobars::InfoBarContainer { ...@@ -21,6 +21,13 @@ class InfoBarContainerIOS : public infobars::InfoBarContainer {
id<InfobarContainerConsumer> legacyConsumer); id<InfobarContainerConsumer> legacyConsumer);
~InfoBarContainerIOS() override; ~InfoBarContainerIOS() override;
// Changes the InfoBarManager for which this container is showing infobars.
// This will hide all current infobars, remove them from the container, add
// the infobars from |infobar_manager|, and show them all. If
// |infobar_manager| is nullptr, it will hide all current Infobars but won't
// be able to present new ones.
void ChangeInfoBarManager(infobars::InfoBarManager* infobar_manager);
protected: protected:
void PlatformSpecificAddInfoBar(infobars::InfoBar* infobar, void PlatformSpecificAddInfoBar(infobars::InfoBar* infobar,
size_t position) override; size_t position) override;
......
...@@ -24,6 +24,12 @@ InfoBarContainerIOS::~InfoBarContainerIOS() { ...@@ -24,6 +24,12 @@ InfoBarContainerIOS::~InfoBarContainerIOS() {
RemoveAllInfoBarsForDestruction(); RemoveAllInfoBarsForDestruction();
} }
void InfoBarContainerIOS::ChangeInfoBarManager(
infobars::InfoBarManager* infobar_manager) {
[consumer_ infobarManagerWillChange];
InfoBarContainer::ChangeInfoBarManager(infobar_manager);
}
void InfoBarContainerIOS::PlatformSpecificAddInfoBar(infobars::InfoBar* infobar, void InfoBarContainerIOS::PlatformSpecificAddInfoBar(infobars::InfoBar* infobar,
size_t position) { size_t position) {
InfoBarIOS* infobar_ios = static_cast<InfoBarIOS*>(infobar); InfoBarIOS* infobar_ios = static_cast<InfoBarIOS*>(infobar);
......
...@@ -18,6 +18,7 @@ source_set("infobars") { ...@@ -18,6 +18,7 @@ source_set("infobars") {
"//ios/chrome/browser", "//ios/chrome/browser",
"//ios/chrome/browser/infobars", "//ios/chrome/browser/infobars",
"//ios/chrome/browser/infobars:badge", "//ios/chrome/browser/infobars:badge",
"//ios/chrome/browser/infobars:public",
"//ios/chrome/browser/ui/commands", "//ios/chrome/browser/ui/commands",
"//ios/chrome/browser/ui/coordinators:chrome_coordinators", "//ios/chrome/browser/ui/coordinators:chrome_coordinators",
"//ios/chrome/browser/ui/fullscreen", "//ios/chrome/browser/ui/fullscreen",
......
...@@ -68,7 +68,7 @@ ...@@ -68,7 +68,7 @@
// from memory. // from memory.
self.delegate->RemoveInfoBar(); self.delegate->RemoveInfoBar();
_confirmInfobarDelegate = nil; _confirmInfobarDelegate = nil;
[self.infobarContainer childCoordinatorStopped]; [self.infobarContainer childCoordinatorStopped:self.infobarType];
} }
} }
......
...@@ -97,7 +97,7 @@ ...@@ -97,7 +97,7 @@
// from memory. // from memory.
self.delegate->RemoveInfoBar(); self.delegate->RemoveInfoBar();
_passwordInfoBarDelegate = nil; _passwordInfoBarDelegate = nil;
[self.infobarContainer childCoordinatorStopped]; [self.infobarContainer childCoordinatorStopped:self.infobarType];
} }
} }
......
...@@ -7,15 +7,15 @@ ...@@ -7,15 +7,15 @@
#import <Foundation/Foundation.h> #import <Foundation/Foundation.h>
enum class InfobarType;
// Protocol for the InfobarCoordinators to communicate with the InfobarContainer // Protocol for the InfobarCoordinators to communicate with the InfobarContainer
// Coordinator. // Coordinator.
@protocol InfobarContainer @protocol InfobarContainer
// Informs the InfobarContainer Coordinator that its child coordinator has // Informs the InfobarContainer Coordinator that its child coordinator of type
// stopped. // |infobarType| has stopped.
// TODO(crbug.com/961343): Add support to indicate which Coordinator has - (void)childCoordinatorStopped:(InfobarType)infobarType;
// stopped.
- (void)childCoordinatorStopped;
@end @end
......
...@@ -15,6 +15,11 @@ ...@@ -15,6 +15,11 @@
// Adds |infoBarDelegate|'s Infobar to the InfobarContainer. // Adds |infoBarDelegate|'s Infobar to the InfobarContainer.
- (void)addInfoBarWithDelegate:(id<InfobarUIDelegate>)infoBarDelegate; - (void)addInfoBarWithDelegate:(id<InfobarUIDelegate>)infoBarDelegate;
// Informs InfobarContainerConsumer that the backing infobarManager will change.
// This most likely means that the WebState is changing and a new set of
// Infobars will/may be presented.
- (void)infobarManagerWillChange;
// Sets the Infobar container user interaction to |enabled|. // Sets the Infobar container user interaction to |enabled|.
- (void)setUserInteractionEnabled:(BOOL)enabled; - (void)setUserInteractionEnabled:(BOOL)enabled;
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#import "base/mac/foundation_util.h" #import "base/mac/foundation_util.h"
#include "ios/chrome/browser/infobars/infobar_manager_impl.h" #include "ios/chrome/browser/infobars/infobar_manager_impl.h"
#import "ios/chrome/browser/infobars/infobar_type.h"
#import "ios/chrome/browser/ui/commands/application_commands.h" #import "ios/chrome/browser/ui/commands/application_commands.h"
#import "ios/chrome/browser/ui/commands/command_dispatcher.h" #import "ios/chrome/browser/ui/commands/command_dispatcher.h"
#import "ios/chrome/browser/ui/fullscreen/fullscreen_controller_factory.h" #import "ios/chrome/browser/ui/fullscreen/fullscreen_controller_factory.h"
...@@ -43,6 +44,10 @@ ...@@ -43,6 +44,10 @@
// TODO(crbug.com/927064): Remove this once the legacy container is no longer // TODO(crbug.com/927064): Remove this once the legacy container is no longer
// needed. // needed.
@property(nonatomic, assign) BOOL legacyContainerFullscrenSupportDisabled; @property(nonatomic, assign) BOOL legacyContainerFullscrenSupportDisabled;
// infobarCoordinators holds all InfobarCoordinators this ContainerCoordinator
// can display.
@property(nonatomic, strong)
NSMutableDictionary<NSNumber*, InfobarCoordinator*>* infobarCoordinators;
@end @end
...@@ -56,6 +61,7 @@ ...@@ -56,6 +61,7 @@
browserState:browserState]; browserState:browserState];
if (self) { if (self) {
_webStateList = webStateList; _webStateList = webStateList;
_infobarCoordinators = [NSMutableDictionary dictionary];
} }
return self; return self;
} }
...@@ -134,10 +140,27 @@ ...@@ -134,10 +140,27 @@
- (void)dismissInfobarBannerAnimated:(BOOL)animated - (void)dismissInfobarBannerAnimated:(BOOL)animated
completion:(void (^)())completion { completion:(void (^)())completion {
DCHECK(IsInfobarUIRebootEnabled()); DCHECK(IsInfobarUIRebootEnabled());
InfobarCoordinator* infobarCoordinator =
static_cast<InfobarCoordinator*>(self.activeChildCoordinator); for (InfobarCoordinator* infobarCoordinator in
[infobarCoordinator dismissInfobarBannerAnimated:animated [self.infobarCoordinators allValues]) {
completion:completion]; if (infobarCoordinator.infobarBannerState !=
InfobarBannerPresentationState::NotPresented) {
// Since only one Banner can be presented at any time, dismiss it.
[infobarCoordinator dismissInfobarBannerAnimated:animated
completion:completion];
return;
}
}
// If no banner was presented make sure the completion block still runs.
if (completion)
completion();
}
#pragma mark - ChromeCoordinator
- (MutableCoordinatorArray*)childCoordinators {
return static_cast<MutableCoordinatorArray*>(
[self.infobarCoordinators allValues]);
} }
#pragma mark - Accessors #pragma mark - Accessors
...@@ -159,9 +182,15 @@ ...@@ -159,9 +182,15 @@
- (InfobarBannerPresentationState)infobarBannerState { - (InfobarBannerPresentationState)infobarBannerState {
DCHECK(IsInfobarUIRebootEnabled()); DCHECK(IsInfobarUIRebootEnabled());
InfobarCoordinator* infobarCoordinator = for (InfobarCoordinator* infobarCoordinator in
static_cast<InfobarCoordinator*>(self.activeChildCoordinator); [self.infobarCoordinators allValues]) {
return infobarCoordinator.infobarBannerState; if (infobarCoordinator.infobarBannerState !=
InfobarBannerPresentationState::NotPresented) {
// Since only one Banner can be presented at any time, early return.
return infobarCoordinator.infobarBannerState;
}
}
return InfobarBannerPresentationState::NotPresented;
} }
#pragma mark - InfobarConsumer #pragma mark - InfobarConsumer
...@@ -171,6 +200,10 @@ ...@@ -171,6 +200,10 @@
InfobarCoordinator* infobarCoordinator = InfobarCoordinator* infobarCoordinator =
static_cast<InfobarCoordinator*>(infoBarDelegate); static_cast<InfobarCoordinator*>(infoBarDelegate);
NSNumber* infobarKey =
[NSNumber numberWithInt:static_cast<int>(infoBarDelegate.infobarType)];
self.infobarCoordinators[infobarKey] = infobarCoordinator;
// Present the InfobarBanner, and set the Coordinator and View hierarchies. // Present the InfobarBanner, and set the Coordinator and View hierarchies.
[infobarCoordinator start]; [infobarCoordinator start];
infobarCoordinator.badgeDelegate = self.mediator; infobarCoordinator.badgeDelegate = self.mediator;
...@@ -181,7 +214,6 @@ ...@@ -181,7 +214,6 @@
if (!infobarCoordinator.bannerWasPresented) if (!infobarCoordinator.bannerWasPresented)
[infobarCoordinator presentInfobarBannerAnimated:YES completion:nil]; [infobarCoordinator presentInfobarBannerAnimated:YES completion:nil];
self.infobarViewController = infobarCoordinator.bannerViewController; self.infobarViewController = infobarCoordinator.bannerViewController;
[self.childCoordinators addObject:infobarCoordinator];
// Dismisses the presented InfobarCoordinator banner after // Dismisses the presented InfobarCoordinator banner after
// kInfobarBannerPresentationDurationInSeconds seconds. // kInfobarBannerPresentationDurationInSeconds seconds.
...@@ -195,6 +227,10 @@ ...@@ -195,6 +227,10 @@
} }
} }
- (void)infobarManagerWillChange {
self.infobarCoordinators = [NSMutableDictionary dictionary];
}
- (void)setUserInteractionEnabled:(BOOL)enabled { - (void)setUserInteractionEnabled:(BOOL)enabled {
[self.infobarViewController.view setUserInteractionEnabled:enabled]; [self.infobarViewController.view setUserInteractionEnabled:enabled];
} }
...@@ -207,18 +243,17 @@ ...@@ -207,18 +243,17 @@
#pragma mark InfobarContainer #pragma mark InfobarContainer
- (void)childCoordinatorStopped { - (void)childCoordinatorStopped:(InfobarType)infobarType {
DCHECK(IsInfobarUIRebootEnabled()); DCHECK(IsInfobarUIRebootEnabled());
// TODO(crbug.com/961343): When more than one InfobarCoordinator can exist NSNumber* infobarKey = [NSNumber numberWithInt:static_cast<int>(infobarType)];
// concurrently, delete only the one that stopped. [self.infobarCoordinators removeObjectForKey:infobarKey];
[self.childCoordinators removeAllObjects];
} }
#pragma mark - InfobarCommands #pragma mark - InfobarCommands
- (void)displayModalInfobar { - (void)displayModalInfobar {
InfobarCoordinator* infobarCoordinator = NSArray* allCoordinators = [self.infobarCoordinators allValues];
static_cast<InfobarCoordinator*>(self.activeChildCoordinator); InfobarCoordinator* infobarCoordinator = [allCoordinators lastObject];
[infobarCoordinator presentInfobarModal]; [infobarCoordinator presentInfobarModal];
} }
......
...@@ -134,6 +134,22 @@ class InfobarContainerCoordinatorTest : public PlatformTest { ...@@ -134,6 +134,22 @@ class InfobarContainerCoordinatorTest : public PlatformTest {
coordinator_, std::move(infobar_delegate_))); coordinator_, std::move(infobar_delegate_)));
} }
// Adds an Infobar to the InfobarManager, triggering an InfobarBanner
// presentation.
void AddSecondInfobar() {
// Setup the InfobarCoordinator and InfobarDelegate.
TestInfoBarDelegate* test_infobar_delegate =
new TestInfoBarDelegate(@"Title 2");
InfobarConfirmCoordinator* coordinator = [[InfobarConfirmCoordinator alloc]
initWithInfoBarDelegate:test_infobar_delegate
type:InfobarType::kInfobarTypePasswordSave];
std::unique_ptr<ConfirmInfoBarDelegate> infobar_delegate =
std::unique_ptr<ConfirmInfoBarDelegate>(test_infobar_delegate);
GetInfobarManager()->AddInfoBar(
std::make_unique<InfoBarIOS>(coordinator, std::move(infobar_delegate)));
}
void AddSecondWebstate() { void AddSecondWebstate() {
std::unique_ptr<web::TestWebState> second_web_state = std::unique_ptr<web::TestWebState> second_web_state =
std::make_unique<web::TestWebState>(); std::make_unique<web::TestWebState>();
...@@ -455,10 +471,9 @@ TEST_F(InfobarContainerCoordinatorTest, ...@@ -455,10 +471,9 @@ TEST_F(InfobarContainerCoordinatorTest,
InfobarBannerPresentationState::NotPresented); InfobarBannerPresentationState::NotPresented);
} }
// Tests that the ChildCoordinator is deleted once it stops. // Tests that the ChildCoordinators are deleted once the Webstate is closed.
// TODO(crbug.com/961343): Update test when more than one Child Coordinator is TEST_F(InfobarContainerCoordinatorTest,
// supported. TestInfobarChildCoordinatorCountWebstate) {
TEST_F(InfobarContainerCoordinatorTest, TestInfobarChildCoordinatorCount) {
AddInfobar(); AddInfobar();
EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout( EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
...@@ -482,6 +497,25 @@ TEST_F(InfobarContainerCoordinatorTest, TestInfobarChildCoordinatorCount) { ...@@ -482,6 +497,25 @@ TEST_F(InfobarContainerCoordinatorTest, TestInfobarChildCoordinatorCount) {
InfobarBannerPresentationState::NotPresented; InfobarBannerPresentationState::NotPresented;
})); }));
AddSecondInfobar();
EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
base::test::ios::kWaitForUIElementTimeout, ^bool {
return infobar_container_coordinator_.infobarBannerState ==
InfobarBannerPresentationState::Presented;
}));
ASSERT_EQ(infobar_container_coordinator_.infobarBannerState,
InfobarBannerPresentationState::Presented);
ASSERT_EQ(NSUInteger(2),
infobar_container_coordinator_.childCoordinators.count);
[infobar_container_coordinator_ dismissInfobarBannerAnimated:NO
completion:nil];
ASSERT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
base::test::ios::kWaitForUIElementTimeout, ^bool {
return infobar_container_coordinator_.infobarBannerState ==
InfobarBannerPresentationState::NotPresented;
}));
web_state_list_->CloseWebStateAt(0, 0); web_state_list_->CloseWebStateAt(0, 0);
ASSERT_TRUE(base::test::ios::WaitUntilConditionOrTimeout( ASSERT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
...@@ -494,3 +528,53 @@ TEST_F(InfobarContainerCoordinatorTest, TestInfobarChildCoordinatorCount) { ...@@ -494,3 +528,53 @@ TEST_F(InfobarContainerCoordinatorTest, TestInfobarChildCoordinatorCount) {
ASSERT_EQ(NSUInteger(0), ASSERT_EQ(NSUInteger(0),
infobar_container_coordinator_.childCoordinators.count); infobar_container_coordinator_.childCoordinators.count);
} }
// Tests that the ChildCoordinators are deleted once they stop.
TEST_F(InfobarContainerCoordinatorTest, TestInfobarChildCoordinatorCountStop) {
AddInfobar();
EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
base::test::ios::kWaitForUIElementTimeout, ^bool {
return infobar_container_coordinator_.infobarBannerState ==
InfobarBannerPresentationState::Presented;
}));
ASSERT_EQ(infobar_container_coordinator_.infobarBannerState,
InfobarBannerPresentationState::Presented);
ASSERT_EQ(NSUInteger(1),
infobar_container_coordinator_.childCoordinators.count);
ASSERT_EQ(infobar_container_coordinator_.infobarBannerState,
InfobarBannerPresentationState::Presented);
[infobar_container_coordinator_ dismissInfobarBannerAnimated:NO
completion:nil];
ASSERT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
base::test::ios::kWaitForUIElementTimeout, ^bool {
return infobar_container_coordinator_.infobarBannerState ==
InfobarBannerPresentationState::NotPresented;
}));
AddSecondInfobar();
EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
base::test::ios::kWaitForUIElementTimeout, ^bool {
return infobar_container_coordinator_.infobarBannerState ==
InfobarBannerPresentationState::Presented;
}));
ASSERT_EQ(infobar_container_coordinator_.infobarBannerState,
InfobarBannerPresentationState::Presented);
ASSERT_EQ(NSUInteger(2),
infobar_container_coordinator_.childCoordinators.count);
[infobar_container_coordinator_ dismissInfobarBannerAnimated:NO
completion:nil];
ASSERT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
base::test::ios::kWaitForUIElementTimeout, ^bool {
return infobar_container_coordinator_.infobarBannerState ==
InfobarBannerPresentationState::NotPresented;
}));
// Stop the first Coordinator.
[coordinator_ stop];
ASSERT_EQ(NSUInteger(1),
infobar_container_coordinator_.childCoordinators.count);
}
...@@ -92,6 +92,11 @@ const CGFloat kAlphaChangeAnimationDuration = 0.35; ...@@ -92,6 +92,11 @@ const CGFloat kAlphaChangeAnimationDuration = 0.35;
]]; ]];
} }
- (void)infobarManagerWillChange {
// NO-OP. This legacy container doesn't need to clean up any state after the
// InfobarManager has changed.
}
- (void)setUserInteractionEnabled:(BOOL)enabled { - (void)setUserInteractionEnabled:(BOOL)enabled {
[self.view setUserInteractionEnabled:enabled]; [self.view setUserInteractionEnabled:enabled];
} }
......
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