Bug 176621

Summary: Switch MediaPlayerPrivateAVFoundation to release logging
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: MediaAssignee: Eric Carlson <eric.carlson>
Status: ASSIGNED    
Severity: Normal CC: commit-queue, jer.noble, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Proposed patch.
none
Patch
none
Patch.
none
Patch for landing.
none
Patch for landing. none

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
DetailsFormatted DiffDiff
Patch (57.92 KB, patch)
2017-09-10 18:55 PDT, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Patch. (58.50 KB, patch)
2017-09-11 08:50 PDT, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Patch for landing. (59.36 KB, patch)
2017-09-11 14:33 PDT, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Patch for landing. (59.36 KB, patch)
2017-09-12 08:59 PDT, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2017-09-08 11:34:33 PDT
<rdar://problem/34335311>
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
Created attachment 320412 [details] Patch
Eric Carlson
Comment 4 2017-09-11 08:50:50 PDT
Created attachment 320430 [details] Patch.
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.