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

Oilpan: Prepare to move SQLError to oilpan heap.

Without this CL, a SQLError object 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, we should not
construct SQLError objects in a database thread. Unlike the SQLResultSet case
[1], SQLError is created by multiple classes and it's hard to make sure they
create SQLError in a context thread.

So, this CL introduce SQLErrorData, which is an immutable data container. We can
construct a SQLErrorData object in a database thread, and move it to a context
thread, then construct a SQLError with the SQLErrorData object.

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

[1] https://src.chromium.org/viewvc/blink?revision=169929&view=revision

BUG=347902

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

git-svn-id: svn://svn.chromium.org/blink/trunk@169989 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 20896bee
......@@ -38,7 +38,7 @@ public:
virtual ~AbstractSQLStatementBackend() { }
virtual void trace(Visitor*) = 0;
virtual PassRefPtr<SQLError> sqlError() const = 0;
virtual SQLErrorData* sqlError() const = 0;
virtual SQLResultSet* sqlResultSet() const = 0;
};
......
......@@ -45,7 +45,7 @@ public:
virtual void requestTransitToState(SQLTransactionState) = 0;
virtual PassRefPtr<SQLError> transactionError() = 0;
virtual SQLErrorData* transactionError() = 0;
virtual AbstractSQLStatement* currentStatement() = 0;
virtual void setShouldRetryCurrentStatement(bool) = 0;
......
......@@ -51,14 +51,14 @@ bool ChangeVersionWrapper::performPreflight(SQLTransactionBackend* transaction)
if (!database->getVersionFromDatabase(actualVersion)) {
int sqliteError = database->sqliteDatabase().lastError();
database->reportChangeVersionResult(1, SQLError::UNKNOWN_ERR, sqliteError);
m_sqlError = SQLError::create(SQLError::UNKNOWN_ERR, "unable to read the current version",
sqliteError, database->sqliteDatabase().lastErrorMsg());
m_sqlError = SQLErrorData::create(SQLError::UNKNOWN_ERR, "unable to read the current version",
sqliteError, database->sqliteDatabase().lastErrorMsg());
return false;
}
if (actualVersion != m_oldVersion) {
database->reportChangeVersionResult(2, SQLError::VERSION_ERR, 0);
m_sqlError = SQLError::create(SQLError::VERSION_ERR, "current version of the database and `oldVersion` argument do not match");
m_sqlError = SQLErrorData::create(SQLError::VERSION_ERR, "current version of the database and `oldVersion` argument do not match");
return false;
}
......@@ -74,8 +74,8 @@ bool ChangeVersionWrapper::performPostflight(SQLTransactionBackend* transaction)
if (!database->setVersionInDatabase(m_newVersion)) {
int sqliteError = database->sqliteDatabase().lastError();
database->reportChangeVersionResult(3, SQLError::UNKNOWN_ERR, sqliteError);
m_sqlError = SQLError::create(SQLError::UNKNOWN_ERR, "unable to set new version in database",
sqliteError, database->sqliteDatabase().lastErrorMsg());
m_sqlError = SQLErrorData::create(SQLError::UNKNOWN_ERR, "unable to set new version in database",
sqliteError, database->sqliteDatabase().lastErrorMsg());
return false;
}
......
......@@ -28,12 +28,13 @@
#ifndef ChangeVersionWrapper_h
#define ChangeVersionWrapper_h
#include "heap/Handle.h"
#include "modules/webdatabase/SQLTransactionBackend.h"
#include "wtf/Forward.h"
namespace WebCore {
class SQLError;
class SQLErrorData;
class ChangeVersionWrapper FINAL : public SQLTransactionWrapper {
public:
......@@ -41,7 +42,7 @@ public:
virtual bool performPreflight(SQLTransactionBackend*) OVERRIDE;
virtual bool performPostflight(SQLTransactionBackend*) OVERRIDE;
virtual SQLError* sqlError() const OVERRIDE { return m_sqlError.get(); }
virtual SQLErrorData* sqlError() const OVERRIDE { return m_sqlError.get(); }
virtual void handleCommitFailedAfterPostflight(SQLTransactionBackend*) OVERRIDE;
private:
......@@ -49,7 +50,7 @@ private:
String m_oldVersion;
String m_newVersion;
RefPtr<SQLError> m_sqlError;
OwnPtr<SQLErrorData> m_sqlError;
};
} // namespace WebCore
......
......@@ -129,8 +129,9 @@ void Database::readTransaction(PassOwnPtr<SQLTransactionCallback> callback, Pass
runTransaction(callback, errorCallback, successCallback, true);
}
static void callTransactionErrorCallback(ExecutionContext*, PassOwnPtr<SQLTransactionErrorCallback> callback, PassRefPtr<SQLError> error)
static void callTransactionErrorCallback(ExecutionContext*, PassOwnPtr<SQLTransactionErrorCallback> callback, PassOwnPtr<SQLErrorData> errorData)
{
RefPtrWillBeRawPtr<SQLError> error = SQLError::create(*errorData);
callback->handleEvent(error.get());
}
......@@ -149,7 +150,7 @@ void Database::runTransaction(PassOwnPtr<SQLTransactionCallback> callback, PassO
OwnPtr<SQLTransactionErrorCallback> callback = transaction->releaseErrorCallback();
ASSERT(callback == originalErrorCallback);
if (callback) {
RefPtr<SQLError> error = SQLError::create(SQLError::UNKNOWN_ERR, "database has been closed");
OwnPtr<SQLErrorData> error = SQLErrorData::create(SQLError::UNKNOWN_ERR, "database has been closed");
executionContext()->postTask(createCallbackTask(&callTransactionErrorCallback, callback.release(), error.release()));
}
}
......
......@@ -35,17 +35,41 @@
namespace WebCore {
class SQLError : public ThreadSafeRefCounted<SQLError>, public ScriptWrappable {
class SQLErrorData {
public:
static PassRefPtr<SQLError> create(unsigned code, const String& message) { return adoptRef(new SQLError(code, message)); }
static PassRefPtr<SQLError> create(unsigned code, const char* message, int sqliteCode, const char* sqliteMessage)
static PassOwnPtr<SQLErrorData> create(unsigned code, const String& message)
{
return adoptPtr(new SQLErrorData(code, message));
}
static PassOwnPtr<SQLErrorData> create(unsigned code, const char* message, int sqliteCode, const char* sqliteMessage)
{
return create(code, String::format("%s (%d %s)", message, sqliteCode, sqliteMessage));
}
static PassOwnPtr<SQLErrorData> create(const SQLErrorData& data)
{
return create(data.code(), data.message());
}
unsigned code() const { return m_code; }
String message() const { return m_message.isolatedCopy(); }
private:
SQLErrorData(unsigned code, const String& message) : m_code(code), m_message(message.isolatedCopy()) { }
unsigned m_code;
String m_message;
};
class SQLError : public ThreadSafeRefCountedWillBeGarbageCollectedFinalized<SQLError>, public ScriptWrappable {
public:
static PassRefPtrWillBeRawPtr<SQLError> create(const SQLErrorData& data) { return adoptRefWillBeNoop(new SQLError(data)); }
void trace(Visitor*) { }
unsigned code() const { return m_data.code(); }
String message() const { return m_data.message(); }
enum SQLErrorCode {
UNKNOWN_ERR = 0,
DATABASE_ERR = 1,
......@@ -62,13 +86,12 @@ public:
static const char versionErrorMessage[];
private:
SQLError(unsigned code, const String& message) : m_code(code), m_message(message.isolatedCopy())
explicit SQLError(const SQLErrorData& data) : m_data(data)
{
ScriptWrappable::init(this);
}
unsigned m_code;
String m_message;
const SQLErrorData m_data;
};
}
......
......@@ -27,6 +27,7 @@
*/
[
WillBeGarbageCollected,
NoInterfaceObject
] interface SQLError {
readonly attribute unsigned long code;
......
......@@ -79,13 +79,15 @@ bool SQLStatement::performCallback(SQLTransaction* transaction)
OwnPtr<SQLStatementCallback> callback = m_statementCallbackWrapper.unwrap();
OwnPtr<SQLStatementErrorCallback> errorCallback = m_statementErrorCallbackWrapper.unwrap();
RefPtr<SQLError> error = m_backend->sqlError();
SQLErrorData* error = m_backend->sqlError();
// Call the appropriate statement callback and track if it resulted in an error,
// because then we need to jump to the transaction error callback.
if (error) {
if (errorCallback)
callbackError = errorCallback->handleEvent(transaction, error.get());
if (errorCallback) {
RefPtrWillBeRawPtr<SQLError> sqlError = SQLError::create(*error);
callbackError = errorCallback->handleEvent(transaction, sqlError.get());
}
} else if (callback) {
RefPtrWillBeRawPtr<SQLResultSet> resultSet = m_backend->sqlResultSet();
callbackError = !callback->handleEvent(transaction, resultSet.get());
......
......@@ -100,9 +100,9 @@ AbstractSQLStatement* SQLStatementBackend::frontend()
return m_frontend.get();
}
PassRefPtr<SQLError> SQLStatementBackend::sqlError() const
SQLErrorData* SQLStatementBackend::sqlError() const
{
return m_error;
return m_error.get();
}
SQLResultSet* SQLStatementBackend::sqlResultSet() const
......@@ -132,9 +132,9 @@ bool SQLStatementBackend::execute(DatabaseBackend* db)
if (result != SQLResultOk) {
WTF_LOG(StorageAPI, "Unable to verify correctness of statement %s - error %i (%s)", m_statement.ascii().data(), result, database->lastErrorMsg());
if (result == SQLResultInterrupt)
m_error = SQLError::create(SQLError::DATABASE_ERR, "could not prepare statement", result, "interrupted");
m_error = SQLErrorData::create(SQLError::DATABASE_ERR, "could not prepare statement", result, "interrupted");
else
m_error = SQLError::create(SQLError::SYNTAX_ERR, "could not prepare statement", result, database->lastErrorMsg());
m_error = SQLErrorData::create(SQLError::SYNTAX_ERR, "could not prepare statement", result, database->lastErrorMsg());
db->reportExecuteStatementResult(1, m_error->code(), result);
return false;
}
......@@ -143,7 +143,7 @@ bool SQLStatementBackend::execute(DatabaseBackend* db)
// If this is the case, they might be trying to do something fishy or malicious
if (statement.bindParameterCount() != m_arguments.size()) {
WTF_LOG(StorageAPI, "Bind parameter count doesn't match number of question marks");
m_error = SQLError::create(db->isInterrupted() ? SQLError::DATABASE_ERR : SQLError::SYNTAX_ERR, "number of '?'s in statement string does not match argument count");
m_error = SQLErrorData::create(db->isInterrupted() ? SQLError::DATABASE_ERR : SQLError::SYNTAX_ERR, "number of '?'s in statement string does not match argument count");
db->reportExecuteStatementResult(2, m_error->code(), 0);
return false;
}
......@@ -158,7 +158,7 @@ bool SQLStatementBackend::execute(DatabaseBackend* db)
if (result != SQLResultOk) {
WTF_LOG(StorageAPI, "Failed to bind value index %i to statement for query '%s'", i + 1, m_statement.ascii().data());
db->reportExecuteStatementResult(3, SQLError::DATABASE_ERR, result);
m_error = SQLError::create(SQLError::DATABASE_ERR, "could not bind value", result, database->lastErrorMsg());
m_error = SQLErrorData::create(SQLError::DATABASE_ERR, "could not bind value", result, database->lastErrorMsg());
return false;
}
}
......@@ -181,7 +181,7 @@ bool SQLStatementBackend::execute(DatabaseBackend* db)
if (result != SQLResultDone) {
db->reportExecuteStatementResult(4, SQLError::DATABASE_ERR, result);
m_error = SQLError::create(SQLError::DATABASE_ERR, "could not iterate results", result, database->lastErrorMsg());
m_error = SQLErrorData::create(SQLError::DATABASE_ERR, "could not iterate results", result, database->lastErrorMsg());
return false;
}
} else if (result == SQLResultDone) {
......@@ -194,11 +194,11 @@ bool SQLStatementBackend::execute(DatabaseBackend* db)
return false;
} else if (result == SQLResultConstraint) {
db->reportExecuteStatementResult(6, SQLError::CONSTRAINT_ERR, result);
m_error = SQLError::create(SQLError::CONSTRAINT_ERR, "could not execute statement due to a constaint failure", result, database->lastErrorMsg());
m_error = SQLErrorData::create(SQLError::CONSTRAINT_ERR, "could not execute statement due to a constaint failure", result, database->lastErrorMsg());
return false;
} else {
db->reportExecuteStatementResult(5, SQLError::DATABASE_ERR, result);
m_error = SQLError::create(SQLError::DATABASE_ERR, "could not execute statement", result, database->lastErrorMsg());
m_error = SQLErrorData::create(SQLError::DATABASE_ERR, "could not execute statement", result, database->lastErrorMsg());
return false;
}
......@@ -215,14 +215,14 @@ void SQLStatementBackend::setVersionMismatchedError(DatabaseBackend* database)
{
ASSERT(!m_error && !m_resultSet->isValid());
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 = SQLErrorData::create(SQLError::VERSION_ERR, "current version of the database and `oldVersion` argument do not match");
}
void SQLStatementBackend::setFailureDueToQuota(DatabaseBackend* database)
{
ASSERT(!m_error && !m_resultSet->isValid());
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 = SQLErrorData::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");
}
void SQLStatementBackend::clearFailureDueToQuota()
......
......@@ -39,7 +39,7 @@ namespace WebCore {
class AbstractSQLStatement;
class DatabaseBackend;
class SQLError;
class SQLErrorData;
class SQLTransactionBackend;
class SQLStatementBackend FINAL : public AbstractSQLStatementBackend {
......@@ -57,7 +57,7 @@ public:
void setVersionMismatchedError(DatabaseBackend*);
AbstractSQLStatement* frontend();
virtual PassRefPtr<SQLError> sqlError() const OVERRIDE;
virtual SQLErrorData* sqlError() const OVERRIDE;
virtual SQLResultSet* sqlResultSet() const OVERRIDE;
private:
......@@ -73,7 +73,7 @@ private:
bool m_hasCallback;
bool m_hasErrorCallback;
RefPtr<SQLError> m_error;
OwnPtr<SQLErrorData> m_error;
RefPtrWillBeMember<SQLResultSet> m_resultSet;
int m_permissions;
......
......@@ -157,7 +157,7 @@ SQLTransactionState SQLTransaction::deliverTransactionCallback()
SQLTransactionState nextState = SQLTransactionState::RunStatements;
if (shouldDeliverErrorCallback) {
m_database->reportStartTransactionResult(5, SQLError::UNKNOWN_ERR, 0);
m_transactionError = SQLError::create(SQLError::UNKNOWN_ERR, "the SQLTransactionCallback was null or threw an exception");
m_transactionError = SQLErrorData::create(SQLError::UNKNOWN_ERR, "the SQLTransactionCallback was null or threw an exception");
nextState = SQLTransactionState::DeliverTransactionErrorCallback;
}
m_database->reportStartTransactionResult(0, -1, 0); // OK
......@@ -174,11 +174,13 @@ SQLTransactionState SQLTransaction::deliverTransactionErrorCallback()
// must be waiting in the idle state waiting for this state to finish.
// Hence, it's thread safe to fetch the backend transactionError without
// a lock.
if (!m_transactionError)
m_transactionError = m_backend->transactionError();
if (!m_transactionError) {
ASSERT(m_backend->transactionError());
m_transactionError = SQLErrorData::create(*m_backend->transactionError());
}
ASSERT(m_transactionError);
errorCallback->handleEvent(m_transactionError.get());
RefPtrWillBeRawPtr<SQLError> error = SQLError::create(*m_transactionError);
errorCallback->handleEvent(error.get());
m_transactionError = nullptr;
}
......@@ -205,7 +207,7 @@ SQLTransactionState SQLTransaction::deliverStatementCallback()
if (result) {
m_database->reportCommitTransactionResult(2, SQLError::UNKNOWN_ERR, 0);
m_transactionError = SQLError::create(SQLError::UNKNOWN_ERR, "the statement callback raised an exception or statement error callback did not return false");
m_transactionError = SQLErrorData::create(SQLError::UNKNOWN_ERR, "the statement callback raised an exception or statement error callback did not return false");
return nextStateForTransactionError();
}
return SQLTransactionState::RunStatements;
......
......@@ -43,7 +43,7 @@ namespace WebCore {
class AbstractSQLTransactionBackend;
class Database;
class ExceptionState;
class SQLError;
class SQLErrorData;
class SQLStatementCallback;
class SQLStatementErrorCallback;
class SQLTransactionCallback;
......@@ -104,7 +104,7 @@ private:
SQLCallbackWrapper<SQLTransactionErrorCallback> m_errorCallbackWrapper;
bool m_executeSqlAllowed;
RefPtr<SQLError> m_transactionError;
OwnPtr<SQLErrorData> m_transactionError;
bool m_readOnly;
};
......
......@@ -433,9 +433,9 @@ AbstractSQLStatement* SQLTransactionBackend::currentStatement()
return m_currentStatementBackend->frontend();
}
PassRefPtr<SQLError> SQLTransactionBackend::transactionError()
SQLErrorData* SQLTransactionBackend::transactionError()
{
return m_transactionError;
return m_transactionError.get();
}
void SQLTransactionBackend::setShouldRetryCurrentStatement(bool shouldRetry)
......@@ -573,7 +573,7 @@ SQLTransactionState SQLTransactionBackend::openTransactionAndPreflight()
if (!m_sqliteTransaction->inProgress()) {
ASSERT(!m_database->sqliteDatabase().transactionInProgress());
m_database->reportStartTransactionResult(2, SQLError::DATABASE_ERR, m_database->sqliteDatabase().lastError());
m_transactionError = SQLError::create(SQLError::DATABASE_ERR, "unable to begin transaction",
m_transactionError = SQLErrorData::create(SQLError::DATABASE_ERR, "unable to begin transaction",
m_database->sqliteDatabase().lastError(), m_database->sqliteDatabase().lastErrorMsg());
m_sqliteTransaction.clear();
return nextStateForTransactionError();
......@@ -585,7 +585,7 @@ SQLTransactionState SQLTransactionBackend::openTransactionAndPreflight()
String actualVersion;
if (!m_database->getActualVersionForTransaction(actualVersion)) {
m_database->reportStartTransactionResult(3, SQLError::DATABASE_ERR, m_database->sqliteDatabase().lastError());
m_transactionError = SQLError::create(SQLError::DATABASE_ERR, "unable to read version",
m_transactionError = SQLErrorData::create(SQLError::DATABASE_ERR, "unable to read version",
m_database->sqliteDatabase().lastError(), m_database->sqliteDatabase().lastErrorMsg());
m_database->disableAuthorizer();
m_sqliteTransaction.clear();
......@@ -599,10 +599,11 @@ SQLTransactionState SQLTransactionBackend::openTransactionAndPreflight()
m_database->disableAuthorizer();
m_sqliteTransaction.clear();
m_database->enableAuthorizer();
m_transactionError = m_wrapper->sqlError();
if (!m_transactionError) {
if (m_wrapper->sqlError()) {
m_transactionError = SQLErrorData::create(*m_wrapper->sqlError());
} else {
m_database->reportStartTransactionResult(4, SQLError::UNKNOWN_ERR, 0);
m_transactionError = SQLError::create(SQLError::UNKNOWN_ERR, "unknown error occurred during transaction preflight");
m_transactionError = SQLErrorData::create(SQLError::UNKNOWN_ERR, "unknown error occurred during transaction preflight");
}
return nextStateForTransactionError();
}
......@@ -699,10 +700,11 @@ SQLTransactionState SQLTransactionBackend::nextStateForCurrentStatementError()
if (m_currentStatementBackend->hasStatementErrorCallback() && !m_sqliteTransaction->wasRolledBackBySqlite())
return SQLTransactionState::DeliverStatementCallback;
m_transactionError = m_currentStatementBackend->sqlError();
if (!m_transactionError) {
if (m_currentStatementBackend->sqlError()) {
m_transactionError = SQLErrorData::create(*m_currentStatementBackend->sqlError());
} else {
m_database->reportCommitTransactionResult(1, SQLError::DATABASE_ERR, 0);
m_transactionError = SQLError::create(SQLError::DATABASE_ERR, "the statement failed to execute");
m_transactionError = SQLErrorData::create(SQLError::DATABASE_ERR, "the statement failed to execute");
}
return nextStateForTransactionError();
}
......@@ -713,10 +715,11 @@ SQLTransactionState SQLTransactionBackend::postflightAndCommit()
// Spec 4.3.2.7: Perform postflight steps, jumping to the error callback if they fail.
if (m_wrapper && !m_wrapper->performPostflight(this)) {
m_transactionError = m_wrapper->sqlError();
if (!m_transactionError) {
if (m_wrapper->sqlError()) {
m_transactionError = SQLErrorData::create(*m_wrapper->sqlError());
} else {
m_database->reportCommitTransactionResult(3, SQLError::UNKNOWN_ERR, 0);
m_transactionError = SQLError::create(SQLError::UNKNOWN_ERR, "unknown error occurred during transaction postflight");
m_transactionError = SQLErrorData::create(SQLError::UNKNOWN_ERR, "unknown error occurred during transaction postflight");
}
return nextStateForTransactionError();
}
......@@ -733,7 +736,7 @@ SQLTransactionState SQLTransactionBackend::postflightAndCommit()
if (m_wrapper)
m_wrapper->handleCommitFailedAfterPostflight(this);
m_database->reportCommitTransactionResult(4, SQLError::DATABASE_ERR, m_database->sqliteDatabase().lastError());
m_transactionError = SQLError::create(SQLError::DATABASE_ERR, "unable to commit transaction",
m_transactionError = SQLErrorData::create(SQLError::DATABASE_ERR, "unable to commit transaction",
m_database->sqliteDatabase().lastError(), m_database->sqliteDatabase().lastErrorMsg());
return nextStateForTransactionError();
}
......
......@@ -42,7 +42,7 @@ namespace WebCore {
class AbstractSQLTransaction;
class DatabaseBackend;
class SQLError;
class SQLErrorData;
class SQLiteTransaction;
class SQLStatementBackend;
class SQLTransactionBackend;
......@@ -53,7 +53,7 @@ public:
virtual ~SQLTransactionWrapper() { }
virtual bool performPreflight(SQLTransactionBackend*) = 0;
virtual bool performPostflight(SQLTransactionBackend*) = 0;
virtual SQLError* sqlError() const = 0;
virtual SQLErrorData* sqlError() const = 0;
virtual void handleCommitFailedAfterPostflight(SQLTransactionBackend*) = 0;
};
......@@ -78,7 +78,7 @@ private:
// APIs called from the frontend published via AbstractSQLTransactionBackend:
virtual void requestTransitToState(SQLTransactionState) OVERRIDE;
virtual PassRefPtr<SQLError> transactionError() OVERRIDE;
virtual SQLErrorData* transactionError() OVERRIDE;
virtual AbstractSQLStatement* currentStatement() OVERRIDE;
virtual void setShouldRetryCurrentStatement(bool) OVERRIDE;
virtual void executeSQL(PassOwnPtr<AbstractSQLStatement>, const String& statement,
......@@ -114,7 +114,7 @@ private:
RefPtrWillBeMember<DatabaseBackend> m_database;
RefPtr<SQLTransactionWrapper> m_wrapper;
RefPtr<SQLError> m_transactionError;
OwnPtr<SQLErrorData> m_transactionError;
bool m_hasCallback;
bool m_hasSuccessCallback;
......
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