| Summary: | Move MediaConstraintsImpl to MediaConstraintsPrivate in platform/ | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Philippe Normand <pnormand> | ||||
| Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||
| Status: | RESOLVED INVALID | ||||||
| Severity: | Normal | CC: | agouaillard, annle10996, annylake, ap, buildbot, commit-queue, eric.carlson, fabri.ll, glenn, gta5hex, gyuyoung.kim, hta, jennyhannb, jer.noble, jet, jonlee, karishmasingh4097, nick.diego, pnormand, rakuco, rniwa, thiago.lacerda, tommyw | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 124288 | ||||||
| Attachments: |
|
||||||
|
Description
Philippe Normand
2013-11-12 23:15:58 PST
Created attachment 216776 [details]
patch
Doesn't yet include XCode project changes.
Comment on attachment 216776 [details] patch Attachment 216776 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/21249958 Comment on attachment 216776 [details] patch Attachment 216776 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/21249959 +1 for this change. There is no reason to have an impl class if there will not have platform dependent implementation of that It is a layering violation to pass an object defined in Modules/mediastream to code in platform. What will we use in platform, for example what gets passed to MediaStreamCenter::validateRequestConstraints? (In reply to comment #5) > It is a layering violation to pass an object defined in Modules/mediastream to code in platform. > > What will we use in platform, for example what gets passed to MediaStreamCenter::validateRequestConstraints? You are right. I don't know why, but I always thought that the platform implementation was the Impl one, but in this case is MediaConstraints. But indeed, we have to leave a platform implementation. We could leave MediaConstraints in Modules/mediastream, and create a MediaConstraintsPrivate that would be in platform. To summarize, would only be a rename of classes. Comment on attachment 216776 [details]
patch
Ok let's finally do this properly then :)
Well, I'm not sure anymore this is a real issue, if we move the Impl class to platform/ and rename it to Private it's not exactly like other Private implementations we have in platform/mediastream. MediaConstraintsImpl is the actual implementation of an abstract class whereas other Private classes don't inherit from the interface they implement, it's a different pattern... For bug 123158 I got a PoC that is able to send MediaConstraintsImpl over the wire but I'm not sure if this is a layer violation or not. I'd prefer to handle MediaConstraints directly but being an abstract class I'm having issues for the IPC communication. This has been removed via 172132. |