ASSIGNED 176621
Switch MediaPlayerPrivateAVFoundation to release logging
https://bugs.webkit.org/show_bug.cgi?id=176621
Summary Switch MediaPlayerPrivateAVFoundation to release logging
Eric Carlson
Reported 2017-09-08 11:33:46 PDT
Switch MediaPlayerPrivateAVFoundation to release logging
Attachments
Proposed patch. (57.72 KB, patch)
2017-09-10 10:51 PDT, Eric Carlson
no flags
Patch (57.92 KB, patch)
2017-09-10 18:55 PDT, Eric Carlson
no flags
Patch. (58.50 KB, patch)
2017-09-11 08:50 PDT, Eric Carlson
no flags
Patch for landing. (59.36 KB, patch)
2017-09-11 14:33 PDT, Eric Carlson
no flags
Patch for landing. (59.36 KB, patch)
2017-09-12 08:59 PDT, Eric Carlson
no flags
Radar WebKit Bug Importer
Comment 1 2017-09-08 11:34:33 PDT
Eric Carlson
Comment 2 2017-09-10 10:51:03 PDT
Created attachment 320397 [details] Proposed patch.
Eric Carlson
Comment 3 2017-09-10 18:55:15 PDT
Eric Carlson
Comment 4 2017-09-11 08:50:50 PDT
Jer Noble
Comment 5 2017-09-11 13:47:42 PDT
Comment on attachment 320430 [details] Patch. View in context: https://bugs.webkit.org/attachment.cgi?id=320430&action=review r=me, with nit. > Source/WebCore/html/HTMLMediaElement.h:904 > + const PAL::Logger* mediaPlayerLogger() final { return &logger(); } Can we just make this a ref rather than a pointer? > Source/WebCore/platform/graphics/MediaPlayer.h:306 > + virtual const PAL::Logger* mediaPlayerLogger() { return nullptr; } You would just make this a pure virtual. (But that's ok; the HTMLMediaElement is the only MediaPlayerClient, currently.)
Eric Carlson
Comment 6 2017-09-11 14:33:33 PDT
Comment on attachment 320430 [details] Patch. View in context: https://bugs.webkit.org/attachment.cgi?id=320430&action=review >> Source/WebCore/html/HTMLMediaElement.h:904 >> + const PAL::Logger* mediaPlayerLogger() final { return &logger(); } > > Can we just make this a ref rather than a pointer? Fixed. >> Source/WebCore/platform/graphics/MediaPlayer.h:306 >> + virtual const PAL::Logger* mediaPlayerLogger() { return nullptr; } > > You would just make this a pure virtual. (But that's ok; the HTMLMediaElement is the only MediaPlayerClient, currently.) Fixed.
Eric Carlson
Comment 7 2017-09-11 14:33:59 PDT
Created attachment 320481 [details] Patch for landing.
Eric Carlson
Comment 8 2017-09-12 08:59:51 PDT
Created attachment 320550 [details] Patch for landing.
WebKit Commit Bot
Comment 9 2017-09-12 11:34:26 PDT
Comment on attachment 320550 [details] Patch for landing. Clearing flags on attachment: 320550 Committed r221929: <http://trac.webkit.org/changeset/221929>
Note You need to log in before you can comment on or make changes to this bug.