1354054 - Network panel could display request priority

Status: RESOLVED FIXED

(DevTools :: Netmonitor, enhancement, P3)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

The Network panel could display HTTP request priority so, dev folks are able to debug it.

Here is meta for the CDP (Context Driven Priority) project.
https://bugzilla.mozilla.org/showdependencytree.cgi?id=1312741&hide_resolved=1

We should track the progress of CDP an discuss what DevTools can do for it.

Contact: Jan Bambas [:mayhemer]

Honza

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Julien, the Profiler is show in HTTP request priorities correct? Can you please point us to the place where it's done? (to see what platform API to use)

Honza

Comment 2

[Julien Wajsberg [:julienw]]

Hey Honza!

There are different ways:
In [1] we use the instance variable mPriority directly. This is part of HttpChannel.
In [2] we use the method GetPriority, but that's simply getting the value of mPriority (unless this can be overriden?). So I'm not sure why we use it over using mPriority directly.

[1] https://searchfox.org/mozilla-central/rev/d2cec90777d573585f8477d5170892e5dcdfb0ab/netwerk/protocol/http/nsHttpChannel.cpp#6454
[2] https://searchfox.org/mozilla-central/rev/d2cec90777d573585f8477d5170892e5dcdfb0ab/netwerk/protocol/http/nsHttpChannel.cpp#6006

Hey Randell, maybe you can explain why you used 2 different ways sometimes? Do you remember?

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Initially they all used GetPriority(), perhaps because I was copying from elsewhere. At some point that one marker got added with mPriority, perhaps because if compiles.

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thanks Julien and Randell!

Harald, what about starting with the following:

• Adding the priority into Summary section in the Headers panel (as soon as Bug 1617167 is fixed)
• Create a new Priority column (hidden by default), so the list of requests can be sorted by priority (can be done immediately, perhaps good-first-bug).

Honza

Comment 5

[:Harald Kirschner :digitarald]

LGTM, Honza.

FWIW, we should not expose the raw numbers but the mapping to lowest/low/normal/high/highest: https://searchfox.org/mozilla-central/rev/41c3ea3ee8eab9ce7b82932257cb80b703cbba67/xpcom/threads/nsISupportsPriority.idl#24-28

Comment 6

[Ajitesh13]

Hello, I would like to work on this bug. Can I proceed ?

Comment 7

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Ajitesh13 from comment #6)

Hello, I would like to work on this bug. Can I proceed ?

Ah sorry, for the delay!

Assigning to you. Feel free to let me know if you are not interested any more (so, I can unassign you and make the bug available to someone else)

Thanks,
Honza

Comment 8

[Ajitesh13]

Thanks for assigning me this bug, but as currently I am fixing another bug, I am unassigning myself and opening it to others. Once I am done there, I will immediately move to this one.

Comment 9

[Abid999]

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #7)

(In reply to Ajitesh13 from comment #6)

Hello, I would like to work on this bug. Can I proceed ?

Ah sorry, for the delay!

Assigning to you. Feel free to let me know if you are not interested any more (so, I can unassign you and make the bug available to someone else)

Thanks,
Honza

i would like to work on this , Can you assign me?

Comment 10

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Assigned to you, thank you for helping.
Btw. are you an outreachy applicant?

Comment 11

[Abid999]

yes, I'm an Outreachy applicant. Hopefully i'll update my progress by tomorrow. thanks a lot for assigning it to me.

Comment 12

[Abid999]

:Honza I'm little bit confused went through the source code quite a few time. Can you help me where to start from? I'm really interested to contribute.

Comment 13

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Some comments and instructions

1. The priority (for the selected request) should be rendered in the Headers panel within the Summary section, underneath the "Referrer Policy". Here is the piece of code that collects data for the summary section

https://searchfox.org/mozilla-central/rev/0f3e259c24e52932387318ac503bfad3c82baa44/devtools/client/netmonitor/src/components/request-details/HeadersPanel.js#730-737

2. The label for the "Priority" value should be properly localized. See how it's done for "Referrer Policy"
   https://searchfox.org/mozilla-central/rev/0f3e259c24e52932387318ac503bfad3c82baa44/devtools/client/netmonitor/src/components/request-details/HeadersPanel.js#94

The localization file is here:
https://searchfox.org/mozilla-central/rev/0f3e259c24e52932387318ac503bfad3c82baa44/devtools/client/locales/en-US/netmonitor.properties#1066

3. The panel should display a string not a number, see the available values here:
   https://searchfox.org/mozilla-central/rev/0f3e259c24e52932387318ac503bfad3c82baa44/xpcom/threads/nsISupportsPriority.idl#24-28
   So, we should render (Highest, High, Normal, Low, Lowest) - all localized

4. The Priority value should be part of the "request" object, which is fetched from the backend (just like the referrer policy value)
   You can check out this piece
   https://searchfox.org/mozilla-central/rev/0f3e259c24e52932387318ac503bfad3c82baa44/devtools/client/netmonitor/src/components/request-details/HeadersPanel.js#519-536

5. The actual priority value should be collected on the backend from the actual HTTP channel (when it's created)
   It should happen when we create the "request" object around this place (again, when the referrer policy is also collected)
   https://searchfox.org/mozilla-central/rev/0f3e259c24e52932387318ac503bfad3c82baa44/devtools/server/actors/network-monitor/utils/network-utils.js#722

6. You need to use "nsISupportsPriority" interface to get the value from the HTTP channel
   https://searchfox.org/mozilla-central/source/xpcom/threads/nsISupportsPriority.idl#19

Something like:

  if (channel instanceof Ci.nsISupportsPriority) {
    event.priority = channel.priority;
  }

I am not sure whether the priority value will be available at this point in time, this needs to be tested and we'll see.

7. There are other places that needs to be modified to properly pass the priority all the way to the Headers panel on the front end. Just follow what's done for "Referrer Policy"
   https://searchfox.org/mozilla-central/search?q=_referrerPolicy&path=devtools
   Or you might also follow "fromServiceWorker" flag etc.

8. You also need to add "priority" to the list of known properties
   https://searchfox.org/mozilla-central/rev/0f3e259c24e52932387318ac503bfad3c82baa44/devtools/client/netmonitor/src/constants.js#228

9. We also need a test, the test for referrer policy might be a good inspiration
   https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/test/browser_net_header-ref-policy.js

10. Check out the following bugs to see how new columns are added
    Bug 1344523 - Add IP address column to the Network Monitor
    Bug 1345489 - Introduce a new column for protocol version
    Bug 1356867 - Add scheme column to netmonitor
    This might be done in a follow up bug.

Honza

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit auto_nag documentation.

Comment 15

[colin.cazabet]

Hello,

I would be interested to work on this one if nobody is interested, I think I have enough information to get started.

Thank you,
Colin

Comment 16

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Great, thank you for helping. Assigned to you.

Comment 17

[colin.cazabet]

Attachment: Bug 1354054 - [devtools] Network panel could display request priority r=Honza

Comment 18

[colin.cazabet]

Hello,

just in case you did not see, I updated the phabricator diff with the sorting on the priority field.

Colin

Comment 19

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Yes, thank you for the ping I missed the patch update. Review comments posted now.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/abd6d47a6f57
[devtools] Network panel could display request priority r=Honza

Comment 21

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/abd6d47a6f57