1377206 - Set DontThrottle class of service flag on background audio downloads

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect, P1)

Comment 1

[Honza Bambas (:mayhemer)]

Attachment: v1 (media element loading channel explicitly unthrottled)

The DontThrottle flag instructs the http backgroung throttling algorithm to never throttle the download.  We may throttle a common responses (with no special flags) when happening in background tabs.  This will prevent background audio to stutter.

Chris, are there any other media loads we should mark with this flag?  We are specifically concerned about background audio like radio.  Should NP (plugin initiated) requests be marked the same way?  Not sure how to recognize they are media, tho.

Thanks!

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8892629 [details] [diff] [review]
v1 (media element loading channel explicitly unthrottled)

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

I think we also need to cover the channel created in ChannelMediaResource::RecreateChannel(), in MediaResource.cpp.

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Honza Bambas (:mayhemer) from comment #1)
> Should NP
> (plugin initiated) requests be marked the same way?  Not sure how to
> recognize they are media, tho.


I don't know how you could recognize them.

Comment 4

[Honza Bambas (:mayhemer)]

Attachment: v2 (media element loading channels explicitly unthrottled)

Looks like there are no more channels to mark with this flag.

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8892961 [details] [diff] [review]
v2 (media element loading channels explicitly unthrottled)

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

::: dom/html/HTMLMediaElement.cpp
@@ +1199,5 @@
> +        // interaction.
> +        aElement->mUseUrgentStartForChannel = false;
> +      }
> +
> +      // Uncoditionally disable throttling since we want the media fluently

s/Uncoditionally/Unconditionally/
s/the media fluently/the media to fluently/

::: dom/media/MediaResource.cpp
@@ +869,5 @@
>    NS_ENSURE_SUCCESS(rv, rv);
>  
> +  nsCOMPtr<nsIClassOfService> cos(do_QueryInterface(mChannel));
> +  if (cos) {
> +    // Uncoditionally disable throttling since we want the media fluently

s/Uncoditionally/Unconditionally/
s/the media fluently/the media to fluently/

Note also that the MediaCache does its own throttling when it thinks we've downloaded "enough" media data.

Comment 6

[Honza Bambas (:mayhemer)]

(In reply to Chris Pearce (:cpearce) from comment #5)
> Note also that the MediaCache does its own throttling when it thinks we've
> downloaded "enough" media data.

Thanks.  This is good to know.  I presume the cache lives on the child process?  How exactly does it throttle?  Just checking since this could be broken with bug 1280629.

Comment 7

[Honza Bambas (:mayhemer)]

Attachment: v2.1 (media element loading channels explicitly unthrottled)

Comment 8

[Honza Bambas (:mayhemer)]

(the blocking bug has to land first)

Comment 9

[Honza Bambas (:mayhemer)]

(no, it's not needed!)

Comment 10

[Honza Bambas (:mayhemer)]

(and maybe yes, there could potentially be leaks, rather pushing to try before requesting checkin..)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4031471f84965274750b839759b39da488092aed

Comment 11

[Honza Bambas (:mayhemer)]

OK, safe to go now.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/73c8cb113321
Explicitly disable throttling of media HTTP responses, r=cpearce

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/73c8cb113321