Bug 108638 - [Chromium] Move IDBFactoryBackendInterface to WebCore
Summary: [Chromium] Move IDBFactoryBackendInterface to WebCore
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Pilgrim (Google)
URL:
Keywords:
Depends on:
Blocks: 106829
  Show dependency treegraph
 
Reported: 2013-02-01 07:52 PST by Mark Pilgrim (Google)
Modified: 2013-02-04 10:53 PST (History)
9 users (show)

See Also:


Attachments
Patch (14.09 KB, patch)
2013-02-01 07:54 PST, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff
Patch (12.96 KB, patch)
2013-02-01 15:19 PST, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff
Patch (13.00 KB, patch)
2013-02-04 07:24 PST, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Pilgrim (Google) 2013-02-01 07:52:33 PST
[Chromium] Move IDBFactoryBackendInterface to WebCore
Comment 1 Mark Pilgrim (Google) 2013-02-01 07:54:08 PST
Created attachment 186048 [details]
Patch
Comment 2 Mark Pilgrim (Google) 2013-02-01 07:55:20 PST
Comment on attachment 186048 [details]
Patch

Same technique as bug 108488.
Comment 3 Adam Barth 2013-02-01 11:10:37 PST
Comment on attachment 186048 [details]
Patch

Ok.
Comment 4 Adam Barth 2013-02-01 11:13:56 PST
Comment on attachment 186048 [details]
Patch

I'm not super excited about this patch.  The difference between this patch and the media player patch is that IndexedDB should be scoped to some sort of storage context.  For example, the PageGroup, rather than being a static.

That said, I think we should go forward with this patch for two reasons:

1) We're already using a static, so we're not really losing anything.
2) We don't currently have the notion of a "storage context", so there isn't really anything to hang this off of today.

I suspect we'll eventually revisit this issue, but at least the problem is more visible now that there is code in the initialize function that mentions the static.
Comment 5 Mark Pilgrim (Google) 2013-02-01 15:19:52 PST
Created attachment 186159 [details]
Patch
Comment 6 Mark Pilgrim (Google) 2013-02-01 15:20:43 PST
Comment on attachment 186159 [details]
Patch

Resubmitting same patch due to SVN weirdness. Let's make sure EWS goes green before committing.
Comment 7 Mark Pilgrim (Google) 2013-02-01 15:23:09 PST
(In reply to comment #4)
> (From update of attachment 186048 [details])
> I'm not super excited about this patch.  The difference between this patch and the media player patch is that IndexedDB should be scoped to some sort of storage context.  For example, the PageGroup, rather than being a static.

Can't use PageGroup because workers. (We tried.)

> I suspect we'll eventually revisit this issue, but at least the problem is more visible now that there is code in the initialize function that mentions the static.

Agreed.
Comment 8 WebKit Review Bot 2013-02-01 16:03:24 PST
Comment on attachment 186159 [details]
Patch

Attachment 186159 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16341195
Comment 9 Adam Barth 2013-02-02 00:05:30 PST
Comment on attachment 186159 [details]
Patch

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

> Source/WebCore/Modules/indexeddb/chromium/IDBFactoryBackendInterfaceChromium.cpp:44
> +    ASSERT(s_idbFactoryCreateFunction);

Looks like you've got the wrong name in this ASSERT.
Comment 10 Mark Pilgrim (Google) 2013-02-04 07:24:48 PST
Created attachment 186372 [details]
Patch
Comment 11 Mark Pilgrim (Google) 2013-02-04 07:25:27 PST
Comment on attachment 186372 [details]
Patch

FIxed ASSERT.
Comment 12 WebKit Review Bot 2013-02-04 10:53:38 PST
Comment on attachment 186372 [details]
Patch

Clearing flags on attachment: 186372

Committed r141784: <http://trac.webkit.org/changeset/141784>
Comment 13 WebKit Review Bot 2013-02-04 10:53:43 PST
All reviewed patches have been landed.  Closing bug.