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

Side by Side Diff: ios/shared/chrome/browser/tabs/web_state_list.mm

Issue 2710183003: Fix WebStateListObserver::WebStateDetachedAt() parameter lifetime. (Closed)
Patch Set: Fix typos. Created 3 years, 9 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 unified diff | Download patch
OLDNEW
1 // Copyright 2017 The Chromium Authors. All rights reserved. 1 // Copyright 2017 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #import "ios/shared/chrome/browser/tabs/web_state_list.h" 5 #import "ios/shared/chrome/browser/tabs/web_state_list.h"
6 6
7 #include <utility> 7 #include <utility>
8 8
9 #include "base/logging.h" 9 #include "base/logging.h"
10 #include "base/memory/ptr_util.h" 10 #include "base/memory/ptr_util.h"
11 #import "ios/shared/chrome/browser/tabs/web_state_list_observer.h" 11 #import "ios/shared/chrome/browser/tabs/web_state_list_observer.h"
12 #import "ios/shared/chrome/browser/tabs/web_state_list_order_controller.h" 12 #import "ios/shared/chrome/browser/tabs/web_state_list_order_controller.h"
13 #import "ios/web/public/navigation_manager.h" 13 #import "ios/web/public/navigation_manager.h"
14 #import "ios/web/public/web_state/web_state.h" 14 #import "ios/web/public/web_state/web_state.h"
15 15
16 #if !defined(__has_feature) || !__has_feature(objc_arc) 16 #if !defined(__has_feature) || !__has_feature(objc_arc)
17 #error "This file requires ARC support." 17 #error "This file requires ARC support."
18 #endif 18 #endif
19 19
20 // Wrapper around a WebState stored in a WebStateList. May own the WebState 20 // Wrapper around a WebState stored in a WebStateList.
21 // dependending on the WebStateList ownership setting (should always be true
22 // once ownership of Tab is sane, see http://crbug.com/546222 for progress).
23 class WebStateList::WebStateWrapper { 21 class WebStateList::WebStateWrapper {
24 public: 22 public:
25 WebStateWrapper(web::WebState* web_state, bool assume_ownership); 23 explicit WebStateWrapper(web::WebState* web_state);
26 ~WebStateWrapper(); 24 ~WebStateWrapper();
27 25
28 web::WebState* web_state() const { return web_state_; } 26 web::WebState* web_state() const { return web_state_; }
29 web::WebState* opener() const { return opener_; } 27 web::WebState* opener() const { return opener_; }
30 28
31 // Replaces the wrapped WebState (and clear associated state) and returns the 29 // Replaces the wrapped WebState (and clear associated state) and returns the
32 // old WebState after forfeiting ownership. 30 // old WebState after forfeiting ownership.
33 web::WebState* ReplaceWebState(web::WebState* web_state); 31 web::WebState* ReplaceWebState(web::WebState* web_state);
34 32
35 // Sets the opener for the wrapped WebState and record the opener navigation 33 // Sets the opener for the wrapped WebState and record the opener navigation
36 // index to allow detecting navigation changes during the same session. 34 // index to allow detecting navigation changes during the same session.
37 void SetOpener(web::WebState* opener); 35 void SetOpener(web::WebState* opener);
38 36
39 // Returns whether |opener| spawned the wrapped WebState. If |use_group| is 37 // Returns whether |opener| spawned the wrapped WebState. If |use_group| is
40 // true, also use the opener navigation index to detect navigation changes 38 // true, also use the opener navigation index to detect navigation changes
41 // during the same session. 39 // during the same session.
42 bool WasOpenedBy(const web::WebState* opener, 40 bool WasOpenedBy(const web::WebState* opener,
43 int opener_navigation_index, 41 int opener_navigation_index,
44 bool use_group) const; 42 bool use_group) const;
45 43
46 private: 44 private:
47 web::WebState* web_state_; 45 web::WebState* web_state_;
48 web::WebState* opener_ = nullptr; 46 web::WebState* opener_ = nullptr;
49 int opener_last_committed_index_; 47 int opener_last_committed_index_;
50 const bool has_web_state_ownership_;
51 48
52 DISALLOW_COPY_AND_ASSIGN(WebStateWrapper); 49 DISALLOW_COPY_AND_ASSIGN(WebStateWrapper);
53 }; 50 };
54 51
55 WebStateList::WebStateWrapper::WebStateWrapper(web::WebState* web_state, 52 WebStateList::WebStateWrapper::WebStateWrapper(web::WebState* web_state)
56 bool assume_ownership) 53 : web_state_(web_state) {
57 : web_state_(web_state), has_web_state_ownership_(assume_ownership) {
58 DCHECK(web_state_); 54 DCHECK(web_state_);
59 } 55 }
60 56
61 WebStateList::WebStateWrapper::~WebStateWrapper() { 57 WebStateList::WebStateWrapper::~WebStateWrapper() = default;
62 if (has_web_state_ownership_)
63 delete web_state_;
64 }
65 58
66 web::WebState* WebStateList::WebStateWrapper::ReplaceWebState( 59 web::WebState* WebStateList::WebStateWrapper::ReplaceWebState(
67 web::WebState* web_state) { 60 web::WebState* web_state) {
68 DCHECK(web_state); 61 DCHECK(web_state);
69 DCHECK_NE(web_state, web_state_); 62 DCHECK_NE(web_state, web_state_);
70 std::swap(web_state, web_state_); 63 std::swap(web_state, web_state_);
71 opener_ = nullptr; 64 opener_ = nullptr;
72 return web_state; 65 return web_state;
73 } 66 }
74 67
(...skipping 15 matching lines...) Expand all
90 if (!use_group) 83 if (!use_group)
91 return true; 84 return true;
92 85
93 return opener_last_committed_index_ == opener_navigation_index; 86 return opener_last_committed_index_ == opener_navigation_index;
94 } 87 }
95 88
96 WebStateList::WebStateList(WebStateOwnership ownership) 89 WebStateList::WebStateList(WebStateOwnership ownership)
97 : web_state_ownership_(ownership), 90 : web_state_ownership_(ownership),
98 order_controller_(base::MakeUnique<WebStateListOrderController>(this)) {} 91 order_controller_(base::MakeUnique<WebStateListOrderController>(this)) {}
99 92
100 WebStateList::~WebStateList() = default; 93 WebStateList::~WebStateList() {
94 // Once WebStateList owns the WebState and has a CloseWebStateAt() method,
95 // then change this to close all the WebState. See http://crbug.com/546222
96 // for progress.
97 if (web_state_ownership_ == WebStateOwned) {
98 for (auto& web_state_wrapper : web_state_wrappers_) {
99 web::WebState* web_state = web_state_wrapper->web_state();
100 delete web_state;
101 }
102 }
103 }
101 104
102 bool WebStateList::ContainsIndex(int index) const { 105 bool WebStateList::ContainsIndex(int index) const {
103 return 0 <= index && index < count(); 106 return 0 <= index && index < count();
104 } 107 }
105 108
106 web::WebState* WebStateList::GetWebStateAt(int index) const { 109 web::WebState* WebStateList::GetWebStateAt(int index) const {
107 DCHECK(ContainsIndex(index)); 110 DCHECK(ContainsIndex(index));
108 return web_state_wrappers_[index]->web_state(); 111 return web_state_wrappers_[index]->web_state();
109 } 112 }
110 113
(...skipping 25 matching lines...) Expand all
136 int WebStateList::GetIndexOfLastWebStateOpenedBy(const web::WebState* opener, 139 int WebStateList::GetIndexOfLastWebStateOpenedBy(const web::WebState* opener,
137 int start_index, 140 int start_index,
138 bool use_group) const { 141 bool use_group) const {
139 return GetIndexOfNthWebStateOpenedBy(opener, start_index, use_group, INT_MAX); 142 return GetIndexOfNthWebStateOpenedBy(opener, start_index, use_group, INT_MAX);
140 } 143 }
141 144
142 void WebStateList::InsertWebState(int index, 145 void WebStateList::InsertWebState(int index,
143 web::WebState* web_state, 146 web::WebState* web_state,
144 web::WebState* opener) { 147 web::WebState* opener) {
145 DCHECK(ContainsIndex(index) || index == count()); 148 DCHECK(ContainsIndex(index) || index == count());
146 web_state_wrappers_.insert( 149 web_state_wrappers_.insert(web_state_wrappers_.begin() + index,
147 web_state_wrappers_.begin() + index, 150 base::MakeUnique<WebStateWrapper>(web_state));
148 base::MakeUnique<WebStateWrapper>(web_state,
149 web_state_ownership_ == WebStateOwned));
150 151
151 if (opener) 152 if (opener)
152 SetOpenerOfWebStateAt(index, opener); 153 SetOpenerOfWebStateAt(index, opener);
153 154
154 for (auto& observer : observers_) 155 for (auto& observer : observers_)
155 observer.WebStateInsertedAt(this, web_state, index); 156 observer.WebStateInsertedAt(this, web_state, index);
156 } 157 }
157 158
158 void WebStateList::AppendWebState(ui::PageTransition transition, 159 void WebStateList::AppendWebState(ui::PageTransition transition,
159 web::WebState* web_state, 160 web::WebState* web_state,
(...skipping 33 matching lines...) Expand 10 before | Expand all | Expand 10 after
193 194
194 if (opener && opener != old_web_state) 195 if (opener && opener != old_web_state)
195 SetOpenerOfWebStateAt(index, opener); 196 SetOpenerOfWebStateAt(index, opener);
196 197
197 for (auto& observer : observers_) 198 for (auto& observer : observers_)
198 observer.WebStateReplacedAt(this, old_web_state, web_state, index); 199 observer.WebStateReplacedAt(this, old_web_state, web_state, index);
199 200
200 return old_web_state; 201 return old_web_state;
201 } 202 }
202 203
203 void WebStateList::DetachWebStateAt(int index) { 204 web::WebState* WebStateList::DetachWebStateAt(int index) {
204 DCHECK(ContainsIndex(index)); 205 DCHECK(ContainsIndex(index));
205 ClearOpenersReferencing(index); 206 ClearOpenersReferencing(index);
206 207
207 web::WebState* web_state = web_state_wrappers_[index]->web_state(); 208 web::WebState* old_web_state = web_state_wrappers_[index]->web_state();
208 web_state_wrappers_.erase(web_state_wrappers_.begin() + index); 209 web_state_wrappers_.erase(web_state_wrappers_.begin() + index);
209 210
210 for (auto& observer : observers_) 211 for (auto& observer : observers_)
211 observer.WebStateDetachedAt(this, web_state, index); 212 observer.WebStateDetachedAt(this, old_web_state, index);
213
214 return old_web_state;
212 } 215 }
213 216
214 void WebStateList::AddObserver(WebStateListObserver* observer) { 217 void WebStateList::AddObserver(WebStateListObserver* observer) {
215 observers_.AddObserver(observer); 218 observers_.AddObserver(observer);
216 } 219 }
217 220
218 void WebStateList::RemoveObserver(WebStateListObserver* observer) { 221 void WebStateList::RemoveObserver(WebStateListObserver* observer) {
219 observers_.RemoveObserver(observer); 222 observers_.RemoveObserver(observer);
220 } 223 }
221 224
(...skipping 25 matching lines...) Expand all
247 } else if (found_index != kInvalidIndex) { 250 } else if (found_index != kInvalidIndex) {
248 return found_index; 251 return found_index;
249 } 252 }
250 } 253 }
251 254
252 return found_index; 255 return found_index;
253 } 256 }
254 257
255 // static 258 // static
256 const int WebStateList::kInvalidIndex; 259 const int WebStateList::kInvalidIndex;
OLDNEW
« no previous file with comments | « ios/shared/chrome/browser/tabs/web_state_list.h ('k') | ios/shared/chrome/browser/tabs/web_state_list_unittest.mm » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698