Bug 124385 – Move execution of IDBTransactionBackendOperations to the IDBServerConnection

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Brady Eidson]

Move execution of IDBTransactionBackendOperations to the IDBServerConnection

This almost removes all knowledge of the backing stores from the front end.

[Brady Eidson]

Created attachment 217010 [details]
Patch v1

[Build Bot]

Comment on attachment 217010 [details]
Patch v1 

Attachment 217010 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/23778008

[Build Bot]

Comment on attachment 217010 [details]
Patch v1 

Attachment 217010 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/23718007

[Brady Eidson]

(In reply to comment #3)
> (From update of attachment 217010 [details])
> Attachment 217010 [details] did not pass mac-ews (mac):
> Output: http://webkit-queues.appspot.com/results/23718007

Whoops, forgot to include .xcodeproj changes to export 2 more headers.

I'm reworking part of the patch right now anyways, will fix in the next version.

[Brady Eidson]

Created attachment 217016 [details]
Patch v2 - Less macros, more variadic templates...?

check-webkit-style will complain:
indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:60:  Place brace on its own line for function definitions.  [whitespace/braces] [4]

But I think it needs updates for C++ lambdas.

[WebKit Commit Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/Modules/indexeddb/IDBDatabaseBackend.cpp', u'Source/WebCore/Modules/indexeddb/IDBDatabaseBackend.h', u'Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp', u'Source/WebCore/Modules/indexeddb/IDBServerConnection.h', u'Source/WebCore/Modules/indexeddb/IDBTransactionBackend.cpp', u'Source/WebCore/Modules/indexeddb/IDBTransactionBackend.h', u'Source/WebCore/Modules/indexeddb/IDBTransactionBackendOperations.cpp', u'Source/WebCore/Modules/indexeddb/IDBTransactionBackendOperations.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBBackingStoreLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBBackingStoreLevelDB.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.h', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/WebProcess/Databases/IndexedDB/WebProcessIDBDatabaseBackend.h']" exit_code: 1
Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:60:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 1 in 14 files


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

[EFL EWS Bot]

Comment on attachment 217016 [details]
Patch v2 - Less macros, more variadic templates...?

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

[EFL EWS Bot]

Comment on attachment 217016 [details]
Patch v2 - Less macros, more variadic templates...?

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

[kov's GTK+ EWS bot]

Comment on attachment 217016 [details]
Patch v2 - Less macros, more variadic templates...?

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

[Brady Eidson]

Hmmm  the EFL/GTK errors seem to be deficiencies in their respective compilers parameter packs and lamdbas.  That's too bad...

[Anders Carlsson]

Comment on attachment 217016 [details]
Patch v2 - Less macros, more variadic templates...?

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

> Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:68
> +template <typename FunctionType, typename... Arguments>
> +class CallOnDestruct {
> +public:
> +    CallOnDestruct(FunctionType callback, Arguments... arguments)
> +        : m_originalCallback(callback)
> +    {
> +        setArguments(arguments...);
> +    }
> +
> +    ~CallOnDestruct()
> +    {
> +        auto callback = m_callbackToPerform;
> +        callOnMainThread([callback]() {
> +            callback();
> +        });
> +    }
> +
> +    void setArguments(Arguments... arguments)
> +    {
> +        auto callback = m_originalCallback;
> +        m_callbackToPerform = [callback, arguments...]() {
> +            callback(arguments...);
> +        };
> +    }
> +
> +private:
> +    FunctionType m_originalCallback;
> +    std::function<void()> m_callbackToPerform;
> +};

I think this is more confusing than what you had earlier.

[Brady Eidson]

(In reply to comment #11)
> (From update of attachment 217016 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=217016&action=review
> 
> > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:68
> > +template <typename FunctionType, typename... Arguments>
> > +class CallOnDestruct {
> > +public:
> > +    CallOnDestruct(FunctionType callback, Arguments... arguments)
> > +        : m_originalCallback(callback)
> > +    {
> > +        setArguments(arguments...);
> > +    }
> > +
> > +    ~CallOnDestruct()
> > +    {
> > +        auto callback = m_callbackToPerform;
> > +        callOnMainThread([callback]() {
> > +            callback();
> > +        });
> > +    }
> > +
> > +    void setArguments(Arguments... arguments)
> > +    {
> > +        auto callback = m_originalCallback;
> > +        m_callbackToPerform = [callback, arguments...]() {
> > +            callback(arguments...);
> > +        };
> > +    }
> > +
> > +private:
> > +    FunctionType m_originalCallback;
> > +    std::function<void()> m_callbackToPerform;
> > +};
> 
> I think this is more confusing than what you had earlier.

I know we're still looking for the right idiom here, and I definitely don't think the macros are it.

But since the EFL/GTK compilers can't handle the variadic class anyways, and this code is only for them for now, I'll go back to the macros + fixing the mac build.

[Brady Eidson]

Created attachment 217058 [details]
Patch v3 - Macros, and hopefully a successful build

[Build Bot]

Comment on attachment 217058 [details]
Patch v3 - Macros, and hopefully a successful build

Attachment 217058 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/23658197

[Brady Eidson]

Created attachment 217061 [details]
Patch v4 - All the WebCore.xcodeproj changes needed...

[Brady Eidson]

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