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:

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

Brady Eidson

Reported 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.

Attachments
Patch v1 (354.96 KB, patch) 2013-09-24 16:42 PDT, Brady Eidson	ap: review+ eflews.bot: commit-queue-	Details Formatted Diff Diff
Patch v2 - Try to appease all the bots (347.68 KB, patch) 2013-09-25 09:09 PDT, Brady Eidson	no flags	Details Formatted Diff Diff
Patch v3 - Hopefully this applies! (347.62 KB, patch) 2013-09-25 09:51 PDT, Brady Eidson	eflews.bot: commit-queue-	Details Formatted Diff Diff
Landed patch (for ews) (348.35 KB, patch) 2013-09-25 11:02 PDT, Brady Eidson	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Brady Eidson

Comment 1 2013-09-24 16:42:37 PDT

Created attachment 212521 [details] Patch v1

WebKit Commit Bot

Comment 2 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.

Alexey Proskuryakov

Comment 3 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.

Brady Eidson

Comment 4 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.

Brady Eidson

Comment 5 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)

EFL EWS Bot

Comment 6 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

EFL EWS Bot

Comment 7 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

Build Bot

Comment 8 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

kov's GTK+ EWS bot

Comment 9 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

Brady Eidson

Comment 10 2013-09-25 09:09:14 PDT

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

Brady Eidson

Comment 11 2013-09-25 09:51:56 PDT

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

WebKit Commit Bot

Comment 12 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.

EFL EWS Bot

Comment 13 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

Brady Eidson

Comment 14 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.

Brady Eidson

Comment 15 2013-09-25 10:59:33 PDT

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

Brady Eidson

Comment 16 2013-09-25 11:02:35 PDT

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

Brady Eidson

Comment 17 2013-09-25 11:02:57 PDT

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

Note You need to log in before you can comment on or make changes to this bug.