1. Create and use the TopSites database file....

1. Create and use the TopSites database file.
2. Timed updates of the database based on the number of sites changed.

BUG=None
TEST=TopSitesTest


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

[Nik Shkrob, 2010-06-08 18:24:56]

http://codereview.chromium.org/2746002/diff/40001/4005
File chrome/browser/history/top_sites_unittest.cc (right):

http://codereview.chromium.org/2746002/diff/40001/4005#newcode645
chrome/browser/history/top_sites_unittest.cc:645: (const
NotificationDetails&)details);
I couldn't find an easy way to avoid the cast here, and left it at that since
this is a test.

[Nik Shkrob, 2010-06-08 20:50:41]

Uploaded a new patch - should fix compile issues on win and runtime errors for
linux and mac.

On 2010/06/08 18:24:56, Nik Shkrob wrote:
> http://codereview.chromium.org/2746002/diff/40001/4005
> File chrome/browser/history/top_sites_unittest.cc (right):
> 
> http://codereview.chromium.org/2746002/diff/40001/4005#newcode645
> chrome/browser/history/top_sites_unittest.cc:645: (const
> NotificationDetails&)details);
> I couldn't find an easy way to avoid the cast here, and left it at that since
> this is a test.

[brettw, 2010-06-08 23:22:42]

LGTM, just some style nits.

http://codereview.chromium.org/2746002/diff/27011/13002
File chrome/browser/history/top_sites.cc (right):

http://codereview.chromium.org/2746002/diff/27011/13002#newcode5
chrome/browser/history/top_sites.cc:5: #include <algorithm>
This should go after top_sites.h (the corresponding h for the cc file should
always be first).

http://codereview.chromium.org/2746002/diff/27011/13002#newcode335
chrome/browser/history/top_sites.cc:335: // Delete all
This comment probably isn't needed.

http://codereview.chromium.org/2746002/diff/27011/13002#newcode338
chrome/browser/history/top_sites.cc:338: file_util::Delete(db_path_, false);
Note that when you do threading, you'll need to put this on the background
thread. Can you add a TODO for now?

http://codereview.chromium.org/2746002/diff/27011/13003
File chrome/browser/history/top_sites.h (right):

http://codereview.chromium.org/2746002/diff/27011/13003#newcode195
chrome/browser/history/top_sites.h:195: size_t num_urls_changed;
Should have an underscore at the end of this. It would make more sense to me if
you called this "last_num_urls_changed_" instead

http://codereview.chromium.org/2746002/diff/27011/13004
File chrome/browser/history/top_sites_database.h (right):

http://codereview.chromium.org/2746002/diff/27011/13004#newcode57
chrome/browser/history/top_sites_database.h:57: class TopSitesDatabaseImpl:
public TopSitesDatabase {
Can you put an extra space before the colon?

http://codereview.chromium.org/2746002/diff/27011/13007
File chrome/common/chrome_constants.cc (right):

http://codereview.chromium.org/2746002/diff/27011/13007#newcode102
chrome/common/chrome_constants.cc:102: const FilePath::CharType
kTopSitesFilename[] = FPL("TopSites");
I'd put a space in here like we do for the rest of the filenames.