Changeset 52631 in webkit

Timestamp:

Dec 29, 2009 12:51:32 PM (14 years ago)

Author:

dumi@chromium.org

Message:

1. Changes the order in which some functions are called to match

the pre-r52536 order. Namely, when a new Database object is
created, DatabaseTracker::addOpenDatabase() is called in the
constructor, before doing anything else related to that database
(like trying to get a file handle to the database
file). Chromium's implementation depends on this ordering.

2. Changes Database::performOpenAndVerify() to close the open

handle to the database file immediately if the database version
does not match the expected one. The current behavior is to add
the Database object to a DatabaseThread collection and let the
database thread close the handle when it's destroyed.

Reviewed by Maciej Stachowiak.

​https://bugs.webkit.org/show_bug.cgi?id=33005

All LayoutTests/storage tests pass in clean WebKit and Chromium
clients.

• storage/Database.cpp:

(WebCore::Database::openDatabase): Notify DatabaseTracker and
Document that a Database object is about to be destroyed (when a
database file cannot be opened, or its version doesn't match the
expected one).
(WebCore::Database::Database): Notify DatabaseTracker and Document
that a new Database object was created.
(WebCore::Database::performOpenAndVerify): If a database version
does not match the expected one, immediately close the open file
handle to the database file.

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• storage/Database.cpp (modified) (6 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2009-12-28  Dumitru Daniliuc  <dumi@chromium.org>
+
+        Reviewed by Maciej Stachowiak.
+
+        1. Changes the order in which some functions are called to match
+        the pre-r52536 order. Namely, when a new Database object is
+        created, DatabaseTracker::addOpenDatabase() is called in the
+        constructor, before doing anything else related to that database
+        (like trying to get a file handle to the database
+        file). Chromium's implementation depends on this ordering.
+        2. Changes Database::performOpenAndVerify() to close the open
+        handle to the database file immediately if the database version
+        does not match the expected one. The current behavior is to add
+        the Database object to a DatabaseThread collection and let the
+        database thread close the handle when it's destroyed.
+
+        https://bugs.webkit.org/show_bug.cgi?id=33005
+
+        All LayoutTests/storage tests pass in clean WebKit and Chromium
+        clients.
+
+        * storage/Database.cpp:
+        (WebCore::Database::openDatabase): Notify DatabaseTracker and
+        Document that a Database object is about to be destroyed (when a
+        database file cannot be opened, or its version doesn't match the
+        expected one).
+        (WebCore::Database::Database): Notify DatabaseTracker and Document
+        that a new Database object was created.
+        (WebCore::Database::performOpenAndVerify): If a database version
+        does not match the expected one, immediately close the open file
+        handle to the database file.
+
 2009-12-29  Nikolas Zimmermann  <nzimmermann@rim.com>
 

trunk/WebCore/storage/Database.cpp

@@ -132 +132 @@
 
     if (!database->openAndVerifyVersion(e)) {
-       LOG(StorageAPI, "Failed to open and verify version (expected %s) of database %s", expectedVersion.ascii().data(), database->databaseDebugName().ascii().data());
-       return 0;
-    }
-
-    DatabaseTracker::tracker().addOpenDatabase(database.get());
-    document->addOpenDatabase(database.get());
+        LOG(StorageAPI, "Failed to open and verify version (expected %s) of database %s", expectedVersion.ascii().data(), database->databaseDebugName().ascii().data());
+        document->removeOpenDatabase(database.get());
+        DatabaseTracker::tracker().removeOpenDatabase(database.get());
+        return 0;
+    }
 
     DatabaseTracker::tracker().setDatabaseDetails(document->securityOrigin(), name, displayName, estimatedSize);
@@ -190 +189 @@
 
     m_filename = DatabaseTracker::tracker().fullPathForDatabase(m_mainThreadSecurityOrigin.get(), m_name);
+
+    DatabaseTracker::tracker().addOpenDatabase(this);
+    m_document->addOpenDatabase(this);
 }
 
@@ -481 +483 @@
                     LOG_ERROR("Unable to create table %s in database %s", databaseInfoTableName().ascii().data(), databaseDebugName().ascii().data());
                     e = INVALID_STATE_ERR;
+                    // Close the handle to the database file.
+                    m_sqliteDatabase.close();
                     return false;
                 }
@@ -488 +492 @@
                 LOG_ERROR("Failed to get current version from database %s", databaseDebugName().ascii().data());
                 e = INVALID_STATE_ERR;
+                // Close the handle to the database file.
+                m_sqliteDatabase.close();
                 return false;
             }
@@ -497 +503 @@
                     LOG_ERROR("Failed to set version %s in database %s", m_expectedVersion.ascii().data(), databaseDebugName().ascii().data());
                     e = INVALID_STATE_ERR;
+                    // Close the handle to the database file.
+                    m_sqliteDatabase.close();
                     return false;
                 }
@@ -517 +525 @@
             databaseDebugName().ascii().data(), currentVersion.ascii().data());
         e = INVALID_STATE_ERR;
+        // Close the handle to the database file.
+        m_sqliteDatabase.close();
         return false;
     }
 
+    // All checks passed and we still have a handle to this database file.
+    // Make sure DatabaseThread closes it when DatabaseThread goes away.
     m_opened = true;
     if (m_document->databaseThread())