1407498 - Don't query loadingprincipal attribute in common case

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P2)

Description

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

I added querying loadingprincipal attribute back in bug 1376971.
However in the follow-up, BZ pointed out some problems in https://bugzilla.mozilla.org/show_bug.cgi?id=1405195#c7

We shouldn't do this for the common case, also we need to update the comments.

Comment 1

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Patch.

Baku, sorry for bothering you again, can you check if my patch addresses bz's comment?

Thanks

Comment 2

[Andrea Marchesini [:baku]]

Comment on attachment 8917307 [details] [diff] [review]
Patch.

Review of attachment 8917307 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/html/HTMLMediaElement.cpp
@@ +1203,1 @@
>                                      aElement->mLoadingSrcTriggeringPrincipal,

strange indentation.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Some comments:

1)  The code is mixing loadingPrincipal/triggeringPrincipal naming.  Should probably be consistent.
2)  "aLoadingNode->OwnerDoc()->NodePrincipal()" could just be "aLoadingNode->NodePrincipal()".
3)  The "MOZ_ASSERT(nsContentUtils::IsSystemPrincipal(aLoadingNode->NodePrincipal())" is now
    clearly redundant, because we already made that exact check earlier in the function.
4)  We should probably rename the attribute to "triggeringprincipal" or so.

Thank you for cleaning this up!

Comment 4

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Patch v2.

Comment 5

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Patch. v3

rebase after bug 1376973.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bad2101d54e4
Don't query loadingprincipal in common case. r=baku

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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