Closed
Bug 1343409
Opened 7 years ago
Closed 7 years ago
HTMLMediaElement::MozRequestDebugInfo should also return EMEInfo
Categories
(Core :: Audio/Video: Playback, enhancement)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: kkoorts, Unassigned)
Details
Attachments
(1 file)
It would be useful, for debugging purposes, to also return EMEInfo with MediaKeys' ids and their session information.
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8842252 [details] Bug 1343409 - HTMLMediaElement::MozRequestDebugInfo returns EMEInfo. https://reviewboard.mozilla.org/r/116128/#review117694 ::: dom/media/eme/MediaKeys.cpp:589 (Diff revision 1) > + !it.Done(); > + it.Next()) { > + MediaKeySession* keySession = it.Data(); > + nsString sessionID; > + keySession->GetSessionId(sessionID); > + sessionsInfo.AppendPrintf("(sid:%s ", sessionID); Here you AppendPrintf an nsString with a %s and without calling .get() on the nsString, and elsewhere you do use %S and use .get(). You should at least be consistent. I expect you're supposed to use %S here (and call get()), and I expect you won't see a problem with your code as-is unless you try to compile this with clang or gcc, or run it on MacOS or Linux. You could instead use: sessionsInfo.AppendLiteral("sid:"); sessionsInfo.Append(sessionID); And that would be slightly safer than a printf (as the string's length would be known in advance), and unambiguous.
Attachment #8842252 -
Flags: review?(cpearce) → review-
Comment hidden (mozreview-request) |
Comment on attachment 8842252 [details] Bug 1343409 - HTMLMediaElement::MozRequestDebugInfo returns EMEInfo. https://reviewboard.mozilla.org/r/116128/#review117704
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8842252 [details] Bug 1343409 - HTMLMediaElement::MozRequestDebugInfo returns EMEInfo. https://reviewboard.mozilla.org/r/116128/#review117706 ::: dom/media/eme/MediaKeys.cpp:589 (Diff revisions 1 - 2) > !it.Done(); > it.Next()) { > MediaKeySession* keySession = it.Data(); > nsString sessionID; > keySession->GetSessionId(sessionID); > - sessionsInfo.AppendPrintf("(sid:%s ", sessionID); > + sessionsInfo.AppendLiteral("(sid:"); So now it's not consistent; you're using Append(nsString) in one place and AppendPrintf("%S", nsString.get()) in others. I'd be happy for you to use one or the other, consistently.
Attachment #8842252 -
Flags: review?(cpearce) → review+
Comment hidden (mozreview-request) |
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/cd7f919c7583 HTMLMediaElement::MozRequestDebugInfo returns EMEInfo. r=cpearce
Keywords: checkin-needed
Comment 8•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/cd7f919c7583
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•