[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373

dimich at chromium.org dimich at chromium.org
Wed Apr 7 23:17:07 UTC 2010


The following commit has been merged in the webkit-1.2 branch:
commit b769f723d78a8cd543e8da46f610ba8dc4cab1ed
Author: dimich at chromium.org <dimich at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Fri Oct 30 22:26:32 2009 +0000

    Refactor DatabaseTask in preparation for removing threadsafe refcounting from it.
    Move the synchronizer object out of the DatabaseTask so there is no need to keep
    the pointer to Databasetask around after passing it to MessageQueue.
    Also pass the references to return parameters to the task so it can update them.
    https://bugs.webkit.org/show_bug.cgi?id=30941
    
    Reviewed by David Levin.
    
    No new tests, since this is just moving the code around, no change in functionality.
    
    * storage/Database.cpp:
    (WebCore::Database::Database):
    (WebCore::Database::openAndVerifyVersion): Use new DatabaseTaskSynchronizer to wait for task completion.
    (WebCore::Database::markAsDeletedAndClose): Ditto.
    (WebCore::Database::tableNames): Ditto.
    (WebCore::Database::stop): Use the boolean flag rather then 'killed' flag built into MessageQueue.
    (WebCore::Database::scheduleTransaction): Transaction queue is a Deque now, change the way to fetch the transaction.
    * storage/Database.h: Change the SQLTransaction queue to be a Deque rather then a MessageQueue.
    * storage/DatabaseTask.cpp:
    (WebCore::DatabaseTaskSynchronizer::DatabaseTaskSynchronizer):
    (WebCore::DatabaseTaskSynchronizer::waitForTaskCompletion):
    (WebCore::DatabaseTaskSynchronizer::taskCompleted):
    (WebCore::DatabaseTask::DatabaseTask): Ctor takes DatabaseTaskSynchronizer which can be 0.
    (WebCore::DatabaseTask::performTask): Signal completion. m_synchronizer should still be around since main thread is waiting on it.
    (WebCore::DatabaseOpenTask::DatabaseOpenTask): Pass synchronizer and return parameters via constructor.
    (WebCore::DatabaseCloseTask::DatabaseCloseTask): Ditto.
    (WebCore::DatabaseTransactionTask::DatabaseTransactionTask): Ditto.
    (WebCore::DatabaseTableNamesTask::DatabaseTableNamesTask): Ditto.
    * storage/DatabaseTask.h:
    (WebCore::DatabaseOpenTask::create):
    (WebCore::DatabaseCloseTask::create):
    (WebCore::DatabaseTransactionTask::create):
    (WebCore::DatabaseTableNamesTask::create):
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@50360 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 060f541..9b8f3ba 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,39 @@
+2009-10-30  Dmitry Titov  <dimich at chromium.org>
+
+        Reviewed by David Levin.
+
+        Refactor DatabaseTask in preparation for removing threadsafe refcounting from it.
+        Move the synchronizer object out of the DatabaseTask so there is no need to keep
+        the pointer to Databasetask around after passing it to MessageQueue.
+        Also pass the references to return parameters to the task so it can update them.
+        https://bugs.webkit.org/show_bug.cgi?id=30941
+
+        No new tests, since this is just moving the code around, no change in functionality.
+
+        * storage/Database.cpp:
+        (WebCore::Database::Database):
+        (WebCore::Database::openAndVerifyVersion): Use new DatabaseTaskSynchronizer to wait for task completion.
+        (WebCore::Database::markAsDeletedAndClose): Ditto.
+        (WebCore::Database::tableNames): Ditto.
+        (WebCore::Database::stop): Use the boolean flag rather then 'killed' flag built into MessageQueue.
+        (WebCore::Database::scheduleTransaction): Transaction queue is a Deque now, change the way to fetch the transaction.
+        * storage/Database.h: Change the SQLTransaction queue to be a Deque rather then a MessageQueue.
+        * storage/DatabaseTask.cpp:
+        (WebCore::DatabaseTaskSynchronizer::DatabaseTaskSynchronizer):
+        (WebCore::DatabaseTaskSynchronizer::waitForTaskCompletion):
+        (WebCore::DatabaseTaskSynchronizer::taskCompleted):
+        (WebCore::DatabaseTask::DatabaseTask): Ctor takes DatabaseTaskSynchronizer which can be 0.
+        (WebCore::DatabaseTask::performTask): Signal completion. m_synchronizer should still be around since main thread is waiting on it.
+        (WebCore::DatabaseOpenTask::DatabaseOpenTask): Pass synchronizer and return parameters via constructor.
+        (WebCore::DatabaseCloseTask::DatabaseCloseTask): Ditto.
+        (WebCore::DatabaseTransactionTask::DatabaseTransactionTask): Ditto.
+        (WebCore::DatabaseTableNamesTask::DatabaseTableNamesTask): Ditto.
+        * storage/DatabaseTask.h:
+        (WebCore::DatabaseOpenTask::create):
+        (WebCore::DatabaseCloseTask::create):
+        (WebCore::DatabaseTransactionTask::create):
+        (WebCore::DatabaseTableNamesTask::create):
+
 2009-10-30  Enrica Casucci  <enrica at apple.com>
 
         Reviewed by Darin Adler.
diff --git a/WebCore/storage/Database.cpp b/WebCore/storage/Database.cpp
index 411c1d3..29b9555 100644
--- a/WebCore/storage/Database.cpp
+++ b/WebCore/storage/Database.cpp
@@ -149,6 +149,7 @@ PassRefPtr<Database> Database::openDatabase(Document* document, const String& na
 
 Database::Database(Document* document, const String& name, const String& expectedVersion, const String& displayName, unsigned long estimatedSize)
     : m_transactionInProgress(false)
+    , m_isTransactionQueueEnabled(true)
     , m_document(document)
     , m_name(name.crossThreadString())
     , m_guid(0)
@@ -205,15 +206,14 @@ bool Database::openAndVerifyVersion(ExceptionCode& e)
         return false;
     m_databaseAuthorizer = DatabaseAuthorizer::create();
 
-    RefPtr<DatabaseOpenTask> task = DatabaseOpenTask::create(this);
+    bool success = false;
+    DatabaseTaskSynchronizer synchronizer;
+    RefPtr<DatabaseOpenTask> task = DatabaseOpenTask::create(this, &synchronizer, e, success);
 
-    task->lockForSynchronousScheduling();
     m_document->databaseThread()->scheduleImmediateTask(task);
-    task->waitForSynchronousCompletion();
+    synchronizer.waitForTaskCompletion();
 
-    ASSERT(task->isComplete());
-    e = task->exceptionCode();
-    return task->openSuccessful();
+    return success;
 }
 
 
@@ -318,11 +318,11 @@ void Database::markAsDeletedAndClose()
 
     m_document->databaseThread()->unscheduleDatabaseTasks(this);
 
-    RefPtr<DatabaseCloseTask> task = DatabaseCloseTask::create(this);
+    DatabaseTaskSynchronizer synchronizer;
+    RefPtr<DatabaseCloseTask> task = DatabaseCloseTask::create(this, &synchronizer);
 
-    task->lockForSynchronousScheduling();
     m_document->databaseThread()->scheduleImmediateTask(task);
-    task->waitForSynchronousCompletion();
+    synchronizer.waitForTaskCompletion();
 }
 
 void Database::close()
