Reverting 20097.

Review URL: http://codereview.chromium.org/155186

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@20101 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/profile.cc

@@ -1016,7 +1016,7 @@ TabRestoreService* ProfileImpl::GetTabRestoreService() {
 ThumbnailStore* ProfileImpl::GetThumbnailStore() {
   if (!thumbnail_store_.get()) {
     thumbnail_store_ = new ThumbnailStore;
-    thumbnail_store_->Init(GetPath().AppendASCII("Top Thumbnails"), this);
+    thumbnail_store_->Init(GetPath().AppendASCII("thumbnailstore"), this);
   }
   return thumbnail_store_.get();
 }

chrome/browser/tab_contents/tab_contents.cc

@@ -1650,8 +1650,8 @@ void TabContents::UpdateThumbnail(const GURL& url,
                                   const ThumbnailScore& score) {
   // Tell History about this thumbnail
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kThumbnailStore)) {
-    if (!profile()->IsOffTheRecord())
-      profile()->GetThumbnailStore()->SetPageThumbnail(url, bitmap, score);
+    profile()->GetThumbnailStore()->
+        SetPageThumbnail(url, bitmap, score, !profile()->IsOffTheRecord());
   } else {
     HistoryService* hs;
     if (!profile()->IsOffTheRecord() &&

chrome/browser/thumbnail_store.h

@@ -15,19 +15,17 @@
 #include "base/timer.h"
 #include "chrome/browser/cancelable_request.h"
 #include "chrome/browser/history/history.h"
-#include "chrome/browser/history/url_database.h"  // For DBCloseScoper
 #include "chrome/common/pref_names.h"
 #include "chrome/common/ref_counted_util.h"
-#include "chrome/common/sqlite_compiled_statement.h"
-#include "chrome/common/thumbnail_score.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"
 
 class DictionaryValue;
 class GURL;
 class HistoryService;
+class Pickle;
 class Profile;
 class SkBitmap;
-struct sqlite3;
+struct ThumbnailScore;
 namespace base {
 class Time;
 }
@@ -40,13 +38,17 @@ class ThumbnailStore : public base::RefCountedThreadSafe<ThumbnailStore> {
   ~ThumbnailStore();
 
   // Must be called after creation but before other methods are called.
-  void Init(const FilePath& db_name,      // The location of the database.
-            Profile* profile);            // To get to the HistoryService.
+  // file_path is a directory where a new database should be created
+  // or the location of an existing databse.
+  void Init(const FilePath& file_path, Profile* profile);
 
   // Stores the given thumbnail and score with the associated url in the cache.
+  // If write_to_disk is true, the thumbnail data is written to disk on the
+  // file_thread.
   bool SetPageThumbnail(const GURL& url,
                         const SkBitmap& thumbnail,
-                        const ThumbnailScore& score);
+                        const ThumbnailScore& score,
+                        bool write_to_disk);
 
   // Sets *data to point to the thumbnail for the given url.
   // Returns false if no thumbnail available.
@@ -59,22 +61,9 @@ class ThumbnailStore : public base::RefCountedThreadSafe<ThumbnailStore> {
   FRIEND_TEST(ThumbnailStoreTest, FollowRedirects);
   friend class ThumbnailStoreTest;
 
-  struct CacheEntry {
-    scoped_refptr<RefCountedBytes> data_;
-    ThumbnailScore score_;
-    bool dirty_;
-
-    CacheEntry() : data_(NULL), score_(ThumbnailScore()), dirty_(false) {}
-    CacheEntry(RefCountedBytes* data,
-               const ThumbnailScore& score,
-               bool dirty)
-        : data_(data),
-          score_(score),
-          dirty_(dirty) {}
-  };
-
   // Data structure used to store thumbnail data in memory.
-  typedef std::map<GURL, CacheEntry> Cache;
+  typedef std::map<GURL, std::pair<scoped_refptr<RefCountedBytes>,
+      ThumbnailScore> > Cache;
 
   // Most visited URLs and their redirect lists -------------------------------
 
@@ -83,7 +72,8 @@ class ThumbnailStore : public base::RefCountedThreadSafe<ThumbnailStore> {
   // callback is OnURLDataAvailable.
   void UpdateURLData();
 
-  // The callback for UpdateURLData.
+  // The callback for UpdateURLData. The ThumbnailStore takes ownership of
+  // the most visited urls list and redirect lists passed in.
   void OnURLDataAvailable(std::vector<GURL>* urls,
                           history::RedirectMap* redirects);
 
@@ -94,26 +84,43 @@ class ThumbnailStore : public base::RefCountedThreadSafe<ThumbnailStore> {
   // visited sites.
   void CleanCacheData();
 
-  // Disk operations ----------------------------------------------------------
+  // Deletes thumbnail data from disk for the given list of urls.
+  void DeleteThumbnails(
+      scoped_refptr<RefCountedVector<GURL> > thumbnail_urls) const;
 
-  // Initialize |db_| to the database specified in |db_name|.  If |cb_loop|
-  // is non-null, calls GetAllThumbnailsFromDisk.  Done on the file_thread.
-  void InitializeFromDB(const FilePath& db_name, MessageLoop* cb_loop);
+  // Disk operations ----------------------------------------------------------
 
   // Read all thumbnail data from the specified FilePath into a Cache object.
   // Done on the file_thread and returns to OnDiskDataAvailable on the thread
   // owning the specified MessageLoop.
-  void GetAllThumbnailsFromDisk(MessageLoop* cb_loop);
+  void GetAllThumbnailsFromDisk(FilePath filepath, MessageLoop* cb_loop);
+
+  // Read the thumbnail data from the given file and stores it in the
+  // out parameters GURL, SkBitmap, and ThumbnailScore.
+  bool GetPageThumbnailFromDisk(const FilePath& file,
+                                GURL* url,
+                                RefCountedBytes* data,
+                                ThumbnailScore* score) const;
 
   // Once thumbnail data from the disk is available from the file_thread,
   // this function is invoked on the main thread.  It takes ownership of the
   // Cache* passed in and retains this Cache* for the lifetime of the object.
   void OnDiskDataAvailable(Cache* cache);
 
-  // Delete each URL in the given vector from the DB and write all dirty
-  // cache entries to the DB.
-  void CommitCacheToDB(
-      scoped_refptr<RefCountedVector<GURL> > stale_urls) const;
+  // Write thumbnail data to disk for a given url.
+  bool WriteThumbnailToDisk(const GURL& url,
+                            scoped_refptr<RefCountedBytes> data,
+                            const ThumbnailScore& score) const;
+
+
+  // Pack the given ThumbnailScore into the given Pickle.
+  void PackScore(const ThumbnailScore& score, Pickle* packed) const;
+
+  // Unpack a ThumbnailScore from a given Pickle and associated iterator.
+  // Returns false is a ThumbnailScore could not be unpacked.
+  bool UnpackScore(ThumbnailScore* score,
+                   const Pickle& packed,
+                   void*& iter) const;
 
   // Decide whether to store data ---------------------------------------------
 
@@ -132,11 +139,10 @@ class ThumbnailStore : public base::RefCountedThreadSafe<ThumbnailStore> {
 
   // The Cache maintained by the object.
   scoped_ptr<Cache> cache_;
+  bool cache_initialized_;
 
-  // The database holding the thumbnails on disk.
-  sqlite3* db_;
-  SqliteStatementCache* statement_cache_;
-  history::DBCloseScoper close_scoper_;
+  // The location of the thumbnail store.
+  FilePath file_path_;
 
   // We hold a reference to the history service to query for most visited URLs
   // and redirect information.

chrome/browser/thumbnail_store_unittest.cc

@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 #include <string.h>
-#include <algorithm>
 #include <iostream>
 #include <vector>
 
@@ -13,13 +12,12 @@
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/gfx/jpeg_codec.h"
+#include "base/md5.h"
 #include "base/path_service.h"
 #include "base/ref_counted.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/ref_counted_util.h"
 #include "chrome/common/thumbnail_score.h"
-#include "chrome/common/sqlite_compiled_statement.h"
-#include "chrome/common/sqlite_utils.h"
 #include "chrome/tools/profiles/thumbnail-inl.h"
 #include "googleurl/src/gurl.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -32,10 +30,9 @@ inline unsigned int diff(unsigned int a, unsigned int b) {
 
 class ThumbnailStoreTest : public testing::Test {
  public:
-  ThumbnailStoreTest() : score_(.5, true, false),
-                         url_("http://www.google.com/") {
+  ThumbnailStoreTest() : score1_(.5, true, false),
+    url1_("http://www.google.com/"), url2_("http://www.elgoog.com") {
   }
-
   ~ThumbnailStoreTest() {
   }
 
@@ -43,32 +40,33 @@ class ThumbnailStoreTest : public testing::Test {
   void SetUp();
 
   void TearDown() {
-    file_util::Delete(db_name_, false);
+    file_util::Delete(file_path_.AppendASCII(url1_.host()), false);
+    file_util::Delete(file_path_.AppendASCII(url2_.host()), false);
   }
 
   // Compute the max difference over all pixels for each RGBA component.
   void PrintPixelDiff(SkBitmap* image_a, SkBitmap* image_b);
 
   // The directory where ThumbnailStore will store data.
-  FilePath db_name_;
+  FilePath file_path_;
 
   scoped_refptr<ThumbnailStore> store_;
   scoped_ptr<SkBitmap> google_;
   scoped_ptr<SkBitmap> weewar_;
   scoped_refptr<RefCountedBytes> jpeg_google_;
   scoped_refptr<RefCountedBytes> jpeg_weewar_;
-  ThumbnailScore score_;
-  GURL url_;
+  ThumbnailScore score1_;
+  GURL url1_, url2_;
   base::Time time_;
 };
 
 void ThumbnailStoreTest::SetUp() {
-  if (!file_util::GetTempDir(&db_name_))
+  if (!file_util::GetTempDir(&file_path_))
     FAIL();
 
   // Delete any old thumbnail files if they exist.
-  db_name_ = db_name_.AppendASCII("ThumbnailDB");
-  file_util::Delete(db_name_, false);
+  file_util::Delete(file_path_.AppendASCII(url1_.host()), false);
+  file_util::Delete(file_path_.AppendASCII(url2_.host()), false);
 
   google_.reset(JPEGCodec::Decode(kGoogleThumbnail, sizeof(kGoogleThumbnail)));
   weewar_.reset(JPEGCodec::Decode(kWeewarThumbnail, sizeof(kWeewarThumbnail)));
@@ -92,12 +90,12 @@ void ThumbnailStoreTest::SetUp() {
       &(jpeg_weewar_->data));
 
   store_ = new ThumbnailStore;
-
+  store_->cache_initialized_ = true;
+  store_->file_path_ = file_path_;
+  store_->most_visited_urls_.reset(new std::vector<GURL>);
+  store_->most_visited_urls_->push_back(url1_);
   store_->cache_.reset(new ThumbnailStore::Cache);
   store_->redirect_urls_.reset(new history::RedirectMap);
-
-  store_->most_visited_urls_.reset(new std::vector<GURL>);
-  store_->most_visited_urls_->push_back(url_);
 }
 
 void ThumbnailStoreTest::PrintPixelDiff(SkBitmap* image_a, SkBitmap* image_b) {
@@ -141,14 +139,21 @@ void ThumbnailStoreTest::PrintPixelDiff(SkBitmap* image_a, SkBitmap* image_b) {
 TEST_F(ThumbnailStoreTest, UpdateThumbnail) {
   RefCountedBytes* read_image = NULL;
   ThumbnailScore score2(0.1, true, true);
+  store_->cache_->clear();
+  store_->redirect_urls_->clear();
 
   // store_ google_ with a low score, then weewar_ with a higher score
   // and check that weewar_ overwrote google_.
 
-  EXPECT_TRUE(store_->SetPageThumbnail(url_, *google_, score_));
-  EXPECT_TRUE(store_->SetPageThumbnail(url_, *weewar_, score2));
+  EXPECT_TRUE(store_->SetPageThumbnail(url1_, *google_, score1_, false));
+  EXPECT_TRUE(store_->SetPageThumbnail(url1_, *weewar_, score2, false));
+
+  // Set fake redirects list.
+  scoped_ptr<std::vector<GURL> > redirects(new std::vector<GURL>);
+  redirects->push_back(url1_);
+  (*store_->redirect_urls_)[url1_] = new RefCountedVector<GURL>(*redirects);
 
-  EXPECT_TRUE(store_->GetPageThumbnail(url_, &read_image));
+  EXPECT_TRUE(store_->GetPageThumbnail(url1_, &read_image));
   EXPECT_EQ(read_image->data.size(), jpeg_weewar_->data.size());
   EXPECT_EQ(0, memcmp(&read_image->data[0], &jpeg_weewar_->data[0],
                       jpeg_weewar_->data.size()));
@@ -158,16 +163,24 @@ TEST_F(ThumbnailStoreTest, UpdateThumbnail) {
 
 TEST_F(ThumbnailStoreTest, RetrieveFromCache) {
   RefCountedBytes* read_image = NULL;
+  store_->cache_->clear();
+  store_->redirect_urls_->clear();
 
   // Retrieve a thumbnail/score for a page not in the cache.
 
-  EXPECT_FALSE(store_->GetPageThumbnail(GURL("nonexistent"), &read_image));
+  EXPECT_FALSE(store_->GetPageThumbnail(url2_, &read_image));
+
+  // store_ a thumbnail into the cache and retrieve it.
 
-  // Store a thumbnail into the cache and retrieve it.
+  EXPECT_TRUE(store_->SetPageThumbnail(url1_, *google_, score1_, false));
 
-  EXPECT_TRUE(store_->SetPageThumbnail(url_, *google_, score_));
-  EXPECT_TRUE(store_->GetPageThumbnail(url_, &read_image));
-  EXPECT_TRUE(score_.Equals((*store_->cache_)[url_].score_));
+  // Set fake redirects list.
+  scoped_ptr<std::vector<GURL> > redirects(new std::vector<GURL>);
+  redirects->push_back(url1_);
+  (*store_->redirect_urls_)[url1_] = new RefCountedVector<GURL>(*redirects);
+
+  EXPECT_TRUE(store_->GetPageThumbnail(url1_, &read_image));
+  EXPECT_TRUE(score1_.Equals((*store_->cache_)[url1_].second));
   EXPECT_TRUE(read_image->data.size() == jpeg_google_->data.size());
   EXPECT_EQ(0, memcmp(&read_image->data[0], &jpeg_google_->data[0],
                       jpeg_google_->data.size()));
@@ -176,47 +189,46 @@ TEST_F(ThumbnailStoreTest, RetrieveFromCache) {
 }
 
 TEST_F(ThumbnailStoreTest, RetrieveFromDisk) {
-  EXPECT_TRUE(store_->SetPageThumbnail(url_, *google_, score_));
-
-  // Write the thumbnail to disk and retrieve it.
-
-  store_->InitializeFromDB(db_name_, NULL);
-  store_->CommitCacheToDB(NULL);  // Write to the DB (dirty bit sould be set)
-  store_->cache_->clear();        // Clear it from the cache.
-
-  // Read from the DB.
-  SQLITE_UNIQUE_STATEMENT(statement, *store_->statement_cache_,
-      "SELECT * FROM thumbnails");
-  EXPECT_TRUE(statement->step() == SQLITE_ROW);
-  GURL url(statement->column_string(0));
-  ThumbnailScore score(statement->column_double(1),
-                       statement->column_bool(2),
-                       statement->column_bool(3),
-                       base::Time::FromInternalValue(
-                          statement->column_int64(4)));
-  scoped_refptr<RefCountedBytes> data = new RefCountedBytes;
-  EXPECT_TRUE(statement->column_blob_as_vector(5, &data->data));
-
-  EXPECT_TRUE(url == url_);
-  EXPECT_TRUE(score.Equals(score_));
-  EXPECT_TRUE(data->data.size() == jpeg_google_->data.size());
-  EXPECT_EQ(0, memcmp(&data->data[0], &jpeg_google_->data[0],
+  scoped_refptr<RefCountedBytes> read_image = new RefCountedBytes;
+  ThumbnailScore score2;
+  store_->cache_->clear();
+  store_->redirect_urls_->clear();
+
+  // store_ a thumbnail onto the disk and retrieve it.
+
+  EXPECT_TRUE(store_->SetPageThumbnail(url1_, *google_, score1_, false));
+  EXPECT_TRUE(store_->WriteThumbnailToDisk(url1_, jpeg_google_, score1_));
+  EXPECT_TRUE(store_->GetPageThumbnailFromDisk(file_path_.AppendASCII(
+      MD5String(url1_.spec())), &url2_, read_image, &score2));
+  EXPECT_TRUE(url1_ == url2_);
+  EXPECT_TRUE(score1_.Equals(score2));
+  EXPECT_TRUE(read_image->data.size() == jpeg_google_->data.size());
+  EXPECT_EQ(0, memcmp(&read_image->data[0], &jpeg_google_->data[0],
                       jpeg_google_->data.size()));
 }
 
 TEST_F(ThumbnailStoreTest, FollowRedirects) {
   RefCountedBytes* read_image = NULL;
-  std::vector<GURL> redirects;
+  scoped_ptr<std::vector<GURL> > redirects(new std::vector<GURL>);
+  store_->cache_->clear();
+  store_->redirect_urls_->clear();
 
   GURL my_url("google");
-  redirects.push_back(GURL("google.com"));
-  redirects.push_back(GURL("www.google.com"));
-  redirects.push_back(url_);  // url_ = http://www.google.com/
-  (*store_->redirect_urls_)[my_url] = new RefCountedVector<GURL>(redirects);
+  redirects->push_back(GURL("google.com"));
+  redirects->push_back(GURL("www.google.com"));
+  redirects->push_back(url1_);  // url1_ = http://www.google.com/
 
   store_->most_visited_urls_->push_back(my_url);
 
-  EXPECT_TRUE(store_->SetPageThumbnail(GURL("google.com"), *google_, score_));
+  (*store_->redirect_urls_)[my_url] = new RefCountedVector<GURL>(*redirects);
+  EXPECT_TRUE(store_->SetPageThumbnail(url1_, *google_, score1_, false));
+  EXPECT_TRUE(store_->GetPageThumbnail(my_url, &read_image));
+
+  read_image->Release();
+  store_->cache_->erase(store_->cache_->find(url1_));
+
+  EXPECT_TRUE(store_->SetPageThumbnail(GURL("google.com"), *google_, score1_,
+      false));
   EXPECT_TRUE(store_->GetPageThumbnail(my_url, &read_image));
 
   read_image->Release();