Summary: | Use new Web IDL dictionary support for MediaSession.setMetadata() | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||
Component: | Bindings | Assignee: | Chris Dumez <cdumez> | ||||
Status: | REOPENED --- | ||||||
Severity: | Normal | CC: | ahmad.saleem792, ap, bfulgham, cdumez, commit-queue, darin, eric.carlson, jer.noble, rniwa | ||||
Priority: | P2 | ||||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | 157725 | ||||||
Bug Blocks: | |||||||
Attachments: |
|
Description
Chris Dumez
2016-05-14 13:31:12 PDT
Created attachment 278944 [details]
Patch
Comment on attachment 278944 [details] Patch Clearing flags on attachment: 278944 Committed r200925: <http://trac.webkit.org/changeset/200925> All reviewed patches have been landed. Closing bug. Comment on attachment 278944 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=278944&action=review > Source/WebCore/Modules/mediasession/MediaSession.h:56 > + struct MediaMetadataInit { I am surprised this works. Maybe the bindings are inside an #if and not compiling. The name for the struct is supposed to be MediaSession::MetadataInit, not MediaSession::MediaMetadataInit. Maybe there is a bug in the GetNestedClassName function in CodeGeneratorJS.pm? Yes, looks like this will fail to compile if ENABLE(MEDIA_SESSION) is true. Re-opened since this is blocked by bug 157725 (In reply to comment #5) > Yes, looks like this will fail to compile if ENABLE(MEDIA_SESSION) is true. Ok, I will roll out :( I was convinced this was enabled on Mac for some reason. We can definitely redo it; we just need to turn on compiling on some platform. I find it really hard to maintain code that is not compiled! This was rolled out and I noticed that there are difference in Webkit MediaSession.IDL with Chrome / Blink: Webkit Link - https://github.com/WebKit/WebKit/blob/main/Source/WebCore/Modules/mediasession/MediaSession.idl Chromium / Blink - https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/mediasession/media_session.idl?q=media_Session.idl Chris - is this needed? Thanks! |