Move Quota related code out of DOMWindow and into the quota/ folder. (Separated out from bug 88396)
Created attachment 146245 [details] Patch
Created attachment 146246 [details] Patch
Comment on attachment 146246 [details] Patch Attachment 146246 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12921196 New failing tests: fast/table/multiple-captions-display.xhtml
Created attachment 146295 [details] Archive of layout-test-results from ec2-cr-linux-02 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-02 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
(In reply to comment #3) > (From update of attachment 146246 [details]) > Attachment 146246 [details] did not pass chromium-ews (chromium-xvfb): > Output: http://queues.webkit.org/results/12921196 > > New failing tests: > fast/table/multiple-captions-display.xhtml This test failure is unrelated to the change.
Comment on attachment 146246 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=146246&action=review The main thing to address before landing is caching the StorageInfo object. I'm happy to take a look at an updated patch if you're unsure about the Supplement pattern. > Source/WebCore/Modules/quota/DOMWindowStorageInfo.cpp:45 > + return StorageInfo::create(); Do we really want to return a new one each time? I would have thought we'd want to cache it so that window.webkitStorageInfo === window.webkitStorageInfo > Source/WebCore/Modules/quota/DOMWindowStorageInfo.h:43 > +class DOMWindowStorageInfo { DOMWindowStorageInfo -> DOMWindowQuota (we're trying to have these names match the module name) > Source/WebCore/Modules/quota/StorageInfo.cpp:64 > +#if !PLATFORM(CHROMIUM) > +void StorageInfo::queryUsageAndQuota(ScriptExecutionContext*, int, PassRefPtr<StorageInfoUsageCallback>, PassRefPtr<StorageInfoErrorCallback>) > +{ > + notImplemented(); > +} > + > +void StorageInfo::requestQuota(ScriptExecutionContext*, int, unsigned long long, PassRefPtr<StorageInfoQuotaCallback>, PassRefPtr<StorageInfoErrorCallback>) > +{ > + notImplemented(); > +} > +#endif Is there a reason to turn ENABLE(QUOTA) on without implementing these functions? If so, we do this is with a StorageInfoNone.cpp that folks can link in if they don't implement the feature. > Source/WebCore/Modules/quota/StorageInfo.idl:30 > + JSGenerateToNativeObject Why is this needed? > Source/WebCore/page/DOMWindow.cpp:-1946 > -StorageInfo* DOMWindow::webkitStorageInfo() const > -{ > - if (!isCurrentlyDisplayedInFrame()) > - return 0; > - if (!m_storageInfo) > - m_storageInfo = StorageInfo::create(); > - return m_storageInfo.get(); > -} Ah, so it used to be cached. We'll need to implement something like this logic in DOMWindowQuota. If you look at some of the other modules, you can see how to do this with the Supplement pattern. > Source/WebCore/storage/StorageInfo.cpp:-54 > -#if !PLATFORM(CHROMIUM) Ah, I see this already existed. It's fine to keep this in this patch since you're just moving the file, but consider a followup patch that removes the PLATFORM(CHROMIUM) ifdefs. > Source/WebCore/storage/StorageInfo.idl:-30 > - JSGenerateToNativeObject Ah, this is here too. Ok. I wonder if its needed...
Created attachment 146503 [details] Patch
Comment on attachment 146246 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=146246&action=review >> Source/WebCore/Modules/quota/DOMWindowStorageInfo.cpp:45 >> + return StorageInfo::create(); > > Do we really want to return a new one each time? I would have thought we'd want to cache it so that window.webkitStorageInfo === window.webkitStorageInfo Done. >> Source/WebCore/Modules/quota/DOMWindowStorageInfo.h:43 >> +class DOMWindowStorageInfo { > > DOMWindowStorageInfo -> DOMWindowQuota (we're trying to have these names match the module name) Done. >> Source/WebCore/Modules/quota/StorageInfo.cpp:64 >> +#endif > > Is there a reason to turn ENABLE(QUOTA) on without implementing these functions? If so, we do this is with a StorageInfoNone.cpp that folks can link in if they don't implement the feature. Good point, I don't think we need this actually. Just dropped them. >> Source/WebCore/Modules/quota/StorageInfo.idl:30 >> + JSGenerateToNativeObject > > Why is this needed? Hmm.. doesn't look we need it. Removed.
Created attachment 146506 [details] Patch
(In reply to comment #6) > (From update of attachment 146246 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=146246&action=review > > The main thing to address before landing is caching the StorageInfo object. I'm happy to take a look at an updated patch if you're unsure about the Supplement pattern. Updated the patch to use the Supplement pattern to cache the StorageInfo object. Adam, can you take one more quick look? Thanks!
Comment on attachment 146506 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=146506&action=review Looks great. Thanks! > Source/WebCore/Modules/quota/DOMWindowQuota.cpp:51 > +// static We usually skip these sorts of comments.
Committed r119954: <http://trac.webkit.org/changeset/119954>