Skip to content

Commit

Permalink
[Merge-103] Ignore unrecognized legacy MediaConstraints being parsed.
Browse files Browse the repository at this point in the history 
Edit: Not renaming unused constants to "kOBSOLETE_" in the backmerge in
order to reduce the size of the backmerge.

This is consistent with how WebIDL parses JS dictionary objects and
allows us to delete old constants and use counters.

Bonus: Delete RtpDataChannel legacy constraints, it was already ignored
(and the comment about it only being available on Fuchsia was wrong).

(cherry picked from commit b341786)

Bug: 1328047, 1328049
Change-Id: Ib6b0e22303918d9c408a382ee2f5720e63127947
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3660263
Reviewed-by: Harald Alvestrand <hta@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
Reviewed-by: Ari Chivukula <arichiv@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1006820}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3689557
Cr-Commit-Position: refs/branch-heads/5060@{#631}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
@henbos
henbos authored and Chromium LUCI CQ committed Jun 7, 2022
1 parent 1abbbcd commit a10f66e
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 154 deletions.
21 changes: 5 additions & 16 deletions chrome/test/data/extensions/api_test/tab_capture/constraints/constraints.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ chrome.test.runTests([
});
},

function rejectsInvalidConstraints() {
function ignoresInvalidConstraints() {
chrome.tabCapture.capture({
video: true,
audio: true,
Expand All @@ -58,21 +58,10 @@ chrome.test.runTests([
}
}
}, function(stream) {
assertBindingsPassedWebKitErrorMessage();
chrome.test.assertTrue(!stream);

chrome.tabCapture.capture({
audio: true,
audioConstraints: {
mandatory: {
notValid: '123'
}
}
}, function(stream) {
assertBindingsPassedWebKitErrorMessage();
chrome.test.assertTrue(!stream);
chrome.test.succeed();
});
chrome.test.assertTrue(!!stream);
stream.getVideoTracks()[0].stop();
stream.getAudioTracks()[0].stop();
chrome.test.succeed();
});
}
]);
94 changes: 5 additions & 89 deletions third_party/blink/renderer/modules/mediastream/media_constraints_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ const char kMediaStreamSourceInfoId[] = "sourceId"; // mapped to deviceId
const char kMediaStreamRenderToAssociatedSink[] =
"chromeRenderToAssociatedSink";
// RenderToAssociatedSink will be going away some time.
const char kMediaStreamAudioHotword[] = "googHotword";
const char kEchoCancellation[] = "echoCancellation";
const char kDisableLocalEcho[] = "disableLocalEcho";
const char kGoogEchoCancellation[] = "googEchoCancellation";
Expand All @@ -105,10 +104,7 @@ const char kGoogAutoGainControl[] = "googAutoGainControl";
const char kGoogExperimentalAutoGainControl[] = "googAutoGainControl2";
const char kGoogNoiseSuppression[] = "googNoiseSuppression";
const char kGoogExperimentalNoiseSuppression[] = "googNoiseSuppression2";
const char kGoogBeamforming[] = "googBeamforming";
const char kGoogArrayGeometry[] = "googArrayGeometry";
const char kGoogHighpassFilter[] = "googHighpassFilter";
const char kGoogTypingNoiseDetection[] = "googTypingNoiseDetection";
const char kGoogAudioMirroring[] = "googAudioMirroring";
// Audio constraints.
const char kDAEchoCancellation[] = "googDAEchoCancellation";
Expand All @@ -117,37 +113,13 @@ const char kNoiseReduction[] = "googNoiseReduction";

// Legacy RTCPeerConnection constructor constraints.

