Bug 121880 – More refactoring to make the LevelDB dependencies of IDB more obvious

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Brady Eidson]

More refactoring to make the LevelDB dependencies of IDB more obvious

The beginnings of a clean platform abstraction were there, but not quite realized.

Moving more files to the leveldb subdirectory and renaming them will help.

[Brady Eidson]

Created attachment 212521 [details]
Patch v1

[WebKit Commit Bot]

Attachment 212521 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp', u'Source/WebCore/Modules/indexeddb/IDBBackingStore.h', u'Source/WebCore/Modules/indexeddb/IDBCursorBackendImpl.cpp', u'Source/WebCore/Modules/indexeddb/IDBCursorBackendImpl.h', u'Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp', u'Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.h', u'Source/WebCore/Modules/indexeddb/IDBFactoryBackendImpl.cpp', u'Source/WebCore/Modules/indexeddb/IDBFactoryBackendImpl.h', u'Source/WebCore/Modules/indexeddb/IDBFactoryBackendInterface.cpp', u'Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp', u'Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.h', u'Source/WebCore/Modules/indexeddb/IDBTransactionBackendImpl.cpp', u'Source/WebCore/Modules/indexeddb/IDBTransactionBackendImpl.h', u'Source/WebCore/Modules/indexeddb/IDBTransactionCoordinator.cpp', u'Source/WebCore/Modules/indexeddb/IDBTransactionCoordinator.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBBackingStoreLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBBackingStoreLevelDB.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBCursorBackendLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBCursorBackendLevelDB.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBDatabaseBackendLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBDatabaseBackendLevelDB.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBFactoryBackendLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBFactoryBackendLevelDB.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBLevelDBCoding.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBLevelDBCoding.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBObjectStoreBackendLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBObjectStoreBackendLevelDB.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBTransactionBackendLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBTransactionBackendLevelDB.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBTransactionCoordinatorLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBTransactionCoordinatorLevelDB.h', u'Source/WebCore/Target.pri', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj']" exit_code: 1
Source/WebCore/Modules/indexeddb/leveldb/IDBBackingStoreLevelDB.h:26:  #ifndef header guard has wrong style, please use: IDBBackingStoreLevelDB_h  [build/header_guard] [5]
Source/WebCore/Modules/indexeddb/leveldb/IDBObjectStoreBackendLevelDB.h:47:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 2 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alexey Proskuryakov]

Comment on attachment 212521 [details]
Patch v1 

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

> Source/WebCore/ChangeLog:33
> +        (WebCore::LevelDBFactory::~LevelDBFactory):
> +        (WebCore::IDBBackingStore::createWeakPtr):
> +        (WebCore::IDBBackingStore::RecordIdentifier::RecordIdentifier):
> +        (WebCore::IDBBackingStore::RecordIdentifier::primaryKey):
> +        (WebCore::IDBBackingStore::RecordIdentifier::version):
> +        (WebCore::IDBBackingStore::RecordIdentifier::reset):
> +        (WebCore::IDBBackingStore::Cursor::key):
> +        (WebCore::IDBBackingStore::Cursor::primaryKey):
> +        (WebCore::IDBBackingStore::Cursor::recordIdentifier):
> +        (WebCore::IDBBackingStore::Cursor::~Cursor):
> +        (WebCore::IDBBackingStore::Cursor::Cursor):
> +        (WebCore::IDBBackingStore::Transaction::reset):
> +        (WebCore::IDBBackingStore::Transaction::levelDBTransactionFrom):

Can you please remove all the function names? They are of no use whatsoever, but waste space in svn log, buildbot console and such. And make reviewing harder.

[Brady Eidson]

