Chromium Code Reviews| Index: ios/shared/chrome/browser/tabs/web_state_list.mm |
| diff --git a/ios/shared/chrome/browser/tabs/web_state_list.mm b/ios/shared/chrome/browser/tabs/web_state_list.mm |
| index 283ddeed94cf9d66f914727c8c0bb15f36218bc3..67c1c404fb98e8714fe0038e545c0c7d870689f3 100644 |
| --- a/ios/shared/chrome/browser/tabs/web_state_list.mm |
| +++ b/ios/shared/chrome/browser/tabs/web_state_list.mm |
| @@ -16,12 +16,10 @@ |
| #error "This file requires ARC support." |
| #endif |
| -// Wrapper around a WebState stored in a WebStateList. May own the WebState |
| -// dependending on the WebStateList ownership setting (should always be true |
| -// once ownership of Tab is sane, see http://crbug.com/546222 for progress). |
| +// Wrapper around a WebState stored in a WebStateList. |
| class WebStateList::WebStateWrapper { |
| public: |
| - WebStateWrapper(web::WebState* web_state, bool assume_ownership); |
| + WebStateWrapper(web::WebState* web_state); |
|
pkl (ping after 24h if needed)
2017/02/27 19:32:09
I think the style guide mandates "explicit" for si
sdefresne
2017/02/28 05:52:52
Done. Thank you.
|
| ~WebStateWrapper(); |
| web::WebState* web_state() const { return web_state_; } |
| @@ -46,21 +44,16 @@ class WebStateList::WebStateWrapper { |
| web::WebState* web_state_; |
| web::WebState* opener_ = nullptr; |
| int opener_last_committed_index_; |
| - const bool has_web_state_ownership_; |
| DISALLOW_COPY_AND_ASSIGN(WebStateWrapper); |
| }; |
| -WebStateList::WebStateWrapper::WebStateWrapper(web::WebState* web_state, |
| - bool assume_ownership) |
| - : web_state_(web_state), has_web_state_ownership_(assume_ownership) { |
| +WebStateList::WebStateWrapper::WebStateWrapper(web::WebState* web_state) |
| + : web_state_(web_state) { |
| DCHECK(web_state_); |
| } |
| -WebStateList::WebStateWrapper::~WebStateWrapper() { |
| - if (has_web_state_ownership_) |
| - delete web_state_; |
| -} |
| +WebStateList::WebStateWrapper::~WebStateWrapper() = default; |
| web::WebState* WebStateList::WebStateWrapper::ReplaceWebState( |
| web::WebState* web_state) { |
| @@ -95,7 +88,17 @@ bool WebStateList::WebStateWrapper::WasOpenedBy(const web::WebState* opener, |
| WebStateList::WebStateList(WebStateOwnership ownership) |
| : web_state_ownership_(ownership) {} |
| -WebStateList::~WebStateList() = default; |
| +WebStateList::~WebStateList() { |
| + // Once WebStateList owns the WebState and has a CloseWebStateAt() method, |
| + // then change this to close all the WebState. See http://crbug.com/546222 |
| + // for progress. |
| + if (web_state_ownership_ == WebStateOwned) { |
| + for (auto& web_state_wrapper : web_state_wrappers_) { |
| + web::WebState* web_state = web_state_wrapper->web_state(); |
| + delete web_state; |
| + } |
| + } |
| +} |
| bool WebStateList::ContainsIndex(int index) const { |
| return 0 <= index && index < count(); |
| @@ -141,10 +144,8 @@ void WebStateList::InsertWebState(int index, |
| web::WebState* web_state, |
| web::WebState* opener) { |
| DCHECK(ContainsIndex(index) || index == count()); |
| - web_state_wrappers_.insert( |
| - web_state_wrappers_.begin() + index, |
| - base::MakeUnique<WebStateWrapper>(web_state, |
| - web_state_ownership_ == WebStateOwned)); |
| + web_state_wrappers_.insert(web_state_wrappers_.begin() + index, |
| + base::MakeUnique<WebStateWrapper>(web_state)); |
| if (opener) |
| SetOpenerOfWebStateAt(index, opener); |
| @@ -188,15 +189,17 @@ web::WebState* WebStateList::ReplaceWebStateAt(int index, |
| return old_web_state; |
| } |
| -void WebStateList::DetachWebStateAt(int index) { |
| +web::WebState* WebStateList::DetachWebStateAt(int index) { |
| DCHECK(ContainsIndex(index)); |
| ClearOpenersReferencing(index); |
| - web::WebState* web_state = web_state_wrappers_[index]->web_state(); |
| + web::WebState* old_web_state = web_state_wrappers_[index]->web_state(); |
| web_state_wrappers_.erase(web_state_wrappers_.begin() + index); |
| for (auto& observer : observers_) |
| - observer.WebStateDetachedAt(this, web_state, index); |
| + observer.WebStateDetachedAt(this, old_web_state, index); |
| + |
| + return old_web_state; |
| } |
| void WebStateList::AddObserver(WebStateListObserver* observer) { |