Bug 120811 – Get MEDIA_STREAM compiling for other ports (EFL and GTK)

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Thiago de Barros Lacerda]

Now these ports can be compiled with media-stream enabled

[Thiago de Barros Lacerda]

Created attachment 210682 [details]
Patch

[Thiago de Barros Lacerda]

Created attachment 210687 [details]
Patch

[Eric Carlson]

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?

[Thiago de Barros Lacerda]

(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?

[Thiago de Barros Lacerda]

Created attachment 210697 [details]
Patch

[Zan Dobersek]

(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

[Thiago de Barros Lacerda]

(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

[Eric Carlson]

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.

[Thiago de Barros Lacerda]

Created attachment 210772 [details]
Patch

[WebKit Commit Bot]

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

[Thiago de Barros Lacerda]

Created attachment 210773 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 210773 [details]
Patch

Clearing flags on attachment: 210773

Committed r155248: <http://trac.webkit.org/changeset/155248>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.