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

[ios] Fixes InfobarCoordinator <-> Consumer memory leak.

There was a retain cycle per bug 1004690.
The ContainerCoordinator had a strong pointer to the Mediator, which in
turn had a strong pointer to infobar_container_ios, which had a strong
pointer to its consumer (ContainerCoordinator).

This would only happen on non Messages, since on the legacy implementation
the consumer is the ContainerViewController. Still I think both consumers
should be weakly retained since the mediator(infobar_container_ios)
don't really own these objects.

Bug: 1004690
Change-Id: Ic5d30b444177e216ffe2f8cdce9b0cf7d012383c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1809744Reviewed-by: default avatarChris Lu <thegreenfrog@chromium.org>
Commit-Queue: Sergio Collazos <sczs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697490}
parent 0c3e8777
......@@ -35,9 +35,9 @@ class InfoBarContainerIOS : public infobars::InfoBarContainer {
void PlatformSpecificInfoBarStateChanged(bool is_animating) override;
private:
id<InfobarContainerConsumer> consumer_;
id<InfobarContainerConsumer> legacyConsumer_;
infobars::InfoBarManager* info_bar_manager_ = nullptr;
__weak id<InfobarContainerConsumer> consumer_;
__weak id<InfobarContainerConsumer> legacyConsumer_;
DISALLOW_COPY_AND_ASSIGN(InfoBarContainerIOS);
};
......
......@@ -327,7 +327,7 @@ TEST_F(InfobarContainerCoordinatorTest,
// TODO(crbug.com/1004514): This test fails due to
// infobarBannerWasDismissed:forWebState:.
TEST_F(InfobarContainerCoordinatorTest,
DISABLED_TestInfobarBannerDismissAtWebStateChange) {
TestInfobarBannerDismissAtWebStateChange) {
AddInfobar();
AddSecondWebstate();
......@@ -429,7 +429,7 @@ TEST_F(InfobarContainerCoordinatorTest,
// TODO(crbug.com/1004514): This test fails due to
// infobarBannerWasDismissed:forWebState:.
TEST_F(InfobarContainerCoordinatorTest,
DISABLED_TestInfobarBannerDismissedClosingWebstate) {
TestInfobarBannerDismissedClosingWebstate) {
AddInfobar();
// Close the Webstate without calling WaitUntilConditionOrTimeout.
web_state_list_->CloseWebStateAt(0, 0);
......@@ -445,8 +445,7 @@ TEST_F(InfobarContainerCoordinatorTest,
// Tests that the Infobar is dismissed when both the VC and Webstate are closed.
// TODO(crbug.com/1004514): This test fails due to
// infobarBannerWasDismissed:forWebState:.
TEST_F(InfobarContainerCoordinatorTest,
DISABLED_TestDismissingAndClosingWebstate) {
TEST_F(InfobarContainerCoordinatorTest, TestDismissingAndClosingWebstate) {
AddInfobar();
ASSERT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
base::test::ios::kWaitForUIElementTimeout, ^bool {
......@@ -473,7 +472,7 @@ TEST_F(InfobarContainerCoordinatorTest,
// TODO(crbug.com/1004514): This test fails due to
// infobarBannerWasDismissed:forWebState:.
TEST_F(InfobarContainerCoordinatorTest,
DISABLED_TestDismissingAndClosingWebstateSecondWebstate) {
TestDismissingAndClosingWebstateSecondWebstate) {
AddInfobar();
AddSecondWebstate();
EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
......
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