Some DatabaseTracker cleanup.

* Remove a DCHECK about an empty database name that should not be there.
* Properly update the cached copy of the database_description.
* unit tests for the above

BUG=91651
Review URL: http://codereview.chromium.org/7748025

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

webkit/database/database_tracker.cc

@@ -141,11 +141,14 @@ void DatabaseTracker::DatabaseOpened(const string16& origin_identifier,
   InsertOrUpdateDatabaseDetails(origin_identifier, database_name,
                                 database_description, estimated_size);
   if (database_connections_.AddConnection(origin_identifier, database_name)) {
-    *database_size = SeedOpenDatabaseSize(origin_identifier, database_name);
+    *database_size = SeedOpenDatabaseInfo(origin_identifier,
+                                          database_name,
+                                          database_description);
     return;
   }
-  *database_size  = UpdateOpenDatabaseSizeAndNotify(origin_identifier,
-                                                    database_name);
+  *database_size  = UpdateOpenDatabaseInfoAndNotify(origin_identifier,
+                                                    database_name,
+                                                    &database_description);
 }
 
 void DatabaseTracker::DatabaseModified(const string16& origin_identifier,
@@ -278,7 +281,6 @@ FilePath DatabaseTracker::GetFullDBFilePath(
     const string16& origin_identifier,
     const string16& database_name) {
   DCHECK(!origin_identifier.empty());
-  DCHECK(!database_name.empty());
   if (!LazyInit())
     return FilePath();
 
@@ -531,13 +533,16 @@ void DatabaseTracker::ClearAllCachedOriginInfo() {
   origins_info_map_.clear();
 }
 
-DatabaseTracker::CachedOriginInfo* DatabaseTracker::GetCachedOriginInfo(
-    const string16& origin_identifier) {
+DatabaseTracker::CachedOriginInfo* DatabaseTracker::MaybeGetCachedOriginInfo(
+    const string16& origin_identifier, bool create_if_needed) {
   if (!LazyInit())
     return NULL;
 
   // Populate the cache with data for this origin if needed.
   if (origins_info_map_.find(origin_identifier) == origins_info_map_.end()) {
+    if (!create_if_needed)
+      return NULL;
+
     std::vector<DatabaseDetails> details;
     if (!databases_table_->GetAllDatabaseDetailsForOrigin(
             origin_identifier, &details)) {
@@ -573,25 +578,33 @@ int64 DatabaseTracker::GetDBFileSize(const string16& origin_identifier,
   return db_file_size;
 }
 
-int64 DatabaseTracker::SeedOpenDatabaseSize(
-    const string16& origin_id, const string16& name) {
+int64 DatabaseTracker::SeedOpenDatabaseInfo(
+    const string16& origin_id, const string16& name,
+    const string16& description) {
   DCHECK(database_connections_.IsDatabaseOpened(origin_id, name));
   int64 size = GetDBFileSize(origin_id, name);
   database_connections_.SetOpenDatabaseSize(origin_id, name,  size);
-  if (origins_info_map_.find(origin_id) != origins_info_map_.end())
-    origins_info_map_[origin_id].SetDatabaseSize(name, size);
+  CachedOriginInfo* info = MaybeGetCachedOriginInfo(origin_id, false);
+  if (info) {
+    info->SetDatabaseSize(name, size);
+    info->SetDatabaseDescription(name, description);
+  }
   return size;
 }
 
-int64 DatabaseTracker::UpdateOpenDatabaseSizeAndNotify(
-    const string16& origin_id, const string16& name) {
+int64 DatabaseTracker::UpdateOpenDatabaseInfoAndNotify(
+    const string16& origin_id, const string16& name,
+    const string16* opt_description) {
   DCHECK(database_connections_.IsDatabaseOpened(origin_id, name));
   int64 new_size = GetDBFileSize(origin_id, name);
   int64 old_size = database_connections_.GetOpenDatabaseSize(origin_id, name);
+  CachedOriginInfo* info = MaybeGetCachedOriginInfo(origin_id, false);
+  if (info && opt_description)
+    info->SetDatabaseDescription(name, *opt_description);
   if (old_size != new_size) {
     database_connections_.SetOpenDatabaseSize(origin_id, name, new_size);
-    if (origins_info_map_.find(origin_id) != origins_info_map_.end())
-      origins_info_map_[origin_id].SetDatabaseSize(name, new_size);
+    if (info)
+      info->SetDatabaseSize(name, new_size);
     if (quota_manager_proxy_)
       quota_manager_proxy_->NotifyStorageModified(
           quota::QuotaClient::kDatabase,

webkit/database/database_tracker.h

@@ -221,14 +221,26 @@ class DatabaseTracker
                                      int64 estimated_size);
 
   void ClearAllCachedOriginInfo();
-  CachedOriginInfo* GetCachedOriginInfo(const string16& origin_identifier);
+  CachedOriginInfo* MaybeGetCachedOriginInfo(const string16& origin_identifier,
+                                             bool create_if_needed);
+  CachedOriginInfo* GetCachedOriginInfo(const string16& origin_identifier) {
+    return MaybeGetCachedOriginInfo(origin_identifier, true);
+  }
 
   int64 GetDBFileSize(const string16& origin_identifier,
                       const string16& database_name);
-  int64 SeedOpenDatabaseSize(const string16& origin_identifier,
-                             const string16& database_name);
+  int64 SeedOpenDatabaseInfo(const string16& origin_identifier,
+                             const string16& database_name,
+                             const string16& description);
+  int64 UpdateOpenDatabaseInfoAndNotify(const string16& origin_identifier,
+                                        const string16& database_name,
+                                        const string16* opt_description);
   int64 UpdateOpenDatabaseSizeAndNotify(const string16& origin_identifier,
-                                        const string16& database_name);
+                                        const string16& database_name) {
+    return UpdateOpenDatabaseInfoAndNotify(
+        origin_identifier, database_name, NULL);
+  }
+
 
   void ScheduleDatabaseForDeletion(const string16& origin_identifier,
                                    const string16& database_name);

webkit/database/database_tracker_unittest.cc

@@ -607,6 +607,52 @@ class DatabaseTracker_TestHelper_Test {
     // The origin directory should be gone as well.
     EXPECT_FALSE(file_util::PathExists(origin1_db_dir));
   }
+
+  static void EmptyDatabaseNameIsValid() {
+    const GURL kOrigin(kOrigin1Url);
+    const string16 kOriginId = DatabaseUtil::GetOriginIdentifier(kOrigin);
+    const string16 kEmptyName;
+    const string16 kDescription(ASCIIToUTF16("description"));
+    const string16 kChangedDescription(ASCIIToUTF16("changed_description"));
+
+    // Initialize a tracker database, no need to put it on disk.
+    const bool kUseInMemoryTrackerDatabase = true;
+    ScopedTempDir temp_dir;
+    ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+    scoped_refptr<DatabaseTracker> tracker(
+        new DatabaseTracker(temp_dir.path(), kUseInMemoryTrackerDatabase,
+                            false, NULL, NULL, NULL));
+
+    // Starts off with no databases.
+    std::vector<OriginInfo> infos;
+    EXPECT_TRUE(tracker->GetAllOriginsInfo(&infos));
+    EXPECT_TRUE(infos.empty());
+
+    // Create a db with an empty name.
+    int64 database_size = -1;
+    tracker->DatabaseOpened(kOriginId, kEmptyName, kDescription, 0,
+                            &database_size);
+    EXPECT_EQ(0, database_size);
+    tracker->DatabaseModified(kOriginId, kEmptyName);
+    EXPECT_TRUE(tracker->GetAllOriginsInfo(&infos));
+    EXPECT_EQ(1u, infos.size());
+    EXPECT_EQ(kDescription, infos[0].GetDatabaseDescription(kEmptyName));
+    EXPECT_FALSE(tracker->GetFullDBFilePath(kOriginId, kEmptyName).empty());
+    tracker->DatabaseOpened(kOriginId, kEmptyName, kChangedDescription, 0,
+                            &database_size);
+    infos.clear();
+    EXPECT_TRUE(tracker->GetAllOriginsInfo(&infos));
+    EXPECT_EQ(1u, infos.size());
+    EXPECT_EQ(kChangedDescription, infos[0].GetDatabaseDescription(kEmptyName));
+    tracker->DatabaseClosed(kOriginId, kEmptyName);
+    tracker->DatabaseClosed(kOriginId, kEmptyName);
+
+    // Deleting it should return to the initial state.
+    EXPECT_EQ(net::OK, tracker->DeleteDatabase(kOriginId, kEmptyName, NULL));
+    infos.clear();
+    EXPECT_TRUE(tracker->GetAllOriginsInfo(&infos));
+    EXPECT_TRUE(infos.empty());
+  }
 };
 
 TEST(DatabaseTrackerTest, DeleteOpenDatabase) {
@@ -635,4 +681,8 @@ TEST(DatabaseTrackerTest, DatabaseTrackerClearLocalStateOnExit) {
   DatabaseTracker_TestHelper_Test::DatabaseTrackerClearLocalStateOnExit();
 }
 
+TEST(DatabaseTrackerTest, EmptyDatabaseNameIsValid) {
+  DatabaseTracker_TestHelper_Test::EmptyDatabaseNameIsValid();
+}
+
 }  // namespace webkit_database