@@ -362,7 +362,7 @@ void Database::stop()
 
     {
         MutexLocker locker(m_transactionInProgressMutex);
-        m_transactionQueue.kill();
+        m_isTransactionQueueEnabled = false;
         m_transactionInProgress = false;
     }
 }
@@ -532,7 +532,13 @@ void Database::scheduleTransaction()
 {
     ASSERT(!m_transactionInProgressMutex.tryLock()); // Locked by caller.
     RefPtr<SQLTransaction> transaction;
-    if (m_transactionQueue.tryGetMessage(transaction) && m_document->databaseThread()) {
+
+    if (m_isTransactionQueueEnabled && !m_transactionQueue.isEmpty()) {
+        transaction = m_transactionQueue.first();
+        m_transactionQueue.removeFirst();
+    }
+
+    if (transaction && m_document->databaseThread()) {
         RefPtr<DatabaseTransactionTask> task = DatabaseTransactionTask::create(transaction);
         LOG(StorageAPI, "Scheduling DatabaseTransactionTask %p for transaction %p\n", task.get(), task->transaction());
         m_transactionInProgress = true;
@@ -616,15 +622,19 @@ void Database::deliverPendingCallback(void* context)
 
 Vector<String> Database::tableNames()
 {
+    // FIXME: Not using threadsafeCopy on these strings looks ok since threads take strict turns
+    // in dealing with them. However, if the code changes, this may not be true anymore.
+    Vector<String> result;
     if (!m_document->databaseThread())
-        return Vector<String>();
-    RefPtr<DatabaseTableNamesTask> task = DatabaseTableNamesTask::create(this);
+        return result;
+
+    DatabaseTaskSynchronizer synchronizer;
+    RefPtr<DatabaseTableNamesTask> task = DatabaseTableNamesTask::create(this, &synchronizer, result);
 
-    task->lockForSynchronousScheduling();
     m_document->databaseThread()->scheduleImmediateTask(task);
-    task->waitForSynchronousCompletion();
+    synchronizer.waitForTaskCompletion();
 
-    return task->tableNames();
+    return result;
 }
 
 void Database::setExpectedVersion(const String& version)
diff --git a/WebCore/storage/Database.h b/WebCore/storage/Database.h
index f275027..61c9b66 100644
--- a/WebCore/storage/Database.h
+++ b/WebCore/storage/Database.h
@@ -30,7 +30,6 @@
 #define Database_h
 
 #if ENABLE(DATABASE)
-#include <wtf/MessageQueue.h>
 #include "PlatformString.h"
 #include "SecurityOrigin.h"
 #include "SQLiteDatabase.h"
@@ -133,9 +132,10 @@ private:
     void scheduleTransactionCallback(SQLTransaction*);
     void scheduleTransactionStep(SQLTransaction* transaction, bool immediately = false);
 
-    MessageQueue<RefPtr<SQLTransaction> > m_transactionQueue;
+    Deque<RefPtr<SQLTransaction> > m_transactionQueue;
     Mutex m_transactionInProgressMutex;
     bool m_transactionInProgress;
+    bool m_isTransactionQueueEnabled;
 
     static void deliverPendingCallback(void*);
 
diff --git a/WebCore/storage/DatabaseTask.cpp b/WebCore/storage/DatabaseTask.cpp
index 755da7c..702c96b 100644
--- a/WebCore/storage/DatabaseTask.cpp
+++ b/WebCore/storage/DatabaseTask.cpp
@@ -35,9 +35,33 @@
 
 namespace WebCore {
 
-DatabaseTask::DatabaseTask(Database* database)
+DatabaseTaskSynchronizer::DatabaseTaskSynchronizer()
+    : m_taskCompleted(false)
+{
+}
+
+void DatabaseTaskSynchronizer::waitForTaskCompletion()
+{
+    m_synchronousMutex.lock();
+    if (!m_taskCompleted)
+        m_synchronousCondition.wait(m_synchronousMutex);
+    m_synchronousMutex.unlock();
+}
+
+void DatabaseTaskSynchronizer::taskCompleted()
+{
+    m_synchronousMutex.lock();
+    m_taskCompleted = true;
+    m_synchronousCondition.signal();
+    m_synchronousMutex.unlock();
+}
+
+DatabaseTask::DatabaseTask(Database* database, DatabaseTaskSynchronizer* synchronizer)
     : m_database(database)
+    , m_synchronizer(synchronizer)
+#ifndef NDEBUG
     , m_complete(false)
+#endif
 {
 }
 
@@ -56,43 +80,19 @@ void DatabaseTask::performTask()
     doPerformTask();
     m_database->performPolicyChecks();
 
-    if (m_synchronousMutex)
-        m_synchronousMutex->lock();
-
-    m_complete = true;
-
-    if (m_synchronousMutex) {
-        m_synchronousCondition->signal();
-        m_synchronousMutex->unlock();
-    }
-}
-
-void DatabaseTask::lockForSynchronousScheduling()
-{
-    // Called from main thread.
-    ASSERT(!m_synchronousMutex);
-    ASSERT(!m_synchronousCondition);
-    m_synchronousMutex.set(new Mutex);
-    m_synchronousCondition.set(new ThreadCondition);
-}
-
-void DatabaseTask::waitForSynchronousCompletion()
-{
-    // Called from main thread.
-    m_synchronousMutex->lock();
-    if (!m_complete)
-        m_synchronousCondition->wait(*m_synchronousMutex);
-    m_synchronousMutex->unlock();
+    if (m_synchronizer)
+        m_synchronizer->taskCompleted();
 }
 
 // *** DatabaseOpenTask ***
 // Opens the database file and verifies the version matches the expected version.
 
-DatabaseOpenTask::DatabaseOpenTask(Database* database)
-    : DatabaseTask(database)
-    , m_code(0)
-    , m_success(false)
+DatabaseOpenTask::DatabaseOpenTask(Database* database, DatabaseTaskSynchronizer* synchronizer, ExceptionCode& code, bool& success)
+    : DatabaseTask(database, synchronizer)
+    , m_code(code)
+    , m_success(success)
 {
+    ASSERT(synchronizer); // A task with output parameters is supposed to be synchronous.
 }
 
 void DatabaseOpenTask::doPerformTask()
@@ -110,8 +110,8 @@ const char* DatabaseOpenTask::debugTaskName() const
 // *** DatabaseCloseTask ***
 // Closes the database.
 
-DatabaseCloseTask::DatabaseCloseTask(Database* database)
-    : DatabaseTask(database)
+DatabaseCloseTask::DatabaseCloseTask(Database* database, DatabaseTaskSynchronizer* synchronizer)
+    : DatabaseTask(database, synchronizer)
 {
 }
 
@@ -131,7 +131,7 @@ const char* DatabaseCloseTask::debugTaskName() const
 // Starts a transaction that will report its results via a callback.
 
 DatabaseTransactionTask::DatabaseTransactionTask(PassRefPtr<SQLTransaction> transaction)
-    : DatabaseTask(transaction->database())
+    : DatabaseTask(transaction->database(), 0)
     , m_transaction(transaction)
 {
 }
@@ -159,9 +159,11 @@ const char* DatabaseTransactionTask::debugTaskName() const
 // *** DatabaseTableNamesTask ***
 // Retrieves a list of all tables in the database - for WebInspector support.
 
-DatabaseTableNamesTask::DatabaseTableNamesTask(Database* database)
-    : DatabaseTask(database)
+DatabaseTableNamesTask::DatabaseTableNamesTask(Database* database, DatabaseTaskSynchronizer* synchronizer, Vector<String>& names)
+    : DatabaseTask(database, synchronizer)
+    , m_tableNames(names)
 {
+    ASSERT(synchronizer); // A task with output parameters is supposed to be synchronous.
 }
 
 void DatabaseTableNamesTask::doPerformTask()
diff --git a/WebCore/storage/DatabaseTask.h b/WebCore/storage/DatabaseTask.h
index 4aef892..65a55af 100644
--- a/WebCore/storage/DatabaseTask.h
+++ b/WebCore/storage/DatabaseTask.h
@@ -39,12 +39,32 @@
 namespace WebCore {
 
 class Database;
+class DatabaseTask;
 class DatabaseThread;
 class SQLValue;
 class SQLCallback;
 class SQLTransaction;
 class VersionChangeCallback;
 
+// Can be used to wait until DatabaseTask is completed.
+// Has to be passed into DatabaseTask::create to be associated with the task.
+class DatabaseTaskSynchronizer : public Noncopyable {
+    friend class DatabaseTask;
+public:
+    DatabaseTaskSynchronizer();
+
+    // Called from main thread to wait until task is completed.
+    void waitForTaskCompletion();
+
+private:
+    // Called by the task.
+    void taskCompleted();
+
+    bool m_taskCompleted;
+    Mutex m_synchronousMutex;
+    ThreadCondition m_synchronousCondition;
+};
+
 class DatabaseTask : public ThreadSafeShared<DatabaseTask> {
     friend class Database;
 public:
@@ -53,53 +73,50 @@ public:
     void performTask();
 
     Database* database() const { return m_database; }
-    bool isComplete() const { return m_complete; }
 
 protected:
-    DatabaseTask(Database*);
+    DatabaseTask(Database*, DatabaseTaskSynchronizer*);
 
 private:
     virtual void doPerformTask() = 0;
-#ifndef NDEBUG
-    virtual const char* debugTaskName() const = 0;
-#endif
-
-    void lockForSynchronousScheduling();
-    void waitForSynchronousCompletion();
 
     Database* m_database;
+    DatabaseTaskSynchronizer* m_synchronizer;
 
-    bool m_complete;
-
-    OwnPtr<Mutex> m_synchronousMutex;
-    OwnPtr<ThreadCondition> m_synchronousCondition;
+#ifndef NDEBUG
+     virtual const char* debugTaskName() const = 0;
+     bool m_complete;
+#endif
 };
 
 class DatabaseOpenTask : public DatabaseTask {
 public:
-    static PassRefPtr<DatabaseOpenTask> create(Database* db) { return adoptRef(new DatabaseOpenTask(db)); }
-
-    ExceptionCode exceptionCode() const { return m_code; }
-    bool openSuccessful() const { return m_success; }
+    static PassRefPtr<DatabaseOpenTask> create(Database* db, DatabaseTaskSynchronizer* synchronizer, ExceptionCode& code, bool& success)
+    {
+        return adoptRef(new DatabaseOpenTask(db, synchronizer, code, success));
+    }
 
 private:
-    DatabaseOpenTask(Database*);
+    DatabaseOpenTask(Database*, DatabaseTaskSynchronizer*, ExceptionCode&, bool& success);
 
     virtual void doPerformTask();
 #ifndef NDEBUG
     virtual const char* debugTaskName() const;
 #endif
 
-    ExceptionCode m_code;
-    bool m_success;
+    ExceptionCode& m_code;
+    bool& m_success;
 };
 
 class DatabaseCloseTask : public DatabaseTask {
 public:
-    static PassRefPtr<DatabaseCloseTask> create(Database* db) { return adoptRef(new DatabaseCloseTask(db)); }
+    static PassRefPtr<DatabaseCloseTask> create(Database* db, DatabaseTaskSynchronizer* synchronizer)
+    { 
+        return adoptRef(new DatabaseCloseTask(db, synchronizer));
+    }
 
 private:
-    DatabaseCloseTask(Database*);
+    DatabaseCloseTask(Database*, DatabaseTaskSynchronizer*);
 
     virtual void doPerformTask();
 #ifndef NDEBUG
@@ -109,7 +126,11 @@ private:
 
 class DatabaseTransactionTask : public DatabaseTask {
 public:
-    static PassRefPtr<DatabaseTransactionTask> create(PassRefPtr<SQLTransaction> transaction) { return adoptRef(new DatabaseTransactionTask(transaction)); }
+    // Transaction task is never synchronous, so no 'synchronizer' parameter.
+    static PassRefPtr<DatabaseTransactionTask> create(PassRefPtr<SQLTransaction> transaction)
+    {
+        return adoptRef(new DatabaseTransactionTask(transaction));
+    }
 
     SQLTransaction* transaction() const { return m_transaction.get(); }
 
@@ -127,19 +148,20 @@ private:
 
 class DatabaseTableNamesTask : public DatabaseTask {
 public:
-    static PassRefPtr<DatabaseTableNamesTask> create(Database* db) { return adoptRef(new DatabaseTableNamesTask(db)); }
-
-    Vector<String>& tableNames() { return m_tableNames; }
+    static PassRefPtr<DatabaseTableNamesTask> create(Database* db, DatabaseTaskSynchronizer* synchronizer, Vector<String>& names)
+    {
+        return adoptRef(new DatabaseTableNamesTask(db, synchronizer, names));
+    }
 
 private:
-    DatabaseTableNamesTask(Database*);
+    DatabaseTableNamesTask(Database*, DatabaseTaskSynchronizer*, Vector<String>& names);
 
     virtual void doPerformTask();
 #ifndef NDEBUG
     virtual const char* debugTaskName() const;
 #endif
 
-    Vector<String> m_tableNames;
+    Vector<String>& m_tableNames;
 };
 
 } // namespace WebCore

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list