[webkit-reviews] review granted: [Bug 232424] Have CachedRawResourceClient and related networking actors use SharedBuffer. : [Attachment 448464] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Jean-Yves Avenard [:jya]
<jean-yves.avenard at apple.com>'s request for review:
Bug 232424: Have CachedRawResourceClient and related networking actors use
SharedBuffer.
https://bugs.webkit.org/show_bug.cgi?id=232424

Attachment 448464: Patch

https://bugs.webkit.org/attachment.cgi?id=448464&action=review




--- Comment #16 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 448464
  --> https://bugs.webkit.org/attachment.cgi?id=448464
Patch

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

> Source/WebCore/Modules/fetch/FetchLoader.cpp:151
> +void FetchLoader::didReceiveData(const SharedBuffer& buffer, bool)

This bool is mysterious.
Ideally, we would remove it, or use a bool enum.

> Source/WebCore/loader/DocumentThreadableLoader.cpp:454
> +    m_client->didReceiveData(buffer, false);

It does not seem that we have a case where we call didReceiveData(buffer,
true).
Maybe we can just not pass this bool, especially since we probably no longer
have -1 as the length in buffer.

> Source/WebCore/workers/WorkerScriptLoader.cpp:218
> +    size_t len = useStrLen ? strlen(reinterpret_cast<const
char*>(buffer.data())) : buffer.size();

Can we try removing this -1 code path?

> Source/WebCore/xml/XMLHttpRequest.cpp:1058
> +    size_t len = useStrLen ? strlen(reinterpret_cast<const
char*>(buffer.data())) : buffer.size();

Can we try removing this -1 code path?

>
Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerSoftUpdateLoader.cpp:18
9
> +    if (!buffer.isEmpty()) {

Should we move that check in forEachSegment?
Or are we expecting that other forEachSegment call sites will not benefit of
this?