Onion soup content/renderer/media/webrtc/stun_field_trial.h

This is phase 4 on the design document [1].

[1] https://docs.google.com/document/d/1AJKVA5U4nDkyDB9p4ROrggWXadCxyy-grKaE9KS5vOU/

BUG=787254
R=guidou@chromium.org, haraken@chromium.org

Change-Id: I171f2732e318eda664a292132cc8597ed5c42207
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1792859
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695158}

content/renderer/BUILD.gn

@@ -216,8 +216,6 @@ target(link_target_type, "renderer") {
     "media/webrtc/rtc_rtp_sender.h",
     "media/webrtc/rtc_rtp_transceiver.cc",
     "media/webrtc/rtc_rtp_transceiver.h",
-    "media/webrtc/stun_field_trial.cc",
-    "media/webrtc/stun_field_trial.h",
     "media/webrtc/transceiver_state_surfacer.cc",
     "media/webrtc/transceiver_state_surfacer.h",
     "media/webrtc/webrtc_media_stream_track_adapter.cc",

content/renderer/media/webrtc/peer_connection_dependency_factory.cc

@@ -28,7 +28,6 @@
 #include "content/public/common/webrtc_ip_handling_policy.h"
 #include "content/public/renderer/content_renderer_client.h"
 #include "content/renderer/media/webrtc/rtc_peer_connection_handler.h"
-#include "content/renderer/media/webrtc/stun_field_trial.h"
 #include "content/renderer/p2p/ipc_socket_factory.h"
 #include "content/renderer/p2p/mdns_responder_adapter.h"
 #include "content/renderer/p2p/port_allocator.h"
@@ -46,6 +45,7 @@
 #include "third_party/blink/public/platform/modules/p2p/filtering_network_manager.h"
 #include "third_party/blink/public/platform/modules/p2p/ipc_network_manager.h"
 #include "third_party/blink/public/platform/modules/peerconnection/audio_codec_factory.h"
+#include "third_party/blink/public/platform/modules/peerconnection/stun_field_trial.h"
 #include "third_party/blink/public/platform/modules/peerconnection/video_codec_factory.h"
 #include "third_party/blink/public/platform/modules/webrtc/webrtc_logging.h"
 #include "third_party/blink/public/platform/web_media_constraints.h"
@@ -565,14 +565,14 @@ void PeerConnectionDependencyFactory::TryScheduleStunProbeTrial() {
       base::BindOnce(
           &PeerConnectionDependencyFactory::StartStunProbeTrialOnWorkerThread,
           base::Unretained(this), params),
-      base::TimeDelta::FromMilliseconds(kExperimentStartDelayMs));
+      base::TimeDelta::FromMilliseconds(blink::kExperimentStartDelayMs));
 }
 
 void PeerConnectionDependencyFactory::StartStunProbeTrialOnWorkerThread(
     const std::string& params) {
   DCHECK(network_manager_);
   DCHECK(chrome_worker_thread_.task_runner()->BelongsToCurrentThread());
-  stun_trial_.reset(new StunProberTrial(network_manager_.get(), params,
+  stun_trial_.reset(new blink::StunProberTrial(network_manager_.get(), params,
                                                socket_factory_.get()));
 }
 

content/renderer/media/webrtc/peer_connection_dependency_factory.h

@@ -14,9 +14,9 @@
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread.h"
 #include "content/common/content_export.h"
-#include "content/renderer/media/webrtc/stun_field_trial.h"
 #include "content/renderer/p2p/socket_dispatcher.h"
 #include "ipc/ipc_platform_file.h"
+#include "third_party/blink/public/platform/modules/peerconnection/stun_field_trial.h"
 #include "third_party/webrtc/api/peer_connection_interface.h"
 #include "third_party/webrtc/p2p/stunprober/stun_prober.h"
 
@@ -166,7 +166,7 @@ class CONTENT_EXPORT PeerConnectionDependencyFactory
   scoped_refptr<P2PSocketDispatcher> p2p_socket_dispatcher_;
   scoped_refptr<blink::WebRtcAudioDeviceImpl> audio_device_;
 
-  std::unique_ptr<StunProberTrial> stun_trial_;
+  std::unique_ptr<blink::StunProberTrial> stun_trial_;
 
   // PeerConnection threads. signaling_thread_ is created from the
   // "current" chrome thread.

content/test/BUILD.gn

@@ -1950,7 +1950,6 @@ test("content_unittests") {
     "../renderer/media/webrtc/rtc_rtp_receiver_unittest.cc",
     "../renderer/media/webrtc/rtc_rtp_sender_unittest.cc",
     "../renderer/media/webrtc/rtc_rtp_transceiver_unittest.cc",
-    "../renderer/media/webrtc/stun_field_trial_unittest.cc",
     "../renderer/media/webrtc/task_queue_factory_unittest.cc",
     "../renderer/media/webrtc/transceiver_state_surfacer_unittest.cc",
     "../renderer/media/webrtc/webrtc_audio_renderer_unittest.cc",

third_party/blink/public/BUILD.gn

@@ -162,6 +162,7 @@ source_set("blink_headers") {
     "platform/modules/peerconnection/rtc_event_log_output_sink_proxy_util.h",
     "platform/modules/peerconnection/rtc_video_decoder_factory_util.h",
     "platform/modules/peerconnection/rtc_video_encoder_factory_util.h",
+    "platform/modules/peerconnection/stun_field_trial.h",
     "platform/modules/peerconnection/two_keys_adapter_map.h",
     "platform/modules/peerconnection/video_codec_factory.h",
     "platform/modules/peerconnection/webrtc_audio_sink.h",
@@ -561,6 +562,7 @@ source_set("blink_headers") {
     "//third_party/webrtc/modules/audio_device:audio_device_api",
     "//third_party/webrtc/modules/audio_processing:api",
     "//third_party/webrtc/modules/video_coding:video_codec_interface",
+    "//third_party/webrtc/p2p:libstunprober",
     "//third_party/webrtc/pc:peerconnection",
     "//third_party/webrtc/rtc_base:rtc_base",
     "//third_party/webrtc/rtc_base:rtc_task_queue",

third_party/blink/public/platform/modules/peerconnection/DEPS

 include_rules = [
+    "+base/timer/timer.h",
     "+media/base/audio_parameters.h",
     "+media/base/audio_push_fifo.h",
     "+media/base/video_codecs.h",

content/renderer/media/webrtc/stun_field_trial.h → third_party/blink/public/platform/modules/peerconnection/stun_field_trial.h

@@ -2,20 +2,19 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CONTENT_RENDERER_MEDIA_WEBRTC_STUN_FIELD_TRIAL_H_
-#define CONTENT_RENDERER_MEDIA_WEBRTC_STUN_FIELD_TRIAL_H_
+#ifndef THIRD_PARTY_BLINK_PUBLIC_PLATFORM_MODULES_PEERCONNECTION_STUN_FIELD_TRIAL_H_
+#define THIRD_PARTY_BLINK_PUBLIC_PLATFORM_MODULES_PEERCONNECTION_STUN_FIELD_TRIAL_H_
 
 #include <memory>
 #include <string>
 #include <vector>
 
-#include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/threading/thread_checker.h"
 #include "base/timer/timer.h"
-#include "content/common/content_export.h"
 #include "third_party/blink/public/platform/modules/p2p/network_list_manager.h"
 #include "third_party/blink/public/platform/modules/p2p/network_list_observer.h"
+#include "third_party/blink/public/platform/web_common.h"
 #include "third_party/webrtc/p2p/stunprober/stun_prober.h"
 #include "third_party/webrtc/rtc_base/network.h"
 #include "third_party/webrtc/rtc_base/third_party/sigslot/sigslot.h"
@@ -25,7 +24,7 @@ class PacketSocketFactory;
 class SocketAddress;
 }  // namespace rtc
 
-namespace content {
+namespace blink {
 
 // Wait for 30 seconds to avoid high CPU usage during browser start-up which
 // might affect the accuracy of the trial. The trial wakes up the browser every
@@ -33,10 +32,13 @@ namespace content {
 // stun probe.
 static const int kExperimentStartDelayMs = 30000;
 
+// TODO(crbug.com/787254): Move this class out of the Blink exposed API when
+// all users of it have been Onion souped. Also migrate away from std::vector
+// and std::string.
 class StunProberTrial : public stunprober::StunProber::Observer,
                         public sigslot::has_slots<> {
  public:
-  struct CONTENT_EXPORT Param {
+  struct BLINK_PLATFORM_EXPORT Param {
     Param();
     ~Param();
     int requests_per_ip = 0;
@@ -47,7 +49,7 @@ class StunProberTrial : public stunprober::StunProber::Observer,
     std::vector<rtc::SocketAddress> servers;
   };
 
-  StunProberTrial(rtc::NetworkManager* network_manager,
+  BLINK_PLATFORM_EXPORT StunProberTrial(rtc::NetworkManager* network_manager,
                                         const std::string& params,
                                         rtc::PacketSocketFactory* factory);
   ~StunProberTrial() override;
@@ -59,7 +61,8 @@ class StunProberTrial : public stunprober::StunProber::Observer,
   void OnNetworksChanged();
 
   // Parsing function to decode the '/' separated parameter string |params|.
-  static CONTENT_EXPORT bool ParseParameters(const std::string& param_line,
+  static BLINK_PLATFORM_EXPORT bool ParseParameters(
+      const std::string& param_line,
       Param* params);
 
   // stunprober::StunProber::Observer:
@@ -85,7 +88,7 @@ class StunProberTrial : public stunprober::StunProber::Observer,
   int started_probers_ = 0;
   int finished_probers_ = 0;
   std::vector<stunprober::StunProber*> probers_;
-  base::ThreadChecker thread_checker_;
+  THREAD_CHECKER(thread_checker_);
 
   // The reason we use a timer instead of depending on the OnFinished callback
   // of each prober is that the OnFinished is not fired at the last of STUN
@@ -98,6 +101,6 @@ class StunProberTrial : public stunprober::StunProber::Observer,
   DISALLOW_COPY_AND_ASSIGN(StunProberTrial);
 };
 
-}  // namespace content
+}  // namespace blink
 
-#endif  // CONTENT_RENDERER_MEDIA_WEBRTC_STUN_FIELD_TRIAL_H_
+#endif  // THIRD_PARTY_BLINK_PUBLIC_PLATFORM_MODULES_PEERCONNECTION_STUN_FIELD_TRIAL_H_

third_party/blink/renderer/platform/BUILD.gn

@@ -1254,6 +1254,7 @@ jumbo_component("platform") {
     "peerconnection/rtc_video_encoder_factory.cc",
     "peerconnection/rtc_video_encoder_factory.h",
     "peerconnection/rtc_void_request.h",
+    "peerconnection/stun_field_trial.cc",
     "peerconnection/transmission_encoding_info_handler.cc",
     "peerconnection/transmission_encoding_info_handler.h",
     "peerconnection/video_codec_factory.cc",
@@ -1815,6 +1816,7 @@ jumbo_source_set("blink_platform_unittests_sources") {
     "peerconnection/rtc_stats_test.cc",
     "peerconnection/rtc_video_decoder_adapter_test.cc",
     "peerconnection/rtc_video_encoder_test.cc",
+    "peerconnection/stun_field_trial_test.cc",
     "peerconnection/transmission_encoding_info_handler_test.cc",
     "peerconnection/two_keys_adapter_map_unittest.cc",
     "peerconnection/webrtc_video_track_source_test.cc",

third_party/blink/renderer/platform/peerconnection/DEPS

@@ -8,6 +8,8 @@ include_rules = [
     "+third_party/blink/renderer/platform/webrtc",
 
     # Dependencies.
+    "+base/strings/string_number_conversions.h",
+    "+base/strings/string_split.h",
     "+base/threading/thread_restrictions.h",
     "+media/base",
     "+media/media_buildflags.h",

content/renderer/media/webrtc/stun_field_trial.cc → third_party/blink/renderer/platform/peerconnection/stun_field_trial.cc

@@ -2,12 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "content/renderer/media/webrtc/stun_field_trial.h"
+#include "third_party/blink/public/platform/modules/peerconnection/stun_field_trial.h"
 
 #include <math.h>
 
-#include "base/bind.h"
-#include "base/location.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/rand_util.h"
@@ -26,7 +24,7 @@
 
 using stunprober::StunProber;
 
-namespace content {
+namespace blink {
 
 namespace {
 
@@ -40,8 +38,8 @@ enum NatType {
 };
 
 // This needs to match "NatType" in histograms.xml.
-const char* const NatTypeNames[] =
-    {"NoNAT", "UnknownNAT", "SymNAT", "NonSymNAT"};
+const char* const NatTypeNames[] = {"NoNAT", "UnknownNAT", "SymNAT",
+                                    "NonSymNAT"};
 static_assert(base::size(NatTypeNames) == NAT_TYPE_MAX,
               "NatType enums must match names");
 
@@ -91,7 +89,7 @@ StunProberTrial::StunProberTrial(rtc::NetworkManager* network_manager,
 StunProberTrial::~StunProberTrial() {}
 
 void StunProberTrial::SaveHistogramData() {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   NatType nat_type = NAT_TYPE_UNKNOWN;
   int interval_ms = 0;
   int count = 0;
@@ -238,7 +236,7 @@ bool StunProberTrial::ParseParameters(const std::string& param_line,
 }
 
 void StunProberTrial::OnNetworksChanged() {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   DVLOG(1) << "Starting stun trial with params: " << param_line_;
   rtc::NetworkManager::NetworkList networks;
   network_manager_->GetNetworks(&networks);
@@ -277,7 +275,7 @@ void StunProberTrial::OnNetworksChanged() {
 
 void StunProberTrial::OnFinished(StunProber* prober,
                                  StunProber::Status result) {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   if (result == StunProber::SUCCESS)
     ++finished_probers_;
 
@@ -287,7 +285,7 @@ void StunProberTrial::OnFinished(StunProber* prober,
 
 void StunProberTrial::OnPrepared(StunProber* prober,
                                  StunProber::Status result) {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   if (result == StunProber::SUCCESS)
     ++ready_probers_;
 
@@ -295,14 +293,15 @@ void StunProberTrial::OnPrepared(StunProber* prober,
     // TODO(guoweis) estimated_execution_time() is the same for all probers. It
     // could be moved up to the StunProberTrial class once the DNS resolution
     // part is moved up too.
-    timer_.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(
+    timer_.Start(FROM_HERE,
+                 base::TimeDelta::FromMilliseconds(
                      probers_.front()->estimated_execution_time()),
                  this, &StunProberTrial::OnTimer);
   }
 }
 
 void StunProberTrial::OnTimer() {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   probers_[started_probers_]->Start(this);
   started_probers_++;
 
@@ -310,4 +309,4 @@ void StunProberTrial::OnTimer() {
     timer_.Stop();
 }
 
-}  // namespace content
+}  // namespace blink

content/renderer/media/webrtc/stun_field_trial_unittest.cc → third_party/blink/renderer/platform/peerconnection/stun_field_trial_test.cc

@@ -2,12 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "content/renderer/media/webrtc/stun_field_trial.h"
+#include "third_party/blink/public/platform/modules/peerconnection/stun_field_trial.h"
 
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/webrtc/rtc_base/socket_address.h"
 
-namespace content {
+namespace blink {
 
 TEST(StunProbeTrial, VerifyParameterParsing) {
   StunProberTrial::Param params;
@@ -38,4 +38,4 @@ TEST(StunProbeTrial, VerifyParameterParsing) {
   EXPECT_FALSE(StunProberTrial::ParseParameters(param_line, &params));
 }
 
-}  // namespace content
+}  // namespace blink