Bug 205961

Created attachment 387148 [details] Patch

Created attachment 387154 [details] Patch

Comment on attachment 387154 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387154&action=review > Source/WebCore/Modules/indexeddb/shared/IDBCursorRecord.h:67 > + encoder << !!value; How did this ever work? Why does it need changing? It was an still is a std::unique_ptr.

Comment on attachment 387154 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387154&action=review >> Source/WebCore/Modules/indexeddb/shared/IDBCursorRecord.h:67 >> + encoder << !!value; > > How did this ever work? > Why does it need changing? It was an still is a std::unique_ptr. If the pointer is not nullptr, we deref the pointer and encode IDBValue object. We encode !!value to indicate whether there is a subsequent encoded IDBValue.

(In reply to Sihui Liu from comment #4) > Comment on attachment 387154 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=387154&action=review > > >> Source/WebCore/Modules/indexeddb/shared/IDBCursorRecord.h:67 > >> + encoder << !!value; > > > > How did this ever work? > > Why does it need changing? It was an still is a std::unique_ptr. > > If the pointer is not nullptr, we deref the pointer and encode IDBValue > object. We encode !!value to indicate whether there is a subsequent encoded > IDBValue. It seems like we cannot encode a unique_ptr and pass that to another process.

Comment on attachment 387154 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387154&action=review > Source/WebCore/Modules/indexeddb/IDBCursor.cpp:355 > + m_prefetchedRecords.append(record); Can we write it as the following: if (auto prefetchedRecords = getResult.prefetchedRecords()) m_prefetchedRecords = WTFMove(prefetchedRecords) That would require IDBGetResult prefetchedRecords to return a Deque. > Source/WebCore/Modules/indexeddb/IDBGetResult.h:80 > + IDBGetResult(const IDBKeyData& keyData, const IDBKeyData& primaryKeyData, IDBValue&& value, const Optional<IDBKeyPath>& keyPath, const Optional<Vector<IDBCursorRecord>>& prefetechedRecords) Can we change all/some of these to r-values, in particular prefetechedRecords? > Source/WebCore/Modules/indexeddb/IDBGetResult.h:100 > + const Optional<Vector<IDBCursorRecord>> prefetchedRecords() const { return m_prefetchedRecords; } s/const Optional<Vector<IDBCursorRecord>>/const Optional<Vector<IDBCursorRecord>>&/ What is the difference between m_prefetchedRecords = {} and m_prefetchedRecords = Vector<IDBCursorRecord> { }? Can we change it to a Deque<IDBCursorRecord>? > Source/WebCore/Modules/indexeddb/shared/IDBCursorRecord.h:38 > std::unique_ptr<IDBValue> value; Why is it a unique_ptr in IDBCursorRecord but not in other places like IDBCursor? Can we use an IDBValue or an Optional<IDBValue>? That may allow removing these move/copy assignmenst/constructors.

This is part of https://bugs.webkit.org/show_bug.cgi?id=207602, which has been fixed. *** This bug has been marked as a duplicate of bug 207602 ***