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

Side by Side Diff: components/enhanced_bookmarks/bookmark_image_service.cc

Issue 1031293002: Fix crashes due to gfx::Image unsafe thread passing (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Remove unnecessary Pass() Created 5 years, 8 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 2014 The Chromium Authors. All rights reserved. 1 // Copyright 2014 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 #include "components/enhanced_bookmarks/bookmark_image_service.h" 5 #include "components/enhanced_bookmarks/bookmark_image_service.h"
6 6
7 #include "base/single_thread_task_runner.h" 7 #include "base/single_thread_task_runner.h"
8 #include "base/task_runner_util.h" 8 #include "base/task_runner_util.h"
9 #include "base/thread_task_runner_handle.h" 9 #include "base/thread_task_runner_handle.h"
10 #include "base/threading/sequenced_worker_pool.h" 10 #include "base/threading/sequenced_worker_pool.h"
(...skipping 130 matching lines...) Expand 10 before | Expand all | Expand 10 after
141 RetrieveSalientImage( 141 RetrieveSalientImage(
142 page_url, 142 page_url,
143 image_url, 143 image_url,
144 "", 144 "",
145 net::URLRequest::CLEAR_REFERRER_ON_TRANSITION_FROM_SECURE_TO_INSECURE, 145 net::URLRequest::CLEAR_REFERRER_ON_TRANSITION_FROM_SECURE_TO_INSECURE,
146 false); 146 false);
147 } 147 }
148 148
149 void BookmarkImageService::FetchCallback(const GURL& page_url, 149 void BookmarkImageService::FetchCallback(const GURL& page_url,
150 ImageCallback original_callback, 150 ImageCallback original_callback,
151 const ImageRecord& record) { 151 scoped_refptr<ImageRecord> record) {
152 DCHECK(CalledOnValidThread()); 152 DCHECK(CalledOnValidThread());
153 if (!record.image.IsEmpty() || !record.url.is_empty()) { 153 if (!record->image->IsEmpty() || !record->url.is_empty()) {
154 // Either the record was in the store or there is no image in the store, but 154 // Either the record was in the store or there is no image in the store, but
155 // an URL for a record is present, indicating that a previous attempt to 155 // an URL for a record is present, indicating that a previous attempt to
156 // download the image failed. Just return the record. 156 // download the image failed. Just return the record.
157 original_callback.Run(record); 157 original_callback.Run(record);
158 } else { 158 } else {
159 // There is no record in the store, and no previous attempts to retrieve 159 // There is no record in the store, and no previous attempts to retrieve
160 // one. Start a request to retrieve a salient image if there is an image 160 // one. Start a request to retrieve a salient image if there is an image
161 // url set on a bookmark, and then enqueue the request for the record to 161 // url set on a bookmark, and then enqueue the request for the record to
162 // be triggered when the retrieval is finished. 162 // be triggered when the retrieval is finished.
163 RetrieveSalientImageForPageUrl(page_url); 163 RetrieveSalientImageForPageUrl(page_url);
(...skipping 20 matching lines...) Expand all
184 base::Bind(&BookmarkImageService::FetchCallback, 184 base::Bind(&BookmarkImageService::FetchCallback,
185 base::Unretained(this), 185 base::Unretained(this),
186 page_url, 186 page_url,
187 callback)); 187 callback));
188 } 188 }
189 } 189 }
190 190
191 void BookmarkImageService::ProcessNewImage(const GURL& page_url, 191 void BookmarkImageService::ProcessNewImage(const GURL& page_url,
192 bool update_bookmarks, 192 bool update_bookmarks,
193 const GURL& image_url, 193 const GURL& image_url,
194 const gfx::Image& image) { 194 scoped_ptr<gfx::Image> image) {
195 DCHECK(CalledOnValidThread()); 195 DCHECK(CalledOnValidThread());
196 PostTaskToStoreImage(image, image_url, page_url); 196
197 gfx::Size size = image->Size();
198 PostTaskToStoreImage(image.Pass(), image_url, page_url);
199
197 if (update_bookmarks && image_url.is_valid()) { 200 if (update_bookmarks && image_url.is_valid()) {
198 const BookmarkNode* bookmark = 201 const BookmarkNode* bookmark =
199 enhanced_bookmark_model_->bookmark_model() 202 enhanced_bookmark_model_->bookmark_model()
200 ->GetMostRecentlyAddedUserNodeForURL(page_url); 203 ->GetMostRecentlyAddedUserNodeForURL(page_url);
201 if (bookmark) { 204 if (bookmark) {
202 const gfx::Size& size = image.Size(); 205
203 bool result = enhanced_bookmark_model_->SetOriginalImage( 206 bool result = enhanced_bookmark_model_->SetOriginalImage(
204 bookmark, image_url, size.width(), size.height()); 207 bookmark, image_url, size.width(), size.height());
205 DCHECK(result); 208 DCHECK(result);
206 } 209 }
207 } 210 }
208 } 211 }
209 212
210 bool BookmarkImageService::IsPageUrlInProgress(const GURL& page_url) { 213 bool BookmarkImageService::IsPageUrlInProgress(const GURL& page_url) {
211 DCHECK(CalledOnValidThread()); 214 DCHECK(CalledOnValidThread());
212 return in_progress_page_urls_.find(page_url) != in_progress_page_urls_.end(); 215 return in_progress_page_urls_.find(page_url) != in_progress_page_urls_.end();
213 } 216 }
214 217
215 ImageRecord BookmarkImageService::ResizeAndStoreImage(const gfx::Image& image, 218 scoped_refptr<ImageRecord> BookmarkImageService::ResizeAndStoreImage(
216 const GURL& image_url, 219 scoped_refptr<ImageRecord> image_info,
217 const GURL& page_url) { 220 const GURL& page_url) {
218 gfx::Image resized_image = ResizeImage(image); 221
219 ImageRecord image_info(resized_image, image_url, SK_ColorBLACK); 222 if (!image_info->image->IsEmpty()) {
220 if (!resized_image.IsEmpty()) { 223 image_info->image = ResizeImage(*image_info->image);
221 image_info.dominant_color = DominantColorForImage(resized_image); 224 image_info->dominant_color = DominantColorForImage(*image_info->image);
222 // TODO(lpromero): this should be saved all the time, even when there is an 225 // TODO(lpromero): this should be saved all the time, even when there is an
223 // empty image. http://crbug.com/451450 226 // empty image. http://crbug.com/451450
224 pool_->PostNamedSequencedWorkerTask( 227 pool_->PostNamedSequencedWorkerTask(
225 kSequenceToken, FROM_HERE, 228 kSequenceToken, FROM_HERE,
226 base::Bind(&ImageStore::Insert, base::Unretained(store_.get()), 229 base::Bind(&ImageStore::Insert, base::Unretained(store_.get()),
227 page_url, image_info)); 230 page_url, image_info));
228 } 231 }
232
229 return image_info; 233 return image_info;
230 } 234 }
231 235
232 void BookmarkImageService::PostTaskToStoreImage(const gfx::Image& image, 236 void BookmarkImageService::PostTaskToStoreImage(
233 const GURL& image_url, 237 scoped_ptr<gfx::Image> image,
234 const GURL& page_url) { 238 const GURL& image_url,
239 const GURL& page_url) {
235 DCHECK(CalledOnValidThread()); 240 DCHECK(CalledOnValidThread());
236 241
237 base::Callback<ImageRecord(void)> task = 242 scoped_refptr<ImageRecord> image_info(
243 new ImageRecord(image.Pass(), image_url));
244
245 // TODO ensure image thread safety.
246 base::Callback<scoped_refptr<ImageRecord>(void)> task =
238 base::Bind(&BookmarkImageService::ResizeAndStoreImage, 247 base::Bind(&BookmarkImageService::ResizeAndStoreImage,
239 base::Unretained(this), image, image_url, page_url); 248 base::Unretained(this), image_info, page_url);
240 base::Callback<void(const ImageRecord&)> reply = 249 base::Callback<void(scoped_refptr<ImageRecord>)> reply =
241 base::Bind(&BookmarkImageService::OnStoreImagePosted, 250 base::Bind(&BookmarkImageService::OnStoreImagePosted,
242 base::Unretained(this), page_url); 251 base::Unretained(this), page_url);
243 252
244 base::PostTaskAndReplyWithResult(pool_.get(), FROM_HERE, task, reply); 253 base::PostTaskAndReplyWithResult(pool_.get(), FROM_HERE, task, reply);
245 } 254 }
246 255
247 void BookmarkImageService::OnStoreImagePosted(const GURL& page_url, 256 void BookmarkImageService::OnStoreImagePosted(
248 const ImageRecord& image) { 257 const GURL& page_url,
258 scoped_refptr<ImageRecord> image) {
249 DCHECK(CalledOnValidThread()); 259 DCHECK(CalledOnValidThread());
250 in_progress_page_urls_.erase(page_url); 260 in_progress_page_urls_.erase(page_url);
251 ProcessRequests(page_url, image); 261 ProcessRequests(page_url, image);
252 } 262 }
253 263
254 void BookmarkImageService::RemoveImageForUrl(const GURL& page_url) { 264 void BookmarkImageService::RemoveImageForUrl(const GURL& page_url) {
255 DCHECK(CalledOnValidThread()); 265 DCHECK(CalledOnValidThread());
256 pool_->PostNamedSequencedWorkerTask( 266 pool_->PostNamedSequencedWorkerTask(
257 kSequenceToken, 267 kSequenceToken,
258 FROM_HERE, 268 FROM_HERE,
259 base::Bind(&ImageStore::Erase, base::Unretained(store_.get()), page_url)); 269 base::Bind(&ImageStore::Erase, base::Unretained(store_.get()), page_url));
260 in_progress_page_urls_.erase(page_url); 270 in_progress_page_urls_.erase(page_url);
261 ProcessRequests(page_url, ImageRecord()); 271 ProcessRequests(page_url, scoped_refptr<ImageRecord>(new ImageRecord()));
262 } 272 }
263 273
264 void BookmarkImageService::ChangeImageURL(const GURL& from, const GURL& to) { 274 void BookmarkImageService::ChangeImageURL(const GURL& from, const GURL& to) {
265 DCHECK(CalledOnValidThread()); 275 DCHECK(CalledOnValidThread());
266 pool_->PostNamedSequencedWorkerTask(kSequenceToken, 276 pool_->PostNamedSequencedWorkerTask(kSequenceToken,
267 FROM_HERE, 277 FROM_HERE,
268 base::Bind(&ImageStore::ChangeImageURL, 278 base::Bind(&ImageStore::ChangeImageURL,
269 base::Unretained(store_.get()), 279 base::Unretained(store_.get()),
270 from, 280 from,
271 to)); 281 to));
272 in_progress_page_urls_.erase(from); 282 in_progress_page_urls_.erase(from);
273 ProcessRequests(from, ImageRecord()); 283 ProcessRequests(from, scoped_refptr<ImageRecord>(new ImageRecord()));
274 } 284 }
275 285
276 void BookmarkImageService::ClearAll() { 286 void BookmarkImageService::ClearAll() {
277 DCHECK(CalledOnValidThread()); 287 DCHECK(CalledOnValidThread());
278 // Clears and executes callbacks. 288 // Clears and executes callbacks.
279 pool_->PostNamedSequencedWorkerTask( 289 pool_->PostNamedSequencedWorkerTask(
280 kSequenceToken, 290 kSequenceToken,
281 FROM_HERE, 291 FROM_HERE,
282 base::Bind(&ImageStore::ClearAll, base::Unretained(store_.get()))); 292 base::Bind(&ImageStore::ClearAll, base::Unretained(store_.get())));
283 293
284 for (std::map<const GURL, std::vector<ImageCallback>>::const_iterator it = 294 for (std::map<const GURL, std::vector<ImageCallback>>::const_iterator it =
285 callbacks_.begin(); 295 callbacks_.begin();
286 it != callbacks_.end(); ++it) { 296 it != callbacks_.end(); ++it) {
287 ProcessRequests(it->first, ImageRecord()); 297 ProcessRequests(it->first, scoped_refptr<ImageRecord>(new ImageRecord()));
288 } 298 }
289 299
290 in_progress_page_urls_.erase(in_progress_page_urls_.begin(), 300 in_progress_page_urls_.erase(in_progress_page_urls_.begin(),
291 in_progress_page_urls_.end()); 301 in_progress_page_urls_.end());
292 } 302 }
293 303
294 void BookmarkImageService::ProcessRequests(const GURL& page_url, 304 void BookmarkImageService::ProcessRequests(const GURL& page_url,
295 const ImageRecord& record) { 305 scoped_refptr<ImageRecord> record) {
296 DCHECK(CalledOnValidThread()); 306 DCHECK(CalledOnValidThread());
297 307
298 std::vector<ImageCallback> callbacks = callbacks_[page_url]; 308 std::vector<ImageCallback> callbacks = callbacks_[page_url];
299 for (std::vector<ImageCallback>::const_iterator it = callbacks.begin(); 309 for (std::vector<ImageCallback>::const_iterator it = callbacks.begin();
300 it != callbacks.end(); ++it) { 310 it != callbacks.end(); ++it) {
301 it->Run(record); 311 it->Run(record);
302 } 312 }
303 313
304 callbacks_.erase(page_url); 314 callbacks_.erase(page_url);
305 } 315 }
(...skipping 54 matching lines...) Expand 10 before | Expand all | Expand 10 after
360 const BookmarkNode* node) { 370 const BookmarkNode* node) {
361 } 371 }
362 372
363 void BookmarkImageService::BookmarkAllUserNodesRemoved( 373 void BookmarkImageService::BookmarkAllUserNodesRemoved(
364 BookmarkModel* model, 374 BookmarkModel* model,
365 const std::set<GURL>& removed_urls) { 375 const std::set<GURL>& removed_urls) {
366 ClearAll(); 376 ClearAll();
367 } 377 }
368 378
369 } // namespace enhanced_bookmarks 379 } // namespace enhanced_bookmarks
OLDNEW
« no previous file with comments | « components/enhanced_bookmarks/bookmark_image_service.h ('k') | components/enhanced_bookmarks/image_record.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698