Invalidate LastReturnedParameters slot after setParameters()

The standard got updated to invalidate the internal slot after
a setParameters() call to enforce having a single matching
getParameters() call.

Bug: 803494
Change-Id: Ia6b1483b908c3ef29c31fdcf2fb8014735cea322
Reviewed-on: https://chromium-review.googlesource.com/1047426
Commit-Queue: Florent Castelli <orphis@chromium.org>
Reviewed-by: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556853}

third_party/WebKit/LayoutTests/fast/peerconnection/RTCRtpSender-setParameters-expected.txt

 This is a testharness.js-based test.
 PASS setParameters() changes getParameters() returned values
 FAIL setParameters() with large maxBitrate changes getParameters() returned values assert_equals: expected 4294967295 but got 2147483647
-PASS setParameters() with multiple calls
+PASS setParameters() with multiple calls after single getParameters()
 PASS setParameters() fails without getParameters()
 PASS video setParameters() check for transactionId modification
 FAIL video setParameters() check for transactionId removed assert_unreached: Should have rejected: undefined Reached unreachable code

third_party/WebKit/LayoutTests/fast/peerconnection/RTCRtpSender-setParameters.html

@@ -43,7 +43,7 @@ promise_test(async () => {
   assert_equals(videoParameters.encodings[0].maxBitrate, 0xFFFFFFFF);
 }, 'setParameters() with large maxBitrate changes getParameters() returned values');
 
-promise_test(async () => {
+promise_test(async (t) => {
   let pc1 = new RTCPeerConnection();
   let pc2 = new RTCPeerConnection();
 
@@ -60,14 +60,13 @@ promise_test(async () => {
   videoParameters.encodings[0].active = true;
   videoParameters.encodings[0].priority = 'low';
   videoParameters.encodings[0].maxBitrate = 1337001;
-  await videoSender.setParameters(videoParameters);
-
-  videoParameters = videoSender.getParameters();
-  assert_equals(videoParameters.encodings[0].active, true);
-  assert_equals(videoParameters.encodings[0].priority, 'low');
-  assert_equals(videoParameters.encodings[0].maxBitrate, 1337001);
-
-}, 'setParameters() with multiple calls');
+  return promise_rejects(
+    t,
+    new DOMException(
+      "getParameters() needs to be called before setParameters().",
+      "InvalidStateError"),
+    videoSender.setParameters(videoParameters));
+}, 'setParameters() with multiple calls after single getParameters()');
 
 promise_test(async (t) => {
   let pc1 = new RTCPeerConnection();

third_party/blink/renderer/modules/peerconnection/rtc_rtp_sender.cc

@@ -49,6 +49,30 @@ class ReplaceTrackRequest : public RTCVoidRequest {
   Member<ScriptPromiseResolver> resolver_;
 };
 
+class SetParametersRequest : public RTCVoidRequestScriptPromiseResolverImpl {
+ public:
+  SetParametersRequest(ScriptPromiseResolver* resolver, RTCRtpSender* sender)
+      : RTCVoidRequestScriptPromiseResolverImpl(resolver), sender_(sender) {}
+
+  void RequestSucceeded() override {
+    sender_->ClearLastReturnedParameters();
+    RTCVoidRequestScriptPromiseResolverImpl::RequestSucceeded();
+  }
+
+  void RequestFailed(const webrtc::RTCError& error) override {
+    sender_->ClearLastReturnedParameters();
+    RTCVoidRequestScriptPromiseResolverImpl::RequestFailed(error);
+  }
+
+  void Trace(blink::Visitor* visitor) override {
+    visitor->Trace(sender_);
+    RTCVoidRequestScriptPromiseResolverImpl::Trace(visitor);
+  }
+
+ private:
+  Member<RTCRtpSender> sender_;
+};
+
 bool HasInvalidModification(const RTCRtpParameters& parameters,
                             const RTCRtpParameters& new_parameters) {
   if (parameters.hasTransactionId() != new_parameters.hasTransactionId() ||
@@ -296,10 +320,10 @@ ScriptPromise RTCRtpSender::setParameters(ScriptState* script_state,
         "getParameters() needs to be called before setParameters()."));
     return promise;
   }
-  // The specification mentions that some fields in the dictionnary should not
+  // The specification mentions that some fields in the dictionary should not
   // be modified. Some of those checks are done in the lower WebRTC layer, but
   // there is no perfect 1-1 mapping between the Javascript layer and native.
-  // So we save the last returned dictionnary and enforce the check at this
+  // So we save the last returned dictionary and enforce the check at this
   // level instead.
   if (HasInvalidModification(last_returned_parameters_.value(), parameters)) {
     resolver->Reject(
@@ -321,12 +345,16 @@ ScriptPromise RTCRtpSender::setParameters(ScriptState* script_state,
     degradation_preference = blink::WebRTCDegradationPreference::Balanced;
   }
 
-  auto* request = RTCVoidRequestScriptPromiseResolverImpl::Create(resolver);
+  auto* request = new SetParametersRequest(resolver, this);
   sender_->SetParameters(std::move(encodings), degradation_preference.value(),
                          request);
   return promise;
 }
 
+void RTCRtpSender::ClearLastReturnedParameters() {
+  last_returned_parameters_.reset();
+}
+
 ScriptPromise RTCRtpSender::getStats(ScriptState* script_state) {
   ScriptPromiseResolver* resolver = ScriptPromiseResolver::Create(script_state);
   ScriptPromise promise = resolver->Promise();

third_party/blink/renderer/modules/peerconnection/rtc_rtp_sender.h

@@ -45,6 +45,7 @@ class RTCRtpSender final : public ScriptWrappable {
   // Sets the track. This must be called when the |WebRTCRtpSender| has its
   // track updated, and the |track| must match the |WebRTCRtpSender::Track|.
   void SetTrack(MediaStreamTrack*);
+  void ClearLastReturnedParameters();
   MediaStreamVector streams() const;
 
   void Trace(blink::Visitor*) override;

third_party/blink/renderer/modules/peerconnection/rtc_void_request_script_promise_resolver_impl.h

@@ -12,7 +12,7 @@ namespace blink {
 
 class ScriptPromiseResolver;
 
-class RTCVoidRequestScriptPromiseResolverImpl final : public RTCVoidRequest {
+class RTCVoidRequestScriptPromiseResolverImpl : public RTCVoidRequest {
  public:
   static RTCVoidRequestScriptPromiseResolverImpl* Create(
       ScriptPromiseResolver*);
@@ -24,7 +24,7 @@ class RTCVoidRequestScriptPromiseResolverImpl final : public RTCVoidRequest {
 
   void Trace(blink::Visitor*) override;
 
- private:
+ protected:
   RTCVoidRequestScriptPromiseResolverImpl(ScriptPromiseResolver*);
 
   Member<ScriptPromiseResolver> resolver_;