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

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

Issue 2710183003: Fix WebStateListObserver::WebStateDetachedAt() parameter lifetime. (Closed)
Patch Set: Fix typos. Created 3 years, 10 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 | « no previous file | ios/shared/chrome/browser/tabs/web_state_list.h » ('j') | 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 310c4a190e9eabfbffdca09249af53cd9bfa5c49..d3733af805c2b21291280463685e09db8fde7efb 100644
--- a/ios/chrome/browser/tabs/tab_model.mm
+++ b/ios/chrome/browser/tabs/tab_model.mm
@@ -627,8 +627,14 @@ Tab* GetOpenerForTab(id<NSFastEnumeration> tabs, Tab* tab) {
[_tabRetainer addObject:newTab];
[newTab setParentTabModel:self];
- _webStateList.ReplaceWebStateAt(index, newTab.webState,
- GetOpenerForTab(self, newTab).webState);
+ // The WebState is owned by the associated Tab, so it is safe to ignore
+ // the result and won't cause a memory leak. Once the ownership is moved
+ // to WebStateList, this function will return a std::unique_ptr<> and the
+ // object destroyed as expected, so it will fine to ignore the result then
+ // too. See http://crbug.com/546222 for progress of changing the ownership
+ // of the WebStates.
+ ignore_result(_webStateList.ReplaceWebStateAt(
+ index, newTab.webState, GetOpenerForTab(self, newTab).webState));
if (self.currentTab == oldTab)
[self changeSelectedTabFrom:nil to:newTab persistState:NO];
@@ -792,7 +798,13 @@ Tab* GetOpenerForTab(id<NSFastEnumeration> tabs, Tab* tab) {
DCHECK([_tabRetainer containsObject:closedTab]);
[_tabRetainer removeObject:closedTab];
- _webStateList.DetachWebStateAt(closedTabIndex);
+ // The WebState is owned by the associated Tab, so it is safe to ignore
+ // the result and won't cause a memory leak. Once the ownership is moved
+ // to WebStateList, this function will return a std::unique_ptr<> and the
+ // object destroyed as expected, so it will fine to ignore the result then
+ // too. See http://crbug.com/546222 for progress of changing the ownership
+ // of the WebStates.
+ ignore_result(_webStateList.DetachWebStateAt(closedTabIndex));
// Current tab has closed, update the selected tab and swap in its
// contents. There is nothing to do if a non-selected tab is closed as
« no previous file with comments | « no previous file | ios/shared/chrome/browser/tabs/web_state_list.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698