Commit 7978ea6f authored by justincohen's avatar justincohen Committed by Commit Bot

Revert "[ios] Ensure proper shutdown of Chrome on iOS."

This reverts commit 1c7e3e09.

Breaks PhoneSimulator and TabletSimulator bots:

Failed steps failed ios_chrome_integration_egtests (iphone 5 ios 10.0) on mac failed ios_chrome_settings_egtests (iphone 5 ios 10.0) on mac failed ios_chrome_ui_egtests (iphone 5 ios 10.0) on mac failed ios_chrome_external_url_egtests (iphone 5 ios 10.0) on mac failed ios_internal_chrome_external_url_egtests (iphone 5 ios 10.0) on mac failed failure reason

[0612/124347.435892:FATAL:browser_view_controller.mm(995)] Check failed: _isShutdown. -shutdown must be called before dealloc.

TBR=sdefresne@chromium.org
BUG=

Review-Url: https://codereview.chromium.org/2930363003
Cr-Commit-Position: refs/heads/master@{#478783}
parent 4396b7a7
...@@ -77,7 +77,6 @@ typedef Tab* (^mock_gurl_nsuinteger_pagetransition)(const GURL&, ...@@ -77,7 +77,6 @@ typedef Tab* (^mock_gurl_nsuinteger_pagetransition)(const GURL&,
- (void)expectNewForegroundTab; - (void)expectNewForegroundTab;
- (void)setActive:(BOOL)active; - (void)setActive:(BOOL)active;
- (TabModel*)tabModel; - (TabModel*)tabModel;
- (void)shutdown;
@end @end
@implementation URLOpenerMockBVC @implementation URLOpenerMockBVC
...@@ -111,19 +110,10 @@ typedef Tab* (^mock_gurl_nsuinteger_pagetransition)(const GURL&, ...@@ -111,19 +110,10 @@ typedef Tab* (^mock_gurl_nsuinteger_pagetransition)(const GURL&,
return nil; return nil;
} }
- (void)shutdown {
// no-op
}
@end @end
class URLOpenerTest : public PlatformTest { class URLOpenerTest : public PlatformTest {
protected: protected:
void TearDown() override {
[main_controller_ stopChromeMain];
PlatformTest::TearDown();
}
MainController* GetMainController() { MainController* GetMainController() {
if (!main_controller_) { if (!main_controller_) {
main_controller_ = [[MainController alloc] init]; main_controller_ = [[MainController alloc] init];
......
...@@ -680,7 +680,6 @@ enum class StackViewDismissalMode { NONE, NORMAL, INCOGNITO }; ...@@ -680,7 +680,6 @@ enum class StackViewDismissalMode { NONE, NORMAL, INCOGNITO };
// Initialize and set the main browser state. // Initialize and set the main browser state.
[self initializeBrowserState:chromeBrowserState]; [self initializeBrowserState:chromeBrowserState];
_mainBrowserState = chromeBrowserState; _mainBrowserState = chromeBrowserState;
[_browserViewWrangler shutdown];
_browserViewWrangler = _browserViewWrangler =
[[BrowserViewWrangler alloc] initWithBrowserState:_mainBrowserState [[BrowserViewWrangler alloc] initWithBrowserState:_mainBrowserState
tabModelObserver:self]; tabModelObserver:self];
...@@ -911,7 +910,6 @@ enum class StackViewDismissalMode { NONE, NORMAL, INCOGNITO }; ...@@ -911,7 +910,6 @@ enum class StackViewDismissalMode { NONE, NORMAL, INCOGNITO };
[_spotlightManager shutdown]; [_spotlightManager shutdown];
_spotlightManager = nil; _spotlightManager = nil;
[_browserViewWrangler shutdown];
_browserViewWrangler = nil; _browserViewWrangler = nil;
_chromeMain.reset(); _chromeMain.reset();
...@@ -2553,7 +2551,6 @@ enum class StackViewDismissalMode { NONE, NORMAL, INCOGNITO }; ...@@ -2553,7 +2551,6 @@ enum class StackViewDismissalMode { NONE, NORMAL, INCOGNITO };
// Create a BrowserViewWrangler with a null browser state. This will trigger // Create a BrowserViewWrangler with a null browser state. This will trigger
// assertions if the BrowserViewWrangler is asked to create any BVC or // assertions if the BrowserViewWrangler is asked to create any BVC or
// tabModel objects, but it will accept assignments to them. // tabModel objects, but it will accept assignments to them.
[_browserViewWrangler shutdown];
_browserViewWrangler = _browserViewWrangler =
[[BrowserViewWrangler alloc] initWithBrowserState:nullptr [[BrowserViewWrangler alloc] initWithBrowserState:nullptr
tabModelObserver:self]; tabModelObserver:self];
......
...@@ -56,11 +56,6 @@ typedef void (^HandleLaunchOptions)(id self, ...@@ -56,11 +56,6 @@ typedef void (^HandleLaunchOptions)(id self,
class TabOpenerTest : public PlatformTest { class TabOpenerTest : public PlatformTest {
protected: protected:
void TearDown() override {
[main_controller_ stopChromeMain];
PlatformTest::TearDown();
}
BOOL swizzleHasBeenCalled() { return swizzle_block_executed_; } BOOL swizzleHasBeenCalled() { return swizzle_block_executed_; }
void swizzleHandleLaunchOptions( void swizzleHandleLaunchOptions(
......
...@@ -155,9 +155,6 @@ extern NSString* const kLocationBarResignsFirstResponderNotification; ...@@ -155,9 +155,6 @@ extern NSString* const kLocationBarResignsFirstResponderNotification;
- (void)removeExternalFilesImmediately:(BOOL)immediately - (void)removeExternalFilesImmediately:(BOOL)immediately
completionHandler:(ProceduralBlock)completionHandler; completionHandler:(ProceduralBlock)completionHandler;
// Called before the instance is deallocated.
- (void)shutdown;
@end @end
#endif // IOS_CHROME_BROWSER_UI_BROWSER_VIEW_CONTROLLER_H_ #endif // IOS_CHROME_BROWSER_UI_BROWSER_VIEW_CONTROLLER_H_
...@@ -471,9 +471,6 @@ NSString* const kNativeControllerTemporaryKey = @"NativeControllerTemporaryKey"; ...@@ -471,9 +471,6 @@ NSString* const kNativeControllerTemporaryKey = @"NativeControllerTemporaryKey";
// YES if waiting for a foreground tab due to expectNewForegroundTab. // YES if waiting for a foreground tab due to expectNewForegroundTab.
BOOL _expectingForegroundTab; BOOL _expectingForegroundTab;
// Whether or not -shutdown has been called.
BOOL _isShutdown;
// The ChromeBrowserState associated with this BVC. // The ChromeBrowserState associated with this BVC.
ios::ChromeBrowserState* _browserState; // weak ios::ChromeBrowserState* _browserState; // weak
...@@ -992,7 +989,19 @@ class BrowserBookmarkModelBridge : public bookmarks::BookmarkModelObserver { ...@@ -992,7 +989,19 @@ class BrowserBookmarkModelBridge : public bookmarks::BookmarkModelObserver {
} }
- (void)dealloc { - (void)dealloc {
DCHECK(_isShutdown) << "-shutdown must be called before dealloc."; _tabStripController = nil;
_infoBarContainer = nil;
_readingListMenuNotifier = nil;
if (_bookmarkModel)
_bookmarkModel->RemoveObserver(_bookmarkModelBridge.get());
[_model removeObserver:self];
[[UpgradeCenter sharedInstance] unregisterClient:self];
[[NSNotificationCenter defaultCenter] removeObserver:self];
[_toolbarController setDelegate:nil];
if (_voiceSearchController)
_voiceSearchController->SetDelegate(nil);
[_rateThisAppDialog setDelegate:nil];
[_model closeAllTabs];
} }
#pragma mark - Accessibility #pragma mark - Accessibility
...@@ -2258,25 +2267,6 @@ class BrowserBookmarkModelBridge : public bookmarks::BookmarkModelObserver { ...@@ -2258,25 +2267,6 @@ class BrowserBookmarkModelBridge : public bookmarks::BookmarkModelObserver {
})); }));
} }
- (void)shutdown {
DCHECK(!_isShutdown);
_isShutdown = YES;
_tabStripController = nil;
_infoBarContainer = nil;
_readingListMenuNotifier = nil;
if (_bookmarkModel)
_bookmarkModel->RemoveObserver(_bookmarkModelBridge.get());
[_model removeObserver:self];
[[UpgradeCenter sharedInstance] unregisterClient:self];
[[NSNotificationCenter defaultCenter] removeObserver:self];
[_toolbarController setDelegate:nil];
if (_voiceSearchController)
_voiceSearchController->SetDelegate(nil);
[_rateThisAppDialog setDelegate:nil];
[_model closeAllTabs];
}
#pragma mark - SnapshotOverlayProvider methods #pragma mark - SnapshotOverlayProvider methods
- (NSArray*)snapshotOverlaysForTab:(Tab*)tab { - (NSArray*)snapshotOverlaysForTab:(Tab*)tab {
......
...@@ -263,8 +263,6 @@ class BrowserViewControllerTest : public BlockCleanupTest { ...@@ -263,8 +263,6 @@ class BrowserViewControllerTest : public BlockCleanupTest {
void TearDown() override { void TearDown() override {
[[bvc_ view] removeFromSuperview]; [[bvc_ view] removeFromSuperview];
[bvc_ shutdown];
BlockCleanupTest::TearDown(); BlockCleanupTest::TearDown();
} }
......
...@@ -57,9 +57,6 @@ class ChromeBrowserState; ...@@ -57,9 +57,6 @@ class ChromeBrowserState;
- (void)deleteIncognitoTabModelState: - (void)deleteIncognitoTabModelState:
(BrowsingDataRemovalController*)removalController; (BrowsingDataRemovalController*)removalController;
// Called before the instance is deallocated.
- (void)shutdown;
@end @end
@interface BrowserViewWrangler (Testing) @interface BrowserViewWrangler (Testing)
......
...@@ -27,7 +27,6 @@ ...@@ -27,7 +27,6 @@
@interface BrowserViewWrangler ()<TabModelObserver> { @interface BrowserViewWrangler ()<TabModelObserver> {
ios::ChromeBrowserState* _browserState; ios::ChromeBrowserState* _browserState;
__unsafe_unretained id<TabModelObserver> _tabModelObserver; __unsafe_unretained id<TabModelObserver> _tabModelObserver;
BOOL _isShutdown;
base::mac::ObjCPropertyReleaser _propertyReleaser_BrowserViewWrangler; base::mac::ObjCPropertyReleaser _propertyReleaser_BrowserViewWrangler;
} }
...@@ -83,7 +82,25 @@ ...@@ -83,7 +82,25 @@
} }
- (void)dealloc { - (void)dealloc {
DCHECK(_isShutdown) << "-shutdown must be called before -dealloc"; if (_tabModelObserver) {
[_mainTabModel removeObserver:_tabModelObserver];
[_otrTabModel removeObserver:_tabModelObserver];
}
[_mainTabModel removeObserver:self];
[_otrTabModel removeObserver:self];
// Stop URL monitoring of the main tab model.
ios_internal::breakpad::StopMonitoringURLsForTabModel(_mainTabModel);
// Stop Breakpad state monitoring of both tab models (if necessary).
ios_internal::breakpad::StopMonitoringTabStateForTabModel(_mainTabModel);
ios_internal::breakpad::StopMonitoringTabStateForTabModel(_otrTabModel);
// Normally other objects will take care of unhooking the tab models from
// the browser state, but this code should ensure that it happens regardless.
[_mainTabModel browserStateDestroyed];
[_otrTabModel browserStateDestroyed];
[super dealloc]; [super dealloc];
} }
...@@ -266,39 +283,6 @@ ...@@ -266,39 +283,6 @@
} }
} }
- (void)shutdown {
DCHECK(!_isShutdown);
_isShutdown = YES;
if (_tabModelObserver) {
[_mainTabModel removeObserver:_tabModelObserver];
[_otrTabModel removeObserver:_tabModelObserver];
_tabModelObserver = nil;
}
[_mainTabModel removeObserver:self];
[_otrTabModel removeObserver:self];
// Stop URL monitoring of the main tab model.
ios_internal::breakpad::StopMonitoringURLsForTabModel(_mainTabModel);
// Stop Breakpad state monitoring of both tab models (if necessary).
ios_internal::breakpad::StopMonitoringTabStateForTabModel(_mainTabModel);
ios_internal::breakpad::StopMonitoringTabStateForTabModel(_otrTabModel);
// Normally other objects will take care of unhooking the tab models from
// the browser state, but this code should ensure that it happens regardless.
[_mainTabModel browserStateDestroyed];
[_otrTabModel browserStateDestroyed];
[_mainBVC shutdown];
[_otrBVC shutdown];
self.mainBVC = nil;
self.otrBVC = nil;
_browserState = nullptr;
}
#pragma mark - Internal methods #pragma mark - Internal methods
- (TabModel*)buildOtrTabModel:(BOOL)empty { - (TabModel*)buildOtrTabModel:(BOOL)empty {
......
...@@ -50,8 +50,6 @@ TEST_F(BrowserViewWranglerTest, TestInitNilObserver) { ...@@ -50,8 +50,6 @@ TEST_F(BrowserViewWranglerTest, TestInitNilObserver) {
EXPECT_NE(bvc, [wrangler otrBVC]); EXPECT_NE(bvc, [wrangler otrBVC]);
EXPECT_NE(tabModel, [wrangler otrTabModel]); EXPECT_NE(tabModel, [wrangler otrTabModel]);
EXPECT_TRUE([wrangler otrTabModel].browserState->IsOffTheRecord()); EXPECT_TRUE([wrangler otrTabModel].browserState->IsOffTheRecord());
[wrangler shutdown];
} }
} // namespace } // namespace
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