Changeset 207818 in webkit

Timestamp:

Oct 25, 2016 8:02:39 AM (8 years ago)

Author:

commit-queue@webkit.org

Message:

MediaEndpointConfiguration does not need to be refcounted
​https://bugs.webkit.org/show_bug.cgi?id=163888

Patch by Youenn Fablet <​youenn@apple.com> on 2016-10-25
Reviewed by Eric Carlson.

No change of behavior.

Updating MediaEndpoint to take a MediaEndpointConfiguration&& instead of a RefPtr<MediaEndpointConfiguration>&&.
This allows ensuring that the configuration is not null and removes the need to ref-count it.

Removing ref-counting from MediaEndpointConfiguration and its related IceServer that is now a struct.
Updated MediaEndpointOwr and mock implementations accordingly.

• Modules/mediastream/MediaEndpointPeerConnection.cpp:

(WebCore::MediaEndpointPeerConnection::setConfiguration):

• platform/mediastream/MediaEndpoint.cpp:
• platform/mediastream/MediaEndpoint.h:
• platform/mediastream/MediaEndpointConfiguration.cpp:

(WebCore::MediaEndpointConfiguration::MediaEndpointConfiguration):

• platform/mediastream/MediaEndpointConfiguration.h:

(WebCore::MediaEndpointConfiguration::iceServers):

• platform/mediastream/openwebrtc/MediaEndpointOwr.cpp:

(WebCore::MediaEndpointOwr::setConfiguration):
(WebCore::MediaEndpointOwr::ensureTransportAgentAndTransceivers):

