Medium chrome Logic Error 📄 Reporter bug report 🔧 Commit mapped

Overview

Medium
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactInsufficient data validation in Navigation
DescriptionInsufficient data validation in Navigation
ComponentNavigation
Bug ClassLogic Error
Tracker526542464
Fix commitc8e29b40bb26 (chromium/src) +9/-19
CISA KEVNot listed
CreditedGoogle
Disclosed2026-07-08

Changed Functions

FunctionChangeNotes
if
content/browser/renderer_host/render_frame_host_manager.cc
modified
if
content/browser/service_worker/service_worker_client.cc
modified

Files Changed

  • content/browser/renderer_host/navigation_request.cc
  • content/browser/renderer_host/navigation_throttle_runner.cc
  • content/browser/renderer_host/render_frame_host_manager.cc
  • content/browser/renderer_host/render_process_host_impl.cc
  • content/browser/service_worker/service_worker_client.cc
  • content/browser/service_worker/service_worker_version.cc
From c8e29b40bb26677126c41feb50b4bc45265f11c6 Mon Sep 17 00:00:00 2001
From: Arthur Sonzogni <arthursonzogni@chromium.org>
Date: Wed, 24 Jun 2026 11:57:55 -0700
Subject: [PATCH] content: Revert 6 reverted CHECKs

Following the systematic conversion of DCHECKs to CHECKs, several CHECKs
were reverted back to DCHECKs to maintain stability. An audit
(documented in go/content-check-to-dcheck-audit) showed that 6 of these
rollbacks were over-broad, unintentionally downgrading legacy checks
that were originally introduced by other engineers (rather than our
migration project).

This CL relands these 6 checks back to CHECKs, as they protect important
invariants.

Note: These checks are currently failing with a low crash rate,
but we want to keep them for now.

Fixed: 526540644
Fixed: 526543416
Fixed: 526542284
Fixed: 526542430
Fixed: 526538771
Fixed: 526542464
Change-Id: I9fdf51338e67f249e03f0a3f93ab5647c5239d0c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7994875
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1651869}
---

diff --git a/content/browser/renderer_host/navigation_request.cc b/content/browser/renderer_host/navigation_request.cc
index 8c4959d..3662212c 100644
--- a/content/browser/renderer_host/navigation_request.cc
+++ b/content/browser/renderer_host/navigation_request.cc
@@ -10326,9 +10326,7 @@
   // we only update the value for non-pending commit navigations (i.e. the
   // NavigationRequest owned by the FrameTreeNode). See the comments in
   // `RenderFrameHostManager::CommitPendingIfNecessary()` for more details.
-  // TODO(https://crbug.com/526542284): CHECK-exclusion: Convert to CHECK once
-  // we are sure this isn't hit.
-  DCHECK(state_ < READY_TO_COMMIT || state_ == WILL_FAIL_REQUEST)
+  CHECK(state_ < READY_TO_COMMIT || state_ == WILL_FAIL_REQUEST)
       << "Use GetRenderFrameHost() instead when the final RenderFrameHost "
          "for the navigation has been picked";
   return associated_rfh_type_;
