Needed to create MediaDeviceInfo files in Javascript to ensure that methods like enumarateDevices can work.
(In reply to comment #0) > Needed to create MediaDeviceInfo files in Javascript to ensure that methods > like enumarateDevices can work. Should be "MediaDeviceInfo class in Javascript"
<rdar://problem/21513462>
Created attachment 255475 [details] Patch
Comment on attachment 255475 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255475&action=review > Source/WebCore/Modules/mediastream/MediaDeviceInfo.cpp:14 > +#include "ContextDescructionObserver.h" typo: ContextDes_c_ructionObserver : c -> t How come the build worked on Mac? > Source/WebCore/Modules/mediastream/MediaDeviceInfo.h:14 > +#include "ContextDescructionObserver.h" ditto > Source/WebCore/Modules/mediastream/MediaDeviceInfo.h:21 > +class MediaDeviceInfo : public RefCounted<MediaDeviceInfo>, public ScriptWrappable, public ContextDescructionObserver { ditto
Created attachment 255481 [details] Patch
Sorry, was typo. Fixed now.
Comment on attachment 255481 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255481&action=review > Source/WebCore/Modules/mediastream/MediaDeviceInfo.h:39 > + const String& m_label; > + const String& m_deviceId; > + const String& m_groupId; > + const String& m_kind; Do you really want these to be const string *references*?
The EFL/GTK build is still broken: Source/WebCore/Modules/mediastream/MediaDeviceInfo.h:40:1: error: expected ';' after class definition Please fix them before landing. How come Mac build isn't broken? Have you tested with ENABLE(MEDIA_STREAM) == true configuration?
Created attachment 255484 [details] Patch
Comment on attachment 255484 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255484&action=review > Source/WebCore/ChangeLog:3 > + Adding MediaDeviceInfo. It should be exactly the title of the bug report: "Building MediaDeviceInfo for enumerateDevices" > Source/WebCore/Modules/mediastream/MediaDeviceInfo.cpp:7 > +// > +// MediaDeviceInfo.cpp > +// WebCore > +// > +// Created by Matt Daiter on 6/23/15. > +// > +// Please remove these lines and add proper copyright and license here. > Source/WebCore/Modules/mediastream/MediaDeviceInfo.h:7 > +// > +// MediaDeviceInfo.h > +// WebCore > +// > +// Created by Matt Daiter on 6/23/15. > +// > +// ditto
Created attachment 255488 [details] Patch
Comment on attachment 255488 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255488&action=review > Source/WebCore/Modules/mediastream/MediaDeviceInfo.h:38 > +class MediaDeviceInfo : public RefCounted<MediaDeviceInfo>, public ScriptWrappable, public ContextDestructionObserver { Why does this class inherit from ContextDestructionObserver, will it need to override contextDestroyed() later? > Source/WebCore/Modules/mediastream/MediaDeviceInfo.h:42 > + Nit: this extra blank line isn't necessary. > Source/WebCore/Modules/mediastream/MediaDeviceInfo.idl:2 > +[ > +NoInterfaceObject, .idl files need a copyright header too. > Source/WebCore/Modules/mediastream/MediaDeviceInfo.idl:10 > + readonly attribute DOMString deviceId; > + readonly attribute DOMString groupId; > + readonly attribute DOMString kind; > + readonly attribute DOMString label; Nit: I don't think the extra spaces between "readonly" and "attribute" are necessary.
(In reply to comment #12) > Comment on attachment 255488 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=255488&action=review > > > Source/WebCore/Modules/mediastream/MediaDeviceInfo.h:38 > > +class MediaDeviceInfo : public RefCounted<MediaDeviceInfo>, public ScriptWrappable, public ContextDestructionObserver { > > Why does this class inherit from ContextDestructionObserver, will it need to > override contextDestroyed() later? > Good point! Won't include in the next update. I believe that while debugging I had been experimenting with using it for destroying pointers to strings to free space. Next time, I won't inherit from it unless necessary. > > Source/WebCore/Modules/mediastream/MediaDeviceInfo.h:42 > > + > Sure. I can remove this. > Nit: this extra blank line isn't necessary. > > > Source/WebCore/Modules/mediastream/MediaDeviceInfo.idl:2 > > +[ > > +NoInterfaceObject, > > .idl files need a copyright header too. > Will do. > > Source/WebCore/Modules/mediastream/MediaDeviceInfo.idl:10 > > + readonly attribute DOMString deviceId; > > + readonly attribute DOMString groupId; > > + readonly attribute DOMString kind; > > + readonly attribute DOMString label; > > Nit: I don't think the extra spaces between "readonly" and "attribute" are > necessary. Sorry, will remove on next patch submit
Comment on attachment 255488 [details] Patch Rejecting attachment 255488 [details] from commit-queue. mdaiter@apple.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/contributors.json. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.
Comment on attachment 255488 [details] Patch Clearing flags on attachment: 255488 Committed r185940: <http://trac.webkit.org/changeset/185940>
All reviewed patches have been landed. Closing bug.
This broke builds on several bots. I landed several build fixes (more may be needed): <https://trac.webkit.org/r185944> <https://trac.webkit.org/r185945> I am not sure why EWS did not complain on GTK/EFL.
(In reply to comment #17) > This broke builds on several bots. I landed several build fixes (more may be > needed): > <https://trac.webkit.org/r185944> > <https://trac.webkit.org/r185945> > > I am not sure why EWS did not complain on GTK/EFL. and <http://trac.webkit.org/changeset/185946>...
(In reply to comment #18) > (In reply to comment #17) > > This broke builds on several bots. I landed several build fixes (more may be > > needed): > > <https://trac.webkit.org/r185944> > > <https://trac.webkit.org/r185945> > > > > I am not sure why EWS did not complain on GTK/EFL. > > and <http://trac.webkit.org/changeset/185946>... It seems the build was broken only on the GTK debug bot. How is it possible? Is this feature used somewhere or is it simple dead code?
(In reply to comment #19) > It seems the build was broken only on the GTK debug bot. How is it > possible? Is this feature used somewhere or is it simple dead code? I built r185940 (EFL) and build works fine. It seems JSMediaDeviceInfo.cpp (generated from the idl file) doesn't use MediaDeviceInfo::MediaDeviceInfo and MediaDeviceInfo::create at all. Is it normal behaviour?
Just out of curiosity: Was this patch tested/built on Mac with enabled MEDIA_STREAM ? Can we add a layout test to check if it works or not?
Hey, Planning on tackling larger problems, which uses MediaDeviceInfo. Dead code now, not dead code later.