// DtlsSrtpKeyAgreement and RtpDataChannels are already ignored, except when
// building Fuchsia.
// DtlsSrtpKeyAgreement is already ignored, except when building Fuchsia.
// TODO(crbug.com/804275): Ignore on all platforms when Fuchsia dependency is
// gone to unblock mediaConstraints removal.
const char kEnableDtlsSrtp[] = "DtlsSrtpKeyAgreement";
const char kEnableRtpDataChannels[] = "RtpDataChannels";
// TODO(https://crbug.com/1315576): Deprecate and ignore.
const char kEnableIPv6[] = "googIPv6";
// Legacy goog-constraints that are already ignored.
const char kNumUnsignalledRecvStreams[] = "googNumUnsignalledRecvStreams";
const char kCombinedAudioVideoBwe[] = "googCombinedAudioVideoBwe";
const char kCpuUnderuseThreshold[] = "googCpuUnderuseThreshold";
const char kCpuOveruseThreshold[] = "googCpuOveruseThreshold";
const char kCpuUnderuseEncodeRsdThreshold[] =
"googCpuUnderuseEncodeRsdThreshold";
const char kCpuOveruseEncodeRsdThreshold[] = "googCpuOveruseEncodeRsdThreshold";
const char kCpuOveruseEncodeUsage[] = "googCpuOveruseEncodeUsage";
const char kHighStartBitrate[] = "googHighStartBitrate";
const char kPayloadPadding[] = "googPayloadPadding";
const char kAudioLatency[] = "latencyMs";
const char kUseRtpMux[] = "googUseRtpMUX";
const char kEnableDscp[] = "googDscp";
const char kScreencastMinBitrate[] = "googScreencastMinBitrate";
const char kEnableVideoSuspendBelowMinBitrate[] = "googSuspendBelowMinBitrate";
const char kCpuOveruseDetection[] = "googCpuOveruseDetection";

// Names that have been used in the past, but should now be ignored.
// Kept around for backwards compatibility.
// https://crbug.com/579729
const char kGoogLeakyBucket[] = "googLeakyBucket";
const char kPowerLineFrequency[] = "googPowerLineFrequency";

// Names used for testing.
const char kTestConstraint1[] = "valid_and_supported_1";
const char kTestConstraint2[] = "valid_and_supported_2";
Expand Down Expand Up @@ -276,7 +248,6 @@ static bool ToBoolean(const WebString& as_web_string) {
static void ParseOldStyleNames(
ExecutionContext* context,
const Vector<NameValueStringConstraint>& old_names,
bool report_unknown_names,
MediaTrackConstraintSetPlatform& result,
MediaErrorState& error_state) {
if (old_names.size() > 0) {
Expand Down Expand Up @@ -354,21 +325,7 @@ static void ParseOldStyleNames(
// Special dispensation for Fuchsia to run SDES in 2022
// TODO(crbug.com/804275): Delete when Fuchsia no longer depends on it.
result.enable_dtls_srtp.SetExact(ToBoolean(constraint.value_));
#else
UseCounter::Count(context, WebFeature::kOldConstraintIgnored);
#endif
} else if (constraint.name_.Equals(kEnableRtpDataChannels)) {
// This constraint does not turn on RTP data channels, but we do not
// want it to cause an error, so we parse it and ignore it.
bool value = ToBoolean(constraint.value_);
if (value) {
Deprecation::CountDeprecation(
context, WebFeature::kRTCConstraintEnableRtpDataChannelsTrue);
} else {
Deprecation::CountDeprecation(
context, WebFeature::kRTCConstraintEnableRtpDataChannelsFalse);
}
UseCounter::Count(context, WebFeature::kOldConstraintIgnored);
} else if (constraint.name_.Equals(kEnableIPv6)) {
result.enable_i_pv6.SetExact(ToBoolean(constraint.value_));
// Count deprecated usage of googIPv6, when it is set to false. Setting it
Expand All @@ -378,35 +335,6 @@ static void ParseOldStyleNames(
Deprecation::CountDeprecation(context,
WebFeature::kLegacyConstraintGoogIPv6);
}
} else if (constraint.name_.Equals(kCpuUnderuseThreshold) ||
constraint.name_.Equals(kCpuOveruseThreshold) ||
constraint.name_.Equals(kCpuUnderuseEncodeRsdThreshold) ||
constraint.name_.Equals(kCpuOveruseEncodeRsdThreshold) ||
constraint.name_.Equals(kCpuOveruseEncodeUsage) ||
constraint.name_.Equals(kGoogLeakyBucket) ||
constraint.name_.Equals(kGoogBeamforming) ||
constraint.name_.Equals(kGoogArrayGeometry) ||
constraint.name_.Equals(kPowerLineFrequency) ||
constraint.name_.Equals(kMediaStreamAudioHotword) ||
constraint.name_.Equals(kGoogTypingNoiseDetection) ||
constraint.name_.Equals(kNumUnsignalledRecvStreams) ||
constraint.name_.Equals(kCombinedAudioVideoBwe) ||
constraint.name_.Equals(kHighStartBitrate) ||
constraint.name_.Equals(kAudioLatency) ||
constraint.name_.Equals(kUseRtpMux) ||
constraint.name_.Equals(kPayloadPadding) ||
constraint.name_.Equals(kEnableDscp) ||
constraint.name_.Equals(kScreencastMinBitrate) ||
constraint.name_.Equals(kEnableVideoSuspendBelowMinBitrate) ||
constraint.name_.Equals(kCpuOveruseDetection)) {
// TODO(crbug.com/856176): Remove the kGoogBeamforming and
// kGoogArrayGeometry special cases.
context->AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
mojom::ConsoleMessageSource::kDeprecation,
mojom::ConsoleMessageLevel::kWarning,
"Obsolete constraint named " + String(constraint.name_) +
" is ignored. Please stop using it."));
UseCounter::Count(context, WebFeature::kOldConstraintIgnored);
} else if (constraint.name_.Equals(kTestConstraint1) ||
constraint.name_.Equals(kTestConstraint2)) {
// These constraints are only for testing parsing.
Expand All @@ -415,20 +343,8 @@ static void ParseOldStyleNames(
error_state.ThrowConstraintError("Illegal value for constraint",
constraint.name_);
}
} else {
if (report_unknown_names) {
UseCounter::Count(context, WebFeature::kOldConstraintRejected);
context->AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
mojom::ConsoleMessageSource::kDeprecation,
mojom::ConsoleMessageLevel::kWarning,
"Unknown constraint named " + String(constraint.name_) +
" rejected"));
error_state.ThrowConstraintError("Unknown name of constraint detected",
constraint.name_);
} else {
UseCounter::Count(context, WebFeature::kOldConstraintNotReported);
}
}
// else: Nothing. Unrecognized constraints are simply ignored.
}
}

