Merge 92155 - [Chromium] WebSQLDatabase version handling is broken in multi-process browsers.

Source/WebCore/storage/AbstractDatabase.cpp

 /*
  * Copyright (C) 2010 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 16 matching lines @@
  */
 
 #include "config.h"
 #include "AbstractDatabase.h"
 
 #if ENABLE(DATABASE)
 #include "DatabaseAuthorizer.h"
 #include "DatabaseTracker.h"
 #include "Logging.h"
 #include "SQLiteStatement.h"
+#include "SQLiteTransaction.h"
 #include "ScriptExecutionContext.h"
 #include "SecurityOrigin.h"
 #include <wtf/HashMap.h>
 #include <wtf/HashSet.h>
 #include <wtf/PassRefPtr.h>
 #include <wtf/RefPtr.h>
 #include <wtf/StdLibExtras.h>
 #include <wtf/text/CString.h>
 #include <wtf/text/StringHash.h>
 
@@ skipping 181 matching lines @@
         if (hashSet->isEmpty()) {
             guidToDatabaseMap().remove(m_guid);
             delete hashSet;
             guidToVersionMap().remove(m_guid);
         }
     }
 }
 
 String AbstractDatabase::version() const
 {
-    MutexLocker locker(guidMutex());
-    return guidToVersionMap().get(m_guid).threadsafeCopy();
+    // Note: In multi-process browsers the cached value may be accurate, but we cannot read the
+    // actual version from the database without potentially inducing a deadlock.
+    // FIXME: Add an async version getter to the DatabaseAPI.
+    return getCachedVersion();
 }
 
