Commit e5ed82db authored by Jérôme Lebel's avatar Jérôme Lebel Committed by Commit Bot

Don't reload setting view controller if it has been dismissed

If the setting view controller is being reloaded while being dismissed,
a new SigninPromoViewMediator is created. This new one is not properly
released. -[SigninPromoViewMediator signinPromoViewRemoved] has to be
called before dealloc.

stack trace:
-[SettingsCollectionViewController loadModel]
...
-[SettingsCollectionViewController didFinishSignin:]
...
-[SigninInteractionController cancel]
-[SettingsCollectionViewController settingsWillBeDismissed]
...


Bug introduced with crrev.com/2883093002
DCHECK introduced with crrev.com/c/567923

Bug: 747121
Change-Id: Ida6847e0942c5145ddc8fbd6c4681bfcc87a8904
Reviewed-on: https://chromium-review.googlesource.com/580408
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Reviewed-by: default avatarSylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488624}
parent c6f13d8b
......@@ -230,6 +230,8 @@ void SigninObserverBridge::GoogleSignedOut(const std::string& account_id,
// YES if the user used at least once the sign-in promo view buttons.
BOOL _signinStarted;
// YES if view has been dismissed.
BOOL _settingsHasBeenDismissed;
}
@property(nonatomic, readonly, weak) id<ApplicationCommands> dispatcher;
......@@ -297,7 +299,8 @@ void SigninObserverBridge::GoogleSignedOut(const std::string& account_id,
}
- (void)dealloc {
[self stopBrowserStateServiceObservers];
DCHECK(_settingsHasBeenDismissed)
<< "-settingsWillBeDismissed must be called before -dealloc";
}
- (void)stopBrowserStateServiceObservers {
......@@ -1008,7 +1011,8 @@ void SigninObserverBridge::GoogleSignedOut(const std::string& account_id,
_signinInteractionController = nil;
// The sign-in is done. The sign-in promo cell or account cell can be
// reloaded.
[self reloadData];
if (!_settingsHasBeenDismissed)
[self reloadData];
}
#pragma mark Material Cell Catalog
......@@ -1037,6 +1041,8 @@ void SigninObserverBridge::GoogleSignedOut(const std::string& account_id,
#pragma mark SettingsControllerProtocol
- (void)settingsWillBeDismissed {
DCHECK(!_settingsHasBeenDismissed);
_settingsHasBeenDismissed = YES;
if (!_signinStarted && _signinPromoViewMediator) {
PrefService* prefs = _browserState->GetPrefs();
int displayedCount =
......
......@@ -101,6 +101,7 @@ TEST_F(SettingsNavigationControllerTest, PopController) {
[settingsController popViewControllerAnimated:NO];
EXPECT_NSEQ(viewController, poppedViewController);
EXPECT_EQ(1U, [[settingsController viewControllers] count]);
[settingsController settingsWillBeDismissed];
}
}
......@@ -116,6 +117,7 @@ TEST_F(SettingsNavigationControllerTest, DontPopRootController) {
EXPECT_EQ(1U, [[settingsController viewControllers] count]);
EXPECT_FALSE([settingsController popViewControllerAnimated:NO]);
[settingsController settingsWillBeDismissed];
}
}
......@@ -138,6 +140,7 @@ TEST_F(SettingsNavigationControllerTest,
[settingsController popViewControllerOrCloseSettingsAnimated:NO];
EXPECT_EQ(1U, [[settingsController viewControllers] count]);
EXPECT_OCMOCK_VERIFY(mockDelegate_);
[settingsController settingsWillBeDismissed];
}
}
......@@ -156,6 +159,7 @@ TEST_F(SettingsNavigationControllerTest,
[[mockDelegate_ expect] closeSettings];
[settingsController popViewControllerOrCloseSettingsAnimated:NO];
EXPECT_OCMOCK_VERIFY(mockDelegate_);
[settingsController settingsWillBeDismissed];
}
}
......
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