Closed Bug 1405739 Opened 7 years ago Closed 7 years ago

PerformanceResourceTiming.redirectStart and .redirectEnd expose internal redirect times

Categories

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

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: bkelly, Assigned: bkelly)

References

Details

Attachments

(2 files, 2 obsolete files)

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.
Updated to hide the redirect count for internal redirects as well.
Attachment #8915201 - Attachment is obsolete: true
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 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
Attachment #8915320 - Flags: review?(valentin.gosu)
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.
Attachment #8915322 - Flags: review?(bugmail)
Attachment #8915322 - Flags: review?(bugmail) → review+
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
Attachment #8915320 - Flags: review?(valentin.gosu) → review+
Attachment #8915636 - Flags: review+
Pushed by bkelly@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/6c5488ade824
https://hg.mozilla.org/mozilla-central/rev/ba89a80854b5
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Blocks: 1404524
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.