Bug 121880

Summary: More refactoring to make the LevelDB dependencies of IDB more obvious
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebCore Misc.Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, commit-queue, eflews.bot, gtk-ews, gyuyoung.kim, jsbell, rakuco, rego+ews, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch v1
ap: review+, eflews.bot: commit-queue-
Patch v2 - Try to appease all the bots
none
Patch v3 - Hopefully this applies!
eflews.bot: commit-queue-
Landed patch (for ews) none

Description Brady Eidson 2013-09-24 16:38:58 PDT
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.
Comment 1 Brady Eidson 2013-09-24 16:42:37 PDT
Created attachment 212521 [details]
Patch v1
Comment 2 WebKit Commit Bot 2013-09-24 16:44:40 PDT
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.
Comment 3 Alexey Proskuryakov 2013-09-24 16:46:07 PDT
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.
Comment 4 Brady Eidson 2013-09-24 16:47:06 PDT
(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.
Comment 5 Brady Eidson 2013-09-24 16:54:45 PDT
Style issues are preexisting.  Going to wait for buildbots to churn (would love for my attempt at keeping the build working to have worked)
Comment 6 EFL EWS Bot 2013-09-24 17:14:30 PDT
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
Comment 7 EFL EWS Bot 2013-09-24 17:17:22 PDT
Comment on attachment 212521 [details]
Patch v1 

Attachment 212521 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/2164060
Comment 8 Build Bot 2013-09-24 17:20:44 PDT
Comment on attachment 212521 [details]
Patch v1 

Attachment 212521 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/2195046
Comment 9 kov's GTK+ EWS bot 2013-09-24 19:24:20 PDT
Comment on attachment 212521 [details]
Patch v1 

Attachment 212521 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/2231006
Comment 10 Brady Eidson 2013-09-25 09:09:14 PDT
Created attachment 212586 [details]
Patch v2 - Try to appease all the bots
Comment 11 Brady Eidson 2013-09-25 09:51:56 PDT
Created attachment 212591 [details]
Patch v3 - Hopefully this applies!
Comment 12 WebKit Commit Bot 2013-09-25 09:54:49 PDT
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.
Comment 13 EFL EWS Bot 2013-09-25 10:53:07 PDT
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
Comment 14 Brady Eidson 2013-09-25 10:58:44 PDT
Okay, figured out the GTK include problem and the EFL source problem.  I'll just be landing this next one.
Comment 15 Brady Eidson 2013-09-25 10:59:33 PDT
(But will be uploading to EWS for email notifications, too)
Comment 16 Brady Eidson 2013-09-25 11:02:35 PDT
Created attachment 212605 [details]
Landed patch (for ews)
Comment 17 Brady Eidson 2013-09-25 11:02:57 PDT
http://trac.webkit.org/changeset/156406