Bug 164059 - PeerMediaDescription does not need to be refcounted
Summary: PeerMediaDescription does not need to be refcounted
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-27 05:08 PDT by youenn fablet
Modified: 2016-10-28 06:39 PDT (History)
3 users (show)

See Also:

Attachments
Patch (46.86 KB, patch)
2016-10-27 05:16 PDT, youenn fablet 		no flags Details | Formatted Diff | Diff
Patch (46.80 KB, patch)
2016-10-27 05:27 PDT, youenn fablet 		no flags Details | Formatted Diff | Diff
Patch (51.44 KB, patch)
2016-10-27 08:16 PDT, youenn fablet 		no flags Details | Formatted Diff | Diff
Patch for landing (51.71 KB, patch)
2016-10-28 03:41 PDT, youenn fablet 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2016-10-27 05:08:46 PDT 
And it should be a struct probably
Comment 1 youenn fablet 2016-10-27 05:16:58 PDT 
Created attachment 293013 [details]
Patch
Comment 2 youenn fablet 2016-10-27 05:27:53 PDT 
Created attachment 293014 [details]
Patch
Comment 3 youenn fablet 2016-10-27 08:16:22 PDT 
Created attachment 293019 [details]
Patch
Comment 4 Darin Adler 2016-10-27 14:31:13 PDT 
Comment on attachment 293019 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=293019&action=review

> Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.h:46
> -class PeerMediaDescription;
> +struct PeerMediaDescription;

We normally put struct forward declarations in a separate paragraph, rather than sorting them in with the classes.

> Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.h:49
> -typedef Vector<RefPtr<PeerMediaDescription>> MediaDescriptionVector;
> +typedef Vector<PeerMediaDescription> MediaDescriptionVector;

In code we are touching we want to move from typedef to using.

> Source/WebCore/platform/mediastream/PeerMediaDescription.h:43
>  public:

No need for "public:" in a struct.

> Source/WebCore/platform/mediastream/PeerMediaDescription.h:52
> +    String mode { "sendrecv" };

Slightly nicer to use { ASCIILiteral { "sendrecv" } }.

> Source/WebCore/platform/mediastream/openwebrtc/MediaEndpointOwr.h:46
> -class PeerMediaDescription;
> +struct PeerMediaDescription;

Same comment about putting a struct in a separate paragraph.
Comment 5 youenn fablet 2016-10-28 03:41:49 PDT 
Created attachment 293137 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2016-10-28 04:17:43 PDT 
Comment on attachment 293137 [details]
Patch for landing

Clearing flags on attachment: 293137

Committed r208043: <http://trac.webkit.org/changeset/208043>
Comment 7 WebKit Commit Bot 2016-10-28 04:17:47 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 8 youenn fablet 2016-10-28 06:39:38 PDT 
Thanks for the review.

(In reply to comment #4)
> Comment on attachment 293019 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=293019&action=review
> 
> > Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.h:46
> > -class PeerMediaDescription;
> > +struct PeerMediaDescription;
> 
> We normally put struct forward declarations in a separate paragraph, rather
> than sorting them in with the classes.

OK

> > Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.h:49
> > -typedef Vector<RefPtr<PeerMediaDescription>> MediaDescriptionVector;
> > +typedef Vector<PeerMediaDescription> MediaDescriptionVector;
> 
> In code we are touching we want to move from typedef to using.

OK

> > Source/WebCore/platform/mediastream/PeerMediaDescription.h:43
> >  public:
> 
> No need for "public:" in a struct.

Done

> > Source/WebCore/platform/mediastream/PeerMediaDescription.h:52
> > +    String mode { "sendrecv" };
> 
> Slightly nicer to use { ASCIILiteral { "sendrecv" } }.

Done

> > Source/WebCore/platform/mediastream/openwebrtc/MediaEndpointOwr.h:46
> > -class PeerMediaDescription;
> > +struct PeerMediaDescription;
> 
> Same comment about putting a struct in a separate paragraph.

OK