Add TopSites::GetTemporaryThumbnailScore().

chrome/browser/history/top_sites.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/history/top_sites.h"
 
 #include <algorithm>
 #include <set>
 
 #include "base/command_line.h"
@@ skipping 228 matching lines @@
     }
 
     filtered_urls = thread_safe_cache_->top_sites();
   }
   request->ForwardResult(GetTopSitesCallback::TupleType(filtered_urls));
 }
 
 bool TopSites::GetPageThumbnail(const GURL& url,
                                 scoped_refptr<RefCountedBytes>* bytes) {
   // WARNING: this may be invoked on any thread.
   base::AutoLock lock(lock_);

[sky, 2011/04/11 14:31:09]

The rest of the class doesn't lock around lock_ when using temp_images_, so
using lock_ doesn't make the code any more thread safe.

Instead of trying to change this method to do two things, add a method that
returns if we have a temp thumbnail. The new method won't need any locks as it's
only for use on the UI thread.

[satorux1, 2011/04/11 15:19:09]

That makes a lot of sense. I've updated the patch and added a new method
GetTemporaryPageThumbnailScore(), and changed ThumbnailGenerator to use it.

Please take another look.

+
+  // The thumbnail may be found in the temporary images.
+  for (TempImages::iterator i = temp_images_.begin(); i != temp_images_.end();
+       ++i) {
+    if (i->first == url) {
+      *bytes = i->second.thumbnail;
+      return true;
+    }
+  }
   return thread_safe_cache_->GetPageThumbnail(url, bytes);
 }
 
 bool TopSites::GetPageThumbnailScore(const GURL& url,
                                      ThumbnailScore* score) {
   // WARNING: this may be invoked on any thread.
   base::AutoLock lock(lock_);
+
+  // The thumbnail score may be found in the temporary images.
+  for (TempImages::iterator i = temp_images_.begin(); i != temp_images_.end();
+       ++i) {
+    if (i->first == url) {
+      *score = i->second.thumbnail_score;
+      return true;
+    }
+  }
   return thread_safe_cache_->GetPageThumbnailScore(url, score);
 }
 
 // Returns the index of |url| in |urls|, or -1 if not found.
 static int IndexOf(const MostVisitedURLList& urls, const GURL& url) {
   for (size_t i = 0; i < urls.size(); i++) {
     if (urls[i].url == url)
       return i;
   }
   return -1;
@@ skipping 696 matching lines @@
   SetTopSites(pages);
 
   // Used only in testing.
   NotificationService::current()->Notify(
       NotificationType::TOP_SITES_UPDATED,
       Source<TopSites>(this),
       Details<CancelableRequestProvider::Handle>(&handle));
 }
 
 }  // namespace history