| Summary: | Move IDBFactoryBackend creation to DatabaseStrategy | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Brady Eidson <beidson> | ||||
| Component: | WebCore Misc. | Assignee: | Brady Eidson <beidson> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | alecflett, commit-queue, jsbell, sam | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Attachments: |
|
||||||
|
Description
Brady Eidson
2013-10-03 10:20:38 PDT
Created attachment 213274 [details]
Patch v1
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. 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). (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. |