diff --git a/content/browser/renderer_host/navigation_throttle_runner.cc b/content/browser/renderer_host/navigation_throttle_runner.cc
index e3b7f0c..f6ca45e 100644
--- a/content/browser/renderer_host/navigation_throttle_runner.cc
+++ b/content/browser/renderer_host/navigation_throttle_runner.cc
@@ -152,9 +152,7 @@
 void NavigationThrottleRunner::ProcessInternal() {
   TRACE_EVENT("navigation", "NavigationThrottleRunner::ProcessInternal",
               "current_event", current_event_);
-  // TODO(https://crbug.com/526542430): CHECK-exclusion: Convert to CHECK once
-  // we are sure this isn't hit.
-  DCHECK_NE(NavigationThrottleEvent::kNoEvent, current_event_);
+  CHECK_NE(NavigationThrottleEvent::kNoEvent, current_event_);
   base::Time start_time = base::Time::Now();
   if (!event_process_start_time_.has_value()) {
     event_process_start_time_ = start_time;
diff --git a/content/browser/renderer_host/render_frame_host_manager.cc b/content/browser/renderer_host/render_frame_host_manager.cc
index 1fd43a4..de22853 100644
--- a/content/browser/renderer_host/render_frame_host_manager.cc
+++ b/content/browser/renderer_host/render_frame_host_manager.cc
@@ -4734,9 +4734,7 @@
       SCOPED_CRASH_KEY_STRING64("Bug1400009", "parent_lifecycle",
                                 RenderFrameHostImpl::LifecycleStateImplToString(
                                     parent_rfh->lifecycle_state()));
-      // TODO(https://crbug.com/526542464): CHECK-exclusion: Convert to CHECK
-      // once we are sure this isn't hit.
-      DCHECK(render_view_host);
+      CHECK(render_view_host);
     }
     if (!render_view_host) {
       // Before creating a new RenderFrameProxyHost, ensure a RenderViewHost
diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc
index 597d251..5dca11edd 100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -1707,9 +1707,8 @@
           perfetto::NamedTrack::FromPointer("RenderProcessHostImpl",
                                             this,
                                             GetChildProcessTracingTrack(id_))) {
-  // TODO(https://crbug.com/526543416): CHECK-exclusion: Convert to CHECK once
-  // we are sure this isn't hit.
-  DCHECK(!browser_context->ShutdownStarted());
+
+  CHECK(!browser_context->ShutdownStarted());
   TRACE_EVENT("shutdown", "RenderProcessHostImpl",
               ChromeTrackEvent::kRenderProcessHost, *this);
   TRACE_EVENT_BEGIN("shutdown", "Browser.RenderProcessHostImpl", tracing_track_,
diff --git a/content/browser/service_worker/service_worker_client.cc b/content/browser/service_worker/service_worker_client.cc
index 2a3ff69..d26e2a0 100644
--- a/content/browser/service_worker/service_worker_client.cc
+++ b/content/browser/service_worker/service_worker_client.cc
@@ -714,9 +714,8 @@
   if (controller_registration) {
     CHECK(IsEligibleForServiceWorkerController());
     CHECK(controller_registration->active_version());
-    // TODO(https://crbug.com/526540644): CHECK-exclusion: Convert to CHECK once
-    // we are sure this isn't hit.
-    DCHECK(IsMatchingRegistration(controller_registration.get()));
+
+    CHECK(IsMatchingRegistration(controller_registration.get()));
   }
 
   controller_registration_ = controller_registration;
diff --git a/content/browser/service_worker/service_worker_version.cc b/content/browser/service_worker/service_worker_version.cc
index 72983d1..29c27b3 100644
--- a/content/browser/service_worker/service_worker_version.cc
+++ b/content/browser/service_worker/service_worker_version.cc
@@ -1107,10 +1107,8 @@
 
 void ServiceWorkerVersion::RestoreControlleeFromBackForwardCacheMap(
     const std::string& client_uuid) {
-  // TODO(https://crbug.com/526538771): CHECK-exclusion: Convert to CHECK once
-  // we are sure this isn't hit.
-  DCHECK(IsBackForwardCacheEnabled());
-  DCHECK(!controllee_map_.contains(client_uuid));
+  CHECK(IsBackForwardCacheEnabled());
+  CHECK(!controllee_map_.contains(client_uuid));
   if (!bfcached_controllee_map_.contains(client_uuid)) {
     // We are navigating to the page using BackForwardCache, which is being
     // evicted due to activation, postMessage or claim. In this case, we reload
Loading diff…

Original Bug Report

reported by ar...@chromium.org

[DCHECK-to-CHECK] RenderFrameHostManager::CreateRenderFrameProxy, failing DCHECK(render_view_host);

This bug is part of the project: http://go/check-content-navigation (Systematic Conversion of DCHECKs to CHECKs in Core Navigation).

This specific DCHECK was previously upgraded to a CHECK, but had to be reverted back to a DCHECK because it was reached in the wild (which occurred in about 15% of the converted instances).

This bug is tracked to resolve this specific invariant failure so we can eventually upgrade it back to a CHECK.

Context:

  • File: content/browser/renderer_host/render_frame_host_manager.cc:4758
  • Function: RenderFrameHostManager::CreateRenderFrameProxy
  • Source Link
   4753:                             !!frame_tree_node_->frame_tree().GetRenderViewHost(
   4754:                                 parent_rfh->GetSiteInstance()->group()));
   4755:       SCOPED_CRASH_KEY_STRING64("Bug1400009", "parent_lifecycle",
   4756:                                 RenderFrameHostImpl::LifecycleStateImplToString(
   4757:                                     parent_rfh->lifecycle_state()));
=> 4758:       // TODO(https://crbug.com/526542464): CHECK-exclusion: Convert to CHECK
   4759:       // once we are sure this isn't hit.
   4760:       DCHECK(render_view_host);
   4761:     }
   4762:     if (!render_view_host) {
   4763:       // Before creating a new RenderFrameProxyHost, ensure a RenderViewHost
   4764:       // exists for |group|, as it creates the page level structure in Blink.
   4765:       render_view_host = frame_tree_node_->frame_tree().CreateRenderViewHost(
View on issue tracker