Bug 233923 – Make PreviewConverterProvider not modify the SharedBuffer once returned

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Jean-Yves Avenard [:jya]]

The LegacyPreviewLoader keeps a SharedBuffer, add data to it and pass it to the provideMainResourceForPreviewConverter PreviewConverter and then clear the Sharedbuffer

The original object passed to provideMainResourceForPreviewConverter is now empty.

In practice here is doesn't matter as the PreviewConverter will read the content immediately, however refactoring this code will allow to have the SharedBufferBuilder::get() method always return a new object.

[Radar WebKit Bug Importer]

<rdar://problem/86149850>

[Jean-Yves Avenard [:jya]]

Created attachment 446155 [details]
Patch for EWS

[Jean-Yves Avenard [:jya]]

Created attachment 446156 [details]
Patch

[Jean-Yves Avenard [:jya]]

Created attachment 446266 [details]
Patch

[Jean-Yves Avenard [:jya]]

Created attachment 446267 [details]
Patch for EWS

[Jean-Yves Avenard [:jya]]

Created attachment 446537 [details]
Patch

rebase

[Jean-Yves Avenard [:jya]]

Created attachment 446538 [details]
Patch for EWS

[Jean-Yves Avenard [:jya]]

Created attachment 447121 [details]
Patch

rebase

[youenn fablet]

Comment on attachment 447121 [details]
Patch

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

> Source/WebCore/loader/ios/LegacyPreviewLoader.mm:230
> +    completionHandler(m_originalData.copy());

Unrelated but why do we need a CompletionHandler if we are synchronously returning.

> Source/WebCore/platform/PreviewConverter.cpp:85
> +        appendFromBuffer(WTFMove(buffer));

So we can never go to didFailUpdating, even if buffer is of size 0?
Should we move from const FragmentedSharedBuffer* to RefPtr<FragmentedSharedBuffer>&&?

[Jean-Yves Avenard [:jya]]

Comment on attachment 447121 [details]
Patch

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

>> Source/WebCore/platform/PreviewConverter.cpp:85
>> +        appendFromBuffer(WTFMove(buffer));
> 
> So we can never go to didFailUpdating, even if buffer is of size 0?
> Should we move from const FragmentedSharedBuffer* to RefPtr<FragmentedSharedBuffer>&&?

Buffer previously was a pointer and could never have been null (the SharedBuffer was always created in the constructor)
I made it a Ref<FragmentedaharedBuffer>&&

There’s been no logic change

[EWS]

Committed r287053 (245249@main): <https://commits.webkit.org/245249@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 447121 [details].