Commit b9847a89 authored by tkent@chromium.org's avatar tkent@chromium.org

Oilpan: Prepare to move SQLResultSet and SQLResultSetRowList to oilpan heap.

Without this CL, a SQLResultSet object in SQLStatementBackend can be constructed
in a database thread, moved to main/worker thread, and referred by JavaScript
object in the main/worker thread.

We can't move object ownership over threads in Oilpan. So, SQLStatementBackend
should construct a SQLResultSet object in the SQLStatementBackend constructor,
which runs in main/worker thread. SQLResultSet should have a validity flag to
represent nullness in the code without this CL.

Note:
* Change the return type of AbstractSQLStatementBackend::sqlResultSet from
  PassRefPtr<SQLResultSet> to SQLResultSet* because the function doesn't release
  the ownership of the SQLResultSet object.

* Change the order of data members of SQLResultSet to pack better.

BUG=347902

Review URL: https://codereview.chromium.org/207453006

git-svn-id: svn://svn.chromium.org/blink/trunk@169929 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 3c189d3c
...@@ -39,7 +39,7 @@ public: ...@@ -39,7 +39,7 @@ public:
virtual void trace(Visitor*) = 0; virtual void trace(Visitor*) = 0;
virtual PassRefPtr<SQLError> sqlError() const = 0; virtual PassRefPtr<SQLError> sqlError() const = 0;
virtual PassRefPtr<SQLResultSet> sqlResultSet() const = 0; virtual SQLResultSet* sqlResultSet() const = 0;
}; };
} // namespace WebCore } // namespace WebCore
......
...@@ -31,18 +31,25 @@ ...@@ -31,18 +31,25 @@
#include "bindings/v8/ExceptionState.h" #include "bindings/v8/ExceptionState.h"
#include "core/dom/ExceptionCode.h" #include "core/dom/ExceptionCode.h"
#include "heap/Handle.h"
namespace WebCore { namespace WebCore {
SQLResultSet::SQLResultSet() SQLResultSet::SQLResultSet()
: m_rows(SQLResultSetRowList::create()) : m_rows(SQLResultSetRowList::create())
, m_insertId(0) , m_insertId(0)
, m_insertIdSet(false)
, m_rowsAffected(0) , m_rowsAffected(0)
, m_insertIdSet(false)
, m_isValid(false)
{ {
ScriptWrappable::init(this); ScriptWrappable::init(this);
} }
void SQLResultSet::trace(Visitor* visitor)
{
visitor->trace(m_rows);
}
int64_t SQLResultSet::insertId(ExceptionState& exceptionState) const int64_t SQLResultSet::insertId(ExceptionState& exceptionState) const
{ {
// 4.11.4 - Return the id of the last row inserted as a result of the query // 4.11.4 - Return the id of the last row inserted as a result of the query
...@@ -75,6 +82,7 @@ void SQLResultSet::setInsertId(int64_t id) ...@@ -75,6 +82,7 @@ void SQLResultSet::setInsertId(int64_t id)
void SQLResultSet::setRowsAffected(int count) void SQLResultSet::setRowsAffected(int count)
{ {
m_rowsAffected = count; m_rowsAffected = count;
m_isValid = true;
} }
} }
...@@ -38,9 +38,10 @@ namespace WebCore { ...@@ -38,9 +38,10 @@ namespace WebCore {
class ExceptionState; class ExceptionState;
class SQLResultSet : public ThreadSafeRefCounted<SQLResultSet>, public ScriptWrappable { class SQLResultSet : public ThreadSafeRefCountedWillBeGarbageCollectedFinalized<SQLResultSet>, public ScriptWrappable {
public: public:
static PassRefPtr<SQLResultSet> create() { return adoptRef(new SQLResultSet); } static PassRefPtrWillBeRawPtr<SQLResultSet> create() { return adoptRefWillBeNoop(new SQLResultSet); }
void trace(Visitor*);
SQLResultSetRowList* rows() const; SQLResultSetRowList* rows() const;
...@@ -50,14 +51,16 @@ public: ...@@ -50,14 +51,16 @@ public:
// For internal (non-JS) use // For internal (non-JS) use
void setInsertId(int64_t); void setInsertId(int64_t);
void setRowsAffected(int); void setRowsAffected(int);
bool isValid() { return m_isValid; }
private: private:
SQLResultSet(); SQLResultSet();
RefPtr<SQLResultSetRowList> m_rows; RefPtrWillBeMember<SQLResultSetRowList> m_rows;
int64_t m_insertId; int64_t m_insertId;
bool m_insertIdSet;
int m_rowsAffected; int m_rowsAffected;
bool m_insertIdSet;
bool m_isValid;
}; };
} // namespace WebCore } // namespace WebCore
......
...@@ -27,7 +27,8 @@ ...@@ -27,7 +27,8 @@
*/ */
[ [
NoInterfaceObject NoInterfaceObject,
WillBeGarbageCollected,
] interface SQLResultSet { ] interface SQLResultSet {
readonly attribute SQLResultSetRowList rows; readonly attribute SQLResultSetRowList rows;
......
...@@ -30,15 +30,17 @@ ...@@ -30,15 +30,17 @@
#define SQLResultSetRowList_h #define SQLResultSetRowList_h
#include "bindings/v8/ScriptWrappable.h" #include "bindings/v8/ScriptWrappable.h"
#include "heap/Handle.h"
#include "modules/webdatabase/sqlite/SQLValue.h" #include "modules/webdatabase/sqlite/SQLValue.h"
#include "wtf/PassRefPtr.h" #include "wtf/PassRefPtr.h"
#include "wtf/RefCounted.h" #include "wtf/RefCounted.h"
namespace WebCore { namespace WebCore {
class SQLResultSetRowList : public RefCounted<SQLResultSetRowList>, public ScriptWrappable { class SQLResultSetRowList : public RefCountedWillBeGarbageCollectedFinalized<SQLResultSetRowList>, public ScriptWrappable {
public: public:
static PassRefPtr<SQLResultSetRowList> create() { return adoptRef(new SQLResultSetRowList); } static PassRefPtrWillBeRawPtr<SQLResultSetRowList> create() { return adoptRefWillBeNoop(new SQLResultSetRowList); }
void trace(Visitor*) { }
const Vector<String>& columnNames() const { return m_columns; } const Vector<String>& columnNames() const { return m_columns; }
const Vector<SQLValue>& values() const { return m_result; } const Vector<SQLValue>& values() const { return m_result; }
......
...@@ -28,6 +28,7 @@ ...@@ -28,6 +28,7 @@
[ [
NoInterfaceObject, NoInterfaceObject,
WillBeGarbageCollected,
] interface SQLResultSetRowList { ] interface SQLResultSetRowList {
readonly attribute unsigned long length; readonly attribute unsigned long length;
[Custom] object item(unsigned long index); [Custom] object item(unsigned long index);
......
...@@ -87,7 +87,7 @@ bool SQLStatement::performCallback(SQLTransaction* transaction) ...@@ -87,7 +87,7 @@ bool SQLStatement::performCallback(SQLTransaction* transaction)
if (errorCallback) if (errorCallback)
callbackError = errorCallback->handleEvent(transaction, error.get()); callbackError = errorCallback->handleEvent(transaction, error.get());
} else if (callback) { } else if (callback) {
RefPtr<SQLResultSet> resultSet = m_backend->sqlResultSet(); RefPtrWillBeRawPtr<SQLResultSet> resultSet = m_backend->sqlResultSet();
callbackError = !callback->handleEvent(transaction, resultSet.get()); callbackError = !callback->handleEvent(transaction, resultSet.get());
} }
......
...@@ -84,13 +84,15 @@ SQLStatementBackend::SQLStatementBackend(PassOwnPtr<AbstractSQLStatement> fronte ...@@ -84,13 +84,15 @@ SQLStatementBackend::SQLStatementBackend(PassOwnPtr<AbstractSQLStatement> fronte
, m_arguments(arguments) , m_arguments(arguments)
, m_hasCallback(m_frontend->hasCallback()) , m_hasCallback(m_frontend->hasCallback())
, m_hasErrorCallback(m_frontend->hasErrorCallback()) , m_hasErrorCallback(m_frontend->hasErrorCallback())
, m_resultSet(SQLResultSet::create())
, m_permissions(permissions) , m_permissions(permissions)
{ {
m_frontend->setBackend(this); m_frontend->setBackend(this);
} }
void SQLStatementBackend::trace(Visitor*) void SQLStatementBackend::trace(Visitor* visitor)
{ {
visitor->trace(m_resultSet);
} }
AbstractSQLStatement* SQLStatementBackend::frontend() AbstractSQLStatement* SQLStatementBackend::frontend()
...@@ -103,14 +105,14 @@ PassRefPtr<SQLError> SQLStatementBackend::sqlError() const ...@@ -103,14 +105,14 @@ PassRefPtr<SQLError> SQLStatementBackend::sqlError() const
return m_error; return m_error;
} }
PassRefPtr<SQLResultSet> SQLStatementBackend::sqlResultSet() const SQLResultSet* SQLStatementBackend::sqlResultSet() const
{ {
return m_resultSet; return m_resultSet->isValid() ? m_resultSet.get() : 0;
} }
bool SQLStatementBackend::execute(DatabaseBackend* db) bool SQLStatementBackend::execute(DatabaseBackend* db)
{ {
ASSERT(!m_resultSet); ASSERT(!m_resultSet->isValid());
// If we're re-running this statement after a quota violation, we need to clear that error now // If we're re-running this statement after a quota violation, we need to clear that error now
clearFailureDueToQuota(); clearFailureDueToQuota();
...@@ -161,13 +163,11 @@ bool SQLStatementBackend::execute(DatabaseBackend* db) ...@@ -161,13 +163,11 @@ bool SQLStatementBackend::execute(DatabaseBackend* db)
} }
} }
RefPtr<SQLResultSet> resultSet = SQLResultSet::create();
// Step so we can fetch the column names. // Step so we can fetch the column names.
result = statement.step(); result = statement.step();
if (result == SQLResultRow) { if (result == SQLResultRow) {
int columnCount = statement.columnCount(); int columnCount = statement.columnCount();
SQLResultSetRowList* rows = resultSet->rows(); SQLResultSetRowList* rows = m_resultSet->rows();
for (int i = 0; i < columnCount; i++) for (int i = 0; i < columnCount; i++)
rows->addColumn(statement.getColumnName(i)); rows->addColumn(statement.getColumnName(i));
...@@ -187,7 +187,7 @@ bool SQLStatementBackend::execute(DatabaseBackend* db) ...@@ -187,7 +187,7 @@ bool SQLStatementBackend::execute(DatabaseBackend* db)
} else if (result == SQLResultDone) { } else if (result == SQLResultDone) {
// Didn't find anything, or was an insert // Didn't find anything, or was an insert
if (db->lastActionWasInsert()) if (db->lastActionWasInsert())
resultSet->setInsertId(database->lastInsertRowID()); m_resultSet->setInsertId(database->lastInsertRowID());
} else if (result == SQLResultFull) { } else if (result == SQLResultFull) {
// Return the Quota error - the delegate will be asked for more space and this statement might be re-run // Return the Quota error - the delegate will be asked for more space and this statement might be re-run
setFailureDueToQuota(db); setFailureDueToQuota(db);
...@@ -205,23 +205,22 @@ bool SQLStatementBackend::execute(DatabaseBackend* db) ...@@ -205,23 +205,22 @@ bool SQLStatementBackend::execute(DatabaseBackend* db)
// FIXME: If the spec allows triggers, and we want to be "accurate" in a different way, we'd use // FIXME: If the spec allows triggers, and we want to be "accurate" in a different way, we'd use
// sqlite3_total_changes() here instead of sqlite3_changed, because that includes rows modified from within a trigger // sqlite3_total_changes() here instead of sqlite3_changed, because that includes rows modified from within a trigger
// For now, this seems sufficient // For now, this seems sufficient
resultSet->setRowsAffected(database->lastChanges()); m_resultSet->setRowsAffected(database->lastChanges());
m_resultSet = resultSet;
db->reportExecuteStatementResult(0, -1, 0); // OK db->reportExecuteStatementResult(0, -1, 0); // OK
return true; return true;
} }
void SQLStatementBackend::setVersionMismatchedError(DatabaseBackend* database) void SQLStatementBackend::setVersionMismatchedError(DatabaseBackend* database)
{ {
ASSERT(!m_error && !m_resultSet); ASSERT(!m_error && !m_resultSet->isValid());
database->reportExecuteStatementResult(7, SQLError::VERSION_ERR, 0); database->reportExecuteStatementResult(7, SQLError::VERSION_ERR, 0);
m_error = SQLError::create(SQLError::VERSION_ERR, "current version of the database and `oldVersion` argument do not match"); m_error = SQLError::create(SQLError::VERSION_ERR, "current version of the database and `oldVersion` argument do not match");
} }
void SQLStatementBackend::setFailureDueToQuota(DatabaseBackend* database) void SQLStatementBackend::setFailureDueToQuota(DatabaseBackend* database)
{ {
ASSERT(!m_error && !m_resultSet); ASSERT(!m_error && !m_resultSet->isValid());
database->reportExecuteStatementResult(8, SQLError::QUOTA_ERR, 0); database->reportExecuteStatementResult(8, SQLError::QUOTA_ERR, 0);
m_error = SQLError::create(SQLError::QUOTA_ERR, "there was not enough remaining storage space, or the storage quota was reached and the user declined to allow more space"); m_error = SQLError::create(SQLError::QUOTA_ERR, "there was not enough remaining storage space, or the storage quota was reached and the user declined to allow more space");
} }
......
...@@ -58,7 +58,7 @@ public: ...@@ -58,7 +58,7 @@ public:
AbstractSQLStatement* frontend(); AbstractSQLStatement* frontend();
virtual PassRefPtr<SQLError> sqlError() const OVERRIDE; virtual PassRefPtr<SQLError> sqlError() const OVERRIDE;
virtual PassRefPtr<SQLResultSet> sqlResultSet() const OVERRIDE; virtual SQLResultSet* sqlResultSet() const OVERRIDE;
private: private:
SQLStatementBackend(PassOwnPtr<AbstractSQLStatement>, const String& statement, SQLStatementBackend(PassOwnPtr<AbstractSQLStatement>, const String& statement,
...@@ -74,7 +74,7 @@ private: ...@@ -74,7 +74,7 @@ private:
bool m_hasErrorCallback; bool m_hasErrorCallback;
RefPtr<SQLError> m_error; RefPtr<SQLError> m_error;
RefPtr<SQLResultSet> m_resultSet; RefPtrWillBeMember<SQLResultSet> m_resultSet;
int m_permissions; int m_permissions;
}; };
......
...@@ -50,7 +50,7 @@ SQLStatementSync::SQLStatementSync(const String& statement, const Vector<SQLValu ...@@ -50,7 +50,7 @@ SQLStatementSync::SQLStatementSync(const String& statement, const Vector<SQLValu
ASSERT(!m_statement.isEmpty()); ASSERT(!m_statement.isEmpty());
} }
PassRefPtr<SQLResultSet> SQLStatementSync::execute(DatabaseSync* db, ExceptionState& exceptionState) PassRefPtrWillBeRawPtr<SQLResultSet> SQLStatementSync::execute(DatabaseSync* db, ExceptionState& exceptionState)
{ {
db->setAuthorizerPermissions(m_permissions); db->setAuthorizerPermissions(m_permissions);
...@@ -91,7 +91,7 @@ PassRefPtr<SQLResultSet> SQLStatementSync::execute(DatabaseSync* db, ExceptionSt ...@@ -91,7 +91,7 @@ PassRefPtr<SQLResultSet> SQLStatementSync::execute(DatabaseSync* db, ExceptionSt
} }
} }
RefPtr<SQLResultSet> resultSet = SQLResultSet::create(); RefPtrWillBeRawPtr<SQLResultSet> resultSet = SQLResultSet::create();
// Step so we can fetch the column names. // Step so we can fetch the column names.
result = statement.step(); result = statement.step();
......
...@@ -31,6 +31,7 @@ ...@@ -31,6 +31,7 @@
#ifndef SQLStatementSync_h #ifndef SQLStatementSync_h
#define SQLStatementSync_h #define SQLStatementSync_h
#include "heap/Handle.h"
#include "modules/webdatabase/sqlite/SQLValue.h" #include "modules/webdatabase/sqlite/SQLValue.h"
#include "modules/webdatabase/DatabaseBasicTypes.h" #include "modules/webdatabase/DatabaseBasicTypes.h"
#include "wtf/Forward.h" #include "wtf/Forward.h"
...@@ -47,7 +48,7 @@ class SQLStatementSync { ...@@ -47,7 +48,7 @@ class SQLStatementSync {
public: public:
SQLStatementSync(const String& statement, const Vector<SQLValue>& arguments, int permissions); SQLStatementSync(const String& statement, const Vector<SQLValue>& arguments, int permissions);
PassRefPtr<SQLResultSet> execute(DatabaseSync*, ExceptionState&); PassRefPtrWillBeRawPtr<SQLResultSet> execute(DatabaseSync*, ExceptionState&);
private: private:
String m_statement; String m_statement;
......
...@@ -83,7 +83,7 @@ void SQLTransactionBackendSync::trace(Visitor* visitor) ...@@ -83,7 +83,7 @@ void SQLTransactionBackendSync::trace(Visitor* visitor)
visitor->trace(m_database); visitor->trace(m_database);
} }
PassRefPtr<SQLResultSet> SQLTransactionBackendSync::executeSQL(const String& sqlStatement, const Vector<SQLValue>& arguments, ExceptionState& exceptionState) PassRefPtrWillBeRawPtr<SQLResultSet> SQLTransactionBackendSync::executeSQL(const String& sqlStatement, const Vector<SQLValue>& arguments, ExceptionState& exceptionState)
{ {
ASSERT(m_database->executionContext()->isContextThread()); ASSERT(m_database->executionContext()->isContextThread());
...@@ -114,7 +114,7 @@ PassRefPtr<SQLResultSet> SQLTransactionBackendSync::executeSQL(const String& sql ...@@ -114,7 +114,7 @@ PassRefPtr<SQLResultSet> SQLTransactionBackendSync::executeSQL(const String& sql
m_database->resetAuthorizer(); m_database->resetAuthorizer();
bool retryStatement = true; bool retryStatement = true;
RefPtr<SQLResultSet> resultSet; RefPtrWillBeRawPtr<SQLResultSet> resultSet;
while (retryStatement) { while (retryStatement) {
retryStatement = false; retryStatement = false;
resultSet = statement.execute(m_database.get(), exceptionState); resultSet = statement.execute(m_database.get(), exceptionState);
......
...@@ -55,7 +55,7 @@ public: ...@@ -55,7 +55,7 @@ public:
~SQLTransactionBackendSync(); ~SQLTransactionBackendSync();
void trace(Visitor*); void trace(Visitor*);
PassRefPtr<SQLResultSet> executeSQL(const String& sqlStatement, const Vector<SQLValue>& arguments, ExceptionState&); PassRefPtrWillBeRawPtr<SQLResultSet> executeSQL(const String& sqlStatement, const Vector<SQLValue>& arguments, ExceptionState&);
DatabaseSync* database() { return m_database.get(); } DatabaseSync* database() { return m_database.get(); }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment