[chromium] expose the filename used for a given indexed DB
Created attachment 68082 [details] Patch
Comment on attachment 68082 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=68082&action=review > WebCore/storage/IDBFactoryBackendInterface.cpp:48 > +String IDBFactoryBackendInterface::fileNameForIDB(const String& name, SecurityOrigin* securityOrigin) Let's not duplicate code. See below comment. > WebCore/storage/IDBFactoryBackendInterface.h:57 > + static String fileNameForIDB(const String& name, SecurityOrigin*); Put this on the Impl.h/cpp since the interface.cpp has a chromium and a non-chromium version. It's fine to leave the WebKit side of things as is tho. > WebCore/storage/chromium/IDBFactoryBackendInterface.cpp:32 > +#include "FileSystem.h" remove (per my above comment) > WebCore/storage/chromium/IDBFactoryBackendInterface.cpp:44 > +String IDBFactoryBackendInterface::fileNameForIDB(const String& name, SecurityOrigin* securityOrigin) remove (per my above comment)
Attachment 68082 [details] did not build on chromium: Build output: http://queues.webkit.org/results/4015054
Created attachment 68086 [details] Patch
Comment on attachment 68086 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=68086&action=review > WebCore/storage/IDBFactoryBackendInterface.cpp:31 > +#include "FileSystem.h" revert this file > WebKit/chromium/public/WebIDBFactory.h:68 > + WEBKIT_API static WebString fileNameForIDB(const WebString& name, const WebSecurityOrigin& origin); databaseFileName seems more clear. Also use that name in WebCore? > WebKit/chromium/src/WebIDBFactory.cpp:36 > +#if ENABLE(INDEXED_DATABASE) move this above the #include (the latest style these days) > WebKit/chromium/src/WebIDBFactory.cpp:42 > +// static remove comment Looks close.
Attachment 68086 [details] did not build on chromium: Build output: http://queues.webkit.org/results/4002068
Created attachment 68087 [details] Patch
Comment on attachment 68087 [details] Patch r=me
Comment on attachment 68087 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=68087&action=review > WebKit/chromium/public/WebIDBFactory.h:68 > + WEBKIT_API static WebString databaseFileName(const WebString& name, const WebSecurityOrigin& origin); nit: no need for the "origin" parameter name since it doesn't add any information. > WebKit/chromium/src/WebIDBFactory.cpp:44 > + return WebString(IDBFactoryBackendImpl::databaseFileName(name, origin)); nit: you should not need to call the WebString constructor explicitly here.
(In reply to comment #9) > (From update of attachment 68087 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=68087&action=review > > > WebKit/chromium/public/WebIDBFactory.h:68 > > + WEBKIT_API static WebString databaseFileName(const WebString& name, const WebSecurityOrigin& origin); > > nit: no need for the "origin" parameter name since it doesn't add any information. > > > WebKit/chromium/src/WebIDBFactory.cpp:44 > > + return WebString(IDBFactoryBackendImpl::databaseFileName(name, origin)); > > nit: you should not need to call the WebString constructor explicitly here. Ugh. Stupid me on both counts. Sorry.
Comment on attachment 68087 [details] Patch Rejecting patch 68087 from commit-queue. Unexpected failure when processing patch! Please file a bug against webkit-patch. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'land-attachment', '--force-clean', '--non-interactive', '--ignore-builders', '--quiet', '--parent-command=commit-queue', 68087]" exit_code: 1 Last 500 characters of output: .cgi?id=68087&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=46090&ctype=xml Processing 1 patch from 1 bug. Cleaning working directory Updating working directory Processing patch 68087 from bug 46090. NOBODY (OOPS!) found in /Projects/CommitQueue/WebCore/ChangeLog does not appear to be a valid reviewer according to committers.py. ERROR: /Projects/CommitQueue/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).
> Rejecting patch 68087 from commit-queue. Sorry, this is a new bug in the commit-queue now that it's using multiple bots. Will fix soon.
(In reply to comment #12) > > Rejecting patch 68087 from commit-queue. > > Sorry, this is a new bug in the commit-queue now that it's using multiple bots. Will fix soon. Well, the fact that it didn't still go ahead and commit it after cq- seems to be an improvement from before! (In which case it could commit a patch 10+ minutes after it was cq-ed.) Anyway, Jochen, can you roll another patch? Thanks!
This should be fixed by bug 32679.
Created attachment 68195 [details] Patch
Attachment 68195 [details] did not build on chromium: Build output: http://queues.webkit.org/results/4101005
Created attachment 68200 [details] Patch
Attachment 68200 [details] did not build on chromium: Build output: http://queues.webkit.org/results/4097009
Comment on attachment 68200 [details] Patch r=me
Created attachment 68202 [details] Patch
Comment on attachment 68202 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=68202&action=review > WebKit/chromium/src/WebIDBFactory.cpp:45 > + return IDBFactoryBackendImpl::databaseFileName(name, static_cast<PassRefPtr<SecurityOrigin> >(origin).get()); noooooo You probably just need to include WebSecurityOrigin at the top.
(In reply to comment #21) > (From update of attachment 68202 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=68202&action=review > > > WebKit/chromium/src/WebIDBFactory.cpp:45 > > + return IDBFactoryBackendImpl::databaseFileName(name, static_cast<PassRefPtr<SecurityOrigin> >(origin).get()); > > noooooo > > You probably just need to include WebSecurityOrigin at the top. that header is already included from WebIDBFactory.h WebSecurityOrigin defines an PassRefPtr<SecurityOrigin> operator(), the method databaseFileName wants to have a SecurityOrigin* an alternative would be to change IDBFactoryBackendImpl.cpp:openSqliteDatabase to take a PassRefPtr instead of a SecurityOrigin*, then databaseFileName could also take a PassRefPtr?
Created attachment 68203 [details] Patch
Comment on attachment 68203 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=68203&action=review please change....but looks good :-) > WebKit/chromium/src/WebIDBFactory.cpp:46 > + securityOrigin = origin; just combine these two lines?
(In reply to comment #24) > (From update of attachment 68203 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=68203&action=review > > please change....but looks good :-) > > > WebKit/chromium/src/WebIDBFactory.cpp:46 > > + securityOrigin = origin; > > just combine these two lines? That would require an explicit static_cast to PassRefPtr<SecurityOrigin>: when the compiler tries to figure out which RefPtr template to use, it doesn't take into account implicit conversion operators
Comment on attachment 68203 [details] Patch Clearing flags on attachment: 68203 Committed r67941: <http://trac.webkit.org/changeset/67941>
All reviewed patches have been landed. Closing bug.