Diff
Modified: trunk/Source/WebCore/ChangeLog (108014 => 108015)
--- trunk/Source/WebCore/ChangeLog 2012-02-17 02:41:11 UTC (rev 108014)
+++ trunk/Source/WebCore/ChangeLog 2012-02-17 03:18:48 UTC (rev 108015)
@@ -1,3 +1,62 @@
+2012-02-15 Michael Nordman <micha...@google.com>
+
+ [chromium] Fix bugs in the implementation of WebDatabase::closeDatabaseImmediately.
+ https://bugs.webkit.org/show_bug.cgi?id=78841
+
+ WebDatabase now delegates this function entirely to DatabaseTracker,
+ a new closeDatabasesImmediately() has been added for that purpose. That
+ method posts tasks to the appropiate context thread for each database
+ instance that should be closed immediately.
+
+ The DatabaseTracker getAllOpenDatabases() method has been removed from
+ the chromium impl because it's unsafe, refs cannot be safely taken on
+ AbstractDatabase instances in the tracker's collection of open databases.
+
+ Add a message to the console log when a database is forcibly closed.
+
+ Transactions initiated on a database instance that has been forcibly
+ closed complete with a transaction error callback.
+
+ This is part of resolving http://crbug.com/98939
+
+ Reviewed by David Levin.
+
+ No new layout tests, there is no common code way to closeImmediately.
+ We have coverage for this in py automation tests.
+
+ * platform/sql/SQLiteDatabase.cpp:
+ The closeImmediately code path can result in the underlying sqlite3 handle being
+ closed earlier than usual and trip some assertions. Updated the assertions to no
+ longer trigger in this early close case.
+ (WebCore::SQLiteDatabase::close):
+ (WebCore::SQLiteDatabase::setMaximumSize):
+ * platform/sql/SQLiteDatabase.h:
+ (WebCore::SQLiteDatabase::sqlite3Handle):
+
+ * storage/Database.cpp:
+ (WebCore::Database::closeImmediately): Modified to only be called on the context thread and to log a console message.
+ (WebCore::Database::changeVersion): Use the private runTransaction helper method.
+ (WebCore::Database::transaction): Pass a new param required by the runTransaction helper.
+ (WebCore::Database::readTransaction): Ditto.
+ (WebCore::callTransactionErrorCallback): Used to defer invocation of the error callback.
+ (WebCore::Database::runTransaction): Modified to detect when the database has been closed, and
+ to invoke the error callback in that case. This also avoids creating a reference cycle between
+ a newly created transaction and the database that previously existed due to a transction being
+ added and never removed from the Q while in this state.
+ * storage/Database.h:
+ * storage/DatabaseSync.cpp:
+ (WebCore::DatabaseSync::closeImmediately): Modified to only be called on the context thread and to log a console message.
+ * storage/DatabaseTracker.h:
+
+ * storage/chromium/DatabaseTrackerChromium.cpp:
+ Posts tasks to the appropiate context thread for execution without bumping AbstractDatabase refcounts.
+ (DatabaseTracker::CloseOneDatabaseImmediatelyTask):
+ (WebCore::DatabaseTracker::CloseOneDatabaseImmediatelyTask::create):
+ (WebCore::DatabaseTracker::CloseOneDatabaseImmediatelyTask::performTask):
+ (WebCore::DatabaseTracker::CloseOneDatabaseImmediatelyTask::CloseOneDatabaseImmediatelyTask):
+ (WebCore::DatabaseTracker::closeDatabasesImmediately):
+ (WebCore::DatabaseTracker::closeOneDatabaseImmediately):
+
2012-02-16 Dana Jansens <dan...@chromium.org>
[Chromium] Occlusion tracking with CSS filters
Modified: trunk/Source/WebCore/platform/sql/SQLiteDatabase.cpp (108014 => 108015)
--- trunk/Source/WebCore/platform/sql/SQLiteDatabase.cpp 2012-02-17 02:41:11 UTC (rev 108014)
+++ trunk/Source/WebCore/platform/sql/SQLiteDatabase.cpp 2012-02-17 03:18:48 UTC (rev 108015)
@@ -102,7 +102,7 @@
void SQLiteDatabase::close()
{
if (m_db) {
- // FIXME: This is being called on themain thread during JS GC. <rdar://problem/5739818>
+ // FIXME: This is being called on the main thread during JS GC. <rdar://problem/5739818>
// ASSERT(currentThread() == m_openingThread);
sqlite3* db = m_db;
{
@@ -167,7 +167,7 @@
int currentPageSize = pageSize();
- ASSERT(currentPageSize);
+ ASSERT(currentPageSize || !m_db);
int64_t newMaxPageCount = currentPageSize ? size / currentPageSize : 0;
MutexLocker locker(m_authorizerLock);
Modified: trunk/Source/WebCore/platform/sql/SQLiteDatabase.h (108014 => 108015)
--- trunk/Source/WebCore/platform/sql/SQLiteDatabase.h 2012-02-17 02:41:11 UTC (rev 108014)
+++ trunk/Source/WebCore/platform/sql/SQLiteDatabase.h 2012-02-17 03:18:48 UTC (rev 108015)
@@ -104,7 +104,7 @@
const char* lastErrorMsg();
sqlite3* sqlite3Handle() const {
- ASSERT(m_sharable || currentThread() == m_openingThread);
+ ASSERT(m_sharable || currentThread() == m_openingThread || !m_db);
return m_db;
}
Modified: trunk/Source/WebCore/storage/Database.cpp (108014 => 108015)
--- trunk/Source/WebCore/storage/Database.cpp 2012-02-17 02:41:11 UTC (rev 108014)
+++ trunk/Source/WebCore/storage/Database.cpp 2012-02-17 03:18:48 UTC (rev 108015)
@@ -32,6 +32,7 @@
#if ENABLE(SQL_DATABASE)
#include "ChangeVersionWrapper.h"
+#include "CrossThreadTask.h"
#include "DatabaseCallback.h"
#include "DatabaseTask.h"
#include "DatabaseThread.h"
@@ -41,6 +42,7 @@
#include "Logging.h"
#include "NotImplemented.h"
#include "Page.h"
+#include "SQLError.h"
#include "SQLTransactionCallback.h"
#include "SQLTransactionClient.h"
#include "SQLTransactionCoordinator.h"
@@ -230,9 +232,12 @@
void Database::closeImmediately()
{
+ ASSERT(m_scriptExecutionContext->isContextThread());
DatabaseThread* databaseThread = scriptExecutionContext()->databaseThread();
- if (databaseThread && !databaseThread->terminationRequested() && opened())
+ if (databaseThread && !databaseThread->terminationRequested() && opened()) {
+ logErrorMessage("forcibly closing database");
databaseThread->scheduleImmediateTask(DatabaseCloseTask::create(this, 0));
+ }
}
unsigned long long Database::maximumSize() const
@@ -256,30 +261,36 @@
PassRefPtr<SQLTransactionCallback> callback, PassRefPtr<SQLTransactionErrorCallback> errorCallback,
PassRefPtr<VoidCallback> successCallback)
{
- RefPtr<SQLTransaction> transaction =
- SQLTransaction::create(this, callback, errorCallback, successCallback, ChangeVersionWrapper::create(oldVersion, newVersion));
- MutexLocker locker(m_transactionInProgressMutex);
- m_transactionQueue.append(transaction.release());
- if (!m_transactionInProgress)
- scheduleTransaction();
+ runTransaction(callback, errorCallback, successCallback, ChangeVersionWrapper::create(oldVersion, newVersion), false);
}
void Database::transaction(PassRefPtr<SQLTransactionCallback> callback, PassRefPtr<SQLTransactionErrorCallback> errorCallback, PassRefPtr<VoidCallback> successCallback)
{
- runTransaction(callback, errorCallback, successCallback, false);
+ runTransaction(callback, errorCallback, successCallback, 0, false);
}
void Database::readTransaction(PassRefPtr<SQLTransactionCallback> callback, PassRefPtr<SQLTransactionErrorCallback> errorCallback, PassRefPtr<VoidCallback> successCallback)
{
- runTransaction(callback, errorCallback, successCallback, true);
+ runTransaction(callback, errorCallback, successCallback, 0, true);
}
+static void callTransactionErrorCallback(ScriptExecutionContext*, PassRefPtr<SQLTransactionErrorCallback> callback, PassRefPtr<SQLError> error)
+{
+ callback->handleEvent(error.get());
+}
+
void Database::runTransaction(PassRefPtr<SQLTransactionCallback> callback, PassRefPtr<SQLTransactionErrorCallback> errorCallback,
- PassRefPtr<VoidCallback> successCallback, bool readOnly)
+ PassRefPtr<VoidCallback> successCallback, PassRefPtr<SQLTransactionWrapper> wrapper, bool readOnly)
{
- RefPtr<SQLTransaction> transaction =
- SQLTransaction::create(this, callback, errorCallback, successCallback, 0, readOnly);
MutexLocker locker(m_transactionInProgressMutex);
+ if (!m_isTransactionQueueEnabled) {
+ if (errorCallback) {
+ RefPtr<SQLError> error = SQLError::create(SQLError::UNKNOWN_ERR, "database has been closed");
+ scriptExecutionContext()->postTask(createCallbackTask(&callTransactionErrorCallback, errorCallback, error.release()));
+ }
+ return;
+ }
+ RefPtr<SQLTransaction> transaction = SQLTransaction::create(this, callback, errorCallback, successCallback, wrapper, readOnly);
m_transactionQueue.append(transaction.release());
if (!m_transactionInProgress)
scheduleTransaction();
@@ -297,9 +308,8 @@
ASSERT(!m_transactionInProgressMutex.tryLock()); // Locked by caller.
RefPtr<SQLTransaction> transaction;
- if (m_isTransactionQueueEnabled && !m_transactionQueue.isEmpty()) {
+ if (m_isTransactionQueueEnabled && !m_transactionQueue.isEmpty())
transaction = m_transactionQueue.takeFirst();
- }
if (transaction && m_scriptExecutionContext->databaseThread()) {
OwnPtr<DatabaseTransactionTask> task = DatabaseTransactionTask::create(transaction);
Modified: trunk/Source/WebCore/storage/Database.h (108014 => 108015)
--- trunk/Source/WebCore/storage/Database.h 2012-02-17 02:41:11 UTC (rev 108014)
+++ trunk/Source/WebCore/storage/Database.h 2012-02-17 03:18:48 UTC (rev 108015)
@@ -47,6 +47,7 @@
class SQLTransactionClient;
class SQLTransactionCoordinator;
class SQLTransactionErrorCallback;
+class SQLTransactionWrapper;
class VoidCallback;
typedef int ExceptionCode;
@@ -93,7 +94,7 @@
Database(ScriptExecutionContext*, const String& name, const String& expectedVersion,
const String& displayName, unsigned long estimatedSize);
void runTransaction(PassRefPtr<SQLTransactionCallback>, PassRefPtr<SQLTransactionErrorCallback>,
- PassRefPtr<VoidCallback> successCallback, bool readOnly);
+ PassRefPtr<VoidCallback> successCallback, PassRefPtr<SQLTransactionWrapper>, bool readOnly);
bool openAndVerifyVersion(bool setVersionInNewDatabase, ExceptionCode&, String& errorMessage);
virtual bool performOpenAndVerify(bool setVersionInNewDatabase, ExceptionCode&, String& errorMessage);
Modified: trunk/Source/WebCore/storage/DatabaseSync.cpp (108014 => 108015)
--- trunk/Source/WebCore/storage/DatabaseSync.cpp 2012-02-17 02:41:11 UTC (rev 108014)
+++ trunk/Source/WebCore/storage/DatabaseSync.cpp 2012-02-17 03:18:48 UTC (rev 108015)
@@ -205,16 +205,13 @@
void DatabaseSync::closeImmediately()
{
- if (!m_scriptExecutionContext->isContextThread()) {
- m_scriptExecutionContext->postTask(CloseSyncDatabaseOnContextThreadTask::create(this));
- return;
- }
+ ASSERT(m_scriptExecutionContext->isContextThread());
if (!opened())
return;
+ logErrorMessage("forcibly closing database");
DatabaseTracker::tracker().removeOpenDatabase(this);
-
closeDatabase();
}
Modified: trunk/Source/WebCore/storage/DatabaseTracker.h (108014 => 108015)
--- trunk/Source/WebCore/storage/DatabaseTracker.h 2012-02-17 02:41:11 UTC (rev 108014)
+++ trunk/Source/WebCore/storage/DatabaseTracker.h 2012-02-17 03:18:48 UTC (rev 108015)
@@ -173,13 +173,16 @@
static void notifyDatabasesChanged(void*);
#else
public:
- void getOpenDatabases(const String& originIdentifier, const String& name, HashSet<RefPtr<AbstractDatabase> >* databases);
+ void closeDatabasesImmediately(const String& originIdentifier, const String& name);
private:
typedef HashSet<AbstractDatabase*> DatabaseSet;
typedef HashMap<String, DatabaseSet*> DatabaseNameMap;
typedef HashMap<String, DatabaseNameMap*> DatabaseOriginMap;
+ class CloseOneDatabaseImmediatelyTask;
+ void closeOneDatabaseImmediately(const String& originIdentifier, const String& name, AbstractDatabase* database);
+
Mutex m_openDatabaseMapGuard;
mutable OwnPtr<DatabaseOriginMap> m_openDatabaseMap;
#endif // !PLATFORM(CHROMIUM)
Modified: trunk/Source/WebCore/storage/chromium/DatabaseTrackerChromium.cpp (108014 => 108015)
--- trunk/Source/WebCore/storage/chromium/DatabaseTrackerChromium.cpp 2012-02-17 02:41:11 UTC (rev 108014)
+++ trunk/Source/WebCore/storage/chromium/DatabaseTrackerChromium.cpp 2012-02-17 03:18:48 UTC (rev 108015)
@@ -157,29 +157,6 @@
DatabaseObserver::databaseClosed(database);
}
-void DatabaseTracker::getOpenDatabases(SecurityOrigin* origin, const String& name, HashSet<RefPtr<AbstractDatabase> >* databases)
-{
- getOpenDatabases(origin->databaseIdentifier(), name, databases);
-}
-
-void DatabaseTracker::getOpenDatabases(const String& originIdentifier, const String& name, HashSet<RefPtr<AbstractDatabase> >* databases)
-{
- MutexLocker openDatabaseMapLock(m_openDatabaseMapGuard);
- if (!m_openDatabaseMap)
- return;
-
- DatabaseNameMap* nameMap = m_openDatabaseMap->get(originIdentifier);
- if (!nameMap)
- return;
-
- DatabaseSet* databaseSet = nameMap->get(name);
- if (!databaseSet)
- return;
-
- for (DatabaseSet::iterator it = databaseSet->begin(); it != databaseSet->end(); ++it)
- databases->add(*it);
-}
-
unsigned long long DatabaseTracker::getMaxSizeForDatabase(const AbstractDatabase* database)
{
unsigned long long spaceAvailable = 0;
@@ -212,6 +189,76 @@
}
}
+class DatabaseTracker::CloseOneDatabaseImmediatelyTask : public ScriptExecutionContext::Task {
+public:
+ static PassOwnPtr<CloseOneDatabaseImmediatelyTask> create(const String& originIdentifier, const String& name, AbstractDatabase* database)
+ {
+ return adoptPtr(new CloseOneDatabaseImmediatelyTask(originIdentifier, name, database));
+ }
+
+ virtual void performTask(ScriptExecutionContext* context)
+ {
+ DatabaseTracker::tracker().closeOneDatabaseImmediately(m_originIdentifier, m_name, m_database);
+ }
+
+private:
+ CloseOneDatabaseImmediatelyTask(const String& originIdentifier, const String& name, AbstractDatabase* database)
+ : m_originIdentifier(originIdentifier.isolatedCopy())
+ , m_name(name.isolatedCopy())
+ , m_database(database)
+ {
+ }
+
+ String m_originIdentifier;
+ String m_name;
+ AbstractDatabase* m_database; // Intentionally a raw pointer.
+};
+
+void DatabaseTracker::closeDatabasesImmediately(const String& originIdentifier, const String& name) {
+ MutexLocker openDatabaseMapLock(m_openDatabaseMapGuard);
+ if (!m_openDatabaseMap)
+ return;
+
+ DatabaseNameMap* nameMap = m_openDatabaseMap->get(originIdentifier);
+ if (!nameMap)
+ return;
+
+ DatabaseSet* databaseSet = nameMap->get(name);
+ if (!databaseSet)
+ return;
+
+ // We have to call closeImmediately() on the context thread and we cannot safely add a reference to
+ // the database in our collection when not on the context thread (which is always the case given
+ // current usage).
+ for (DatabaseSet::iterator it = databaseSet->begin(); it != databaseSet->end(); ++it)
+ (*it)->scriptExecutionContext()->postTask(CloseOneDatabaseImmediatelyTask::create(originIdentifier, name, *it));
}
+void DatabaseTracker::closeOneDatabaseImmediately(const String& originIdentifier, const String& name, AbstractDatabase* database)
+{
+ // First we have to confirm the 'database' is still in our collection.
+ {
+ MutexLocker openDatabaseMapLock(m_openDatabaseMapGuard);
+ if (!m_openDatabaseMap)
+ return;
+
+ DatabaseNameMap* nameMap = m_openDatabaseMap->get(originIdentifier);
+ if (!nameMap)
+ return;
+
+ DatabaseSet* databaseSet = nameMap->get(name);
+ if (!databaseSet)
+ return;
+
+ DatabaseSet::iterator found = databaseSet->find(database);
+ if (found == databaseSet->end())
+ return;
+ }
+
+ // And we have to call closeImmediately() without our collection lock being held.
+ database->closeImmediately();
+}
+
+}
+
#endif // ENABLE(SQL_DATABASE)
Modified: trunk/Source/WebKit/chromium/ChangeLog (108014 => 108015)
--- trunk/Source/WebKit/chromium/ChangeLog 2012-02-17 02:41:11 UTC (rev 108014)
+++ trunk/Source/WebKit/chromium/ChangeLog 2012-02-17 03:18:48 UTC (rev 108015)
@@ -1,3 +1,16 @@
+2012-02-15 Michael Nordman <micha...@google.com>
+
+ Fix bugs in the implementation of WebDatabase::closeDatabaseImmediately.
+ https://bugs.webkit.org/show_bug.cgi?id=78841
+
+ WebDatabase now delegates this function entirely to DatabaseTracker.
+ This is part of resolving http://crbug.com/98939
+
+ Reviewed by David Levin.
+
+ * src/WebDatabase.cpp:
+ (WebKit::WebDatabase::closeDatabaseImmediately):
+
2012-02-16 Dana Jansens <dan...@chromium.org>
[Chromium] Occlusion tracking with CSS filters
Modified: trunk/Source/WebKit/chromium/src/WebDatabase.cpp (108014 => 108015)
--- trunk/Source/WebKit/chromium/src/WebDatabase.cpp 2012-02-17 02:41:11 UTC (rev 108014)
+++ trunk/Source/WebKit/chromium/src/WebDatabase.cpp 2012-02-17 03:18:48 UTC (rev 108015)
@@ -122,10 +122,7 @@
void WebDatabase::closeDatabaseImmediately(const WebString& originIdentifier, const WebString& databaseName)
{
#if ENABLE(SQL_DATABASE)
- HashSet<RefPtr<AbstractDatabase> > databaseHandles;
- DatabaseTracker::tracker().getOpenDatabases(originIdentifier, databaseName, &databaseHandles);
- for (HashSet<RefPtr<AbstractDatabase> >::iterator it = databaseHandles.begin(); it != databaseHandles.end(); ++it)
- it->get()->closeImmediately();
+ DatabaseTracker::tracker().closeDatabasesImmediately(originIdentifier, databaseName);
#endif
}
