1248203 - streamline h2 stream flow control buffer

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Patrick McManus [:mcmanus]]

This was originally posted
https://bugzilla.mozilla.org/show_bug.cgi?id=1247205#c9

It turned out not to solve the problem in that bug, but its a good
refactoring of existing and potentially buggy code. This code has the
advantage of having only one error state (oom) and is lockless.

Comment 1

[Patrick McManus [:mcmanus]]

Attachment: streamline h2 stream flow control buffer

Comment 2

[Patrick McManus [:mcmanus]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=33bdc91b1419

Comment 3

[u408661]

Comment on attachment 8719225 [details] [diff] [review]
streamline h2 stream flow control buffer

Review of attachment 8719225 [details] [diff] [review]:
-----------------------------------------------------------------

r=me w/changes below

::: netwerk/base/SimpleBuffer.h
@@ +6,5 @@
> +
> +#ifndef SimpleBuffer_h__
> +#define SimpleBuffer_h__
> +
> +#if 0

Regular block comment, please :) (|#if 0| is always a red flag to me)

@@ +8,5 @@
> +#define SimpleBuffer_h__
> +
> +#if 0
> +This class is similar to a nsPipe except it does not have any locking, stores
> +an unbounded amount of data, can only be used on one thread, and has much

So while it's obvious from reading the code that this is only safe to use on one thread, there's no asserting current thread == owning thread. It would be nice to have that (at least in debug builds) so if (when) people start using this somewhere other than h2, errors can be caught sooner.

::: netwerk/protocol/http/Http2Stream.cpp
@@ +263,4 @@
>    if (NS_SUCCEEDED(rv)) {
> +    rv = mSimpleBuffer.Write(buf, *countWritten);
> +    if (NS_FAILED(rv)) {
> +      MOZ_ASSERT (rv == NS_ERROR_OUT_OF_MEMORY);

nit: no space before (

Comment 4

[Patrick McManus [:mcmanus]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=251c2c289a84

Comment 5

[Patrick McManus [:mcmanus]]

Attachment: streamline h2 stream flow control buffer

Comment 6

[Patrick McManus [:mcmanus]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1fff1b2ac665d640e0f2e669647aca6d2ec31dc9
Bug 1248203 - streamline h2 stream flow control buffer r=hurley

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1fff1b2ac665