• platform/mediastream/openwebrtc/MediaEndpointOwr.h:
• platform/mock/MockMediaEndpoint.cpp:
• platform/mock/MockMediaEndpoint.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• Modules/mediastream/MediaEndpointPeerConnection.cpp (modified) (1 diff)
• platform/mediastream/MediaEndpoint.cpp (modified) (1 diff)
• platform/mediastream/MediaEndpoint.h (modified) (1 diff)
• platform/mediastream/MediaEndpointConfiguration.cpp (modified) (1 diff)
• platform/mediastream/MediaEndpointConfiguration.h (modified) (4 diffs)
• platform/mediastream/openwebrtc/MediaEndpointOwr.cpp (modified) (3 diffs)
• platform/mediastream/openwebrtc/MediaEndpointOwr.h (modified) (2 diffs)
• platform/mock/MockMediaEndpoint.cpp (modified) (1 diff)
• platform/mock/MockMediaEndpoint.h (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-10-25  Youenn Fablet  <youenn@apple.com>
+
+        MediaEndpointConfiguration does not need to be refcounted
+        https://bugs.webkit.org/show_bug.cgi?id=163888
+
+        Reviewed by Eric Carlson.
+
+        No change of behavior.
+
+        Updating MediaEndpoint to take a MediaEndpointConfiguration&& instead of a RefPtr<MediaEndpointConfiguration>&&.
+        This allows ensuring that the configuration is not null and removes the need to ref-count it.
+
+        Removing ref-counting from MediaEndpointConfiguration and its related IceServer that is now a struct.
+        Updated MediaEndpointOwr and mock implementations accordingly.
+
+        * Modules/mediastream/MediaEndpointPeerConnection.cpp:
+        (WebCore::MediaEndpointPeerConnection::setConfiguration):
+        * platform/mediastream/MediaEndpoint.cpp:
+        * platform/mediastream/MediaEndpoint.h:
+        * platform/mediastream/MediaEndpointConfiguration.cpp:
+        (WebCore::MediaEndpointConfiguration::MediaEndpointConfiguration):
+        * platform/mediastream/MediaEndpointConfiguration.h:
+        (WebCore::MediaEndpointConfiguration::iceServers):
+        * platform/mediastream/openwebrtc/MediaEndpointOwr.cpp:
+        (WebCore::MediaEndpointOwr::setConfiguration):
+        (WebCore::MediaEndpointOwr::ensureTransportAgentAndTransceivers):
+        * platform/mediastream/openwebrtc/MediaEndpointOwr.h:
+        * platform/mock/MockMediaEndpoint.cpp:
+        * platform/mock/MockMediaEndpoint.h:
+
 2016-10-25  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.cpp

@@ -615 +615 @@
 void MediaEndpointPeerConnection::setConfiguration(RTCConfiguration& configuration)
 {
-    Vector<RefPtr<IceServerInfo>> iceServers;
-    for (auto& server : configuration.iceServers())
-        iceServers.append(IceServerInfo::create(server->urls(), server->credential(), server->username()));
-
-    m_mediaEndpoint->setConfiguration(MediaEndpointConfiguration::create(iceServers, configuration.iceTransportPolicy(), configuration.bundlePolicy()));
+    Vector<MediaEndpointConfiguration::IceServerInfo> iceServers;
+    if (configuration.iceServers().size()) {
+        iceServers.reserveInitialCapacity(configuration.iceServers().size());
+        for (auto& server : configuration.iceServers()) {
+            Vector<URL> urls;
+            urls.reserveInitialCapacity(server->urls().size());
+            for (auto& url : server->urls())
+                urls.uncheckedAppend(URL(URL(), url));
+            iceServers.uncheckedAppend(MediaEndpointConfiguration::IceServerInfo { WTFMove(urls), server->credential(), server->username() });
+        }
+    }
+
+    m_mediaEndpoint->setConfiguration({ WTFMove(iceServers), configuration.iceTransportPolicy(), configuration.bundlePolicy() });
 }
 

trunk/Source/WebCore/platform/mediastream/MediaEndpoint.cpp

@@ -58 +58 @@
     EmptyMediaEndpoint(MediaEndpointClient&) { }
 
-    void setConfiguration(RefPtr<MediaEndpointConfiguration>&&) override { }
+    void setConfiguration(MediaEndpointConfiguration&&) override { }
 
     void generateDtlsInfo() override { }

trunk/Source/WebCore/platform/mediastream/MediaEndpoint.h

@@ -64 +64 @@
     using IceTransportState = PeerConnectionStates::IceTransportState;
 
-    virtual void setConfiguration(RefPtr<MediaEndpointConfiguration>&&) = 0;
+    virtual void setConfiguration(MediaEndpointConfiguration&&) = 0;
 
     virtual void generateDtlsInfo() = 0;

trunk/Source/WebCore/platform/mediastream/MediaEndpointConfiguration.cpp

@@ -36 +36 @@
 namespace WebCore {
 
-IceServerInfo::IceServerInfo(const Vector<String>& urls, const String& credential, const String& username)
-    : m_credential(credential)
-    , m_username(username)
+MediaEndpointConfiguration::MediaEndpointConfiguration(Vector<IceServerInfo>&& iceServers, IceTransportPolicy iceTransportPolicy, BundlePolicy bundlePolicy)
+    : iceServers(WTFMove(iceServers))
+    , iceTransportPolicy(iceTransportPolicy)
+    , bundlePolicy(bundlePolicy)
 {
-    m_urls.reserveCapacity(urls.size());
-    for (auto& url : urls)
-        m_urls.append(URL(URL(), url));
 }
 
-MediaEndpointConfiguration::MediaEndpointConfiguration(Vector<RefPtr<IceServerInfo>>& iceServers, IceTransportPolicy iceTransportPolicy, BundlePolicy bundlePolicy)
-    : m_iceServers(iceServers)
-    , m_iceTransportPolicy(iceTransportPolicy)
-    , m_bundlePolicy(bundlePolicy)
+MediaEndpointConfiguration::IceServerInfo::IceServerInfo(Vector<URL>&& urls, const String& credential, const String& username)
+    : urls(WTFMove(urls))
+    , credential(credential)
+    , username(username)
 {
 }

trunk/Source/WebCore/platform/mediastream/MediaEndpointConfiguration.h

@@ -29 +29 @@
  */
 
-#ifndef MediaEndpointConfiguration_h
-#define MediaEndpointConfiguration_h
+#pragma once
 
 #if ENABLE(WEB_RTC)
@@ -36 +35 @@
 #include "PeerConnectionStates.h"
 #include "URL.h"
-#include <wtf/RefCounted.h>
-#include <wtf/RefPtr.h>
 #include <wtf/Vector.h>
 #include <wtf/text/WTFString.h>
@@ -43 +40 @@
 namespace WebCore {
 
-class IceServerInfo : public RefCounted<IceServerInfo> {
-public:
-    static RefPtr<IceServerInfo> create(const Vector<String>& urls, const String& credential, const String& username)
-    {
-        return adoptRef(new IceServerInfo(urls, credential, username));
-    }
-    virtual ~IceServerInfo() { }
+struct MediaEndpointConfiguration {
+    // FIXME: We might be able to remove these constructors once all compilers can handle without it (see https://bugs.webkit.org/show_bug.cgi?id=163255#c15)
+    struct IceServerInfo {
+        Vector<URL> urls;
+        String credential;
+        String username;
 
-    const Vector<URL>& urls() const { return m_urls; }
-    const String& credential() const { return m_credential; }
-    const String& username() const { return m_username; }
+        IceServerInfo(Vector<URL>&&, const String&, const String&);
+    };
 
-private:
-    IceServerInfo(const Vector<String>& urls, const String& credential, const String& username);
-
-    Vector<URL> m_urls;
-    String m_credential;
-    String m_username;
-};
-
-class MediaEndpointConfiguration : public RefCounted<MediaEndpointConfiguration> {
-public:
     using IceTransportPolicy = PeerConnectionStates::IceTransportPolicy;
     using BundlePolicy = PeerConnectionStates::BundlePolicy;
 
-    static RefPtr<MediaEndpointConfiguration> create(Vector<RefPtr<IceServerInfo>>& iceServers, IceTransportPolicy iceTransportPolicy, BundlePolicy bundlePolicy)
-    {
-        return adoptRef(new MediaEndpointConfiguration(iceServers, iceTransportPolicy, bundlePolicy));
-    }
+    MediaEndpointConfiguration(Vector<IceServerInfo>&&, IceTransportPolicy, BundlePolicy);
 
-    const Vector<RefPtr<IceServerInfo>>& iceServers() const { return m_iceServers; }
-    IceTransportPolicy iceTransportPolicy() const { return m_iceTransportPolicy; }
-    BundlePolicy bundlePolicy() const { return m_bundlePolicy; }
-
-private:
-    MediaEndpointConfiguration(Vector<RefPtr<IceServerInfo>>&, IceTransportPolicy, BundlePolicy);
-
-    Vector<RefPtr<IceServerInfo>> m_iceServers;
-    IceTransportPolicy m_iceTransportPolicy;
-    BundlePolicy m_bundlePolicy;
+    Vector<IceServerInfo> iceServers;
+    IceTransportPolicy iceTransportPolicy;
+    BundlePolicy bundlePolicy;
 };
 
@@ -88 +63 @@
 
 #endif // ENABLE(WEB_RTC)
-
-#endif // MediaEndpointConfiguration_h

trunk/Source/WebCore/platform/mediastream/openwebrtc/MediaEndpointOwr.cpp

@@ -90 +90 @@
 }
 
-void MediaEndpointOwr::setConfiguration(RefPtr<MediaEndpointConfiguration>&& configuration)
-{
-    m_configuration = configuration;
+void MediaEndpointOwr::setConfiguration(MediaEndpointConfiguration&& configuration)
+{
+    m_configuration = WTFMove(configuration);
 }
 
@@ -553 +553 @@
         m_transportAgent = owr_transport_agent_new(false);
 
-        for (auto& server : m_configuration->iceServers()) {
-            for (auto& webkitUrl : server->urls()) {
+        ASSERT(m_configuration);
+        for (auto& server : m_configuration->iceServers) {
+            for (auto& webkitUrl : server.urls) {
                 HelperServerUrl url;
                 // WebKit's URL class can't handle ICE helper server urls properly
@@ -571 +572 @@
                     owr_transport_agent_add_helper_server(m_transportAgent, serverType,
                         url.host.ascii().data(), port,
-                        server->username().ascii().data(), server->credential().ascii().data());
+                        server.username.ascii().data(), server.credential.ascii().data());
                 } else
                     ASSERT_NOT_REACHED();

trunk/Source/WebCore/platform/mediastream/openwebrtc/MediaEndpointOwr.h

@@ -83 +83 @@
     ~MediaEndpointOwr();
 
-    void setConfiguration(RefPtr<MediaEndpointConfiguration>&&) override;
+    void setConfiguration(MediaEndpointConfiguration&&) override;
 
     void generateDtlsInfo() override;
@@ -126 +126 @@
     void internalAddRemoteCandidate(OwrSession*, IceCandidate&, const String& ufrag, const String& password);
 
-    RefPtr<MediaEndpointConfiguration> m_configuration;
+    Optional<MediaEndpointConfiguration> m_configuration;
     GRegex* m_helperServerRegEx;
 

trunk/Source/WebCore/platform/mock/MockMediaEndpoint.cpp

@@ -64 +64 @@
 }
 
-void MockMediaEndpoint::setConfiguration(RefPtr<MediaEndpointConfiguration>&& configuration)
-{
-    UNUSED_PARAM(configuration);
-}
-
 void MockMediaEndpoint::generateDtlsInfo()
 {

trunk/Source/WebCore/platform/mock/MockMediaEndpoint.h

@@ -39 +39 @@
 namespace WebCore {
 
-class MockMediaEndpoint : public MediaEndpoint {
+class MockMediaEndpoint final : public MediaEndpoint {
 public:
     WEBCORE_EXPORT static std::unique_ptr<MediaEndpoint> create(MediaEndpointClient&);
@@ -46 +46 @@
     ~MockMediaEndpoint();
 
-    void setConfiguration(RefPtr<MediaEndpointConfiguration>&&) override;
+    void setConfiguration(MediaEndpointConfiguration&&) final { };
 
-    void generateDtlsInfo() override;
-    MediaPayloadVector getDefaultAudioPayloads() override;
-    MediaPayloadVector getDefaultVideoPayloads() override;
-    MediaPayloadVector filterPayloads(const MediaPayloadVector& remotePayloads, const MediaPayloadVector& defaultPayloads) override;
+    void generateDtlsInfo() final;
+    MediaPayloadVector getDefaultAudioPayloads() final;
+    MediaPayloadVector getDefaultVideoPayloads() final;
+    MediaPayloadVector filterPayloads(const MediaPayloadVector& remotePayloads, const MediaPayloadVector& defaultPayloads) final;
 
-    UpdateResult updateReceiveConfiguration(MediaEndpointSessionConfiguration*, bool isInitiator) override;
-    UpdateResult updateSendConfiguration(MediaEndpointSessionConfiguration*, const RealtimeMediaSourceMap&, bool isInitiator) override;
+    UpdateResult updateReceiveConfiguration(MediaEndpointSessionConfiguration*, bool isInitiator) final;
+    UpdateResult updateSendConfiguration(MediaEndpointSessionConfiguration*, const RealtimeMediaSourceMap&, bool isInitiator) final;
 
-    void addRemoteCandidate(IceCandidate&, const String& mid, const String& ufrag, const String& password) override;
+    void addRemoteCandidate(IceCandidate&, const String& mid, const String& ufrag, const String& password) final;
 
-    Ref<RealtimeMediaSource> createMutedRemoteSource(const String& mid, RealtimeMediaSource::Type) override;
-    void replaceSendSource(RealtimeMediaSource&, const String& mid) override;
-    void replaceMutedRemoteSourceMid(const String& oldMid, const String& newMid) override;
+    Ref<RealtimeMediaSource> createMutedRemoteSource(const String& mid, RealtimeMediaSource::Type) final;
+    void replaceSendSource(RealtimeMediaSource&, const String& mid) final;
+    void replaceMutedRemoteSourceMid(const String& oldMid, const String& newMid) final;
 
-    void stop() override;
+    void stop() final;
 
-    void emulatePlatformEvent(const String& action) override;
+    void emulatePlatformEvent(const String& action) final;
 
 private: