Track SDP parse errors for SLD in chrome://webrtc-internals

Just the SLD-equivalent of:
https://codereview.chromium.org/2622453002

Also adds tests.

BUG=chromium:677550

Review-Url: https://codereview.chromium.org/2619343003
Cr-Commit-Position: refs/heads/master@{#442766}

content/renderer/media/rtc_peer_connection_handler.cc

@@ -1245,6 +1245,11 @@ void RTCPeerConnectionHandler::setLocalDescription(
   std::string sdp = description.sdp().utf8();
   std::string type = description.type().utf8();
 
+  if (peer_connection_tracker_) {
+    peer_connection_tracker_->TrackSetSessionDescription(
+        this, sdp, type, PeerConnectionTracker::SOURCE_LOCAL);
+  }
+
   webrtc::SdpParseError error;
   // Since CreateNativeSessionDescription uses the dependency factory, we need
   // to make this call on the current thread to be safe.
@@ -1257,14 +1262,14 @@ void RTCPeerConnectionHandler::setLocalDescription(
     reason_str.append(error.description);
     LOG(ERROR) << reason_str;
     request.requestFailed(blink::WebString::fromUTF8(reason_str));
+    if (peer_connection_tracker_) {
+      peer_connection_tracker_->TrackSessionDescriptionCallback(
+          this, PeerConnectionTracker::ACTION_SET_LOCAL_DESCRIPTION,
+          "OnFailure", reason_str);
+    }
     return;
   }
 
-  if (peer_connection_tracker_) {
-    peer_connection_tracker_->TrackSetSessionDescription(
-        this, sdp, type, PeerConnectionTracker::SOURCE_LOCAL);
-  }
-
   if (!first_local_description_ && IsOfferOrAnswer(native_desc)) {
     first_local_description_.reset(new FirstSessionDescription(native_desc));
     if (first_remote_description_) {

content/renderer/media/rtc_peer_connection_handler_unittest.cc

@@ -184,6 +184,11 @@ class MockPeerConnectionTracker : public PeerConnectionTracker {
       TrackIceGatheringStateChange,
       void(RTCPeerConnectionHandler* pc_handler,
            WebRTCPeerConnectionHandlerClient::ICEGatheringState state));
+  MOCK_METHOD4(TrackSessionDescriptionCallback,
+               void(RTCPeerConnectionHandler* pc_handler,
+                    Action action,
+                    const std::string& type,
+                    const std::string& value));
   MOCK_METHOD1(TrackOnRenegotiationNeeded,
                void(RTCPeerConnectionHandler* pc_handler));
   MOCK_METHOD2(TrackCreateDTMFSender,
@@ -479,6 +484,36 @@ TEST_F(RTCPeerConnectionHandlerTest, setLocalDescription) {
   EXPECT_EQ(kDummySdpType, mock_peer_connection_->local_description()->type());
   mock_peer_connection_->local_description()->ToString(&sdp_string);
   EXPECT_EQ(kDummySdp, sdp_string);
+
+  // TODO(deadbeef): Also mock the "success" callback from the PeerConnection
+  // and ensure that the sucessful result is tracked by PeerConnectionTracker.
+}
+
+// Test that setLocalDescription with invalid SDP will result in a failure, and
+// is tracked as a failure with PeerConnectionTracker.
+TEST_F(RTCPeerConnectionHandlerTest, setLocalDescriptionParseError) {
+  blink::WebRTCVoidRequest request;
+  blink::WebRTCSessionDescription description;
+  description.initialize(kDummySdpType, kDummySdp);
+  testing::InSequence sequence;
+  // Expect two "Track" calls, one for the start of the attempt and one for the
+  // failure.
+  EXPECT_CALL(
+      *mock_tracker_.get(),
+      TrackSetSessionDescription(pc_handler_.get(), kDummySdp, kDummySdpType,
+                                 PeerConnectionTracker::SOURCE_LOCAL));
+  EXPECT_CALL(
+      *mock_tracker_.get(),
+      TrackSessionDescriptionCallback(
+          pc_handler_.get(),
+          PeerConnectionTracker::ACTION_SET_LOCAL_DESCRIPTION, "OnFailure", _));
+
+  // Used to simulate a parse failure.
+  mock_dependency_factory_->SetFailToCreateSessionDescription(true);
+  pc_handler_->setLocalDescription(request, description);
+  base::RunLoop().RunUntilIdle();
+  // A description that failed to be applied shouldn't be stored.
+  EXPECT_TRUE(pc_handler_->localDescription().sdp().isEmpty());
 }
 
 TEST_F(RTCPeerConnectionHandlerTest, setRemoteDescription) {
@@ -505,6 +540,36 @@ TEST_F(RTCPeerConnectionHandlerTest, setRemoteDescription) {
   EXPECT_EQ(kDummySdpType, mock_peer_connection_->remote_description()->type());
   mock_peer_connection_->remote_description()->ToString(&sdp_string);
   EXPECT_EQ(kDummySdp, sdp_string);
+
+  // TODO(deadbeef): Also mock the "success" callback from the PeerConnection
+  // and ensure that the sucessful result is tracked by PeerConnectionTracker.
+}
+
+// Test that setRemoteDescription with invalid SDP will result in a failure, and
+// is tracked as a failure with PeerConnectionTracker.
+TEST_F(RTCPeerConnectionHandlerTest, setRemoteDescriptionParseError) {
+  blink::WebRTCVoidRequest request;
+  blink::WebRTCSessionDescription description;
+  description.initialize(kDummySdpType, kDummySdp);
+  testing::InSequence sequence;
+  // Expect two "Track" calls, one for the start of the attempt and one for the
+  // failure.
+  EXPECT_CALL(
+      *mock_tracker_.get(),
+      TrackSetSessionDescription(pc_handler_.get(), kDummySdp, kDummySdpType,
+                                 PeerConnectionTracker::SOURCE_REMOTE));
+  EXPECT_CALL(*mock_tracker_.get(),
+              TrackSessionDescriptionCallback(
+                  pc_handler_.get(),
+                  PeerConnectionTracker::ACTION_SET_REMOTE_DESCRIPTION,
+                  "OnFailure", _));
+
+  // Used to simulate a parse failure.
+  mock_dependency_factory_->SetFailToCreateSessionDescription(true);
+  pc_handler_->setRemoteDescription(request, description);
+  base::RunLoop().RunUntilIdle();
+  // A description that failed to be applied shouldn't be stored.
+  EXPECT_TRUE(pc_handler_->remoteDescription().sdp().isEmpty());
 }
 
 TEST_F(RTCPeerConnectionHandlerTest, setConfiguration) {

content/renderer/media/webrtc/mock_peer_connection_dependency_factory.cc

@@ -360,6 +360,8 @@ MockPeerConnectionDependencyFactory::CreateSessionDescription(
     const std::string& type,
     const std::string& sdp,
     webrtc::SdpParseError* error) {
+  if (fail_to_create_session_description_)
+    return nullptr;
   return new MockSessionDescription(type, sdp);
 }
 
@@ -376,4 +378,9 @@ MockPeerConnectionDependencyFactory::GetWebRtcSignalingThread() const {
   return signaling_thread_.task_runner();
 }
 
+void MockPeerConnectionDependencyFactory::SetFailToCreateSessionDescription(
+    bool fail) {
+  fail_to_create_session_description_ = fail;
+}
+
 }  // namespace content

content/renderer/media/webrtc/mock_peer_connection_dependency_factory.h

@@ -145,8 +145,14 @@ class MockPeerConnectionDependencyFactory
   scoped_refptr<base::SingleThreadTaskRunner> GetWebRtcSignalingThread()
       const override;
 
+  // If |fail| is true, subsequent calls to CreateSessionDescription will
+  // return nullptr. This can be used to fake a blob of SDP that fails to be
+  // parsed.
+  void SetFailToCreateSessionDescription(bool fail);
+
  private:
   base::Thread signaling_thread_;
+  bool fail_to_create_session_description_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(MockPeerConnectionDependencyFactory);
 };

third_party/WebKit/Source/platform/exported/WebRTCVoidRequest.cpp

@@ -47,13 +47,13 @@ void WebRTCVoidRequest::reset() {
 }
 
 void WebRTCVoidRequest::requestSucceeded() const {
-  ASSERT(m_private.get());
-  m_private->requestSucceeded();
+  if (m_private.get())
+    m_private->requestSucceeded();
 }
 
 void WebRTCVoidRequest::requestFailed(const WebString& error) const {
-  ASSERT(m_private.get());
-  m_private->requestFailed(error);
+  if (m_private.get())
+    m_private->requestFailed(error);
 }
 
 }  // namespace blink