[per-agent scheduling] Don't wait for non-ordinary page frame signals.

Frames for non-ordinary pages don't report signals (such as FMP or
onload). They also have no parent frame, so they are considered main
frames, and they use arbitrary agent cluster IDs. This can create
situations where we will "affect" queues forever.

The solution is to not add such frames to the "waiting for signal" set. This avoids waiting for them indefinitely, while also not throttling them.

The CL also changes it so that we don't try affect queues not associated with a frame (i.e.: have no FrameScheduler), as those are "global" queues that are not associated with a single agent.

Bug: 1090251,1115389
Change-Id: I8e624626694aa5aded7bce1ab6243c57eb69ebd7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2352584
Commit-Queue: Tal Pressman <talp@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797662}

third_party/blink/renderer/platform/scheduler/main_thread/agent_scheduling_strategy.cc

@@ -226,12 +226,21 @@ class TrackMainFrameSignal final : public AgentSchedulingStrategy {
       SetWaitingForInput(true);
 
     main_frames_.insert(&frame_scheduler);
-    main_frames_waiting_for_signal_.insert(&frame_scheduler);
+
+    // Only add ordinary page frames to the set of waiting frames, as
+    // non-ordinary ones don't report any signals.
+    if (frame_scheduler.IsOrdinary())
+      main_frames_waiting_for_signal_.insert(&frame_scheduler);
 
     return ShouldUpdatePolicy::kYes;
   }
 
   bool ShouldAffectQueue(const MainThreadTaskQueue& task_queue) const {
+    // Queues that don't have a frame scheduler are, by definition, not
+    // associated with a frame (or agent).
+    if (!task_queue.GetFrameScheduler())
+      return false;
+
     if (affected_queue_types_ == PerAgentAffectedQueues::kTimerQueues &&
         task_queue.GetPrioritisationType() !=
             PrioritisationType::kJavaScriptTimer) {

third_party/blink/renderer/platform/scheduler/main_thread/agent_scheduling_strategy_unittest.cc

@@ -30,6 +30,7 @@ using ::base::sequence_manager::TaskQueue;
 using ::base::test::ScopedFeatureList;
 using ::testing::_;
 using ::testing::NiceMock;
+using ::testing::Return;
 using ::testing::ReturnRef;
 using ::testing::Test;
 
@@ -43,9 +44,9 @@ class MockDelegate : public AgentSchedulingStrategy::Delegate {
                base::TimeDelta delay));
 };
 
-class MockFrameDelegate : public FrameScheduler::Delegate {
+class MockFrameSchedulerDelegate : public FrameScheduler::Delegate {
  public:
-  MockFrameDelegate() {
+  MockFrameSchedulerDelegate() {
     ON_CALL(*this, GetAgentClusterId)
         .WillByDefault(ReturnRef(agent_cluster_id_));
   }
@@ -70,10 +71,14 @@ class MockFrameScheduler : public FrameSchedulerImpl {
                            /*parent_page_scheduler=*/nullptr,
                            /*delegate=*/&delegate_,
                            /*blame_context=*/nullptr,
-                           /*frame_type=*/frame_type) {}
+                           /*frame_type=*/frame_type) {
+    ON_CALL(*this, IsOrdinary).WillByDefault(Return(true));
+  }
+
+  MOCK_METHOD(bool, IsOrdinary, (), (const));
 
  private:
-  NiceMock<MockFrameDelegate> delegate_;
+  NiceMock<MockFrameSchedulerDelegate> delegate_;
 };
 
 }  // namespace
@@ -494,5 +499,29 @@ TEST_F(PerAgentDefaultIsNoOpStrategyTest, DoesntModifyPolicyDecisions) {
   EXPECT_FALSE(strategy_->QueuePriority(*timer_queue_).has_value());
 }
 
+class PerAgentNonOrdinaryPageTest : public PerAgentSchedulingBaseTest {
+ public:
+  PerAgentNonOrdinaryPageTest()
+      : PerAgentSchedulingBaseTest({{"queues", "timer-queues"},
+                                    {"method", "disable"},
+                                    {"signal", "onload"}}) {
+    ON_CALL(non_ordinary_frame_scheduler_, IsOrdinary)
+        .WillByDefault(Return(false));
+  }
+
+ protected:
+  NiceMock<MockFrameScheduler> non_ordinary_frame_scheduler_{
+      FrameScheduler::FrameType::kMainFrame};
+};
+
+TEST_F(PerAgentNonOrdinaryPageTest, DoesntWaitForNonOrdinaryFrames) {
+  EXPECT_EQ(strategy_->OnFrameAdded(non_ordinary_frame_scheduler_),
+            ShouldUpdatePolicy::kYes);
+  EXPECT_FALSE(strategy_->QueueEnabledState(*timer_queue_).has_value());
+  EXPECT_FALSE(strategy_->QueuePriority(*timer_queue_).has_value());
+  EXPECT_FALSE(strategy_->QueueEnabledState(*non_timer_queue_).has_value());
+  EXPECT_FALSE(strategy_->QueuePriority(*non_timer_queue_).has_value());
+}
+
 }  // namespace scheduler
 }  // namespace blink

third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.cc

@@ -941,6 +941,12 @@ bool FrameSchedulerImpl::IsWaitingForMeaningfulPaint() const {
   return waiting_for_meaningful_paint_;
 }
 
+bool FrameSchedulerImpl::IsOrdinary() const {
+  if (!parent_page_scheduler_)
+    return true;
+  return parent_page_scheduler_->IsOrdinary();
+}
+
 bool FrameSchedulerImpl::ShouldThrottleTaskQueues() const {
   // TODO(crbug.com/1078387): Convert the CHECK to a DCHECK once enough time has
   // passed to confirm that it is correct. (November 2020).

third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.h

@@ -125,6 +125,11 @@ class PLATFORM_EXPORT FrameSchedulerImpl : public FrameScheduler,
   bool IsWaitingForContentfulPaint() const;
   bool IsWaitingForMeaningfulPaint() const;
 
+  // An "ordinary" FrameScheduler is responsible for a frame whose parent page
+  // is a fully-featured page owned by a web view (as opposed to, e.g.: a Page
+  // created by an SVGImage). Virtual for testing.
+  virtual bool IsOrdinary() const;
+
   void AsValueInto(base::trace_event::TracedValue* state) const;
   bool IsExemptFromBudgetBasedThrottling() const override;
   std::unique_ptr<blink::mojom::blink::PauseSubresourceLoadingHandle>