Commit 85928a37 authored by Mark Cogan's avatar Mark Cogan Committed by Commit Bot

[iOS] Add (and use) an explicit -disconnect for TabStripController.

TabStripController relies on -dalloc to stop its various WebStateList
observations. If the WebStateList is destroyed before the dealloc runs,
the non-empty observer DCHECKs are hit. The upcoming change to
WebStateList ownership (see crrev.com/c/1796358) will cause this to
happen, for example.

To fix this, an explicit -disconnect method is added to
TabStripController, and the tab strip coordinator calls it from -stop.

Change-Id: I22c2fc967c0b42c544ab93dce17518e93d272f43
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1809298
Commit-Queue: Mark Cogan <marq@chromium.org>
Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697241}
parent cdcea0d0
...@@ -48,6 +48,11 @@ ...@@ -48,6 +48,11 @@
// Hides or shows the tab strip. // Hides or shows the tab strip.
- (void)hideTabStrip:(BOOL)hidden; - (void)hideTabStrip:(BOOL)hidden;
// Preprare the receiver for destruction, disconnecting from all services.
// It is an error for the receiver to dealloc without this having been called
// first.
- (void)disconnect;
@end @end
#endif // IOS_CHROME_BROWSER_UI_TABS_TAB_STRIP_CONTROLLER_H_ #endif // IOS_CHROME_BROWSER_UI_TABS_TAB_STRIP_CONTROLLER_H_
...@@ -257,6 +257,9 @@ UIColor* BackgroundColor() { ...@@ -257,6 +257,9 @@ UIColor* BackgroundColor() {
@property(nonatomic, readonly, retain) TabStripView* tabStripView; @property(nonatomic, readonly, retain) TabStripView* tabStripView;
@property(nonatomic, readonly, retain) UIButton* buttonNewTab; @property(nonatomic, readonly, retain) UIButton* buttonNewTab;
// YES if the controller has been disconnected.
@property(nonatomic) BOOL disconnected;
// Initializes the tab array based on the the entries in the TabModel. Creates // Initializes the tab array based on the the entries in the TabModel. Creates
// one TabView per Tab and adds it to the tabstrip. A later call to // one TabView per Tab and adds it to the tabstrip. A later call to
// |-layoutTabs| is needed to properly place the tabs in the correct positions. // |-layoutTabs| is needed to properly place the tabs in the correct positions.
...@@ -510,10 +513,16 @@ UIColor* BackgroundColor() { ...@@ -510,10 +513,16 @@ UIColor* BackgroundColor() {
} }
- (void)dealloc { - (void)dealloc {
DCHECK(_disconnected);
}
- (void)disconnect {
[_tabStripView setDelegate:nil]; [_tabStripView setDelegate:nil];
[_tabStripView setLayoutDelegate:nil]; [_tabStripView setLayoutDelegate:nil];
_allWebStateObservationForwarder.reset(); _allWebStateObservationForwarder.reset();
_webStateListFaviconObserver.reset();
_tabModel.webStateList->RemoveObserver(_webStateListObserver.get()); _tabModel.webStateList->RemoveObserver(_webStateListObserver.get());
self.disconnected = YES;
} }
- (void)hideTabStrip:(BOOL)hidden { - (void)hideTabStrip:(BOOL)hidden {
......
...@@ -126,6 +126,7 @@ class TabStripControllerTest : public PlatformTest { ...@@ -126,6 +126,7 @@ class TabStripControllerTest : public PlatformTest {
return; return;
[tab_model_ browserStateDestroyed]; [tab_model_ browserStateDestroyed];
[controller_ disconnect];
} }
web::WebTaskEnvironment task_environment_; web::WebTaskEnvironment task_environment_;
......
...@@ -88,6 +88,7 @@ ...@@ -88,6 +88,7 @@
- (void)stop { - (void)stop {
self.started = NO; self.started = NO;
[self.tabStripController disconnect];
self.tabStripController = nil; self.tabStripController = nil;
self.dispatcher = nil; self.dispatcher = nil;
self.tabModel = nil; self.tabModel = nil;
......
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