Write thumbnail to the database on SetPageThumbnail....

1. Write thumbnail to the database on SetPageThumbnail.
2. Store the thumbnail score in the database.

BUG=None
TEST=TopSitesTest




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

[Nik Shkrob, 2010-06-02 18:42:27]

Hi Scott,

Please review this CL.

Thanks,

Nik

[sky, 2010-06-02 19:46:31]

http://codereview.chromium.org/2499003/diff/5/12001
File chrome/browser/history/top_sites.cc (right):

http://codereview.chromium.org/2499003/diff/5/12001#newcode80
chrome/browser/history/top_sites.cc:80: int index = found->second;
Do you need a static_cast here?

http://codereview.chromium.org/2499003/diff/5/12002
File chrome/browser/history/top_sites.h (right):

http://codereview.chromium.org/2499003/diff/5/12002#newcode96
chrome/browser/history/top_sites.h:96: bool SetPageThumbnailNoDB(const GURL&
url,
Please document the return value.

http://codereview.chromium.org/2499003/diff/5/12003
File chrome/browser/history/top_sites_database.cc (right):

http://codereview.chromium.org/2499003/diff/5/12003#newcode41
chrome/browser/history/top_sites_database.cc:41: "last_updated INTEGER DEFAULT
0) ")) {
Do you need any migration code for the old? Or is this only enabled with a
switch so that we don't care.

http://codereview.chromium.org/2499003/diff/5/12003#newcode122
chrome/browser/history/top_sites_database.cc:122: ThumbnailScore score =
thumbnail.thumbnail_score;
const ThumbnailScore&

http://codereview.chromium.org/2499003/diff/5/12003#newcode126
chrome/browser/history/top_sites_database.cc:126: statement.BindInt64(8,
score.time_at_snapshot.ToTimeT());
Use ToInternalValue when saving times to the db.

http://codereview.chromium.org/2499003/diff/5/12003#newcode200
chrome/browser/history/top_sites_database.cc:200:
base::Time::FromTimeT(statement.ColumnInt64(4));
FromInternalValue

[Nik Shkrob, 2010-06-02 20:20:45]

http://codereview.chromium.org/2499003/diff/5/12001
File chrome/browser/history/top_sites.cc (right):

http://codereview.chromium.org/2499003/diff/5/12001#newcode80
chrome/browser/history/top_sites.cc:80: int index = found->second;
On 2010/06/02 19:46:31, sky wrote:
> Do you need a static_cast here?

Changed to size_t. There was no compiler warning though.
The function SetPageThumbnail (line 83) should probably take size_t as well.

http://codereview.chromium.org/2499003/diff/5/12002
File chrome/browser/history/top_sites.h (right):

http://codereview.chromium.org/2499003/diff/5/12002#newcode96
chrome/browser/history/top_sites.h:96: bool SetPageThumbnailNoDB(const GURL&
url,
On 2010/06/02 19:46:31, sky wrote:
> Please document the return value.

Done.

http://codereview.chromium.org/2499003/diff/5/12003
File chrome/browser/history/top_sites_database.cc (right):

http://codereview.chromium.org/2499003/diff/5/12003#newcode41
chrome/browser/history/top_sites_database.cc:41: "last_updated INTEGER DEFAULT
0) ")) {
On 2010/06/02 19:46:31, sky wrote:
> Do you need any migration code for the old? Or is this only enabled with a
> switch so that we don't care.

For now, this only works with a switch. Migration will be done later.

http://codereview.chromium.org/2499003/diff/5/12003#newcode122
chrome/browser/history/top_sites_database.cc:122: ThumbnailScore score =
thumbnail.thumbnail_score;
On 2010/06/02 19:46:31, sky wrote:
> const ThumbnailScore&

Done.

http://codereview.chromium.org/2499003/diff/5/12003#newcode126
chrome/browser/history/top_sites_database.cc:126: statement.BindInt64(8,
score.time_at_snapshot.ToTimeT());
On 2010/06/02 19:46:31, sky wrote:
> Use ToInternalValue when saving times to the db.

Done - the comment for ToInternalValue says not to use it. Still, all the DBs
use it. Why is that?

http://codereview.chromium.org/2499003/diff/5/12003#newcode200
chrome/browser/history/top_sites_database.cc:200:
base::Time::FromTimeT(statement.ColumnInt64(4));
On 2010/06/02 19:46:31, sky wrote:
> FromInternalValue

Done.

[sky, 2010-06-02 20:43:50]

LGTM