RESOLVED FIXED 136991
Update HTMLMediaElement logging
https://bugs.webkit.org/show_bug.cgi?id=136991
Summary Update HTMLMediaElement logging
Eric Carlson
Reported 2014-09-21 18:45:19 PDT
Some of the logging in HTMLMediaElement does not log 'this', making it difficult to use when a page has multiple media elements.
Attachments
Proposed patch. (44.58 KB, patch)
2014-09-21 18:52 PDT, Eric Carlson
no flags
Eric Carlson
Comment 1 2014-09-21 18:52:10 PDT
Created attachment 238443 [details] Proposed patch.
Alexey Proskuryakov
Comment 2 2014-09-21 18:57:16 PDT
Comment on attachment 238443 [details] Proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=238443&action=review > Source/WebCore/html/HTMLMediaElement.cpp:673 > + LOG(Media, "HTMLMediaElement::removedFrom(%p)", this); I'd log this as HTMLMediaElement %p removedFrom", to make it clear that it's not an argument. Ditto elsewhere.
Eric Carlson
Comment 3 2014-09-22 08:14:50 PDT
(In reply to comment #2) > (From update of attachment 238443 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=238443&action=review > > > Source/WebCore/html/HTMLMediaElement.cpp:673 > > + LOG(Media, "HTMLMediaElement::removedFrom(%p)", this); > > I'd log this as HTMLMediaElement %p removedFrom", to make it clear that it's not an argument. Ditto elsewhere. All "Media" logging uses this pattern. I don't think there is much danger of it being mistaken for an argument because only 'this' is in parenthesis, so I will leave it as is for now at least. Thanks for the review!
WebKit Commit Bot
Comment 4 2014-09-22 08:51:33 PDT
Comment on attachment 238443 [details] Proposed patch. Clearing flags on attachment: 238443 Committed r173838: <http://trac.webkit.org/changeset/173838>
WebKit Commit Bot
Comment 5 2014-09-22 08:51:35 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.