Bug 103921 – IndexedDB: Stub out transaction-backend methods

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Alec Flett]

this patch stubs about 5 methods:

* IDBDatabaseBackendInterface::createTransaction - a new version of this that can be called asynchronously. 
* IDBDatabaseBackendInterface::abort() and ::commit() - the two primary methods of the soon-to-be-deprecated IDBTransactionBackend
* IDBDatabaseCallbacks::onAbort() and ::onComplete() - the two primary callbacks of the soon-to-be-deprecated IDBTransactionCallbacks

[Alec Flett]

Created attachment 177379 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Alec Flett]

Created attachment 177523 [details]
Patch

[Alec Flett]

jsbell/dgrogan - take a look? this is just the method stub-out phase.

[Joshua Bell]

Comment on attachment 177523 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=177523&action=review

> Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp:64
> +    static PassOwnPtr<ScriptExecutionContext::Task> create(PassRefPtr<IDBDatabaseBackendImpl> database, int64_t transactionId, int64_t version, PassRefPtr<IDBCallbacks> callbacks, PassRefPtr<IDBDatabaseCallbacks> databaseCallbacks, PassRefPtr<IDBTransactionBackendImpl> transaction)

Why is the transactionId needed here? This operation must, by definition, exist within a IDBTransactionBackendImpl which must already have been created and should have an ID assigned.

> Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.h:71
> +    virtual void commit(int64_t) { ASSERT_NOT_REACHED(); }

Give a name for the ID parameters.

> Source/WebCore/Modules/indexeddb/IDBDatabaseBackendInterface.h:64
> +    virtual void commit(int64_t) = 0;

Give a name for the ID parameters.

> Source/WebCore/Modules/indexeddb/IDBDatabaseCallbacks.h:47
> +    virtual void onAbort(int64_t, PassRefPtr<IDBDatabaseError>) = 0;

Give a name for the ID parameters.

> Source/WebCore/Modules/indexeddb/IDBDatabaseCallbacksImpl.h:49
> +    virtual void onAbort(int64_t, PassRefPtr<IDBDatabaseError>);

Give a name for the ID parameters.

> Source/WebCore/Modules/indexeddb/IDBOpenDBRequest.cpp:71
> +void IDBOpenDBRequest::onUpgradeNeeded(int64_t oldVersion, int64_t transactionId, PassRefPtr<IDBTransactionBackendInterface> prpTransactionBackend, PassRefPtr<IDBDatabaseBackendInterface> prpDatabaseBackend)

It should not be necessary to return the transaction ID from the back-end to the front-end; the open() call can pass it to the IDBOpenDBRequest which can hold onto it. Passing the ID back could also reveal information about the back-end.

> Source/WebCore/Modules/indexeddb/IDBOpenDBRequest.cpp:107
> +void IDBOpenDBRequest::onUpgradeNeeded(int64_t oldVersion, PassRefPtr<IDBTransactionBackendInterface> prpTransactionBackend, PassRefPtr<IDBDatabaseBackendInterface> prpDatabaseBackend)

Remove the "prp" prefix if these aren't being assigned to local RefPtrs.

> Source/WebKit/chromium/public/WebIDBDatabase.h:65
> +    virtual void abort(long long) { WEBKIT_ASSERT_NOT_REACHED(); }

Give a name for the ID parameters.

> Source/WebKit/chromium/src/IDBDatabaseBackendProxy.h:54
> +    virtual void commit(int64_t);

Give a name for the ID parameters.

> Source/WebKit/chromium/src/IDBDatabaseCallbacksProxy.h:47
> +    virtual void onAbort(int64_t, PassRefPtr<WebCore::IDBDatabaseError>);

Give a name for the ID parameters.

> Source/WebKit/chromium/src/WebIDBDatabaseImpl.h:62
> +    virtual void abort(long long);

Give a name for the ID parameters.

> Source/WebKit/chromium/tests/IDBAbortOnCorruptTest.cpp:111
> +    virtual void onAbort(int64_t, PassRefPtr<IDBDatabaseError>) OVERRIDE { }

Give a name for the ID parameters.

> Source/WebKit/chromium/tests/IDBDatabaseBackendTest.cpp:111
> +    virtual void onAbort(int64_t, PassRefPtr<IDBDatabaseError>) OVERRIDE { }

Give a name for the ID parameters.

> Source/WebKit/chromium/tests/IDBDatabaseBackendTest.cpp:167
> +    virtual void abort(int64_t) { }

Give a name for the ID parameters.

[Alec Flett]

Created attachment 177585 [details]
Patch

[Dimitri Glazkov (Google)]

Comment on attachment 177585 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=177585&action=review

> Source/WebCore/Modules/indexeddb/IDBFactory.cpp:132
> +    // FIXME: pass the transactionId in here

File a bug and reference here?

> Source/WebKit/chromium/src/IDBFactoryBackendProxy.h:52
> +    // FIXME: Remove old method in https://bugs.webkit.org/show_bug.cgi?id=103923.

old -> this here and elsewhere. "This method" (that is, the one just below) is easier to understand out of the context of this patch.

[Alec Flett]

Created attachment 177591 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 177591 [details]
Patch for landing

Rejecting attachment 177591 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
bKit/chromium/src/WebIDBDatabaseImpl.cpp
patching file Source/WebKit/chromium/src/WebIDBDatabaseImpl.h
patching file Source/WebKit/chromium/src/WebIDBFactoryImpl.cpp
patching file Source/WebKit/chromium/src/WebIDBFactoryImpl.h
patching file Source/WebKit/chromium/tests/IDBAbortOnCorruptTest.cpp
patching file Source/WebKit/chromium/tests/IDBDatabaseBackendTest.cpp

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/15138450

[Alec Flett]

Created attachment 177786 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 177786 [details]
Patch for landing

Clearing flags on attachment: 177786

Committed r136714: <http://trac.webkit.org/changeset/136714>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.