Closed Bug 1267754 Opened 4 years ago Closed 4 years ago

Investigate log messages about missing triggeringPrincipal (uri) in LoadInfo

Categories

(Core :: DOM: Security, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: ckerschb, Assigned: ckerschb)

References

Details

(Whiteboard: [domsecurity-active])

Attachments

(1 file)

No description provided.
Potentially we can just remove the warning, but we should quickly investigate why we are seeing so many of those warnings:


  INFO -  [Parent 2516] WARNING: no triggering principal available via loadInfo, assuming load is cross-origin: file c:/builds/moz2_slave/ash-w32-d-00000000000000000000/build/src/netwerk/protocol/http/HttpBaseChannel.cpp, line 1405
Assignee: nobody → ckerschb
Status: NEW → ASSIGNED
Whiteboard: [domsecurity-active]
I think the network predictor code might also have an influence on this bug. See details [1].

Marking this bug being blocked by Bug 1267058 as well. Once the predictor code provides loadInfo args we should reinvestigate and see what's left to fix. Maybe we can then keep the NS_WARNING.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1267591#c4
Depends on: 1267058
I suppose Bug 1267058 fixed those issues as well, but we should make sure:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=33e0e16f6e8d
Within Bug 1267591 we pushed assertions to Treeherder to make sure all channels have a loadInfo attached. This warning however is slightly different in a sense that we only that warning the URI of the triggeringPrincipal is null, which is the case if the triggeringPrincipal is the systemPrincipal. I suppose we could make that warning less verbose. In fact I suppose we can even delete that warning.

Patrick, from a network perspective, are you fine with removing that warning?
Tanvi, what do you think? Can we remove the warning?

[1] https://hg.mozilla.org/mozilla-central/annotate/043082cb7bd8/netwerk/protocol/http/HttpBaseChannel.cpp#l1406
Flags: needinfo?(tanvi)
Flags: needinfo?(mcmanus)
You can delete the warning as far as I'm concerned - could you replace it with a LOG(() ? (that will just be an nspr log.)
Flags: needinfo?(mcmanus)
I assume Tanvi is fine with that change as well. I'll not land before she answers her ni? Thanks!
Attachment #8751184 - Flags: review?(mcmanus)
Attachment #8751184 - Flags: review?(mcmanus) → review+
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #6)
> Created attachment 8751184 [details] [diff] [review]
> bug_1267754_replace_warning_with_nsprlog.patch
> 
> I assume Tanvi is fine with that change as well. I'll not land before she
> answers her ni? Thanks!

This is fine.
Flags: needinfo?(tanvi)
Pushed by kcambridge@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7182dee7bb1c
Replace WARNING about missing triggeringPrincipal with LOG() r=mcmanus
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7182dee7bb1c
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.