Fix issue of localDescription and remoteDescription getter.

Source/modules/mediastream/RTCPeerConnection.cpp

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 223 matching lines @@
     RTCPeerConnection* peerConnection = new RTCPeerConnection(context, configuration, constraints, exceptionState);
     peerConnection->suspendIfNeeded();
     if (exceptionState.hadException())
         return 0;
 
     return peerConnection;
 }
 
 RTCPeerConnection::RTCPeerConnection(ExecutionContext* context, RTCConfiguration* configuration, WebMediaConstraints constraints, ExceptionState& exceptionState)
     : ActiveDOMObject(context)
+    , m_pendingLocalDescription(nullptr)
+    , m_pendingRemoteDescription(nullptr)
     , m_signalingState(SignalingStateStable)
     , m_iceGatheringState(ICEGatheringStateNew)
     , m_iceConnectionState(ICEConnectionStateNew)
     , m_dispatchScheduledEventRunner(this, &RTCPeerConnection::dispatchScheduledEvent)
     , m_stopped(false)
     , m_closed(false)
 {
     Document* document = toDocument(executionContext());
 
     // If we fail, set |m_closed| and |m_stopped| to true, to avoid hitting the assert in the destructor.
@@ skipping 14 matching lines @@
     }
 
     document->frame()->loader().client()->dispatchWillStartUsingPeerConnectionHandler(m_peerHandler.get());
 
     if (!m_peerHandler->initialize(configuration, constraints)) {
         m_closed = true;
         m_stopped = true;
         exceptionState.throwDOMException(NotSupportedError, "Failed to initialize native PeerConnection.");
         return;
     }
+    m_localDescription = RTCSessionDescription::create(m_peerHandler->localDescription());
+    m_remoteDescription = RTCSessionDescription::create(m_peerHandler->remoteDescription());
 }
 
 RTCPeerConnection::~RTCPeerConnection()
 {
     // This checks that close() or stop() is called before the destructor.
     // We are assuming that a wrapper is always created when RTCPeerConnection is created.
     ASSERT(m_closed || m_stopped);
 }
 
 void RTCPeerConnection::createOffer(RTCSessionDescriptionCallback* successCallback, RTCErrorCallback* errorCallback, const Dictionary& rtcOfferOptions, ExceptionState& exceptionState)
@@ skipping 40 matching lines @@
     if (throwExceptionIfSignalingStateClosed(m_signalingState, exceptionState))
         return;
 
     if (!sessionDescription) {
         exceptionState.throwDOMException(TypeMismatchError, ExceptionMessages::argumentNullOrIncorrectType(1, "RTCSessionDescription"));
         return;
     }
 
     RTCVoidRequest* request = RTCVoidRequestImpl::create(executionContext(), this, successCallback, errorCallback);
     m_peerHandler->setLocalDescription(request, sessionDescription->webSessionDescription());
+    m_pendingLocalDescription = sessionDescription;
 }
 
-RTCSessionDescription* RTCPeerConnection::localDescription(ExceptionState& exceptionState)
+RTCSessionDescription* RTCPeerConnection::localDescription()
 {
-    WebRTCSessionDescription webSessionDescription = m_peerHandler->localDescription();
-    if (webSessionDescription.isNull())
+    if (!m_peerHandler || m_peerHandler->localDescription().isNull())
         return nullptr;
 
-    return RTCSessionDescription::create(webSessionDescription);
+    updateLocalSessionDescriptionIfNeeded(false);
+    return m_localDescription;
 }
 
 void RTCPeerConnection::setRemoteDescription(RTCSessionDescription* sessionDescription, VoidCallback* successCallback, RTCErrorCallback* errorCallback, ExceptionState& exceptionState)
 {
     if (throwExceptionIfSignalingStateClosed(m_signalingState, exceptionState))
         return;
 
     if (!sessionDescription) {
         exceptionState.throwDOMException(TypeMismatchError, ExceptionMessages::argumentNullOrIncorrectType(1, "RTCSessionDescription"));
         return;
     }
 
     RTCVoidRequest* request = RTCVoidRequestImpl::create(executionContext(), this, successCallback, errorCallback);
     m_peerHandler->setRemoteDescription(request, sessionDescription->webSessionDescription());
+    m_pendingRemoteDescription = sessionDescription;
 }
 
-RTCSessionDescription* RTCPeerConnection::remoteDescription(ExceptionState& exceptionState)
+RTCSessionDescription* RTCPeerConnection::remoteDescription()
 {
-    WebRTCSessionDescription webSessionDescription = m_peerHandler->remoteDescription();
-    if (webSessionDescription.isNull())
+    if (!m_peerHandler || m_peerHandler->remoteDescription().isNull())
         return nullptr;
 
-    return RTCSessionDescription::create(webSessionDescription);
+    updateRemoteSessionDescriptionIfNeeded(false);
+    return m_remoteDescription;
 }
 
 void RTCPeerConnection::updateIce(const Dictionary& rtcConfiguration, const Dictionary& mediaConstraints, ExceptionState& exceptionState)
 {
     if (throwExceptionIfSignalingStateClosed(m_signalingState, exceptionState))
         return;
 
     RTCConfiguration* configuration = parseConfiguration(rtcConfiguration, exceptionState);
     if (exceptionState.hadException())
         return;
@@ skipping 240 matching lines @@
 }
 
 void RTCPeerConnection::close(ExceptionState& exceptionState)
 {
     if (throwExceptionIfSignalingStateClosed(m_signalingState, exceptionState))
         return;
 
     closeInternal();
 }
 
+void RTCPeerConnection::updateLocalSessionDescriptionIfNeeded(bool pending)
+{
+    if (!m_peerHandler)
+        return;
+    WebRTCSessionDescription webSessionDescription = m_peerHandler->localDescription();
+    if (webSessionDescription.isNull())
+        return;
+
+    if (pending) {

[Jens Widell, 2015/05/05 06:31:20]

This code path has essentially nothing in common with the rest of the function,
so I'd just make this bit its own (very simple) function, called
commitPendingLocalSessionDescription() or something like that.

[changbin, 2015/05/05 07:02:29]

Therefore, your proposal would be,
1. Move out 'if(pending)' logic into a separate function, named
commitPendingLocalSessionDescription().
2. Move updateLocalSessionDescriptionIfNeeded() codes back in localDescription()
getter.
3. In RTCVoidRequestImpl::requestSucceeded(), call
commitPendingLocalSessionDescription().

Is my understanding correct? 

[Jens Widell, 2015/05/05 07:17:21]

Correct.

Though 2 is somewhat optional. Keeping this helper might make sense simply for
the purpose of having a function name that documents what's going on. Or not. I
have no strong opinion. :-)

+        m_localDescription = m_pendingLocalDescription;
+        return;
+    }
+
+    if (!m_localDescription || m_localDescription->webSessionDescription().isNull()
+        || m_localDescription->webSessionDescription() != webSessionDescription) {
+        m_localDescription = RTCSessionDescription::create(webSessionDescription);
+    }
+}
+
+void RTCPeerConnection::updateRemoteSessionDescriptionIfNeeded(bool pending)
+{
+    if (!m_peerHandler)
+        return;
+    WebRTCSessionDescription webSessionDescription = m_peerHandler->remoteDescription();
+    if (webSessionDescription.isNull())
+        return;
+
+    if (pending) {
+        m_remoteDescription = m_pendingRemoteDescription;
+        return;
+    }
+
+    if (!m_remoteDescription || m_remoteDescription->webSessionDescription().isNull()
+        || m_remoteDescription->webSessionDescription() != webSessionDescription) {
+        m_remoteDescription = RTCSessionDescription::create(webSessionDescription);
+    }
+}
+
 void RTCPeerConnection::negotiationNeeded()
 {
     ASSERT(!m_closed);
     scheduleDispatchEvent(Event::create(EventTypeNames::negotiationneeded));
 }
 
 void RTCPeerConnection::didGenerateICECandidate(const WebRTCICECandidate& webCandidate)
 {
     ASSERT(!m_closed);
     ASSERT(executionContext()->isContextThread());
@@ skipping 177 matching lines @@
         dispatchEvent((*it).release());
 
     events.clear();
 }
 
 DEFINE_TRACE(RTCPeerConnection)
 {
     visitor->trace(m_localStreams);
     visitor->trace(m_remoteStreams);
     visitor->trace(m_dataChannels);
+    visitor->trace(m_localDescription);
+    visitor->trace(m_remoteDescription);
+    visitor->trace(m_pendingLocalDescription);
+    visitor->trace(m_pendingRemoteDescription);
 #if ENABLE(OILPAN)
     visitor->trace(m_scheduledEvents);
 #endif
     RefCountedGarbageCollectedEventTargetWithInlineData<RTCPeerConnection>::trace(visitor);
     ActiveDOMObject::trace(visitor);
 }
 
 } // namespace blink