Closed Bug 1608599 Opened 10 months ago Closed 7 months ago

Flag speculative status for requests in the Network panel

Categories

(Core :: Networking, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla76
Tracking Status
firefox76 --- fixed

People

(Reporter: Harald, Assigned: sonakshisaxena1)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [necko-triaged])

User Story

When auditing page load, I want to know which requests got triggered by preloads, so that I can understand how preload changes the waterfall behavior.

Attachments

(1 file)

Off the cuff, the cause column would probably be a good target that could combine type+preload (similar to (raced)), but let's discuss further to scope out a solution.

Stretch goal: Stack trace points to the HTML location that defines the preload entry.

See Also: → 1358038
Summary: Mark preloaded resources in Network panel → Add preloaded status for requests in the Network panel
Whiteboard: [necko-triaged]

preloaded scripts already seems to be an internal content policy type: https://searchfox.org/mozilla-central/source/dom/base/nsContentUtils.h#3372

Honza, could we get access to them to have more fine-grained causes?

Flags: needinfo?(odvarko)

Dragana, could we get access to nsIContentPolicy for a request? Perhaps through channel.loadInfo?

Honza

Flags: needinfo?(odvarko) → needinfo?(dd.mozilla)

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

Dragana, could we get access to nsIContentPolicy for a request? Perhaps through channel.loadInfo?

Honza

the attribute is in loadInfo but it is not accessible from scripts:
https://searchfox.org/mozilla-central/source/netwerk/base/nsILoadInfo.idl#556
You probably need it from a js.

Flags: needinfo?(dd.mozilla)

(In reply to Dragana Damjanovic [:dragana] from comment #3)

the attribute is in loadInfo but it is not accessible from scripts:
https://searchfox.org/mozilla-central/source/netwerk/base/nsILoadInfo.idl#556
You probably need it from a js.

Correct, DevTools are entirely written in JS and so, we are consuming all platform APIs from JS.

Is it possible to expose the nsContentPolicyType internalContentPolicyType(); to scripts?

DevTools would benefit from having access to the attribute.
The Network panel could provide more detailed info so, the user can understand how preload logic works and impacts the page load.

Thanks!
Honza

Flags: needinfo?(dd.mozilla)

We can expose internalContentPolicyType to js.
rel=preload is not ready yet. I will put this bug as a part of the rel=preload project as it should be fix as a part of that project.

Do you need it earlier?

Flags: needinfo?(dd.mozilla)

(In reply to Dragana Damjanovic [:dragana] from comment #5)

We can expose internalContentPolicyType to js.
rel=preload is not ready yet. I will put this bug as a part of the rel=preload project as it should be fix as a part of that project.

Do you need it earlier?

If fixing this bug is relatively small change (e.g. changing idl definition etc.) it would be great to have it earlier (also AFAIK there is no forecast for rel=preload so, it might take a while). GSoC application phase (edit) will start March 16 and there are students that could help us testing this new API in DevTools and build innovative features.

Honza

Flags: needinfo?(dd.mozilla)

Nhi, can you prioritize this?

Flags: needinfo?(dd.mozilla) → needinfo?(nhnguyen)

I think this should be done as part of the rel=preload project, which is in our roadmap. Leaving this as P3.

Flags: needinfo?(nhnguyen)

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

(In reply to Dragana Damjanovic [:dragana] from comment #5)

We can expose internalContentPolicyType to js.
rel=preload is not ready yet. I will put this bug as a part of the rel=preload project as it should be fix as a part of that project.

Do you need it earlier?

If fixing this bug is relatively small change (e.g. changing idl definition etc.) it would be great to have it earlier (also AFAIK there is no forecast for rel=preload so, it might take a while). GSoC application phase (edit) will start March 16 and there are students that could help us testing this new API in DevTools and build innovative features.

Honza

Hey! I am an Outreachy applicant, and I was hoping if you could give me some direction in solving this bug.

Thanks!
Sonakshi

Flags: needinfo?(honzab.moz)

(In reply to Sonakshi Saxena from comment #9)

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

(In reply to Dragana Damjanovic [:dragana] from comment #5)

We can expose internalContentPolicyType to js.
rel=preload is not ready yet. I will put this bug as a part of the rel=preload project as it should be fix as a part of that project.

Do you need it earlier?

If fixing this bug is relatively small change (e.g. changing idl definition etc.) it would be great to have it earlier (also AFAIK there is no forecast for rel=preload so, it might take a while). GSoC application phase (edit) will start March 16 and there are students that could help us testing this new API in DevTools and build innovative features.

Honza

Hey! I am an Outreachy applicant, and I was hoping if you could give me some direction in solving this bug.

Thanks!
Sonakshi

You need to remove noscript and notxpcom from
https://searchfox.org/mozilla-central/rev/c888ecff616737e27286d99167edafb6805e8494/netwerk/base/nsILoadInfo.idl#560
you need to remove that line. I think that is all that is needed.

Okay, sure. Thanks! Will put a patch for this :)

Assignee: nobody → sonakshisaxena1
Status: NEW → ASSIGNED
Flags: needinfo?(honzab.moz)
Blocks: 1624368
Pushed by valentin.gosu@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/0cdc6c8cee2d
Add preloaded status for requests in the Network panel r=valentin

I assume we'll use bug 1624368 for the netmonitor work?

Flags: needinfo?(odvarko)

I assume we'll use bug 1624368 for the netmonitor work?

Just talking to Honza about this. That is correct.

Flags: needinfo?(odvarko)
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla76

Sorry to chime in so late, but what do you expect from this attribute? The content policy type doesn't say ANYTHING about <link preload> invocation of the request, not at all. The new preload code at the moment doesn't exist, so there is nothing to produce the information a request is a rel=preload. You apparently misplaced "speculative load" with "rel=preload". Those are two different things. Only fact is that rel=preload started loads will be nothing more than speculative loads with some additional properties (and those properties are what you are looking for!) There are speculative loads, right now, for regular tags as well, and actually all (or at least most) markup invoked loads are speculative. Will this patch just tell devtools that all our such loads are rel=preloads? That would really be wrong. This bug is definitely not FIXED.

So what did this bug land then? Sounds like we should file a follow up bug to rename the preload label added to speculative?

Flags: needinfo?(honzab.moz)

(In reply to :Harald Kirschner :digitarald from comment #18)

So what did this bug land then?

Nothing at all related to preload.

Sounds like we should file a follow up bug to rename the preload label added to speculative?

Yes, because right now it's pretty confusing if we want to add "preload" info there as well.

Flags: needinfo?(honzab.moz)
Summary: Add preloaded status for requests in the Network panel → Flag speculative status for requests in the Network panel
You need to log in before you can comment on or make changes to this bug.