Summary: | Get MEDIA_STREAM compiling for other ports (EFL and GTK) | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Thiago de Barros Lacerda <thiago.lacerda> | ||||||||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | commit-queue, eric.carlson, glenn, gyuyoung.kim, hta, hugo.lima, jer.noble, lauro.neto, luciano.wolf, rakuco, sergio, tommyw, zan | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Thiago de Barros Lacerda
2013-09-05 16:44:20 PDT
Created attachment 210682 [details]
Patch
Created attachment 210687 [details]
Patch
Comment on attachment 210687 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=210687&action=review > Source/WebCore/Modules/mediastream/LocalMediaStream.h:39 > +#if PLATFORM(MAC) > class LocalMediaStream FINAL : public MediaStream { > +#else > +class LocalMediaStream : public MediaStream { > +#endif Why doesn't FINAL work for you? > Source/WebCore/Modules/mediastream/MediaStreamRegistry.cpp:60 > + RefPtr<MediaStreamDescriptor> descriptor = m_streamDescriptors.get(url); > + return descriptor.get(); This is wrong, for HashMap the peek type is the wrapped pointer so you can just remove the ".get()": return m_streamDescriptors.get(url) > Source/WebCore/Modules/mediastream/RTCStatsResponse.h:46 > +#if PLATFORM(MAC) > class RTCStatsResponse FINAL : public RTCStatsResponseBase { > +#else > +class RTCStatsResponse : public RTCStatsResponseBase { > +#endif Why doesn't FINAL work here? (In reply to comment #3) > (From update of attachment 210687 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=210687&action=review > > > Source/WebCore/Modules/mediastream/LocalMediaStream.h:39 > > +#if PLATFORM(MAC) > > class LocalMediaStream FINAL : public MediaStream { > > +#else > > +class LocalMediaStream : public MediaStream { > > +#endif > > Why doesn't FINAL work for you? Because both LocalMediaStream and RTCStatsResponseBase in the end will be inherited by struct Base in JSDOMBinding.h (line 540) > > > Source/WebCore/Modules/mediastream/MediaStreamRegistry.cpp:60 > > + RefPtr<MediaStreamDescriptor> descriptor = m_streamDescriptors.get(url); > > + return descriptor.get(); > > This is wrong, for HashMap the peek type is the wrapped pointer so you can just remove the ".get()": > > return m_streamDescriptors.get(url) > My first thought was doing this way you are telling, but I thought it was wrong. OK, I will change. > > Source/WebCore/Modules/mediastream/RTCStatsResponse.h:46 > > +#if PLATFORM(MAC) > > class RTCStatsResponse FINAL : public RTCStatsResponseBase { > > +#else > > +class RTCStatsResponse : public RTCStatsResponseBase { > > +#endif > > Why doesn't FINAL work here? Created attachment 210697 [details]
Patch
(In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 210687 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=210687&action=review > > > > > Source/WebCore/Modules/mediastream/LocalMediaStream.h:39 > > > +#if PLATFORM(MAC) > > > class LocalMediaStream FINAL : public MediaStream { > > > +#else > > > +class LocalMediaStream : public MediaStream { > > > +#endif > > > > Why doesn't FINAL work for you? > > Because both LocalMediaStream and RTCStatsResponseBase in the end will be inherited by struct Base in JSDOMBinding.h (line 540) I guess you mean the definition of the Base struct in the HasMemoryCostMemberFunction class? http://trac.webkit.org/browser/trunk/Source/WebCore/bindings/js/JSDOMBinding.h#L613 (In reply to comment #6) > (In reply to comment #4) > > (In reply to comment #3) > > > (From update of attachment 210687 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=210687&action=review > > > > > > > Source/WebCore/Modules/mediastream/LocalMediaStream.h:39 > > > > +#if PLATFORM(MAC) > > > > class LocalMediaStream FINAL : public MediaStream { > > > > +#else > > > > +class LocalMediaStream : public MediaStream { > > > > +#endif > > > > > > Why doesn't FINAL work for you? > > > > Because both LocalMediaStream and RTCStatsResponseBase in the end will be inherited by struct Base in JSDOMBinding.h (line 540) > > I guess you mean the definition of the Base struct in the HasMemoryCostMemberFunction class? > http://trac.webkit.org/browser/trunk/Source/WebCore/bindings/js/JSDOMBinding.h#L613 Yes, sorry about the confusion. It is line 624, inside HasMemoryCostMemberFunction class Comment on attachment 210697 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=210697&action=review > Source/WebCore/Modules/mediastream/LocalMediaStream.h:39 > +#if PLATFORM(MAC) > class LocalMediaStream FINAL : public MediaStream { > +#else > +class LocalMediaStream : public MediaStream { > +#endif I think it is better to not have platform specific code here so I am fine with you just removing the FINAL. > Source/WebCore/Modules/mediastream/RTCStatsResponse.h:46 > +#if PLATFORM(MAC) > class RTCStatsResponse FINAL : public RTCStatsResponseBase { > +#else > +class RTCStatsResponse : public RTCStatsResponseBase { > +#endif Ditto. Created attachment 210772 [details]
Patch
Comment on attachment 210772 [details] Patch Rejecting attachment 210772 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 210772, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit /Volumes/Data/EWS/WebKit/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://webkit-queues.appspot.com/results/1705448 Created attachment 210773 [details]
Patch
Comment on attachment 210773 [details] Patch Clearing flags on attachment: 210773 Committed r155248: <http://trac.webkit.org/changeset/155248> All reviewed patches have been landed. Closing bug. |