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: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Thiago de Barros Lacerda
2013-11-26 14:57:35 PST
Created attachment 217907 [details]
Patch
*** Bug 123093 has been marked as a duplicate of this bug. *** 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. (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. Created attachment 217945 [details]
Patch
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. Created attachment 217961 [details]
Patch for landing.
Created attachment 217962 [details]
Patch for landing.
Comment on attachment 217962 [details] Patch for landing. Clearing flags on attachment: 217962 Committed r159823: <http://trac.webkit.org/changeset/159823> |