1411697 - Make sure to SendDocumentChannelCleanup even when mPreferredCachedAltDataType is set

Status: RESOLVED FIXED

(Core :: Networking, defect, P2)

Comment 1

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1411697 - Make sure to SendDocumentChannelCleanup even when mPreferredCachedAltDataType is set

Review commit: https://reviewboard.mozilla.org/r/193052/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/193052/

Comment 2

[Michal Novotny [:michal]]

Comment on attachment 8922052 [details]
Bug 1411697 - Make sure to SendDocumentChannelCleanup even when mPreferredCachedAltDataType is set

https://reviewboard.mozilla.org/r/193052/#review198552

Some description about what this patch does is needed.

::: netwerk/protocol/http/HttpChannelChild.cpp:1103
(Diff revision 1)
>  
>    // DocumentChannelCleanup actually nulls out mCacheEntry in the parent, which
>    // we might need later to open the Alt-Data output stream, so just return here
>    if (!mPreferredCachedAltDataType.IsEmpty()) {
>      mKeptAlive = true;
> +    SendDocumentChannelCleanup(false); // don't clear cache entry

The comment above is no longer true.

Comment 3

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8922052 [details]
Bug 1411697 - Make sure to SendDocumentChannelCleanup even when mPreferredCachedAltDataType is set

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/193052/diff/1-2/

Comment 4

[Michal Novotny [:michal]]

Comment on attachment 8922052 [details]
Bug 1411697 - Make sure to SendDocumentChannelCleanup even when mPreferredCachedAltDataType is set

https://reviewboard.mozilla.org/r/193052/#review198578

::: netwerk/protocol/http/HttpChannelChild.cpp:1101
(Diff revision 2)
>  
>    CleanupBackgroundChannel();
>  
> -  // DocumentChannelCleanup actually nulls out mCacheEntry in the parent, which
> -  // we might need later to open the Alt-Data output stream, so just return here
> +  // If there is a possibility we might want to write alt data to the cache
> +  // entry, we keep the channel alive. We still send the DocumentChannelCleanup
> +  // message, but request that the cache entry be kept by the parent.

The sentence sounds a bit weird. Wouldn't be better "but request the cache entry to be kept by the parent"?

Comment 5

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8922052 [details]
Bug 1411697 - Make sure to SendDocumentChannelCleanup even when mPreferredCachedAltDataType is set

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/193052/diff/2-3/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/692541a9f99b
Make sure to SendDocumentChannelCleanup even when mPreferredCachedAltDataType is set r=michal

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/692541a9f99b