[Cache API] Add response body storage
Created attachment 318332 [details] Patch
Created attachment 318375 [details] Patch
Created attachment 318377 [details] Patch
Created attachment 318389 [details] Patch
Created attachment 318390 [details] Patch
Comment on attachment 318390 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=318390&action=review > Source/WebCore/Modules/fetch/FetchResponse.h:109 > + static void startFetching(ScriptExecutionContext&, FetchRequest&, std::optional<FetchPromise>&&, std::optional<EndOfLoad>&&); I don't think it's elegant to change FetchResponse from a class that used to always resolve a promise when it's done to a class that either resolves a promise when it's done or calls a lambda. I think instead we should make it a class that always calls a lambda when it's done and the lambda would resolve a promise in the cases where it should, or we should take a Variant<FetchPromise, EndOfLoadCallback> to make it clear that it does one or the other, then make better abstractions for what it is supposed to do when it's done.
> I don't think it's elegant to change FetchResponse from a class that used to > always resolve a promise when it's done to a class that either resolves a > promise when it's done or calls a lambda. I think instead we should make it > a class that always calls a lambda when it's done and the lambda would > resolve a promise in the cases where it should, or we should take a > Variant<FetchPromise, EndOfLoadCallback> to make it clear that it does one > or the other, then make better abstractions for what it is supposed to do > when it's done. The two lambdas will be useful for the same fetch response in the future. I will change the promise into a lambda and add a setter for EndOfLoadCallback.
Created attachment 318514 [details] Patch
Comment on attachment 318514 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=318514&action=review > Source/WebKit/Shared/WebCoreArgumentCoders.cpp:114 > +static void encodeSharedBuffer(Encoder& encoder, const SharedBuffer* buffer) We're just moving this code to use it below? We already kind of have this in SharedBufferDataReference. This should be united with that, and maybe that should be cleaned up.
(In reply to Alex Christensen from comment #9) > Comment on attachment 318514 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=318514&action=review > > > Source/WebKit/Shared/WebCoreArgumentCoders.cpp:114 > > +static void encodeSharedBuffer(Encoder& encoder, const SharedBuffer* buffer) > > We're just moving this code to use it below? We already kind of have this > in SharedBufferDataReference. This should be united with that, and maybe > that should be cleaned up. Didn't know about SharedBufferDataReference. Code is moved and refreshed a bit. It also fixes a potential bug when trying to encode a SharedBuffer whose size is 0.
Comment on attachment 318514 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=318514&action=review > Source/WebKit/Shared/WebCoreArgumentCoders.cpp:367 > + responseBody = buffer.releaseNonNull(); But then, we should only set responseBody if buffer is not null here...
Created attachment 318519 [details] Patch for landing
(In reply to youenn fablet from comment #10) > Didn't know about SharedBufferDataReference. We should probably get rid of it.
Comment on attachment 318519 [details] Patch for landing Clearing flags on attachment: 318519 Committed r220928: <http://trac.webkit.org/changeset/220928>
All reviewed patches have been landed. Closing bug.
<rdar://problem/33967936>