Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(82)

Unified Diff: ios/chrome/browser/tabs/tab_model.mm

Issue 2885803002: [ios] Remove TabModel cleanup from -dealloc to -browserStateDestroyed. (Closed)
Patch Set: Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « ios/chrome/browser/tabs/tab.mm ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: ios/chrome/browser/tabs/tab_model.mm
diff --git a/ios/chrome/browser/tabs/tab_model.mm b/ios/chrome/browser/tabs/tab_model.mm
index b3d60d2cece4a7a975cffeb3d1e03ca06de77f0b..b1ccdfcf7727513b7634d86babbad71f32cc8921 100644
--- a/ios/chrome/browser/tabs/tab_model.mm
+++ b/ios/chrome/browser/tabs/tab_model.mm
@@ -191,36 +191,11 @@ void CleanCertificatePolicyCache(
#pragma mark - Overriden
- (void)dealloc {
- // Make sure the tabs do clean after themselves. It is important for
- // removeObserver: to be called first otherwise a lot of unecessary work will
rohitrao (ping after 24h) 2017/05/16 13:28:10 We are changing the semantics here. It seems like
- // happen on -closeAllTabs.
- DCHECK([_observers empty]);
-
// browserStateDestroyed should always have been called before destruction.
DCHECK(!_browserState);
- [[NSNotificationCenter defaultCenter] removeObserver:self];
-
- // Clear weak pointer to WebStateListMetricsObserver before destroying it.
- _webStateListMetricsObserver = nullptr;
-
- // Close all tabs. Do this in an @autoreleasepool as WebStateList observers
- // will be notified (they are unregistered later). As some of them may be
- // implemented in Objective-C and unregister themselves in their -dealloc
- // method, ensure they -autorelease introduced by ARC are processed before
- // the WebStateList destructor is called.
- @autoreleasepool {
- [self closeAllTabs];
- }
-
- // Unregister all observers after closing all the tabs as some of them are
- // required to properly clean up the Tabs.
- for (const auto& webStateListObserver : _webStateListObservers)
- _webStateList->RemoveObserver(webStateListObserver.get());
- _webStateListObservers.clear();
- _retainedWebStateListObservers = nil;
-
- _clearPoliciesTaskTracker.TryCancelAll();
+ // Make sure the observers do clean after themselves.
+ DCHECK([_observers empty]);
}
#pragma mark - Public methods
@@ -631,11 +606,33 @@ void CleanCertificatePolicyCache(
// NOTE: This can be called multiple times, so must be robust against that.
- (void)browserStateDestroyed {
+ if (!_browserState)
+ return;
+
[[NSNotificationCenter defaultCenter] removeObserver:self];
- if (_browserState) {
- UnregisterTabModelFromChromeBrowserState(_browserState, self);
- }
+ UnregisterTabModelFromChromeBrowserState(_browserState, self);
_browserState = nullptr;
+
+ // Clear weak pointer to WebStateListMetricsObserver before destroying it.
+ _webStateListMetricsObserver = nullptr;
+
+ // Close all tabs. Do this in an @autoreleasepool as WebStateList observers
+ // will be notified (they are unregistered later). As some of them may be
+ // implemented in Objective-C and unregister themselves in their -dealloc
+ // method, ensure they -autorelease introduced by ARC are processed before
+ // the WebStateList destructor is called.
+ @autoreleasepool {
+ [self closeAllTabs];
+ }
+
+ // Unregister all observers after closing all the tabs as some of them are
+ // required to properly clean up the Tabs.
+ for (const auto& webStateListObserver : _webStateListObservers)
+ _webStateList->RemoveObserver(webStateListObserver.get());
+ _webStateListObservers.clear();
+ _retainedWebStateListObservers = nil;
+
+ _clearPoliciesTaskTracker.TryCancelAll();
}
- (void)navigationCommittedInTab:(Tab*)tab
« no previous file with comments | « ios/chrome/browser/tabs/tab.mm ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698