Modify WebRTC logic for disabling throttling.

Currently the existence of an RTCPeerConnection in a LocalDOMWindow
prevents that context from being throttled.

RTCPeerConnection objects are used in fingerprinting libraries
(typically for the IP information that the ICE exploration yields), but
not subsequently actually used to send any data. This prevents these
contexts from being throttled. It has been observed that these same
fingerprinting libraries add RTCDataChannels to the RTCPeerConnection,
and these channels stay forever in the 'connecting' state and never
transition to 'open'.

To allow the context to be throttled this CL modifies the logic to only
notify the scheduler once the RTCPeerConnection is supplemented with an
'open' RTCDataChannel or a 'live' MediaStreamTrack, which indicates with
much higher likelihood that actual data will be sent. The feature handle
is removed when RTCDataChannel or MediaStreamTrack transition to
readyState == 'closed'.

BUG=1127481,1075553,1101731

Change-Id: I9d07992da717a9e36eb004a7af27c4b72f9ea08c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2406685Reviewed-by: Harald Alvestrand <hta@chromium.org>
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808416}

third_party/blink/renderer/modules/mediastream/media_stream_track.cc

@@ -27,7 +27,9 @@
 
 #include <memory>
 
+#include "base/feature_list.h"
 #include "base/strings/stringprintf.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/platform/modules/mediastream/web_media_stream_track.h"
 #include "third_party/blink/public/platform/modules/webrtc/webrtc_logging.h"
 #include "third_party/blink/public/web/modules/mediastream/media_stream_video_source.h"
@@ -40,6 +42,7 @@
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
 #include "third_party/blink/renderer/core/frame/deprecation.h"
 #include "third_party/blink/renderer/core/frame/local_dom_window.h"
+#include "third_party/blink/renderer/core/frame/local_frame.h"
 #include "third_party/blink/renderer/modules/imagecapture/image_capture.h"
 #include "third_party/blink/renderer/modules/mediastream/apply_constraints_request.h"
 #include "third_party/blink/renderer/modules/mediastream/media_constraints_impl.h"
@@ -259,6 +262,11 @@ MediaStreamTrack::MediaStreamTrack(ExecutionContext* context,
     execution_context_->GetTaskRunner(TaskType::kInternalMedia)
         ->PostTask(FROM_HERE, std::move(callback));
   }
+
+  // Note that both 'live' and 'muted' correspond to a 'live' ready state in the
+  // web API.
+  if (ready_state_ != MediaStreamSource::kReadyStateEnded)
+    EnsureFeatureHandleForScheduler();
 }
 
 MediaStreamTrack::~MediaStreamTrack() = default;
@@ -402,6 +410,7 @@ void MediaStreamTrack::stopTrack(ExecutionContext* execution_context) {
     return;
 
   ready_state_ = MediaStreamSource::kReadyStateEnded;
+  feature_handle_for_scheduler_.reset();
   UserMediaController* user_media =
       UserMediaController::From(To<LocalDOMWindow>(execution_context));
   if (user_media)
@@ -730,19 +739,25 @@ void MediaStreamTrack::SourceChangedState() {
   if (Ended())
     return;
 
+  // Note that both 'live' and 'muted' correspond to a 'live' ready state in the
+  // web API, hence the following logic around |feature_handle_for_scheduler_|.
+
   ready_state_ = component_->Source()->GetReadyState();
   switch (ready_state_) {
     case MediaStreamSource::kReadyStateLive:
       component_->SetMuted(false);
       DispatchEvent(*Event::Create(event_type_names::kUnmute));
+      EnsureFeatureHandleForScheduler();
       break;
     case MediaStreamSource::kReadyStateMuted:
       component_->SetMuted(true);
       DispatchEvent(*Event::Create(event_type_names::kMute));
+      EnsureFeatureHandleForScheduler();
       break;
     case MediaStreamSource::kReadyStateEnded:
       DispatchEvent(*Event::Create(event_type_names::kEnded));
       PropagateTrackEnded();
+      feature_handle_for_scheduler_.reset();
       break;
   }
   SendLogMessage(
@@ -813,4 +828,24 @@ void MediaStreamTrack::Trace(Visitor* visitor) const {
   EventTargetWithInlineData::Trace(visitor);
 }
 
+void MediaStreamTrack::EnsureFeatureHandleForScheduler() {
+  if (feature_handle_for_scheduler_)
+    return;
+  LocalDOMWindow* window = DynamicTo<LocalDOMWindow>(GetExecutionContext());
+  // Ideally we'd use To<LocalDOMWindow>, but in unittests the ExecutionContext
+  // may not be a LocalDOMWindow.
+  if (!window)
+    return;
+  // This can happen for detached frames.
+  if (!window->GetFrame())
+    return;
+  feature_handle_for_scheduler_ =
+      window->GetFrame()->GetFrameScheduler()->RegisterFeature(
+          SchedulingPolicy::Feature::kWebRTC,
+          base::FeatureList::IsEnabled(features::kOptOutWebRTCFromAllThrottling)
+              ? SchedulingPolicy{SchedulingPolicy::DisableAllThrottling()}
+              : SchedulingPolicy{
+                    SchedulingPolicy::DisableAggressiveThrottling()});
+}
+
 }  // namespace blink

third_party/blink/renderer/modules/mediastream/media_stream_track.h

@@ -33,6 +33,7 @@
 #include "third_party/blink/renderer/modules/modules_export.h"
 #include "third_party/blink/renderer/platform/mediastream/media_stream_descriptor.h"
 #include "third_party/blink/renderer/platform/mediastream/media_stream_source.h"
+#include "third_party/blink/renderer/platform/scheduler/public/frame_scheduler.h"
 #include "third_party/blink/renderer/platform/wtf/forward.h"
 
 namespace blink {
@@ -125,6 +126,15 @@ class MODULES_EXPORT MediaStreamTrack
 
   std::string GetTrackLogString() const;
 
+  // Ensures that |feature_handle_for_scheduler_| is initialized.
+  void EnsureFeatureHandleForScheduler();
+
+  // This handle notifies the scheduler about a live media stream track
+  // associated with a frame. The handle should be destroyed when the track
+  // is stopped.
+  FrameScheduler::SchedulingAffectingFeatureHandle
+      feature_handle_for_scheduler_;
+
   MediaStreamSource::ReadyState ready_state_;
   HeapHashSet<Member<MediaStream>> registered_media_streams_;
   bool is_iterating_registered_media_streams_ = false;

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

@@ -29,11 +29,14 @@
 #include <string>
 #include <utility>
 
+#include "base/feature_list.h"
 #include "base/metrics/histogram_macros.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/platform/task_type.h"
 #include "third_party/blink/renderer/core/events/message_event.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
 #include "third_party/blink/renderer/core/fileapi/blob.h"
+#include "third_party/blink/renderer/core/frame/local_dom_window.h"
 #include "third_party/blink/renderer/core/typed_arrays/dom_array_buffer.h"
 #include "third_party/blink/renderer/core/typed_arrays/dom_array_buffer_view.h"
 #include "third_party/blink/renderer/modules/peerconnection/adapters/web_rtc_cross_thread_copier.h"
@@ -42,6 +45,7 @@
 #include "third_party/blink/renderer/modules/peerconnection/rtc_peer_connection_handler.h"
 #include "third_party/blink/renderer/platform/bindings/exception_state.h"
 #include "third_party/blink/renderer/platform/scheduler/public/post_cross_thread_task.h"
+#include "third_party/blink/renderer/platform/scheduler/public/scheduling_policy.h"
 #include "third_party/blink/renderer/platform/wtf/cross_thread_functional.h"
 #include "third_party/blink/renderer/platform/wtf/std_lib_extras.h"
 #include "third_party/blink/renderer/platform/wtf/thread_safe_ref_counted.h"
@@ -459,6 +463,7 @@ void RTCDataChannel::ContextDestroyed() {
   Dispose();
   stopped_ = true;
   state_ = webrtc::DataChannelInterface::kClosed;
+  feature_handle_for_scheduler_.reset();
 }
 
 // ActiveScriptWrappable
@@ -507,8 +512,10 @@ void RTCDataChannel::Trace(Visitor* visitor) const {
 }
 
 void RTCDataChannel::SetStateToOpenWithoutEvent() {
+  DCHECK_NE(state_, webrtc::DataChannelInterface::kOpen);
   IncrementCounter(DataChannelCounters::kOpened);
   state_ = webrtc::DataChannelInterface::kOpen;
+  CreateFeatureHandleForScheduler();
 }
 
 void RTCDataChannel::DispatchOpenEvent() {
@@ -536,6 +543,7 @@ void RTCDataChannel::OnStateChange(
   switch (state_) {
     case webrtc::DataChannelInterface::kOpen:
       IncrementCounter(DataChannelCounters::kOpened);
+      CreateFeatureHandleForScheduler();
       DispatchEvent(*Event::Create(event_type_names::kOpen));
       break;
     case webrtc::DataChannelInterface::kClosing:
@@ -544,6 +552,7 @@ void RTCDataChannel::OnStateChange(
       }
       break;
     case webrtc::DataChannelInterface::kClosed:
+      feature_handle_for_scheduler_.reset();
       if (!channel()->error().ok()) {
         DispatchEvent(*MakeGarbageCollected<RTCErrorEvent>(
             event_type_names::kError, channel()->error()));
@@ -647,4 +656,23 @@ bool RTCDataChannel::SendDataBuffer(webrtc::DataBuffer data_buffer) {
   return true;
 }
 
+void RTCDataChannel::CreateFeatureHandleForScheduler() {
+  DCHECK(!feature_handle_for_scheduler_);
+  LocalDOMWindow* window = DynamicTo<LocalDOMWindow>(GetExecutionContext());
+  // Ideally we'd use To<LocalDOMWindow>, but in unittests the ExecutionContext
+  // may not be a LocalDOMWindow.
+  if (!window)
+    return;
+  // This can happen for detached frames.
+  if (!window->GetFrame())
+    return;
+  feature_handle_for_scheduler_ =
+      window->GetFrame()->GetFrameScheduler()->RegisterFeature(
+          SchedulingPolicy::Feature::kWebRTC,
+          base::FeatureList::IsEnabled(features::kOptOutWebRTCFromAllThrottling)
+              ? SchedulingPolicy{SchedulingPolicy::DisableAllThrottling()}
+              : SchedulingPolicy{
+                    SchedulingPolicy::DisableAggressiveThrottling()});
+}
+
 }  // namespace blink

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

@@ -35,6 +35,7 @@
 #include "third_party/blink/renderer/core/typed_arrays/array_buffer_view_helpers.h"
 #include "third_party/blink/renderer/modules/event_target_modules.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
+#include "third_party/blink/renderer/platform/scheduler/public/frame_scheduler.h"
 #include "third_party/blink/renderer/platform/timer.h"
 #include "third_party/webrtc/api/peer_connection_interface.h"
 
@@ -166,6 +167,10 @@ class MODULES_EXPORT RTCDataChannel final
   bool SendRawData(const char* data, size_t length);
   bool SendDataBuffer(webrtc::DataBuffer data_buffer);
 
+  // Initializes |feature_handle_for_scheduler_|, which must not yet have been
+  // initialized.
+  void CreateFeatureHandleForScheduler();
+
   webrtc::DataChannelInterface::DataState state_;
 
   enum BinaryType { kBinaryTypeBlob, kBinaryTypeArrayBuffer };
@@ -178,6 +183,12 @@ class MODULES_EXPORT RTCDataChannel final
   FRIEND_TEST_ALL_PREFIXES(RTCDataChannelTest, Message);
   FRIEND_TEST_ALL_PREFIXES(RTCDataChannelTest, BufferedAmountLow);
 
+  // This handle notifies the scheduler about a connected data channel
+  // associated with a frame. The handle should be destroyed when the channel
+  // is closed.
+  FrameScheduler::SchedulingAffectingFeatureHandle
+      feature_handle_for_scheduler_;
+
   unsigned buffered_amount_low_threshold_;
   unsigned buffered_amount_;
   bool stopped_;

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

@@ -13,9 +13,13 @@
 #include "base/run_loop.h"
 #include "base/test/test_simple_task_runner.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_testing.h"
 #include "third_party/blink/renderer/core/dom/events/event.h"
+#include "third_party/blink/renderer/core/frame/local_frame.h"
 #include "third_party/blink/renderer/core/testing/null_execution_context.h"
 #include "third_party/blink/renderer/modules/peerconnection/mock_rtc_peer_connection_handler_platform.h"
+#include "third_party/blink/renderer/platform/scheduler/public/frame_scheduler.h"
+#include "third_party/blink/renderer/platform/scheduler/public/page_scheduler.h"
 #include "third_party/blink/renderer/platform/scheduler/public/post_cross_thread_task.h"
 #include "third_party/blink/renderer/platform/wtf/cross_thread_functional.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
@@ -358,4 +362,39 @@ TEST_F(RTCDataChannelTest, CloseAfterContextDestroyed) {
   EXPECT_EQ(String::FromUTF8("closed"), channel->readyState());
 }
 
+TEST_F(RTCDataChannelTest, StopsThrottling) {
+  V8TestingScope scope;
+
+  auto* scheduler = scope.GetFrame().GetFrameScheduler()->GetPageScheduler();
+  EXPECT_FALSE(scheduler->OptedOutFromAggressiveThrottlingForTest());
+
+  // Creating an RTCDataChannel doesn't enable the opt-out.
+  scoped_refptr<MockDataChannel> webrtc_channel(
+      new rtc::RefCountedObject<MockDataChannel>(signaling_thread()));
+  std::unique_ptr<MockPeerConnectionHandler> pc(
+      new MockPeerConnectionHandler(signaling_thread()));
+  auto* channel = MakeGarbageCollected<RTCDataChannel>(
+      scope.GetExecutionContext(), webrtc_channel.get(), pc.get());
+  EXPECT_EQ("connecting", channel->readyState());
+  EXPECT_FALSE(scheduler->OptedOutFromAggressiveThrottlingForTest());
+
+  // Transitioning to 'open' enables the opt-out.
+  webrtc_channel->ChangeState(webrtc::DataChannelInterface::kOpen);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ("open", channel->readyState());
+  EXPECT_TRUE(scheduler->OptedOutFromAggressiveThrottlingForTest());
+
+  // Transitioning to 'closing' keeps the opt-out enabled.
+  webrtc_channel->ChangeState(webrtc::DataChannelInterface::kClosing);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ("closing", channel->readyState());
+  EXPECT_TRUE(scheduler->OptedOutFromAggressiveThrottlingForTest());
+
+  // Transitioning to 'closed' stops the opt-out.
+  webrtc_channel->ChangeState(webrtc::DataChannelInterface::kClosed);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ("closed", channel->readyState());
+  EXPECT_FALSE(scheduler->OptedOutFromAggressiveThrottlingForTest());
+}
+
 }  // namespace blink

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

@@ -35,6 +35,7 @@
 #include <string>
 #include <utility>
 
+#include "base/feature_list.h"
 #include "base/lazy_instance.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
@@ -117,6 +118,7 @@
 #include "third_party/blink/renderer/platform/peerconnection/rtc_stats_request.h"
 #include "third_party/blink/renderer/platform/peerconnection/rtc_void_request.h"
 #include "third_party/blink/renderer/platform/runtime_enabled_features.h"
+#include "third_party/blink/renderer/platform/scheduler/public/scheduling_policy.h"
 #include "third_party/blink/renderer/platform/wtf/functional.h"
 #include "third_party/blink/renderer/platform/wtf/std_lib_extras.h"
 #include "third_party/webrtc/api/data_channel_interface.h"
@@ -818,13 +820,8 @@ RTCPeerConnection::RTCPeerConnection(
   feature_handle_for_scheduler_ =
       window->GetFrame()->GetFrameScheduler()->RegisterFeature(
           SchedulingPolicy::Feature::kWebRTC,
-          base::FeatureList::IsEnabled(features::kOptOutWebRTCFromAllThrottling)
-              ? SchedulingPolicy{SchedulingPolicy::DisableAllThrottling(),
-                                 SchedulingPolicy::
-                                     RecordMetricsForBackForwardCache()}
-              : SchedulingPolicy{
-                    SchedulingPolicy::DisableAggressiveThrottling(),
-                    SchedulingPolicy::RecordMetricsForBackForwardCache()});
+          SchedulingPolicy{
+              SchedulingPolicy::RecordMetricsForBackForwardCache()});
 }
 
 RTCPeerConnection::~RTCPeerConnection() {

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

@@ -1150,71 +1150,31 @@ TEST(DeduceSdpUsageCategory, ComplexSdpIsSafeIfMatchingExplicitSdpSemantics) {
                                    true, webrtc::SdpSemantics::kUnifiedPlan));
 }
 
-// This test was originally extracted out of [1], so that core/ does not need
-// to depend on mobules/.
-//
-// [1] core/scheduler_integration_tests/scheduler_affecting_features_test.cc
-//
-// TODO(crbug.com/787254): Consider factorying
-// SchedulingAffectingWebRTCFeaturesTest out to avoid the code duplication.
-class SchedulingAffectingWebRTCFeaturesTest : public SimTest {
- public:
-  PageScheduler* PageScheduler() {
-    return MainFrameScheduler()->GetPageScheduler();
-  }
-
-  FrameScheduler* MainFrameScheduler() { return MainFrame().Scheduler(); }
-
-  // Some features (e.g. document.load) are expected to appear in almost
-  // any output. Filter them out to make most of the tests simpler.
-  Vector<SchedulingPolicy::Feature> GetNonTrivialMainFrameFeatures() {
-    Vector<SchedulingPolicy::Feature> result;
-    for (SchedulingPolicy::Feature feature :
-         MainFrameScheduler()
-             ->GetActiveFeaturesTrackedForBackForwardCacheMetrics()) {
-      if (feature != SchedulingPolicy::Feature::kWebRTC)
-        continue;
-      result.push_back(feature);
-    }
-    return result;
-  }
-
-  std::unique_ptr<RTCPeerConnectionHandler> CreateRTCPeerConnectionHandler() {
-    return std::make_unique<MockRTCPeerConnectionHandlerPlatform>();
-  }
-};
-
-TEST_F(SchedulingAffectingWebRTCFeaturesTest, WebRTCStopsThrottling) {
-  ScopedTestingPlatformSupport<TestingPlatformSupport> platform;
-
-  RTCPeerConnection::SetRtcPeerConnectionHandlerFactoryForTesting(
-      base::BindRepeating(&SchedulingAffectingWebRTCFeaturesTest::
-                              CreateRTCPeerConnectionHandler,
-                          base::Unretained(this)));
-
-  SimRequest main_resource("https://example.com/", "text/html");
-
-  LoadURL("https://example.com/");
-
-  EXPECT_FALSE(PageScheduler()->OptedOutFromAggressiveThrottlingForTest());
-  EXPECT_THAT(GetNonTrivialMainFrameFeatures(),
-              testing::UnorderedElementsAre());
+TEST_F(RTCPeerConnectionTest, MediaStreamTrackStopsThrottling) {
+  V8TestingScope scope;
 
-  main_resource.Complete(
-      "<script>"
-      "  var data_channel = new RTCPeerConnection();"
-      "</script>");
+  auto* scheduler = scope.GetFrame().GetFrameScheduler()->GetPageScheduler();
+  EXPECT_FALSE(scheduler->OptedOutFromAggressiveThrottlingForTest());
 
-  EXPECT_TRUE(PageScheduler()->OptedOutFromAggressiveThrottlingForTest());
-  EXPECT_THAT(
-      GetNonTrivialMainFrameFeatures(),
-      testing::UnorderedElementsAre(SchedulingPolicy::Feature::kWebRTC));
+  // Creating the RTCPeerConnection doesn't disable throttling.
+  RTCPeerConnection* pc = CreatePC(scope);
+  EXPECT_EQ("", GetExceptionMessage(scope));
+  ASSERT_TRUE(pc);
+  EXPECT_FALSE(scheduler->OptedOutFromAggressiveThrottlingForTest());
 
-  MainFrame().ExecuteScript(WebString("data_channel.close();"));
+  // But creating a media stream track does.
+  MediaStreamTrack* track =
+      CreateTrack(scope, MediaStreamSource::kTypeAudio, "audioTrack");
+  HeapVector<Member<MediaStreamTrack>> tracks;
+  tracks.push_back(track);
+  MediaStream* stream =
+      MediaStream::Create(scope.GetExecutionContext(), tracks);
+  ASSERT_TRUE(stream);
+  EXPECT_TRUE(scheduler->OptedOutFromAggressiveThrottlingForTest());
 
-  EXPECT_FALSE(PageScheduler()->OptedOutFromAggressiveThrottlingForTest());
-  EXPECT_THAT(GetNonTrivialMainFrameFeatures(),
-              testing::UnorderedElementsAre());
+  // Stopping the track disables the opt-out.
+  track->stopTrack(scope.GetExecutionContext());
+  EXPECT_FALSE(scheduler->OptedOutFromAggressiveThrottlingForTest());
 }
 
 }  // namespace blink

third_party/blink/renderer/platform/scheduler/public/frame_or_worker_scheduler.h

@@ -63,6 +63,8 @@ class PLATFORM_EXPORT FrameOrWorkerScheduler {
     SchedulingAffectingFeatureHandle& operator=(
         SchedulingAffectingFeatureHandle&&);
 
+    explicit operator bool() const { return scheduler_.get(); }
+
     inline void reset() {
       if (scheduler_)
         scheduler_->OnStoppedUsingFeature(feature_, policy_);