Bug 103921 - IndexedDB: Stub out transaction-backend methods
Summary: IndexedDB: Stub out transaction-backend methods
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alec Flett
URL:
Keywords:
Depends on:
Blocks: 103922
  Show dependency treegraph
 
Reported: 2012-12-03 13:34 PST by Alec Flett
Modified: 2012-12-05 10:53 PST (History)
8 users (show)

See Also:


Attachments
Patch (49.96 KB, patch)
2012-12-03 17:16 PST, Alec Flett
no flags Details | Formatted Diff | Diff
Patch (53.56 KB, patch)
2012-12-04 12:02 PST, Alec Flett
no flags Details | Formatted Diff | Diff
Patch (45.21 KB, patch)
2012-12-04 15:28 PST, Alec Flett
no flags Details | Formatted Diff | Diff
Patch for landing (45.27 KB, patch)
2012-12-04 15:54 PST, Alec Flett
no flags Details | Formatted Diff | Diff
Patch for landing (46.22 KB, patch)
2012-12-05 10:11 PST, Alec Flett
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alec Flett 2012-12-03 13:34:04 PST
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
Comment 1 Alec Flett 2012-12-03 17:16:48 PST
Created attachment 177379 [details]
Patch
Comment 2 WebKit Review Bot 2012-12-03 17:18:22 PST
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 3 Alec Flett 2012-12-04 12:02:15 PST
Created attachment 177523 [details]
Patch
Comment 4 Alec Flett 2012-12-04 12:02:49 PST
jsbell/dgrogan - take a look? this is just the method stub-out phase.
Comment 5 Joshua Bell 2012-12-04 12:21:33 PST
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.
Comment 6 Alec Flett 2012-12-04 15:28:39 PST
Created attachment 177585 [details]
Patch
Comment 7 Dimitri Glazkov (Google) 2012-12-04 15:48:41 PST
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.
Comment 8 Alec Flett 2012-12-04 15:54:29 PST
Created attachment 177591 [details]
Patch for landing
Comment 9 WebKit Review Bot 2012-12-04 20:30:19 PST
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
Comment 10 Alec Flett 2012-12-05 10:11:06 PST
Created attachment 177786 [details]
Patch for landing
Comment 11 WebKit Review Bot 2012-12-05 10:53:13 PST
Comment on attachment 177786 [details]
Patch for landing

Clearing flags on attachment: 177786

Committed r136714: <http://trac.webkit.org/changeset/136714>
Comment 12 WebKit Review Bot 2012-12-05 10:53:17 PST
All reviewed patches have been landed.  Closing bug.