-static const int maxSqliteBusyWaitTime = 30000;
 bool AbstractDatabase::performOpenAndVerify(bool shouldSetVersionInNewDatabase, ExceptionCode& ec)
 {
+    const int maxSqliteBusyWaitTime = 30000;
+
     if (!m_sqliteDatabase.open(m_filename, true)) {
         LOG_ERROR("Unable to open database at path %s", m_filename.ascii().data());
         ec = INVALID_STATE_ERR;
         return false;
     }
     if (!m_sqliteDatabase.turnOnIncrementalAutoVacuum())
         LOG_ERROR("Unable to turn on incremental auto-vacuum for database %s", m_filename.ascii().data());
 
-    ASSERT(m_databaseAuthorizer);
-    m_sqliteDatabase.setAuthorizer(m_databaseAuthorizer);
     m_sqliteDatabase.setBusyTimeout(maxSqliteBusyWaitTime);
 
     String currentVersion;
     {
         MutexLocker locker(guidMutex());
 
         GuidVersionMap::iterator entry = guidToVersionMap().find(m_guid);
         if (entry != guidToVersionMap().end()) {
             // Map null string to empty string (see updateGuidVersionMap()).
             currentVersion = entry->second.isNull() ? String("") : entry->second;
             LOG(StorageAPI, "Current cached version for guid %i is %s", m_guid, currentVersion.ascii().data());
+
+#if PLATFORM(CHROMIUM)
+            // Note: In multi-process browsers the cached value may be inaccurate, but
+            // we cannot read the actual version from the database without potentially
+            // inducing a form of deadlock, a busytimeout error when trying to
+            // access the database. So we'll use the cached value if we're able to read
+            // the value without waiting, and otherwise use the cached value (which may be off).
+            // FIXME: Add an async openDatabase method to the DatabaseAPI.
+            const int noSqliteBusyWaitTime = 0;
+            m_sqliteDatabase.setBusyTimeout(noSqliteBusyWaitTime);
+            String versionFromDatabase;
+            if (getVersionFromDatabase(versionFromDatabase, false)) {
+                currentVersion = versionFromDatabase;
+                updateGuidVersionMap(m_guid, currentVersion);
+            }
+            m_sqliteDatabase.setBusyTimeout(maxSqliteBusyWaitTime);
+#endif
         } else {
             LOG(StorageAPI, "No cached version for guid %i", m_guid);
 
+            SQLiteTransaction transaction(m_sqliteDatabase);
+            transaction.begin();
+            if (!transaction.inProgress()) {
+                LOG_ERROR("Unable to begin transaction while opening %s", databaseDebugName().ascii().data());
+                ec = INVALID_STATE_ERR;
+                m_sqliteDatabase.close();
+                return false;
+            }
+
             if (!m_sqliteDatabase.tableExists(databaseInfoTableName())) {
                 m_new = true;
 
                 if (!m_sqliteDatabase.executeCommand("CREATE TABLE " + databaseInfoTableName() + " (key TEXT NOT NULL ON CONFLICT FAIL UNIQUE ON CONFLICT REPLACE,value TEXT NOT NULL ON CONFLICT FAIL);")) {
                     LOG_ERROR("Unable to create table %s in database %s", databaseInfoTableName().ascii().data(), databaseDebugName().ascii().data());
                     ec = INVALID_STATE_ERR;
-                    // Close the handle to the database file.
+                    transaction.rollback();
                     m_sqliteDatabase.close();
                     return false;
                 }
-            }
-
-            if (!getVersionFromDatabase(currentVersion)) {
+            } else if (!getVersionFromDatabase(currentVersion, false)) {
                 LOG_ERROR("Failed to get current version from database %s", databaseDebugName().ascii().data());
                 ec = INVALID_STATE_ERR;
-                // Close the handle to the database file.
+                transaction.rollback();
                 m_sqliteDatabase.close();
                 return false;
             }
+
             if (currentVersion.length()) {
                 LOG(StorageAPI, "Retrieved current version %s from database %s", currentVersion.ascii().data(), databaseDebugName().ascii().data());
             } else if (!m_new || shouldSetVersionInNewDatabase) {
                 LOG(StorageAPI, "Setting version %s in database %s that was just created", m_expectedVersion.ascii().data(), databaseDebugName().ascii().data());
-                if (!setVersionInDatabase(m_expectedVersion)) {
+                if (!setVersionInDatabase(m_expectedVersion, false)) {
                     LOG_ERROR("Failed to set version %s in database %s", m_expectedVersion.ascii().data(), databaseDebugName().ascii().data());
                     ec = INVALID_STATE_ERR;
-                    // Close the handle to the database file.
+                    transaction.rollback();
                     m_sqliteDatabase.close();
                     return false;
                 }
                 currentVersion = m_expectedVersion;
             }
-
             updateGuidVersionMap(m_guid, currentVersion);
+            transaction.commit();
         }
     }
 
     if (currentVersion.isNull()) {
         LOG(StorageAPI, "Database %s does not have its version set", databaseDebugName().ascii().data());
         currentVersion = "";
     }
 
     // If the expected version isn't the empty string, ensure that the current database version we have matches that version. Otherwise, set an exception.
     // If the expected version is the empty string, then we always return with whatever version of the database we have.
     if ((!m_new || shouldSetVersionInNewDatabase) && m_expectedVersion.length() && m_expectedVersion != currentVersion) {
         LOG(StorageAPI, "page expects version %s from database %s, which actually has version name %s - openDatabase() call will fail", m_expectedVersion.ascii().data(),
             databaseDebugName().ascii().data(), currentVersion.ascii().data());
         ec = INVALID_STATE_ERR;
-        // Close the handle to the database file.
         m_sqliteDatabase.close();
         return false;
     }
 
+    ASSERT(m_databaseAuthorizer);
+    m_sqliteDatabase.setAuthorizer(m_databaseAuthorizer);
+
     m_opened = true;
 
+    if (m_new && !shouldSetVersionInNewDatabase)
+        m_expectedVersion = ""; // The caller provided a creationCallback which will set the expected version.
+
     return true;
 }
 
 ScriptExecutionContext* AbstractDatabase::scriptExecutionContext() const
 {
     return m_scriptExecutionContext.get();
 }
 
 SecurityOrigin* AbstractDatabase::securityOrigin() const
 {
@@ skipping 23 matching lines @@
     return m_filename.threadsafeCopy();
 }
 
 // static
 const String& AbstractDatabase::databaseVersionKey()
 {
     DEFINE_STATIC_LOCAL(String, key, ("WebKitDatabaseVersionKey"));
     return key;
 }
 
-bool AbstractDatabase::getVersionFromDatabase(String& version)
+bool AbstractDatabase::getVersionFromDatabase(String& version, bool shouldCacheVersion)
 {
     DEFINE_STATIC_LOCAL(String, getVersionQuery, ("SELECT value FROM " + databaseInfoTableName() + " WHERE key = '" + databaseVersionKey() + "';"));
 
     m_databaseAuthorizer->disable();
 
     bool result = retrieveTextResultFromDatabase(m_sqliteDatabase, getVersionQuery.threadsafeCopy(), version);
-    if (!result)
+    if (result) {
+        if (shouldCacheVersion)
+            setCachedVersion(version);
+    } else
         LOG_ERROR("Failed to retrieve version from database %s", databaseDebugName().ascii().data());
 
     m_databaseAuthorizer->enable();
 
     return result;
 }
 
-bool AbstractDatabase::setVersionInDatabase(const String& version)
+bool AbstractDatabase::setVersionInDatabase(const String& version, bool shouldCacheVersion)
 {
     // The INSERT will replace an existing entry for the database with the new version number, due to the UNIQUE ON CONFLICT REPLACE
     // clause in the CREATE statement (see Database::performOpenAndVerify()).
     DEFINE_STATIC_LOCAL(String, setVersionQuery, ("INSERT INTO " + databaseInfoTableName() + " (key, value) VALUES ('" + databaseVersionKey() + "', ?);"));
 
     m_databaseAuthorizer->disable();
 
     bool result = setTextValueInDatabase(m_sqliteDatabase, setVersionQuery.threadsafeCopy(), version);
-    if (!result)
+    if (result) {
+        if (shouldCacheVersion)
+            setCachedVersion(version);
+    } else
         LOG_ERROR("Failed to set version %s in database (%s)", version.ascii().data(), setVersionQuery.ascii().data());
 
     m_databaseAuthorizer->enable();
 
     return result;
 }
 
-bool AbstractDatabase::versionMatchesExpected() const
-{
-    if (!m_expectedVersion.isEmpty()) {
-        MutexLocker locker(guidMutex());
-        return m_expectedVersion == guidToVersionMap().get(m_guid);
-    }
-
-    return true;
-}
-
 void AbstractDatabase::setExpectedVersion(const String& version)
 {
     m_expectedVersion = version.threadsafeCopy();
+}
+
+String AbstractDatabase::getCachedVersion() const
+{
+    MutexLocker locker(guidMutex());
+    return guidToVersionMap().get(m_guid).threadsafeCopy();
+}
+
+void AbstractDatabase::setCachedVersion(const String& actualVersion)
+{
     // Update the in memory database version map.
     MutexLocker locker(guidMutex());
-    updateGuidVersionMap(m_guid, version);
+    updateGuidVersionMap(m_guid, actualVersion);
+}
+
+bool AbstractDatabase::getActualVersionForTransaction(String &actualVersion)
+{
+    ASSERT(m_sqliteDatabase.transactionInProgress());
+#if PLATFORM(CHROMIUM)
+    // Note: In multi-process browsers the cached value may be inaccurate.
+    // So we retrieve the value from the database and update the cached value here.
+    return getVersionFromDatabase(actualVersion, true);
+#else
+    actualVersion = getCachedVersion();
+    return true;
+#endif
 }
 
 void AbstractDatabase::disableAuthorizer()
 {
     ASSERT(m_databaseAuthorizer);
     m_databaseAuthorizer->disable();
 }
 
 void AbstractDatabase::enableAuthorizer()
 {
@@ skipping 63 matching lines @@
 
 bool AbstractDatabase::isInterrupted()
 {
     MutexLocker locker(m_sqliteDatabase.databaseMutex());
     return m_sqliteDatabase.isInterrupted();
 }
 
 } // namespace WebCore
 
 #endif // ENABLE(DATABASE)