ResourceLoadScheduler: DCHECK fails if a chosen request was canceled

In OnLifecycleStateChanged(), ShowConsoleMessageIfNeeded() is called
and it chosen the top request from |pending_requests_| to show a
console message if the request is stacked in the queue for a long time.

This code expects the entry in |pending_requests_| is always alive even
in the |pending_request_map_|, but it's wrong. If the request is
canceled, it remains only in |pending_requests_| and DCHECK fires.

This patch changes ShowConsoleMessageIfNeeded() to choose a request
only when it is still alive in |oending_request_map_|.

Bug: 936937
Change-Id: Ic0ea774f62719a3b36e78c49f24da3d5f6ce8365
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1498971Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#637638}

third_party/blink/renderer/platform/loader/fetch/resource_load_scheduler.cc

@@ -604,6 +604,23 @@ ResourceLoadScheduler::ClientId ResourceLoadScheduler::GenerateClientId() {
   return id;
 }
 
+bool ResourceLoadScheduler::IsPendingRequestEffectivelyEmpty(
+    ThrottleOption option) {
+  for (const auto& client : pending_requests_[option]) {
+    // The request in |pending_request_| is erased when it is scheduled. So if
+    // the request is canceled, or Release() is called before firing its Run(),
+    // the entry for the request remains in |pending_request_| until it is
+    // popped in GetNextPendingRequest().
+    if (pending_request_map_.find(client.client_id) !=
+        pending_request_map_.end()) {
+      return false;
+    }
+  }
+  // There is no entry, or no existing entries are alive in
+  // |pending_request_map_|.
+  return true;
+}
+
 bool ResourceLoadScheduler::GetNextPendingRequest(ClientId* id) {
   bool needs_throttling =
       running_throttleable_requests_.size() >= GetOutstandingLimit();
@@ -714,20 +731,23 @@ void ResourceLoadScheduler::ShowConsoleMessageIfNeeded() {
       base::TimeTicks::Now() - base::TimeDelta::FromMinutes(1);
   ThrottleOption target_option;
   if (pending_queue_update_times_[ThrottleOption::kThrottleable] < limit &&
-      !pending_requests_[ThrottleOption::kThrottleable].empty()) {
+      !IsPendingRequestEffectivelyEmpty(ThrottleOption::kThrottleable)) {
     target_option = ThrottleOption::kThrottleable;
   } else if (pending_queue_update_times_[ThrottleOption::kStoppable] < limit &&
-             !pending_requests_[ThrottleOption::kStoppable].empty()) {
+             !IsPendingRequestEffectivelyEmpty(ThrottleOption::kStoppable)) {
     target_option = ThrottleOption::kStoppable;
   } else {
     // At least, one of the top requests in pending queues was handled in the
     // last 1 minutes, or there is no pending requests in the inactive queue.
     return;
   }
-  auto client_it = pending_request_map_.find(
-      pending_requests_[target_option].begin()->client_id);
-  DCHECK_NE(pending_request_map_.end(), client_it);
-  ConsoleLogger* logger = client_it->value->client->GetConsoleLogger();
+  ConsoleLogger* logger = nullptr;
+  for (const auto& client : pending_requests_[target_option]) {
+    auto client_it = pending_request_map_.find(client.client_id);
+    if (pending_request_map_.end() == client_it)
+      continue;
+    logger = client_it->value->client->GetConsoleLogger();
+  }
   DCHECK(logger);
 
   logger->AddInfoMessage(

third_party/blink/renderer/platform/loader/fetch/resource_load_scheduler.h

@@ -249,6 +249,11 @@ class PLATFORM_EXPORT ResourceLoadScheduler final
     int intra_priority;
   };
 
+  // Checks if |pending_requests_| for the specified option is effectively
+  // empty, that means it does not contain any request that is still alive in
+  // |pending_request_map_|.
+  bool IsPendingRequestEffectivelyEmpty(ThrottleOption option);
+
   // Gets the highest priority pending request that is allowed to be run.
   bool GetNextPendingRequest(ClientId* id);
 
@@ -313,11 +318,13 @@ class PLATFORM_EXPORT ResourceLoadScheduler final
     kStopped,
   };
   ThrottlingHistory throttling_history_ = ThrottlingHistory::kInitial;
+
   scheduler::SchedulingLifecycleState frame_scheduler_lifecycle_state_ =
       scheduler::SchedulingLifecycleState::kNotThrottled;
 
   // Holds clients that haven't been granted, and are waiting for a grant.
   HeapHashMap<ClientId, Member<ClientInfo>> pending_request_map_;
+
   // We use std::set here because WTF doesn't have its counterpart.
   // This tracks two sets of requests, throttleable and stoppable.
   std::map<ThrottleOption,