Add TopSites::GetTemporaryThumbnailScore().

Add TopSites::GetTemporaryThumbnailScore().

This will make the in-browser thumbnailing more efficient. Before this
change, we regenerated a thumbnail even when we had a good thumbnail,
if it's not yet saved (this occurred when the top site list was not yet full)

BUG=65936
TEST=confirm that the in-browser thumbnailing doesn't regenerate a thumbnail when we already have one.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=81216

[sky, 2011-04-08 15:59:07]

http://codereview.chromium.org/6813040/diff/1/chrome/browser/history/top_site...
File chrome/browser/history/top_sites.cc (right):

http://codereview.chromium.org/6813040/diff/1/chrome/browser/history/top_site...
chrome/browser/history/top_sites.cc:248: // The thumbnail may be found in the
temporary images.
This breaks the thread safety of this class.

[satorux1, 2011-04-09 02:36:51]

http://codereview.chromium.org/6813040/diff/1/chrome/browser/history/top_site...
File chrome/browser/history/top_sites.cc (right):

http://codereview.chromium.org/6813040/diff/1/chrome/browser/history/top_site...
chrome/browser/history/top_sites.cc:248: // The thumbnail may be found in the
temporary images.
On 2011/04/08 15:59:07, sky wrote:
> This breaks the thread safety of this class.

My bad. I missed WARNING. Moved the lock  to the beginning of the function.

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

http://codereview.chromium.org/6813040/diff/5001/chrome/browser/history/top_s...
File chrome/browser/history/top_sites.cc (right):

http://codereview.chromium.org/6813040/diff/5001/chrome/browser/history/top_s...
chrome/browser/history/top_sites.cc:249: base::AutoLock lock(lock_);
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]

http://codereview.chromium.org/6813040/diff/5001/chrome/browser/history/top_s...
File chrome/browser/history/top_sites.cc (right):

http://codereview.chromium.org/6813040/diff/5001/chrome/browser/history/top_s...
chrome/browser/history/top_sites.cc:249: base::AutoLock lock(lock_);
On 2011/04/11 14:31:09, sky wrote:
> 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.

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.

[sky, 2011-04-11 15:24:02]

http://codereview.chromium.org/6813040/diff/6003/chrome/browser/tab_contents/...
File chrome/browser/tab_contents/thumbnail_generator.cc (right):

http://codereview.chromium.org/6813040/diff/6003/chrome/browser/tab_contents/...
chrome/browser/tab_contents/thumbnail_generator.cc:678: if
(top_sites->GetTemporaryPageThumbnailScore(url, &current_score) &&
Shouldn't this remain as GetPageThumbnailScore?

[satorux1, 2011-04-11 15:31:45]

http://codereview.chromium.org/6813040/diff/6003/chrome/browser/tab_contents/...
File chrome/browser/tab_contents/thumbnail_generator.cc (right):

http://codereview.chromium.org/6813040/diff/6003/chrome/browser/tab_contents/...
chrome/browser/tab_contents/thumbnail_generator.cc:678: if
(top_sites->GetTemporaryPageThumbnailScore(url, &current_score) &&
On 2011/04/11 15:24:03, sky wrote:
> Shouldn't this remain as GetPageThumbnailScore?

Good catch! Fixed. My brain doesn't seem to be working. Time for me to sleep.

[sky, 2011-04-11 15:57:53]

LGTM