[Webkit-unassigned] [Bug 28918] Add support for Database.readTransaction()
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Sep 8 16:22:23 PDT 2009
https://bugs.webkit.org/show_bug.cgi?id=28918
Dimitri Glazkov (Google) <dglazkov at chromium.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #39219|review? |review-
Flag| |
--- Comment #11 from Dimitri Glazkov (Google) <dglazkov at chromium.org> 2009-09-08 16:22:22 PDT ---
(From update of attachment 39219)
These are all style nits. Overall, looks great!
> + * bindings/js/JSDatabaseCustom.cpp:
> + (WebCore::DatabaseTransactionHelper):
> + (WebCore::JSDatabase::transaction):
> + (WebCore::JSDatabase::readTransaction):
> + * bindings/v8/custom/V8CustomBinding.h:
> + * bindings/v8/custom/V8DatabaseCustom.cpp:
> + (WebCore::DatabaseTransactionHelper):
> + (WebCore::DatabaseReadTransaction):
> + * storage/Database.cpp:
> + (WebCore::Database::changeVersion):
> + (WebCore::Database::transaction):
> + * storage/Database.h:
> + * storage/Database.idl:
> + * storage/DatabaseAuthorizer.cpp:
> + (WebCore::DatabaseAuthorizer::createView):
> + (WebCore::DatabaseAuthorizer::createTempView):
> + (WebCore::DatabaseAuthorizer::dropView):
> + (WebCore::DatabaseAuthorizer::dropTempView):
> + * storage/DatabaseAuthorizer.h:
> + * storage/SQLTransaction.cpp:
> + (WebCore::SQLTransaction::create):
> + (WebCore::SQLTransaction::SQLTransaction):
> + (WebCore::SQLTransaction::executeSQL):
> + (WebCore::SQLTransaction::acquireLock):
> + * storage/SQLTransaction.h:
> + * storage/SQLTransactionCoordinator.cpp:
> + (WebCore::SQLTransactionCoordinator::acquireLock):
> + * storage/SQLTransactionCoordinator.h:
Would be good to have short descriptions in the ChangeLog.
> -JSValue JSDatabase::transaction(ExecState* exec, const ArgList& args)
> +static JSValue DatabaseTransactionHelper(ExecState* exec, const ArgList& args, Database* database, bool readOnly)
camelCase, and badName :)
How about createTransaction?
>
> -CALLBACK_FUNC_DECL(DatabaseTransaction)
> +static v8::Handle<v8::Value> DatabaseTransactionHelper(const v8::Arguments& args, bool readOnly)
Ditto.
> + m_transactionQueue.append(SQLTransaction::create(this, callback, errorCallback, successCallback, ChangeVersionWrapper::create(oldVersion, newVersion), false));
It's not obvious what "false" is for here. How about we just make readOnly a
default param and omit the "false" here?
> + static PassRefPtr<SQLTransaction> create(Database*, PassRefPtr<SQLTransactionCallback>, PassRefPtr<SQLTransactionErrorCallback>,
> + PassRefPtr<VoidCallback>, PassRefPtr<SQLTransactionWrapper>, bool readOnly);
bool readOnly = false
> -void SQLTransactionCoordinator::acquireLock(SQLTransaction* transaction)
> +void SQLTransactionCoordinator::acquireLock(SQLTransaction* transaction, bool)
use UNUSED_PARAM here, bool by itself is lonely.
> +function runTest() {
brace on new line.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list