Bug 122284 - Move IDBFactoryBackend creation to DatabaseStrategy
Summary: Move IDBFactoryBackend creation to DatabaseStrategy
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-03 10:20 PDT by Brady Eidson
Modified: 2013-10-08 11:44 PDT (History)
4 users (show)

See Also:


Attachments
Patch v1 (8.96 KB, patch)
2013-10-03 10:23 PDT, Brady Eidson
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2013-10-03 10:20:38 PDT
Move IDBFactoryBackend creation to DatabaseStrategy
Comment 1 Brady Eidson 2013-10-03 10:23:40 PDT
Created attachment 213274 [details]
Patch v1
Comment 2 Alexey Proskuryakov 2013-10-03 10:37:08 PDT
Comment on attachment 213274 [details]
Patch v1

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

> Source/WebCore/Modules/indexeddb/IDBFactory.h:60
> +    // FIXME: getDatabaseNames is no longer a web-facting API, and should be removed from IDBFactory.

*web-facing*

> Source/WebKit2/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:221
> +    return DatabaseStrategy::createIDBFactoryBackend(); // Use the default for now.

I don't think that a "for now" comment helps, it should either be more detailed, or removed.
Comment 3 Brady Eidson 2013-10-03 10:53:59 PDT
http://trac.webkit.org/changeset/156842
Comment 4 Sam Weinig 2013-10-04 15:07:53 PDT
I don't think a PlatformStrategy is the right approach here, and I think it was the wrong to have a DatabaseStrategy in the first place.  IndexDB is not a platform concept, and thus should not be referenced in Platform.  I know we have some of these violations already, but we don't need to add more.

Instead, I think we should stick with good old client interfaces for these (maybe add a new PageClient).
Comment 5 Brady Eidson 2013-10-08 11:44:03 PDT
(In reply to comment #4)
> I don't think a PlatformStrategy is the right approach here, and I think it was the wrong to have a DatabaseStrategy in the first place.  IndexDB is not a platform concept, and thus should not be referenced in Platform.  I know we have some of these violations already, but we don't need to add more.

The backend for the IndexDB is a platform concept.

Every time this comes up I ask for an actual rule to decide whether we use platform strategies or a client, and there's never really been one.

> Instead, I think we should stick with good old client interfaces for these (maybe add a new PageClient).

Bad name, considering a mid-term goal of phasing it out.

Also, does making it a PageClient concept make sense when IDB is accessible from a (possibly shared) worker?

I'll revisit this once we have the right answers, but will be moving forward with the rest of the implementation in the meantime.