Bug 175732 – [Cache API] Add support for being loaded responses

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[youenn fablet]

[Cache API] Add support for being loaded responses

[youenn fablet]

Created attachment 318532 [details]
Patch

[youenn fablet]

Created attachment 318543 [details]
Patch

[Chris Dumez]

orange bubbles!

[youenn fablet]

Created attachment 318551 [details]
Patch

[youenn fablet]

No more orange bubbles related to this patch apparently...

[Chris Dumez]

Comment on attachment 318551 [details]
Patch

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

r=me with comments.

> Source/WebCore/Modules/fetch/FetchResponse.cpp:137
> +        responseDataCallback(m_response.body().consumer().takeData());

Is it ok to just steal the data from m_response?

> Source/WebCore/Modules/fetch/FetchResponse.cpp:152
> +        responseDataCallback(Exception { TypeError });

Would be nice to pass a second parameter, like a couple of lines above.

> Source/WebCore/Modules/fetch/FetchResponse.cpp:282
> +    if (!isLoading())

I think this should either be an assertion OR it should call the callback before returning. Otherwise, it is confusing from the client's point of view that the callback may or may not get called.

[youenn fablet]

Comment on attachment 318551 [details]
Patch

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

>> Source/WebCore/Modules/fetch/FetchResponse.cpp:137
>> +        responseDataCallback(m_response.body().consumer().takeData());
> 
> Is it ok to just steal the data from m_response?

Yes, this is the principle behind consuming a response.
I'll use consume to make that clearer.

>> Source/WebCore/Modules/fetch/FetchResponse.cpp:152
>> +        responseDataCallback(Exception { TypeError });
> 
> Would be nice to pass a second parameter, like a couple of lines above.

OK

>> Source/WebCore/Modules/fetch/FetchResponse.cpp:282
>> +    if (!isLoading())
> 
> I think this should either be an assertion OR it should call the callback before returning. Otherwise, it is confusing from the client's point of view that the callback may or may not get called.

I'll change to an ASSERT(isLoading()).

[youenn fablet]

Created attachment 318564 [details]
Patch

[youenn fablet]

Created attachment 318566 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 318566 [details]
Patch for landing

Clearing flags on attachment: 318566

Committed r220948: <http://trac.webkit.org/changeset/220948>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/33974575>