Bug 124902

Summary:

Adding MediaConstraintsMock class

Product:

WebKit

Reporter:

Thiago de Barros Lacerda <thiago.lacerda>

Component:

WebCore Misc.

Assignee:

Thiago de Barros Lacerda <thiago.lacerda>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, eric.carlson, glenn, gyuyoung.kim, hta, jer.noble, pnormand, rakuco, tommyw

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	eric.carlson: review+
Patch for landing.	thiago.lacerda: commit-queue-
Patch for landing.	none

Thiago de Barros Lacerda

Reported 2013-11-26 14:57:35 PST

Missing block to run RTCPeerConnection LayoutTests

Attachments
Patch (14.77 KB, patch) 2013-11-26 15:01 PST, Thiago de Barros Lacerda	no flags	Details Formatted Diff Diff
Patch (18.92 KB, patch) 2013-11-27 06:06 PST, Thiago de Barros Lacerda	eric.carlson: review+	Details Formatted Diff Diff
Patch for landing. (18.40 KB, patch) 2013-11-27 12:11 PST, Thiago de Barros Lacerda	thiago.lacerda: commit-queue-	Details Formatted Diff Diff
Patch for landing. (18.40 KB, patch) 2013-11-27 12:15 PST, Thiago de Barros Lacerda	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Thiago de Barros Lacerda

Comment 1 2013-11-26 15:01:37 PST

Created attachment 217907 [details] Patch

Thiago de Barros Lacerda

Comment 2 2013-11-26 15:02:45 PST

*** Bug 123093 has been marked as a duplicate of this bug. ***

Eric Carlson

Comment 3 2013-11-26 17:51:24 PST

Comment on attachment 217907 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=217907&action=review > Source/WebCore/platform/mock/MediaConstraintsMock.cpp:52 > +bool isSupported(const String& constraint) > +{ > + return constraint == "valid_and_supported_1" || constraint == "valid_and_supported_2"; > +} > + > +bool isValid(const String& constraint) > +{ > + return isSupported(constraint) || constraint == "valid_but_unsupported_1" || constraint == "valid_but_unsupported_2"; > +} > + > +bool MediaConstraintsMock::verifyConstraints(PassRefPtr<MediaConstraints> prpConstraints) This duplicates functionality in MockMediaStreamCenter. I would rather you use MockMediaStreamCenter::validateRequestConstraints and its supporting functions, and have MockMediaStreamCenter use this.

Thiago de Barros Lacerda

Comment 4 2013-11-26 18:27:29 PST

(In reply to comment #3) > (From update of attachment 217907 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=217907&action=review > > > Source/WebCore/platform/mock/MediaConstraintsMock.cpp:52 > > +bool isSupported(const String& constraint) > > +{ > > + return constraint == "valid_and_supported_1" || constraint == "valid_and_supported_2"; > > +} > > + > > +bool isValid(const String& constraint) > > +{ > > + return isSupported(constraint) || constraint == "valid_but_unsupported_1" || constraint == "valid_but_unsupported_2"; > > +} > > + > > +bool MediaConstraintsMock::verifyConstraints(PassRefPtr<MediaConstraints> prpConstraints) > > This duplicates functionality in MockMediaStreamCenter. I would rather you use MockMediaStreamCenter::validateRequestConstraints and its supporting functions, and have MockMediaStreamCenter use this. I agree, haven't seen that MockMediaStreamCenter was validating constraints too.

Thiago de Barros Lacerda

Comment 5 2013-11-27 06:06:01 PST

Created attachment 217945 [details] Patch

Eric Carlson

Comment 6 2013-11-27 10:13:38 PST

Comment on attachment 217945 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=217945&action=review > Source/WebCore/ChangeLog:8 > + Missing block to run RTCPeerConnection LayoutTests Not: do you mean missing *mock*? > Source/WebCore/platform/mock/MediaConstraintsMock.cpp:2 > + * Copyright (C) 2012 Google Inc. All rights reserved. Nit: I think you can replace "Google" with "Apple" here. > Source/WebCore/platform/mock/MediaConstraintsMock.h:2 > + * Copyright (C) 2012 Google Inc. All rights reserved. Ditto > Source/WebCore/platform/mock/RTCPeerConnectionHandlerMock.cpp:52 > + if (invalidQuery.isEmpty()) { Nit: you could use an early return here.

Thiago de Barros Lacerda

Comment 7 2013-11-27 12:11:27 PST

Created attachment 217961 [details] Patch for landing.

Thiago de Barros Lacerda

Comment 8 2013-11-27 12:15:22 PST

Created attachment 217962 [details] Patch for landing.

WebKit Commit Bot

Comment 9 2013-11-27 13:01:13 PST

Comment on attachment 217962 [details] Patch for landing. Clearing flags on attachment: 217962 Committed r159823: <http://trac.webkit.org/changeset/159823>

Note You need to log in before you can comment on or make changes to this bug.