Bug 205961 - IndexedDB: include prefetched cursor records in result of iterating cursor
Summary: IndexedDB: include prefetched cursor records in result of iterating cursor
Status: RESOLVED DUPLICATE of bug 207602
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sihui Liu
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-01-08 15:03 PST by Sihui Liu
Modified: 2022-02-10 16:50 PST (History)
10 users (show)

See Also:


Attachments
Patch (8.44 KB, patch)
2020-01-08 15:20 PST, Sihui Liu
no flags Details | Formatted Diff | Diff
Patch (9.06 KB, patch)
2020-01-08 16:03 PST, Sihui Liu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sihui Liu 2020-01-08 15:03:14 PST
...
Comment 1 Sihui Liu 2020-01-08 15:20:10 PST
Created attachment 387148 [details]
Patch
Comment 2 Sihui Liu 2020-01-08 16:03:13 PST
Created attachment 387154 [details]
Patch
Comment 3 Alex Christensen 2020-01-08 17:04:05 PST
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 4 Sihui Liu 2020-01-08 17:11:49 PST
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.
Comment 5 Sihui Liu 2020-01-08 17:14:14 PST
(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 6 youenn fablet 2020-01-13 01:39:20 PST
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.
Comment 7 Sihui Liu 2020-02-14 12:37:04 PST
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 ***