Expand All @@ -440,7 +356,7 @@ static MediaConstraints CreateFromNamedConstraints(
MediaTrackConstraintSetPlatform basic;
MediaTrackConstraintSetPlatform advanced;
MediaConstraints constraints;
ParseOldStyleNames(context, mandatory, true, basic, error_state);
ParseOldStyleNames(context, mandatory, basic, error_state);
if (error_state.HadException())
return constraints;
// We ignore unknow names and syntax errors in optional constraints.
Expand All @@ -449,7 +365,7 @@ static MediaConstraints CreateFromNamedConstraints(
for (const auto& optional_constraint : optional) {
MediaTrackConstraintSetPlatform advanced_element;
Vector<NameValueStringConstraint> element_as_list(1, optional_constraint);
ParseOldStyleNames(context, element_as_list, false, advanced_element,
ParseOldStyleNames(context, element_as_list, advanced_element,
ignored_error_state);
if (!advanced_element.IsUnconstrained())
advanced_vector.push_back(advanced_element);
Expand Down
15 changes: 4 additions & 11 deletions third_party/blink/web_tests/fast/mediastream/getusermedia-constraints.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,10 @@
});
}, 'getUserMedia() with audio=true should succeed');

promise_test(function() {
return navigator.mediaDevices.getUserMedia(
{audio: {'mandatory': { 'valid_but_unsupported_1': 0}}})
.then(function(s) {
assert_unreached('getUserMedia should have failed');
})
.catch(function(e) {
assert_equals(e.name, 'OverconstrainedError');
assert_equals(e.constraint, 'valid_but_unsupported_1');
});
}, 'getUserMedia() with unsupported mandatory constraint should fail');
promise_test(async () => {
await navigator.mediaDevices.getUserMedia(
{audio: {'mandatory': { 'valid_but_unsupported_1': 0}}});
}, 'getUserMedia() with unsupported mandatory constraint should succeed');

// The next tests document existing behavior. They seem non-conformant
// with the specs.
Expand Down
14 changes: 4 additions & 10 deletions third_party/blink/web_tests/fast/mediastream/getusermedia-promise.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,10 @@
});
}, 'getUserMedia() audio and video tracks');

