Bug 103920

Summary: IndexedDB: Pass metadata in to IDBOpenDBRequest.onUpgradeNeeded/onSuccess
Product: WebKit Reporter: Joshua Bell <jsbell>
Component: WebCore Misc.Assignee: Alec Flett <alecflett>
Severity: Normal CC: abarth, alecflett, dglazkov, dgrogan, fishd, jamesr, jsbell, tkent+wkapi, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 103763, 108151    
Bug Blocks:    
Description Flags
Patch for landing
Patch for landing none

Description Joshua Bell 2012-12-03 13:33:30 PST
Currently the front-end needs to query the back-end for metadata during these two handlers. In multi-process ports (like Chromium) that may be a synchronous IPC call.

Since the data will be needed anyway, the back-end should just supply it along with the other data in the (async) callbacks.
Comment 1 Alec Flett 2013-01-24 13:58:16 PST
Created attachment 184573 [details]
Comment 2 Alec Flett 2013-01-24 13:58:52 PST
jsbell@ - r?
Comment 3 WebKit Review Bot 2013-01-24 14:38:00 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 4 Joshua Bell 2013-01-24 14:43:56 PST
Comment on attachment 184573 [details]

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

lgtm, knowing that there will be a cleanup pass that removes all the duplicate methods

> Source/WebKit/chromium/src/IDBCallbacksProxy.cpp:185
> +    m_callbacks->onUpgradeNeeded(oldVersion, 0, new WebIDBDatabaseImpl(database, m_databaseCallbacks));

Question for reviewers: 0 is still preferred over nullptr, correct?

> Source/WebKit/chromium/src/WebIDBCallbacksImpl.cpp:82
> +    m_callbacks->onSuccess(localDatabaseProxy, localDatabaseProxy->metadata());

Nit: can use localDatabaseProxy.release() to avoid refcount churn

> Source/WebKit/chromium/src/WebIDBCallbacksImpl.cpp:92
> +    m_callbacks->onSuccess(localDatabaseProxy, metadata);

Nit: can use localDatabaseProxy.release() to avoid refcount churn
Comment 5 Alec Flett 2013-01-24 14:52:18 PST
Created attachment 184587 [details]
Comment 6 Alec Flett 2013-01-24 14:52:58 PST
dglazkov - r? (mostly webkit API refactoring)
Comment 7 Alec Flett 2013-01-28 21:34:21 PST
Comment on attachment 184587 [details]

optimistically landing now that the smoke has cleared from the earlier test failures.
Comment 8 WebKit Review Bot 2013-01-28 21:41:43 PST
Comment on attachment 184587 [details]

Clearing flags on attachment: 184587

Committed r141049: <http://trac.webkit.org/changeset/141049>
Comment 9 WebKit Review Bot 2013-01-28 21:41:47 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 WebKit Review Bot 2013-01-28 22:44:26 PST
Re-opened since this is blocked by bug 108151
Comment 11 Joshua Bell 2013-01-28 23:55:43 PST
Link to failing tests?
Comment 12 Alec Flett 2013-01-29 10:32:58 PST
Created attachment 185261 [details]
Patch for landing
Comment 13 WebKit Review Bot 2013-01-29 10:35:12 PST
Comment on attachment 185261 [details]
Patch for landing

Rejecting attachment 185261 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-04', 'apply-attachment', '--no-update', '--non-interactive', 185261, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:

patching file Source/WebKit/chromium/src/WebIDBCallbacksImpl.cpp
Hunk #2 succeeded at 128 (offset -5 lines).
patching file Source/WebKit/chromium/src/WebIDBCallbacksImpl.h
Hunk #2 succeeded at 59 with fuzz 1 (offset -1 lines).
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', '--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/16022335
Comment 14 Alec Flett 2013-01-29 11:28:48 PST
Created attachment 185274 [details]
Patch for landing
Comment 15 WebKit Review Bot 2013-01-29 12:34:23 PST
Comment on attachment 185274 [details]
Patch for landing

Clearing flags on attachment: 185274

Committed r141142: <http://trac.webkit.org/changeset/141142>
Comment 16 WebKit Review Bot 2013-01-29 12:34:27 PST
All reviewed patches have been landed.  Closing bug.