Bug 120650

Summary: Get MEDIA_STREAM compiling on OS X
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: MediaAssignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, glenn, hta, jer.noble, ossy, tommyw
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 124288    
Attachments:
Description Flags
Proposed patch none

Eric Carlson
Reported 2013-09-03 13:54:19 PDT
Update MEDIA_STREAM flagged code so it compiles for the Mac port.
Attachments
Proposed patch (157.04 KB, patch)
2013-09-03 14:14 PDT, Eric Carlson 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Eric Carlson
Comment 1 2013-09-03 14:14:51 PDT
Created attachment 210410 [details] Proposed patch
Darin Adler
Comment 2 2013-09-03 19:22:19 PDT
Comment on attachment 210410 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=210410&action=review > Source/WebCore/Modules/mediastream/MediaStreamRegistry.cpp:61 > - return m_streamDescriptors.get(url).get(); > + > + RefPtr<MediaStreamDescriptor> descriptor = m_streamDescriptors.get(url); > + return descriptor.get(); What typecast does this avoid? I don’t understand. The old code looks better than the new. > Source/WebCore/Modules/mediastream/RTCSessionDescription.cpp:100 > void RTCSessionDescription::setSdp(const String& sdp, ExceptionCode& ec) Why include this ExceptionCode argument if it‘s always zero? Better to fix the IDL file. > Source/WebCore/Modules/mediastream/RTCStatsResponse.cpp:51 > + return m_idmap.find(name) != m_idmap.end(); Should use contains, not find != end.
Eric Carlson
Comment 3 2013-09-04 12:22:14 PDT
(In reply to comment #2) > (From update of attachment 210410 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=210410&action=review > > > Source/WebCore/Modules/mediastream/MediaStreamRegistry.cpp:61 > > - return m_streamDescriptors.get(url).get(); > > + > > + RefPtr<MediaStreamDescriptor> descriptor = m_streamDescriptors.get(url); > > + return descriptor.get(); > > What typecast does this avoid? I don’t understand. The old code looks better than the new. > The old code wouldn't compile yesterday, but I updated my tools this morning and it works. Removed my change. > > Source/WebCore/Modules/mediastream/RTCSessionDescription.cpp:100 > > void RTCSessionDescription::setSdp(const String& sdp, ExceptionCode& ec) > > Why include this ExceptionCode argument if it‘s always zero? Better to fix the IDL file. > Good point, removed. > > Source/WebCore/Modules/mediastream/RTCStatsResponse.cpp:51 > > + return m_idmap.find(name) != m_idmap.end(); > > Should use contains, not find != end. > Done. Thanks!
Eric Carlson
Comment 4 2013-09-04 12:22:52 PDT
Committed r155057: https://trac.webkit.org/r155057
Csaba Osztrogonác
Comment 5 2013-09-04 14:19:33 PDT
(In reply to comment #4) > Committed r155057: https://trac.webkit.org/r155057 FYI: It broke the build on the Windows bots. Could you fix it please?
Csaba Osztrogonác
Comment 6 2013-09-04 14:21:13 PDT
1>C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\obj32\WebCore\DerivedSources\JSNavigator.cpp(36): fatal error C1083: Cannot open include file: 'NavigatorMediaStream.h': No such file or directory
Csaba Osztrogonác
Comment 7 2013-09-04 15:00:16 PDT
ping?
Eric Carlson
Comment 8 2013-09-04 15:09:05 PDT
(In reply to comment #7) > ping? bfulgham is working on a fix.
Note You need to log in before you can comment on or make changes to this bug.