This change is mostly just adding the plumbing necessary for the IndexedDatabaseRequest and IndexedDatabaseSync (not written yet). This code is non-functional, so no tests (yet).
Created attachment 50326 [details] patch
Attachment 50326 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/storage/IndexedDatabaseImpl.cpp:47: Place brace on its own line for function definitions. [whitespace/braces] [4] WebCore/storage/IndexedDatabaseImpl.cpp:53: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 2 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 50326 [details] patch > +++ b/WebCore/platform/chromium/ChromiumBridge.h ... > + // IndexedDB ---------------------------------------------------------- > + static PassRefPtr<IndexedDatabase> getIndexedDatabase(); ordinarily, we drop the "get" prefix for accessors like this. it should just be indexedDatabase(). > +++ b/WebCore/storage/IndexedDatabase.h ... > +namespace WebCore { > + > +class IndexedDatabase : public ThreadSafeShared<IndexedDatabase> { It might helpful to add some comments here about the threading model. > +++ b/WebCore/storage/IndexedDatabaseImpl.cpp ... > +namespace WebCore { > + > +IndexedDatabaseImpl* IndexedDatabaseImpl::s_indexedDatabaseImpl = 0; I didn't think it was WebKit style to use the "s_" prefix. I prefer it myself since I think it helps code readability. Do you know if there is any rule about this? > + > +PassRefPtr<IndexedDatabaseImpl> IndexedDatabaseImpl::get() > +{ > + if (!s_indexedDatabaseImpl) > + s_indexedDatabaseImpl = new IndexedDatabaseImpl(); > + ASSERT(s_indexedDatabaseImpl); > + return s_indexedDatabaseImpl; > +} > + > +IndexedDatabaseImpl::IndexedDatabaseImpl() { > + // FIXME: Make this thread safe. > + ASSERT(!s_indexedDatabaseImpl); > + s_indexedDatabaseImpl = this; > +} > + > +IndexedDatabaseImpl::~IndexedDatabaseImpl() { brackets on the next line. > +++ b/WebKit/chromium/public/WebKitClient.h > - // Database ------------------------------------------------------------ > + // HTML5 Database ------------------------------------------------------ "SQL Database" would be better since this API is not part of HTML5. > + // Indexed Database ---------------------------------------------------- > + > + virtual WebIndexedDatabase* getIndexedDatabase() { return 0; } indexedDatabase
Those are just style nits. The patch looks great otherwise.
Given that we're in different time zones and there are many more patches coming where I can clean up other issues if they arise, can I get a r+ please? As for s_ IIRC, this is one of the accepted styles, but not the preferred one. I'll figure out which is preferred and change it, but I still think s_ is much better than any of the other ones.
> > +++ b/WebCore/storage/IndexedDatabaseImpl.cpp > ... > > +namespace WebCore { > > + > > +IndexedDatabaseImpl* IndexedDatabaseImpl::s_indexedDatabaseImpl = 0; > > I didn't think it was WebKit style to use the "s_" prefix. I prefer it > myself since I think it helps code readability. Do you know if there > is any rule about this? There isn't. More code justUsesThisStyle. Some uses s_. I think using local variable syntax is really terrible for readability, but I'll change it since you asked. > > > > +++ b/WebKit/chromium/public/WebKitClient.h > > > - // Database ------------------------------------------------------------ > > + // HTML5 Database ------------------------------------------------------ > > "SQL Database" would be better since this API is not part of HTML5. I added the HTML 5 because it's out of order alphabetically without it (and that's what the other file has). If we wanted to change it, it probably should be in a subsequent patch that moves the code as well.
http://trac.webkit.org/changeset/55784