IPC plumbing for IndexedDB to snapshot metadata to the renderer.

IPC plumbing for IndexedDB to snapshot metadata to the renderer.

Allows avoiding trivial sync IPC calls for metadata implemented in 
http://webkit.org/b/88467 and sets the stage for more dramatic 
improvements by moving IndexedDB logic from the browser to the 
renderer and eliminating the V8 utility process.

BUG=129470
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=143202

[jsbell, 2012-06-08 19:06:25]

Full landing sequence:

(1) https://bugs.webkit.org/show_bug.cgi?id=88678 
(2) this
(3) https://bugs.webkit.org/show_bug.cgi?id=88467

Please take a look.

[michaeln, 2012-06-08 19:46:51]

https://chromiumcodereview.appspot.com/10533057/diff/2001/content/browser/in_...
File content/browser/in_process_webkit/indexed_db_dispatcher_host.h (right):

https://chromiumcodereview.appspot.com/10533057/diff/2001/content/browser/in_...
content/browser/in_process_webkit/indexed_db_dispatcher_host.h:124:
std::vector<string16>* object_stores);
once the uber OnMetadata() goes in, is the plan to remove these other sync
getters OnName/OnVersion/OnObjectStoreNames/OnUnique... (etc)?

https://chromiumcodereview.appspot.com/10533057/diff/2001/content/common/inde...
File content/common/indexed_db/indexed_db_metadata.h (right):

https://chromiumcodereview.appspot.com/10533057/diff/2001/content/common/inde...
content/common/indexed_db/indexed_db_metadata.h:21: class CONTENT_EXPORT
IndexedDBDatabaseMetadata {
Do these classes really need the CONTENT_EXPORT?

https://chromiumcodereview.appspot.com/10533057/diff/2001/content/common/inde...
content/common/indexed_db/indexed_db_metadata.h:24: explicit
IndexedDBDatabaseMetadata(const WebKit::WebIDBDatabaseMetadata);
Do you want this to be a const input arg?

Also we generally put param names in method defintions too (unlike webkit).

https://chromiumcodereview.appspot.com/10533057/diff/2001/content/common/inde...
content/common/indexed_db/indexed_db_metadata.h:76: explicit
IndexedDBIndexMetadata(const WebKit::WebIDBIndexMetadata);
ditto const ref + param name

[jsbell, 2012-06-08 20:11:57]

https://chromiumcodereview.appspot.com/10533057/diff/2001/content/browser/in_...
File content/browser/in_process_webkit/indexed_db_dispatcher_host.h (right):

https://chromiumcodereview.appspot.com/10533057/diff/2001/content/browser/in_...
content/browser/in_process_webkit/indexed_db_dispatcher_host.h:124:
std::vector<string16>* object_stores);
On 2012/06/08 19:46:51, michaeln wrote:
> once the uber OnMetadata() goes in, is the plan to remove these other sync
> getters OnName/OnVersion/OnObjectStoreNames/OnUnique... (etc)?

Yes!

https://chromiumcodereview.appspot.com/10533057/diff/2001/content/common/inde...
File content/common/indexed_db/indexed_db_metadata.h (right):

https://chromiumcodereview.appspot.com/10533057/diff/2001/content/common/inde...
content/common/indexed_db/indexed_db_metadata.h:21: class CONTENT_EXPORT
IndexedDBDatabaseMetadata {
On 2012/06/08 19:46:51, michaeln wrote:
> Do these classes really need the CONTENT_EXPORT?

Nope!

https://chromiumcodereview.appspot.com/10533057/diff/2001/content/common/inde...
content/common/indexed_db/indexed_db_metadata.h:24: explicit
IndexedDBDatabaseMetadata(const WebKit::WebIDBDatabaseMetadata);
On 2012/06/08 19:46:51, michaeln wrote:
> Do you want this to be a const input arg?

Oops, missing &.

> Also we generally put param names in method defintions too (unlike webkit).

Fixed, thanks.

https://chromiumcodereview.appspot.com/10533057/diff/2001/content/common/inde...
content/common/indexed_db/indexed_db_metadata.h:76: explicit
IndexedDBIndexMetadata(const WebKit::WebIDBIndexMetadata);
On 2012/06/08 19:46:51, michaeln wrote:
> ditto const ref + param name

Done.

[jsbell, 2012-06-08 20:15:27]

+jam@ for OWNERS review of content/content_common.gypi (and general
wisdom/awesomeness)

[michaeln, 2012-06-08 20:22:30]

lgtm, will be nice to that bunch of sync getters go :)

[jsbell, 2012-06-12 00:22:28]

Based on offline feedback from jam@, used IPC macros to define structs rather
than introducing new class files.

jam@ - is this about what you were thinking?

Note that this relies on restructured Chromium WebKit API which I haven't upload
yet, and the "one top level class per file" restriction 
 means I may need to alter the #includes here.

[jam, 2012-06-12 00:44:33]

lgtm

http://codereview.chromium.org/10533057/diff/12001/content/browser/in_process...
File content/browser/in_process_webkit/indexed_db_dispatcher_host.cc (right):

http://codereview.chromium.org/10533057/diff/12001/content/browser/in_process...
content/browser/in_process_webkit/indexed_db_dispatcher_host.cc:366: 
nit: blank line

http://codereview.chromium.org/10533057/diff/12001/content/browser/in_process...
content/browser/in_process_webkit/indexed_db_dispatcher_host.cc:375: 
ditto

[jsbell, 2012-06-12 16:55:53]

Updated to match WebKit side: https://bugs.webkit.org/show_bug.cgi?id=88678

[jam, 2012-06-13 16:19:05]

are you waiting for another review? for things like this (renaming after a
webkit roll) you don't need another lgtm :)

[jsbell, 2012-06-13 16:22:06]

On 2012/06/13 16:19:05, John Abd-El-Malek wrote:
> are you waiting for another review? for things like this (renaming after a
> webkit roll) you don't need another lgtm :)

I need a review on the WebKit side which needs to land first. :P

https://bugs.webkit.org/show_bug.cgi?id=88678 (which unblocks this CL) and
https://bugs.webkit.org/show_bug.cgi?id=88467 (which is the final change that
actually uses this plumbing)

[jsbell, 2012-06-19 23:48:35]

https://bugs.webkit.org/show_bug.cgi?id=88678 finally landed, so as soon as DEPS
rolls WebKit r120753 into Chromium this can land.

[commit-bot: I haz the power, 2012-06-20 16:15:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jsbell@chromium.org/10533057/12002

[commit-bot: I haz the power, 2012-06-20 17:38:12]

Change committed as 143202