Bug 136991 – Update HTMLMediaElement logging

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Eric Carlson]

Some of the logging in HTMLMediaElement does not log 'this', making it difficult to use when a page has multiple media elements.

[Eric Carlson]

Created attachment 238443 [details]
Proposed patch.

[Alexey Proskuryakov]

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]

(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 on attachment 238443 [details]
Proposed patch.

Clearing flags on attachment: 238443

Committed r173838: <http://trac.webkit.org/changeset/173838>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.