Closed Bug 1525640 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Networking: DNS, enhancement, P2)

Product:
Core
Component:
Networking: DNS
Version:
60 Branch
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla68
Tracking Flags:
Tracking Status
firefox68 --- fixed

People

(Reporter: valentin, Assigned: valentin)

References

(Blocks 3 open bugs)

Details

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

Attachments

(3 files)

Bug 1525640 - Pass TRR status from sockettransport to channel r=dragana
47 bytes, text/x-phabricator-request
Details | Review
Bug 1525640 - Rename nsIHttpChannelInternal.trr to .isHttpServiceChannel to avoid confusion r=dragana
47 bytes, text/x-phabricator-request
Details | Review
Bug 1525640 - Add nsIHttpChannelInternal.isResolvedByTRR r=dragana
47 bytes, text/x-phabricator-request
Details | Review

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
Closed: 5 years ago
Resolution: --- → WONTFIX

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

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

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
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

https://hg.mozilla.org/mozilla-central/rev/e3539a40afdf
https://hg.mozilla.org/mozilla-central/rev/e71641f0465b
https://hg.mozilla.org/mozilla-central/rev/efdd32c00dc6

Status: REOPENED → RESOLVED
Closed: 5 years ago → 5 years ago
status-firefox68: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
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)

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.

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

https://hg.mozilla.org/mozilla-central/rev/406e4a9a3f75
https://hg.mozilla.org/mozilla-central/rev/3b43b5e97f9e
https://hg.mozilla.org/mozilla-central/rev/83b1e5ce0132

Status: REOPENED → RESOLVED
Closed: 5 years ago → 5 years ago
status-firefox68: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Depends on: 1557180
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: