[scheduler] Replace OnActiveConnectionCreated with RegisterFeature

Generalise active connection detection mechanism. It will be used to detect the presence of bfcache-related features and will continue to exempt pages with active connections from throttling. R=haraken@chromium.org BUG=933147 Change-Id: If8aecb95fc4650e0fcc0d7c047b334a794d60cc3 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1508463 Commit-Queue: Alexander Timin <altimin@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Sami Kyöstilä <skyostil@chromium.org> Cr-Commit-Position: refs/heads/master@{#641696}
marcosholgado · Mar 18, 2019 · 30f1e55 · 30f1e55
1 parent fdbc8be
commit 30f1e55
Show file tree

Hide file tree

Showing 18 changed files with 200 additions and 79 deletions.
diff --git a/third_party/blink/renderer/modules/peerconnection/adapters/ice_transport_proxy.cc b/third_party/blink/renderer/modules/peerconnection/adapters/ice_transport_proxy.cc
@@ -22,8 +22,9 @@ IceTransportProxy::IceTransportProxy(
       host_thread_(std::move(host_thread)),
       host_(nullptr, base::OnTaskRunnerDeleter(host_thread_)),
       delegate_(delegate),
-      connection_handle_for_scheduler_(
-          frame.GetFrameScheduler()->OnActiveConnectionCreated()),
+      feature_handle_for_scheduler_(frame.GetFrameScheduler()->RegisterFeature(
+          SchedulingPolicy::Feature::kWebRTC,
+          {SchedulingPolicy::DisableAggressiveThrottling()})),
       weak_ptr_factory_(this) {
   DCHECK(host_thread_);
   DCHECK(delegate_);

diff --git a/third_party/blink/renderer/modules/peerconnection/adapters/ice_transport_proxy.h b/third_party/blink/renderer/modules/peerconnection/adapters/ice_transport_proxy.h
@@ -116,8 +116,8 @@ class IceTransportProxy final {
   // This handle notifies scheduler about an active connection associated
   // with a frame. Handle should be destroyed when connection is closed.
   // This should have the same lifetime as |proxy_|.
-  std::unique_ptr<FrameScheduler::ActiveConnectionHandle>
-      connection_handle_for_scheduler_;
+  FrameScheduler::SchedulingAffectingFeatureHandle
+      feature_handle_for_scheduler_;
 
   THREAD_CHECKER(thread_checker_);
 

diff --git a/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.cc b/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.cc
@@ -778,8 +778,10 @@ RTCPeerConnection::RTCPeerConnection(
     return;
   }
 
-  connection_handle_for_scheduler_ =
-      document->GetFrame()->GetFrameScheduler()->OnActiveConnectionCreated();
+  feature_handle_for_scheduler_ =
+      document->GetFrame()->GetFrameScheduler()->RegisterFeature(
+          SchedulingPolicy::Feature::kWebRTC,
+          {SchedulingPolicy::DisableAggressiveThrottling()});
 }
 
 RTCPeerConnection::~RTCPeerConnection() {
@@ -2869,7 +2871,7 @@ void RTCPeerConnection::ReleasePeerConnectionHandler() {
 
   peer_handler_.reset();
   dispatch_scheduled_events_task_handle_.Cancel();
-  connection_handle_for_scheduler_.reset();
+  feature_handle_for_scheduler_.reset();
 }
 
 void RTCPeerConnection::ClosePeerConnection() {
@@ -3003,7 +3005,7 @@ void RTCPeerConnection::CloseInternal() {
   HostsUsingFeatures::CountAnyWorld(
       *document, HostsUsingFeatures::Feature::kRTCPeerConnectionUsed);
 
-  connection_handle_for_scheduler_.reset();
+  feature_handle_for_scheduler_.reset();
 }
 
 void RTCPeerConnection::ScheduleDispatchEvent(Event* event) {

diff --git a/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.h b/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.h
@@ -508,8 +508,8 @@ class MODULES_EXPORT RTCPeerConnection final
 
   // This handle notifies scheduler about an active connection associated
   // with a frame. Handle should be destroyed when connection is closed.
-  std::unique_ptr<FrameScheduler::ActiveConnectionHandle>
-      connection_handle_for_scheduler_;
+  FrameScheduler::SchedulingAffectingFeatureHandle
+      feature_handle_for_scheduler_;
 
   bool negotiation_needed_;
   bool stopped_;

diff --git a/third_party/blink/renderer/modules/websockets/websocket_channel_impl.cc b/third_party/blink/renderer/modules/websockets/websocket_channel_impl.cc
@@ -219,8 +219,11 @@ bool WebSocketChannelImpl::Connect(
   if (GetBaseFetchContext()->ShouldBlockWebSocketByMixedContentCheck(url))
     return false;
 
-  if (auto* scheduler = execution_context_->GetScheduler())
-    connection_handle_for_scheduler_ = scheduler->OnActiveConnectionCreated();
+  if (auto* scheduler = execution_context_->GetScheduler()) {
+    feature_handle_for_scheduler_ = scheduler->RegisterFeature(
+        SchedulingPolicy::Feature::kWebSocket,
+        {SchedulingPolicy::DisableAggressiveThrottling()});
+  }
 
   if (MixedContentChecker::IsMixedContent(
           execution_context_->GetSecurityOrigin(), url)) {
@@ -396,7 +399,7 @@ void WebSocketChannelImpl::Disconnect() {
         "data", InspectorWebSocketEvent::Data(execution_context_, identifier_));
     probe::DidCloseWebSocket(execution_context_, identifier_);
   }
-  connection_handle_for_scheduler_.reset();
+  feature_handle_for_scheduler_.reset();
   AbortAsyncOperations();
   handshake_throttle_.reset();
   handle_.reset();
@@ -611,7 +614,7 @@ void WebSocketChannelImpl::DidFail(WebSocketHandle* handle,
   NETWORK_DVLOG(1) << this << " DidFail(" << handle << ", " << String(message)
                    << ")";
 
-  connection_handle_for_scheduler_.reset();
+  feature_handle_for_scheduler_.reset();
 
   DCHECK(handle_);
   DCHECK_EQ(handle, handle_.get());
@@ -689,7 +692,7 @@ void WebSocketChannelImpl::DidClose(WebSocketHandle* handle,
   NETWORK_DVLOG(1) << this << " DidClose(" << handle << ", " << was_clean
                    << ", " << code << ", " << String(reason) << ")";
 
-  connection_handle_for_scheduler_.reset();
+  feature_handle_for_scheduler_.reset();
 
   DCHECK(handle_);
   DCHECK_EQ(handle, handle_.get());
@@ -777,7 +780,7 @@ void WebSocketChannelImpl::DidFailLoadingBlob(FileErrorCode error_code) {
 
 void WebSocketChannelImpl::TearDownFailedConnection() {
   // |handle_| and |client_| can be null here.
-  connection_handle_for_scheduler_.reset();
+  feature_handle_for_scheduler_.reset();
   handshake_throttle_.reset();
 
   if (client_)

diff --git a/third_party/blink/renderer/modules/websockets/websocket_channel_impl.h b/third_party/blink/renderer/modules/websockets/websocket_channel_impl.h
@@ -202,8 +202,8 @@ class MODULES_EXPORT WebSocketChannelImpl final
   uint64_t sending_quota_;
   uint64_t received_data_size_for_flow_control_;
   wtf_size_t sent_size_of_top_message_;
-  std::unique_ptr<FrameScheduler::ActiveConnectionHandle>
-      connection_handle_for_scheduler_;
+  FrameScheduler::SchedulingAffectingFeatureHandle
+      feature_handle_for_scheduler_;
 
   std::unique_ptr<SourceLocation> location_at_construction_;
   network::mojom::blink::WebSocketHandshakeRequestPtr handshake_request_;

diff --git a/third_party/blink/renderer/platform/scheduler/BUILD.gn b/third_party/blink/renderer/platform/scheduler/BUILD.gn
@@ -116,6 +116,7 @@ blink_platform_sources("scheduler") {
     "public/post_cancellable_task.h",
     "public/post_cross_thread_task.h",
     "public/scheduling_lifecycle_state.h",
+    "public/scheduling_policy.h",
     "public/thread.h",
     "public/thread_cpu_throttler.h",
     "public/thread_scheduler.h",

diff --git a/third_party/blink/renderer/platform/scheduler/DEPS b/third_party/blink/renderer/platform/scheduler/DEPS
@@ -44,6 +44,7 @@ include_rules = [
   "+base/threading/sequenced_task_runner_handle.h",
   "+base/threading/thread.h",
   "+base/threading/thread_checker.h",
+  "+base/traits_bag.h",
   "+components/scheduling_metrics",
   "+services/metrics",
 

diff --git a/third_party/blink/renderer/platform/scheduler/common/frame_or_worker_scheduler.cc b/third_party/blink/renderer/platform/scheduler/common/frame_or_worker_scheduler.cc
@@ -16,12 +16,44 @@ FrameOrWorkerScheduler::LifecycleObserverHandle::~LifecycleObserverHandle() {
     scheduler_->RemoveLifecycleObserver(observer_);
 }
 
+FrameOrWorkerScheduler::SchedulingAffectingFeatureHandle::
+    SchedulingAffectingFeatureHandle(
+        SchedulingPolicy::Feature feature,
+        SchedulingPolicy policy,
+        base::WeakPtr<FrameOrWorkerScheduler> scheduler)
+    : feature_(feature), policy_(policy), scheduler_(std::move(scheduler)) {
+  DCHECK(scheduler_);
+  scheduler_->OnStartedUsingFeature(feature_, policy_);
+}
+
+FrameOrWorkerScheduler::SchedulingAffectingFeatureHandle::
+    SchedulingAffectingFeatureHandle(SchedulingAffectingFeatureHandle&& other)
+    : feature_(other.feature_), scheduler_(std::move(other.scheduler_)) {
+  other.scheduler_ = nullptr;
+}
+
+FrameOrWorkerScheduler::SchedulingAffectingFeatureHandle&
+FrameOrWorkerScheduler::SchedulingAffectingFeatureHandle::operator=(
+    SchedulingAffectingFeatureHandle&& other) {
+  feature_ = other.feature_;
+  policy_ = std::move(other.policy_);
+  scheduler_ = std::move(other.scheduler_);
+  other.scheduler_ = nullptr;
+  return *this;
+}
+
 FrameOrWorkerScheduler::FrameOrWorkerScheduler() : weak_factory_(this) {}
 
 FrameOrWorkerScheduler::~FrameOrWorkerScheduler() {
   weak_factory_.InvalidateWeakPtrs();
 }
 
+FrameOrWorkerScheduler::SchedulingAffectingFeatureHandle
+FrameOrWorkerScheduler::RegisterFeature(SchedulingPolicy::Feature feature,
+                                        SchedulingPolicy policy) {
+  return SchedulingAffectingFeatureHandle(feature, policy, GetWeakPtr());
+}
+
 std::unique_ptr<FrameOrWorkerScheduler::LifecycleObserverHandle>
 FrameOrWorkerScheduler::AddLifecycleObserver(ObserverType type,
                                              Observer* observer) {

diff --git a/third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.cc b/third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.cc
@@ -118,19 +118,6 @@ std::set<std::string> TaskTypesFromFieldTrialParam(const char* param) {
 
 }  // namespace
 
-FrameSchedulerImpl::ActiveConnectionHandleImpl::ActiveConnectionHandleImpl(
-    FrameSchedulerImpl* frame_scheduler)
-    : frame_scheduler_(frame_scheduler->GetWeakPtr()) {
-  frame_scheduler->DidOpenActiveConnection();
-}
-
-FrameSchedulerImpl::ActiveConnectionHandleImpl::~ActiveConnectionHandleImpl() {
-  if (frame_scheduler_) {
-    static_cast<FrameSchedulerImpl*>(frame_scheduler_.get())
-        ->DidCloseActiveConnection();
-  }
-}
-
 FrameSchedulerImpl::PauseSubresourceLoadingHandleImpl::
     PauseSubresourceLoadingHandleImpl(
         base::WeakPtr<FrameSchedulerImpl> frame_scheduler)
@@ -622,6 +609,22 @@ WebScopedVirtualTimePauser FrameSchedulerImpl::CreateWebScopedVirtualTimePauser(
   return WebScopedVirtualTimePauser(main_thread_scheduler_, duration, name);
 }
 
+void FrameSchedulerImpl::OnStartedUsingFeature(
+    SchedulingPolicy::Feature feature,
+    const SchedulingPolicy& policy) {
+  // TODO(altimin): Rename *ActiveConnection to
+  // Enable/DisableAggressiveThrottling.
+  if (policy.disable_aggressive_throttling)
+    DidOpenActiveConnection();
+}
+
+void FrameSchedulerImpl::OnStoppedUsingFeature(
+    SchedulingPolicy::Feature feature,
+    const SchedulingPolicy& policy) {
+  if (policy.disable_aggressive_throttling)
+    DidCloseActiveConnection();
+}
+
 void FrameSchedulerImpl::DidOpenActiveConnection() {
   ++active_connection_count_;
   has_active_connection_ = static_cast<bool>(active_connection_count_);
@@ -766,11 +769,6 @@ void FrameSchedulerImpl::OnFirstMeaningfulPaint() {
   main_thread_scheduler_->OnFirstMeaningfulPaint();
 }
 
-std::unique_ptr<FrameScheduler::ActiveConnectionHandle>
-FrameSchedulerImpl::OnActiveConnectionCreated() {
-  return std::make_unique<FrameSchedulerImpl::ActiveConnectionHandleImpl>(this);
-}
-
 bool FrameSchedulerImpl::ShouldThrottleTaskQueues() const {
   if (!RuntimeEnabledFeatures::TimerThrottlingForBackgroundTabsEnabled())
     return false;

diff --git a/third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.h b/third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.h
@@ -107,12 +107,16 @@ class PLATFORM_EXPORT FrameSchedulerImpl : public FrameScheduler,
       const WTF::String& name,
       WebScopedVirtualTimePauser::VirtualTaskDuration duration) override;
   void OnFirstMeaningfulPaint() override;
-  std::unique_ptr<ActiveConnectionHandle> OnActiveConnectionCreated() override;
   void AsValueInto(base::trace_event::TracedValue* state) const;
   bool IsExemptFromBudgetBasedThrottling() const override;
   std::unique_ptr<blink::mojom::blink::PauseSubresourceLoadingHandle>
   GetPauseSubresourceLoadingHandle() override;
 
+  void OnStartedUsingFeature(SchedulingPolicy::Feature feature,
+                             const SchedulingPolicy& policy) override;
+  void OnStoppedUsingFeature(SchedulingPolicy::Feature feature,
+                             const SchedulingPolicy& policy) override;
+
   scoped_refptr<base::SingleThreadTaskRunner> ControlTaskRunner();
 
   void UpdatePolicy();
@@ -180,17 +184,6 @@ class PLATFORM_EXPORT FrameSchedulerImpl : public FrameScheduler,
   friend class page_scheduler_impl_unittest::PageSchedulerImplTest;
   friend class ResourceLoadingTaskRunnerHandleImpl;
 
-  class ActiveConnectionHandleImpl : public ActiveConnectionHandle {
-   public:
-    ActiveConnectionHandleImpl(FrameSchedulerImpl* frame_scheduler);
-    ~ActiveConnectionHandleImpl() override;
-
-   private:
-    base::WeakPtr<FrameOrWorkerScheduler> frame_scheduler_;
-
-    DISALLOW_COPY_AND_ASSIGN(ActiveConnectionHandleImpl);
-  };
-
   // A class that adds and removes itself from the passed in weak pointer. While
   // one exists, resource loading is paused.
   class PauseSubresourceLoadingHandleImpl
@@ -224,12 +217,12 @@ class PLATFORM_EXPORT FrameSchedulerImpl : public FrameScheduler,
   void UpdateTaskQueueThrottling(MainThreadTaskQueue* task_queue,
                                  bool should_throttle);
 
-  void DidOpenActiveConnection();
-  void DidCloseActiveConnection();
-
   void AddPauseSubresourceLoadingHandle();
   void RemovePauseSubresourceLoadingHandle();
 
+  void DidOpenActiveConnection();
+  void DidCloseActiveConnection();
+
   std::unique_ptr<ResourceLoadingTaskRunnerHandleImpl>
   CreateResourceLoadingTaskRunnerHandleImpl();
 

diff --git a/third_party/blink/renderer/platform/scheduler/main_thread/page_scheduler_impl_unittest.cc b/third_party/blink/renderer/platform/scheduler/main_thread/page_scheduler_impl_unittest.cc
@@ -1159,8 +1159,10 @@ TEST_F(PageSchedulerImplTest, OpenWebSocketExemptsFromBudgetThrottling) {
                   base::TimeTicks() + base::TimeDelta::FromSeconds(51)));
   run_times.clear();
 
-  std::unique_ptr<FrameScheduler::ActiveConnectionHandle> websocket_connection =
-      frame_scheduler1->OnActiveConnectionCreated();
+  FrameScheduler::SchedulingAffectingFeatureHandle websocket_feature =
+      frame_scheduler1->RegisterFeature(
+          SchedulingPolicy::Feature::kWebSocket,
+          {SchedulingPolicy::DisableAggressiveThrottling()});
 
   for (size_t i = 0; i < 3; ++i) {
     ThrottleableTaskQueueForScheduler(frame_scheduler1.get())
@@ -1203,7 +1205,7 @@ TEST_F(PageSchedulerImplTest, OpenWebSocketExemptsFromBudgetThrottling) {
           base::TimeTicks() + base::TimeDelta::FromMilliseconds(59500)));
   run_times.clear();
 
-  websocket_connection.reset();
+  websocket_feature.reset();
 
   // Wait for 10s to enable throttling back.
   FastForwardTo(base::TimeTicks() + base::TimeDelta::FromMilliseconds(70500));