promise_test(function() {
return navigator.mediaDevices.getUserMedia(
{audio: {'mandatory': { 'valid_but_unsupported_1': 0}}})
.then(function(s) {
fail('getUserMedia should have failed');
}).catch(function(e) {
assert_equals(e.name, 'OverconstrainedError');
assert_equals(e.constraint, 'valid_but_unsupported_1');
});
}, 'getUserMedia() with unsupported mandatory constraint');
promise_test(async () => {
await navigator.mediaDevices.getUserMedia(
{audio: {'mandatory': { 'valid_but_unsupported_1': 0}}});
}, 'getUserMedia() with unsupported mandatory constraint does not throw');


</script>
Expand Down
21 changes: 8 additions & 13 deletions third_party/blink/web_tests/fast/mediastream/getusermedia.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,7 @@
finishJSTest();
}

function gotStreamInError(s) {
testFailed('Stream generated.');
finishJSTest();
}

function gotStream5(s) {
function gotStream6(s) {
stream = s;
testPassed('Stream generated.');
shouldBe('stream.getAudioTracks().length', '1');
Expand All @@ -31,13 +26,13 @@
finishJSTest();
}

function error1(e) {
errorArg = e;
testPassed('Error callback called.');
shouldBeEqualToString('errorArg.name', 'OverconstrainedError');
shouldBeEqualToString('errorArg.constraint', 'valid_but_unsupported_1');
function gotStream5(s) {
stream = s;
testPassed('Stream generated.');
shouldBe('stream.getAudioTracks().length', '1');
shouldBe('stream.getVideoTracks().length', '1');

shouldNotThrow("navigator.getUserMedia({audio:{mandatory:{'valid_and_supported_1':1}, optional:[{'valid_but_unsupported_1':0}]}, video:true}, gotStream5, error);");
shouldNotThrow("navigator.getUserMedia({audio:{mandatory:{'valid_and_supported_1':1}, optional:[{'valid_but_unsupported_1':0}]}, video:true}, gotStream6, error);");
}

function gotStream4(s) {
Expand All @@ -46,7 +41,7 @@
shouldBe('stream.getAudioTracks().length', '1');
shouldBe('stream.getVideoTracks().length', '1');

shouldNotThrow("navigator.getUserMedia({audio:{mandatory:{'valid_but_unsupported_1':0}, optional:[]}, video:true}, gotStreamInError, error1);");
shouldNotThrow("navigator.getUserMedia({audio:{mandatory:{'valid_but_unsupported_1':0}, optional:[]}, video:true}, gotStream5, error);");
}

function gotStream3(s) {
Expand Down
6 changes: 3 additions & 3 deletions third_party/blink/web_tests/fast/peerconnection/RTCPeerConnection.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@
shouldNotThrow("new RTCPeerConnection(null, {optional:[{valid_and_supported_1:0},{valid_and_supported_2:0}]});");
shouldNotThrow("new RTCPeerConnection(null, {optional:[{valid_but_unsupported_1:0},{valid_but_unsupported_2:0}]});");
shouldThrow("new RTCPeerConnection(null, {mandatory:{valid_and_supported_1:66}});");
shouldThrow("new RTCPeerConnection(null, {mandatory:{invalid:1}});");
shouldThrow("new RTCPeerConnection(null, {mandatory:{valid_but_unsupported_1:1}});");
shouldThrow("new RTCPeerConnection(null, {mandatory:{valid_but_unsupported_1:1, valid_and_supported_1:1}});");
shouldNotThrow("new RTCPeerConnection(null, {mandatory:{invalid:1}});");
shouldNotThrow("new RTCPeerConnection(null, {mandatory:{valid_but_unsupported_1:1}});");
shouldNotThrow("new RTCPeerConnection(null, {mandatory:{valid_but_unsupported_1:1, valid_and_supported_1:1}});");
shouldThrow("new RTCPeerConnection(null, {optional:{valid_and_supported_1:0}});");
shouldThrow("new RTCPeerConnection(null, {optional:[{valid_and_supported_1:0,valid_and_supported_2:0}]});");
// Optional constraints are ignored even if they are invalid.
Expand Down
11 changes: 5 additions & 6 deletions third_party/blink/web_tests/platform/generic/fast/mediastream/getusermedia-expected.txt
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
CONSOLE WARNING: Unknown constraint named valid_but_unsupported_1 rejected
Tests webkitGetUserMedia.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
Expand Down Expand Up @@ -30,11 +29,11 @@ PASS navigator.getUserMedia({audio:{mandatory:{}, optional:[]}, video:true}, got
PASS Stream generated.
PASS stream.getAudioTracks().length is 1
PASS stream.getVideoTracks().length is 1
PASS Error callback called.
PASS errorArg.name is "OverconstrainedError"
PASS errorArg.constraint is "valid_but_unsupported_1"
PASS navigator.getUserMedia({audio:{mandatory:{'valid_and_supported_1':1}, optional:[{'valid_but_unsupported_1':0}]}, video:true}, gotStream5, error); did not throw exception.
PASS navigator.getUserMedia({audio:{mandatory:{'valid_but_unsupported_1':0}, optional:[]}, video:true}, gotStreamInError, error1); did not throw exception.
PASS navigator.getUserMedia({audio:{mandatory:{'valid_but_unsupported_1':0}, optional:[]}, video:true}, gotStream5, error); did not throw exception.
PASS Stream generated.
PASS stream.getAudioTracks().length is 1
PASS stream.getVideoTracks().length is 1
PASS navigator.getUserMedia({audio:{mandatory:{'valid_and_supported_1':1}, optional:[{'valid_but_unsupported_1':0}]}, video:true}, gotStream6, error); did not throw exception.
PASS Stream generated.
PASS stream.getAudioTracks().length is 1
PASS stream.getVideoTracks().length is 1
Expand Down
9 changes: 3 additions & 6 deletions ...party/blink/web_tests/platform/generic/fast/peerconnection/RTCPeerConnection-expected.txt
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
CONSOLE WARNING: Unknown constraint named invalid rejected
CONSOLE WARNING: Unknown constraint named valid_but_unsupported_1 rejected
CONSOLE WARNING: Unknown constraint named valid_but_unsupported_1 rejected
Tests the RTCPeerConnection constructor.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
Expand Down Expand Up @@ -53,9 +50,9 @@ PASS new RTCPeerConnection(null, {optional:[{valid_and_supported_1:0}]}); did no
PASS new RTCPeerConnection(null, {optional:[{valid_and_supported_1:0},{valid_and_supported_2:0}]}); did not throw exception.
PASS new RTCPeerConnection(null, {optional:[{valid_but_unsupported_1:0},{valid_but_unsupported_2:0}]}); did not throw exception.
PASS new RTCPeerConnection(null, {mandatory:{valid_and_supported_1:66}}); threw exception NotSupportedError: Failed to construct 'RTCPeerConnection': Unsatisfiable constraint valid_and_supported_1.
PASS new RTCPeerConnection(null, {mandatory:{invalid:1}}); threw exception NotSupportedError: Failed to construct 'RTCPeerConnection': Unsatisfiable constraint invalid.
PASS new RTCPeerConnection(null, {mandatory:{valid_but_unsupported_1:1}}); threw exception NotSupportedError: Failed to construct 'RTCPeerConnection': Unsatisfiable constraint valid_but_unsupported_1.
PASS new RTCPeerConnection(null, {mandatory:{valid_but_unsupported_1:1, valid_and_supported_1:1}}); threw exception NotSupportedError: Failed to construct 'RTCPeerConnection': Unsatisfiable constraint valid_but_unsupported_1.
PASS new RTCPeerConnection(null, {mandatory:{invalid:1}}); did not throw exception.
PASS new RTCPeerConnection(null, {mandatory:{valid_but_unsupported_1:1}}); did not throw exception.
PASS new RTCPeerConnection(null, {mandatory:{valid_but_unsupported_1:1, valid_and_supported_1:1}}); did not throw exception.
PASS new RTCPeerConnection(null, {optional:{valid_and_supported_1:0}}); threw exception TypeError: Failed to construct 'RTCPeerConnection': Malformed constraints object..
PASS new RTCPeerConnection(null, {optional:[{valid_and_supported_1:0,valid_and_supported_2:0}]}); threw exception TypeError: Failed to construct 'RTCPeerConnection': Malformed constraints object..
PASS new RTCPeerConnection(null, {optional:[{invalid:0}]}); did not throw exception.
Expand Down

0 comments on commit a10f66e

Please sign in to comment.