Let consumers of nsIHttpChannel know if resolving the name was done via TRR

RESOLVED FIXED in Firefox 68

Status

()

enhancement
P2
normal
RESOLVED FIXED
4 months ago
16 days ago

People

(Reporter: valentin, Assigned: valentin)

Tracking

(Blocks 3 bugs)

60 Branch
mozilla68
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

(Whiteboard: [necko-triaged][trr])

Attachments

(3 attachments)

This would be useful when gathering telemetry/doing TRR experiments.
Right now we have nsIHttpChannelInternal.trr, but that only specifies if that channel is used to perform DoH requests.

Whiteboard: [necko-triaged] → [necko-triaged][trr]

We ended up finding this information another way, so closing.

Status: NEW → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → WONTFIX

Turns out this would still be useful, for bug 1542331 for example.

Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Assignee

Updated

2 months ago
Blocks: 1542331, 1542357

TODO: make sure it works in the child channel

Depends on D26883

(In reply to Valentin Gosu [:valentin] from comment #5)

TODO: make sure it works in the child channel

I manually checked that it works, but I still need to write a proper unit test for this.

Blocks: 1543010

Comment 7

a month ago
Pushed by valentin.gosu@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e3539a40afdf
Pass TRR status from sockettransport to channel r=dragana
https://hg.mozilla.org/integration/autoland/rev/e71641f0465b
Rename nsIHttpChannelInternal.trr to .isHttpServiceChannel to avoid confusion r=dragana
https://hg.mozilla.org/integration/autoland/rev/efdd32c00dc6
Add nsIHttpChannelInternal.isResolvedByTRR r=dragana

Comment 8

a month ago
bugherder
Status: REOPENED → RESOLVED
Last Resolved: 2 months agoa month ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Comment 10

21 days ago
Pushed by valentin.gosu@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/4294abed890f
Pass TRR status from sockettransport to channel r=dragana
https://hg.mozilla.org/integration/autoland/rev/19aa991328cb
Rename nsIHttpChannelInternal.trr to .isHttpServiceChannel to avoid confusion r=dragana
https://hg.mozilla.org/integration/autoland/rev/d9a454afe9bf
Add nsIHttpChannelInternal.isResolvedByTRR r=dragana
Flags: needinfo?(valentin.gosu)

Backed out 3 changesets (Bug 1525640) for frequent wpt failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=wpt&group_state=expanded&fromchange=6df281b9388839efca4fc89ae62aa2c696aaf76b&tochange=3be80123e3af0bf2aece853e560891b3163e5d7a&selectedJob=244200033

Backout link: https://hg.mozilla.org/integration/autoland/rev/3be80123e3af0bf2aece853e560891b3163e5d7a

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=244200033&repo=autoland&lineNumber=43493

16:10:51 INFO - PROCESS LEAKS /var/folders/p5/tdlp7wrx6pnd3r2h3s1zyh7000000x/T/tmp4ayBBF.mozrunner/runtests_leaks_691.log
16:10:51 INFO - leakcheck | Processing log file /var/folders/p5/tdlp7wrx6pnd3r2h3s1zyh7000000x/T/tmp4ayBBF.mozrunner/runtests_leaks_691.log for scope /referrer-policy/strict-origin-when-cross-origin/attr-referrer
16:10:51 INFO - TEST-INFO | leakcheck | default process: leak threshold set at 2000 bytes
16:10:51 INFO - TEST-INFO | leakcheck | plugin process: leak threshold set at 0 bytes
16:10:51 INFO - TEST-INFO | leakcheck | tab process: leak threshold set at 10000 bytes
16:10:51 INFO - TEST-INFO | leakcheck | gmplugin process: leak threshold set at 20000 bytes
16:10:51 INFO - TEST-INFO | leakcheck | gpu process: leak threshold set at 0 bytes
16:10:51 INFO - TEST-INFO | leakcheck | rdd process: leak threshold set at 400 bytes
16:10:51 INFO - TEST-INFO | leakcheck | vr process: leak threshold set at 0 bytes
16:10:51 INFO - TEST-INFO | leakcheck | socket process: leak threshold set at 0 bytes
16:10:51 INFO - leakcheck | Processing leak log file /var/folders/p5/tdlp7wrx6pnd3r2h3s1zyh7000000x/T/tmp4ayBBF.mozrunner/runtests_leaks_691.log
16:10:51 INFO -
16:10:51 INFO - == BloatView: ALL (cumulative) LEAK AND BLOAT STATISTICS, default process 2237
16:10:51 INFO -
16:10:51 INFO - |<----------------Class--------------->|<-----Bytes------>|<----Objects---->|
16:10:51 INFO - | | Per-Inst Leaked| Total Rem|
16:10:51 INFO - 0 |TOTAL | 40 2004| 5686224 12|
16:10:51 INFO - 544 |Mutex | 104 312| 3431 3|
16:10:51 INFO - 799 |PollableEvent | 32 32| 1 1|
16:10:51 INFO - 851 |ReentrantMonitor | 40 40| 6073 1|
16:10:51 INFO - 1363 |nsAStreamCopier | 40 40| 484 1|
16:10:51 INFO - 1691 |nsPipe | 208 208| 835 1|
16:10:51 INFO - 1692 |nsPipeInputStream | 136 136| 835 1|
16:10:51 INFO - 1741 |nsSocketTransport | 856 856| 19 1|
16:10:51 INFO - 1742 |nsSocketTransportService | 360 360| 1 1|
16:10:51 INFO - 1753 |nsStringBuffer | 12 12| 103519 1|
16:10:51 INFO - 1797 |nsTArray_base | 8 8| 2657075 1|
16:10:51 INFO -
16:10:51 INFO - nsTraceRefcnt::DumpStatistics: 1923 entries
16:10:51 INFO - TEST-INFO | leakcheck | default leaked 3 Mutex
16:10:51 INFO - TEST-INFO | leakcheck | default leaked 1 PollableEvent
16:10:51 INFO - TEST-INFO | leakcheck | default leaked 1 ReentrantMonitor
16:10:51 INFO - TEST-INFO | leakcheck | default leaked 1 nsAStreamCopier
16:10:51 INFO - TEST-INFO | leakcheck | default leaked 1 nsPipe
16:10:51 INFO - TEST-INFO | leakcheck | default leaked 1 nsPipeInputStream
16:10:51 INFO - TEST-INFO | leakcheck | default leaked 1 nsSocketTransport
16:10:51 INFO - TEST-INFO | leakcheck | default leaked 1 nsSocketTransportService
16:10:51 INFO - TEST-INFO | leakcheck | default leaked 1 nsStringBuffer
16:10:51 INFO - TEST-INFO | leakcheck | default leaked 1 nsTArray_base
16:10:51 INFO - TEST-UNEXPECTED-FAIL | leakcheck | default 2004 bytes leaked (Mutex, PollableEvent, ReentrantMonitor, nsAStreamCopier, nsPipe, ...)
16:10:51 INFO -
16:10:51 INFO - leakcheck | Processing leak log file /var/folders/p5/tdlp7wrx6pnd3r2h3s1zyh7000000x/T/tmp4ayBBF.mozrunner/runtests_leaks_691_tab_pid2239.log
16:10:51 INFO -
16:10:51 INFO - == BloatView: ALL (cumulative) LEAK AND BLOAT STATISTICS, tab process 2239
16:10:51 INFO -
16:10:51 INFO - |<----------------Class--------------->|<-----Bytes------>|<----Objects---->|
16:10:51 INFO - | | Per-Inst Leaked| Total Rem|
16:10:51 INFO - 0 |TOTAL | 42 0| 70115 0|
16:10:51 INFO -
16:10:51 INFO - nsTraceRefcnt::DumpStatistics: 766 entries

Flags: needinfo?(valentin.gosu)

I still find it very unlikely that this patch is causing the leaks.
I see the orange count in bug 1515057 is still quite high, even without the patch. But in any case I'll do some more testing on try. Maybe there is some subtle way this is breaking things.

Flags: needinfo?(valentin.gosu)

It looks definitely like triggered by this bug: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception&searchStr=10.10%2Cdebug%2Cwpt&fromchange=76e8b769a7c00797dcaeef19cbaaa564f1f373b6&tochange=bb38c692e51911b0de25bd9846f0ac79aa593176&group_state=expanded&selectedJob=244310314

Bug 1515057 caused some classifications as intermittents from the landing of this bug yesterday.

See also bug 1501108 which also caused a frequent OSX debug leak in wpt tests and had almost the same size.

See Also: → 1501108

So, I figured I'd try this locally, using the instructions from bug 1515057 comment 44

I ran the test with my latest pull from m-c on linux and I was able to reproduce it immediately, without applying my patch.
https://pastebin.com/823LYbCm

Interestingly, if I run the wpt in headless mode, the leak doesn't happen.

Comment 15

17 days ago
Pushed by valentin.gosu@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/406e4a9a3f75
Pass TRR status from sockettransport to channel r=dragana
https://hg.mozilla.org/integration/autoland/rev/3b43b5e97f9e
Rename nsIHttpChannelInternal.trr to .isHttpServiceChannel to avoid confusion r=dragana
https://hg.mozilla.org/integration/autoland/rev/83b1e5ce0132
Add nsIHttpChannelInternal.isResolvedByTRR r=dragana

Comment 16

16 days ago
bugherder
Status: REOPENED → RESOLVED
Last Resolved: a month ago16 days ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.