1405739 - PerformanceResourceTiming.redirectStart and .redirectEnd expose internal redirect times

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Description

[Ben Kelly [:bkelly, not reviewing]]

One of the things bug 1191943 checks is PerformanceResourceTiming.redirectStart.  Currently we fail this check because we expose the internal redirect caused by resetting the intercepted channel.  These internal redirects should not be exposed to script.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Don't expose internal redirect start/end times through performance timing API. r=valentin

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8bb279ee9d0373f940f3146d06d54e936956cd54

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P1 Don't expose internal redirect start/end/count through performance timing API. r=valentin

Updated to hide the redirect count for internal redirects as well.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P2 Update performance-timeline.https.html WPT test expectations. r=asuth

This does regress a test in performance-timeline.https.html.  Just accept this for now, though, since it will be fixed in bug 1191943.

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

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

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8915320 [details] [diff] [review]
P1 Don't expose internal redirect start/end/count through performance timing API. r=valentin

Valentin, this patch avoids update the following values on internal redirect:

  - redirectStart time
  - redirectEnd time
  - redirectCount

The count is still tracked in a separate field.  Rather than decrement the redirectLimit we now enforce the limit by comparing it to both these counts.

To avoid growing the size of the channel I also reduced the redirect count values from uint16_t to uint8_t.  This should be safe since we currently clamp the redirect-limit pref to 0xff here:

https://searchfox.org/mozilla-central/source/netwerk/protocol/http/nsHttpHandler.cpp#1314

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8915322 [details] [diff] [review]
P2 Update performance-timeline.https.html WPT test expectations. r=asuth

The P1 patch here will temporarily regress our test behavior performance-timeline.https.html in non-e10s mode.  This will be fixed in bug 1191943 which I plan to push at the same time.

Comment 7

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

Comment on attachment 8915320 [details] [diff] [review]
P1 Don't expose internal redirect start/end/count through performance timing API. r=valentin

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

::: netwerk/protocol/http/HttpBaseChannel.cpp
@@ +3620,5 @@
>        newTimedChannel->SetRedirectStart(mRedirectStartTimeStamp);
>      }
>  
> +    // For internal redirects just propagate the last redirect end time
> +    // forward.  Otherwise the new redirect end time is the last respond

nit: last response

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P1 Don't expose internal redirect start/end/count through performance timing API. r=valentin

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6c5488ade824
P1 Don't expose internal redirect start/end/count through performance timing API. r=valentin
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba89a80854b5
P2 Update performance-timeline.https.html WPT test expectations. r=asuth

Comment 10

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

https://hg.mozilla.org/mozilla-central/rev/6c5488ade824
https://hg.mozilla.org/mozilla-central/rev/ba89a80854b5