(In reply to comment #3)
> (From update of attachment 212521 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=212521&action=review
> 
> > Source/WebCore/ChangeLog:33
> > +        (WebCore::LevelDBFactory::~LevelDBFactory):
> > +        (WebCore::IDBBackingStore::createWeakPtr):
> > +        (WebCore::IDBBackingStore::RecordIdentifier::RecordIdentifier):
> > +        (WebCore::IDBBackingStore::RecordIdentifier::primaryKey):
> > +        (WebCore::IDBBackingStore::RecordIdentifier::version):
> > +        (WebCore::IDBBackingStore::RecordIdentifier::reset):
> > +        (WebCore::IDBBackingStore::Cursor::key):
> > +        (WebCore::IDBBackingStore::Cursor::primaryKey):
> > +        (WebCore::IDBBackingStore::Cursor::recordIdentifier):
> > +        (WebCore::IDBBackingStore::Cursor::~Cursor):
> > +        (WebCore::IDBBackingStore::Cursor::Cursor):
> > +        (WebCore::IDBBackingStore::Transaction::reset):
> > +        (WebCore::IDBBackingStore::Transaction::levelDBTransactionFrom):
> 
> Can you please remove all the function names? They are of no use whatsoever, but waste space in svn log, buildbot console and such. And make reviewing harder.

Will do.

[Brady Eidson]

Style issues are preexisting.  Going to wait for buildbots to churn (would love for my attempt at keeping the build working to have worked)

[EFL EWS Bot]

Comment on attachment 212521 [details]
Patch v1 

Attachment 212521 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/2161070

[EFL EWS Bot]

Comment on attachment 212521 [details]
Patch v1 

Attachment 212521 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/2164060

[Build Bot]

Comment on attachment 212521 [details]
Patch v1 

Attachment 212521 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/2195046

[kov's GTK+ EWS bot]

Comment on attachment 212521 [details]
Patch v1 

Attachment 212521 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/2231006

[Brady Eidson]

Created attachment 212586 [details]
Patch v2 - Try to appease all the bots

[Brady Eidson]

Created attachment 212591 [details]
Patch v3 - Hopefully this applies!

[WebKit Commit Bot]

Attachment 212591 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp', u'Source/WebCore/Modules/indexeddb/IDBBackingStore.h', u'Source/WebCore/Modules/indexeddb/IDBCursorBackendImpl.cpp', u'Source/WebCore/Modules/indexeddb/IDBCursorBackendImpl.h', u'Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp', u'Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.h', u'Source/WebCore/Modules/indexeddb/IDBFactoryBackendImpl.cpp', u'Source/WebCore/Modules/indexeddb/IDBFactoryBackendImpl.h', u'Source/WebCore/Modules/indexeddb/IDBFactoryBackendInterface.cpp', u'Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp', u'Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.h', u'Source/WebCore/Modules/indexeddb/IDBTransactionBackendImpl.cpp', u'Source/WebCore/Modules/indexeddb/IDBTransactionBackendImpl.h', u'Source/WebCore/Modules/indexeddb/IDBTransactionCoordinator.cpp', u'Source/WebCore/Modules/indexeddb/IDBTransactionCoordinator.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBBackingStoreLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBBackingStoreLevelDB.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBCursorBackendLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBCursorBackendLevelDB.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBDatabaseBackendLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBDatabaseBackendLevelDB.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBFactoryBackendLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBFactoryBackendLevelDB.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBLevelDBCoding.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBLevelDBCoding.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBObjectStoreBackendLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBObjectStoreBackendLevelDB.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBTransactionBackendLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBTransactionBackendLevelDB.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBTransactionCoordinatorLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBTransactionCoordinatorLevelDB.h', u'Source/WebCore/Target.pri', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj']" exit_code: 1
Source/WebCore/Modules/indexeddb/leveldb/IDBBackingStoreLevelDB.h:26:  #ifndef header guard has wrong style, please use: IDBBackingStoreLevelDB_h  [build/header_guard] [5]
Source/WebCore/Modules/indexeddb/leveldb/IDBObjectStoreBackendLevelDB.h:47:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 2 in 23 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EFL EWS Bot]

Comment on attachment 212591 [details]
Patch v3 - Hopefully this applies!

Attachment 212591 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/2281043

[Brady Eidson]

Okay, figured out the GTK include problem and the EFL source problem.  I'll just be landing this next one.

[Brady Eidson]

(But will be uploading to EWS for email notifications, too)

[Brady Eidson]

Created attachment 212605 [details]
Landed patch (for ews)

[Brady Eidson]

http://trac.webkit.org/changeset/156406