Low chrome Logic Error 🔧 Commit mapped

Overview

Low
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactObservable discrepancy in Glic
DescriptionObservable discrepancy in Glic
ComponentGlic
Bug ClassLogic Error
Tracker506539337
Fix commit6937cb76b634 (chromium/src) +169/-56
CISA KEVNot listed
CreditedGoogle
Disclosed2026-08-25

Changed Functions

FunctionChangeNotes
RegisterFrameCallback
third_party/blink/renderer/core/dom/frame_request_callback_collection.cc
modified
if
third_party/blink/renderer/core/dom/frame_request_callback_collection.cc
modified
for
third_party/blink/renderer/core/dom/frame_request_callback_collection.cc
modified

Files Changed

  • third_party/blink/renderer/core/animation_frame/worker_animation_frame_provider.cc
  • third_party/blink/renderer/core/dom/document.cc
  • third_party/blink/renderer/core/dom/document.h
  • third_party/blink/renderer/core/dom/frame_request_callback_collection.cc
From 6937cb76b634b8f44877a3e9443b30baddf28c5f Mon Sep 17 00:00:00 2001
From: William Liu <[email protected]>
Date: Mon, 27 Jul 2026 11:02:16 -0700
Subject: [PATCH] [blink] Distinguish between Web and internal requestAnimationFrame

Fixed: b:506539337
Change-Id: I891083a789417a1d9317b95223091d3c65f30bc1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8144040
Reviewed-by: Adithya Srinivasan <[email protected]>
Commit-Queue: William Liu <[email protected]>
Reviewed-by: Philip Rogers <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1668807}
---

diff --git a/third_party/blink/renderer/core/animation_frame/worker_animation_frame_provider.cc b/third_party/blink/renderer/core/animation_frame/worker_animation_frame_provider.cc
index 7a646f03..2a9b35f 100644
--- a/third_party/blink/renderer/core/animation_frame/worker_animation_frame_provider.cc
+++ b/third_party/blink/renderer/core/animation_frame/worker_animation_frame_provider.cc
@@ -32,13 +32,14 @@
   }
 
   FrameRequestCallbackCollection::CallbackId id =
-      callback_collection_.RegisterFrameCallback(callback);
+      callback_collection_.RegisterFrameCallback(
+          callback, FrameCallbackType::kWebExposed);
   begin_frame_provider_->RequestBeginFrame();
   return id;
 }
 
 void WorkerAnimationFrameProvider::CancelCallback(int id) {
-  callback_collection_.CancelFrameCallback(id);
+  callback_collection_.CancelFrameCallback(id, FrameCallbackType::kWebExposed);
 }
 
 void WorkerAnimationFrameProvider::BeginFrame(const viz::BeginFrameArgs& args) {
diff --git a/third_party/blink/renderer/core/dom/document.cc b/third_party/blink/renderer/core/dom/document.cc
index 460ac2eb..e3961a3 100644
--- a/third_party/blink/renderer/core/dom/document.cc
+++ b/third_party/blink/renderer/core/dom/document.cc
@@ -8849,12 +8849,13 @@
   return *scripted_animation_controller_;
 }
 
-int Document::RequestAnimationFrame(FrameCallback* callback) {
-  return scripted_animation_controller_->RegisterFrameCallback(callback);
+int Document::RequestAnimationFrame(FrameCallback* callback,
+                                    FrameCallbackType type) {
+  return scripted_animation_controller_->RegisterFrameCallback(callback, type);
 }
 
-void Document::CancelAnimationFrame(int id) {
-  scripted_animation_controller_->CancelFrameCallback(id);
+void Document::CancelAnimationFrame(int id, FrameCallbackType type) {
+  scripted_animation_controller_->CancelFrameCallback(id, type);
 }
 
 DocumentLoader* Document::Loader() const {
diff --git a/third_party/blink/renderer/core/dom/document.h b/third_party/blink/renderer/core/dom/document.h
index 6e735c85..a6c0c2e0 100644
--- a/third_party/blink/renderer/core/dom/document.h
+++ b/third_party/blink/renderer/core/dom/document.h
@@ -75,6 +75,7 @@
 #include "third_party/blink/renderer/core/dom/element.h"
 #include "third_party/blink/renderer/core/dom/events/event_path.h"
 #include "third_party/blink/renderer/core/dom/focus_params.h"
+#include "third_party/blink/renderer/core/dom/frame_request_callback_collection.h"
 #include "third_party/blink/renderer/core/dom/live_node_list_registry.h"
 #include "third_party/blink/renderer/core/dom/node_list_invalidation_type.h"
 #include "third_party/blink/renderer/core/dom/qualified_name.h"
@@ -1660,8 +1661,8 @@
            IsInOutermostMainFrame();
   }
 
-  int RequestAnimationFrame(FrameCallback*);
-  void CancelAnimationFrame(int id);
+  int RequestAnimationFrame(FrameCallback*, FrameCallbackType type);
+  void CancelAnimationFrame(int id, FrameCallbackType type);
 
   ScriptedAnimationController& GetScriptedAnimationController();
 
diff --git a/third_party/blink/renderer/core/dom/frame_request_callback_collection.cc b/third_party/blink/renderer/core/dom/frame_request_callback_collection.cc
index d20b374..1b6f2302 100644
--- a/third_party/blink/renderer/core/dom/frame_request_callback_collection.cc
+++ b/third_party/blink/renderer/core/dom/frame_request_callback_collection.cc
@@ -16,12 +16,18 @@
     : context_(context) {}
 
 FrameRequestCallbackCollection::CallbackId
-FrameRequestCallbackCollection::RegisterFrameCallback(FrameCallback* callback) {
-  FrameRequestCallbackCollection::CallbackId id = ++next_callback_id_;
+FrameRequestCallbackCollection::RegisterFrameCallback(FrameCallback* callback,
+                                                      FrameCallbackType type) {
+  FrameRequestCallbackCollection::CallbackId id =
+      (type == FrameCallbackType::kInternal) ? ++next_internal_callback_id_
+                                             : ++next_callback_id_;
   callback->SetIsCancelled(false);
   callback->SetId(id);
-  frame_callbacks_.push_back(callback);
-
+  if (type == FrameCallbackType::kInternal) {
+    internal_frame_callbacks_.push_back(callback);
+  } else {
+    frame_callbacks_.push_back(callback);
+  }
   DEVTOOLS_TIMELINE_TRACE_EVENT_INSTANT("RequestAnimationFrame",
                                         inspector_animation_frame_event::Data,
                                         context_, id);
@@ -30,19 +36,28 @@
   return id;
 }
 
-void FrameRequestCallbackCollection::CancelFrameCallback(CallbackId id) {
-  for (wtf_size_t i = 0; i < frame_callbacks_.size(); ++i) {
-    if (frame_callbacks_[i]->Id() == id) {
-      frame_callbacks_[i]->async_task_context()->Cancel();
+void FrameRequestCallbackCollection::CancelFrameCallback(
+    CallbackId id,
+    FrameCallbackType type) {
+  auto& callbacks = (type == FrameCallbackType::kInternal)
+                        ? internal_frame_callbacks_
+                        : frame_callbacks_;
+  auto& callbacks_to_invoke = (type == FrameCallbackType::kInternal)
+                                  ? internal_callbacks_to_invoke_
+                                  : callbacks_to_invoke_;
+
+  for (wtf_size_t i = 0; i < callbacks.size(); ++i) {
+    if (callbacks[i]->Id() == id) {
+      callbacks[i]->async_task_context()->Cancel();
       probe::BreakableLocation(context_, "cancelAnimationFrame");
-      frame_callbacks_.EraseAt(i);
+      callbacks.EraseAt(i);
       DEVTOOLS_TIMELINE_TRACE_EVENT_INSTANT(
           "CancelAnimationFrame", inspector_animation_frame_event::Data,
           context_.Get(), id);
       return;
     }
   }
-  for (const auto& callback : callbacks_to_invoke_) {
+  for (const auto& callback : callbacks_to_invoke) {
     if (callback->Id() == id) {
       callback->async_task_context()->Cancel();
       probe::BreakableLocation(context_, "cancelAnimationFrame");
@@ -64,19 +79,32 @@
   ExecutionContext::ScopedRequestAnimationFrameStatus scoped_raf_status(
       context_);
 
-  // First, generate a list of callbacks to consider.  Callbacks registered from
-  // this point on are considered only for the "next" frame, not this one.
+  // First, snapshot both lists. Callbacks registered from this point on are
+  // considered only for the "next" frame, not this one.
   DCHECK(callbacks_to_invoke_.empty());
+  DCHECK(internal_callbacks_to_invoke_.empty());
   swap(callbacks_to_invoke_, frame_callbacks_);
+  swap(internal_callbacks_to_invoke_, internal_frame_callbacks_);
 
-  for (const auto& callback : callbacks_to_invoke_) {
+  ExecuteFrameCallbacksImpl(callbacks_to_invoke_, high_res_now_ms,
+                            high_res_now_ms_legacy);
+  ExecuteFrameCallbacksImpl(internal_callbacks_to_invoke_, high_res_now_ms,
+                            high_res_now_ms_legacy);
+}
+
+void FrameRequestCallbackCollection::ExecuteFrameCallbacksImpl(
+    CallbackList& callbacks_to_invoke,
+    double high_res_now_ms,
+    double high_res_now_ms_legacy) {
+  for (const auto& callback : callbacks_to_invoke) {
     // When the ExecutionContext is destroyed (e.g. an iframe is detached),
     // there is no path to perform wrapper tracing for the callbacks. In such a
     // case, the callback functions may already have been collected by V8 GC.
     // Since it's possible that a callback function being invoked detaches an
     // iframe, we need to check the condition for each callback.
-    if (context_->IsContextDestroyed())
+    if (context_->IsContextDestroyed()) {
       break;
+    }
     if (callback->IsCancelled()) {
       // Another requestAnimationFrame callback already cancelled this one
       UseCounter::Count(context_,
@@ -89,18 +117,21 @@
     probe::AsyncTask async_task(context_, callback->async_task_context());
     probe::UserCallback probe(context_, "requestAnimationFrame", AtomicString(),
                               true);
-    if (callback->GetUseLegacyTimeBase())
+    if (callback->GetUseLegacyTimeBase()) {
       callback->Invoke(high_res_now_ms_legacy);
-    else
+    } else {
       callback->Invoke(high_res_now_ms);
+    }
   }
 
-  callbacks_to_invoke_.clear();
+  callbacks_to_invoke.clear();
 }
 
 void FrameRequestCallbackCollection::Trace(Visitor* visitor) const {
   visitor->Trace(frame_callbacks_);
   visitor->Trace(callbacks_to_invoke_);
+  visitor->Trace(internal_frame_callbacks_);
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/third_party/blink/renderer/core/dom/scripted_animation_controller_test.cc b/third_party/blink/renderer/core/dom/scripted_animation_controller_test.cc
index c4001c15..b5473fe 100644
--- a/third_party/blink/renderer/core/dom/scripted_animation_controller_test.cc
+++ b/third_party/blink/renderer/core/dom/scripted_animation_controller_test.cc
@@ -184,7 +184,8 @@
   event->SetTarget(&GetDocument());
 
   Controller().RegisterFrameCallback(
-      MakeGarbageCollected<RunTaskCallback>(observer.CreateTask(1)));
+      MakeGarbageCollected<RunTaskCallback>(observer.CreateTask(1)),
+      FrameCallbackType::kWebExposed);
   Controller().EnqueueTask(observer.CreateTask(2));
   EXPECT_EQ(0u, observer.Order().size());
 
@@ -199,19 +200,22 @@
   TaskOrderObserver observer;
 
   Controller().RegisterFrameCallback(
-      MakeGarbageCollected<RunTaskCallback>(observer.CreateTask(1)));
+      MakeGarbageCollected<RunTaskCallback>(observer.CreateTask(1)),
+      FrameCallbackType::kWebExposed);
   EXPECT_TRUE(Controller().HasFrameCallback());
 
-  Controller().CancelFrameCallback(1);
+  Controller().CancelFrameCallback(1, FrameCallbackType::kWebExposed);
   EXPECT_FALSE(Controller().HasFrameCallback());
 
   Controller().RegisterFrameCallback(
-      MakeGarbageCollected<RunTaskCallback>(observer.CreateTask(1)));
+      MakeGarbageCollected<RunTaskCallback>(observer.CreateTask(1)),
+      FrameCallbackType::kWebExposed);
   Controller().RegisterFrameCallback(
-      MakeGarbageCollected<RunTaskCallback>(observer.CreateTask(2)));
+      MakeGarbageCollected<RunTaskCallback>(observer.CreateTask(2)),
+      FrameCallbackType::kWebExposed);
   EXPECT_TRUE(Controller().HasFrameCallback());
 
-  Controller().CancelFrameCallback(1);
+  Controller().CancelFrameCallback(1, FrameCallbackType::kWebExposed);
   EXPECT_TRUE(Controller().HasFrameCallback());
 
   // Servicing the scripted animations should call the remaining callback and
@@ -233,7 +237,8 @@
             *ran_callback = true;
           },
           WrapPersistent(&Controller()),
-          blink::subtle::UnretainedException(&ran_callback))));
+          blink::subtle::UnretainedException(&ran_callback))),
+      FrameCallbackType::kWebExposed);
 
   PageAnimator::ServiceScriptedAnimations(base::TimeTicks(),
                                           {{Controller(), false}});
@@ -242,4 +247,52 @@
   EXPECT_FALSE(Controller().GetExecutionContext()->IsInRequestAnimationFrame());
 }
 
+TEST_F(ScriptedAnimationControllerTest, TestInternalCallbackIsolation) {
+  TaskOrderObserver observer;
+
+  // Web-exposed callback registration gets ID 1 in web pool.
+  int web_id = Controller().RegisterFrameCallback(
+      MakeGarbageCollected<RunTaskCallback>(observer.CreateTask(1)),
+      FrameCallbackType::kWebExposed);
+  EXPECT_EQ(1, web_id);
+
+  // Internal callback registration gets independent ID 1 in internal pool.
+  int internal_id = Controller().RegisterFrameCallback(
+      MakeGarbageCollected<RunTaskCallback>(observer.CreateTask(2)),
+      FrameCallbackType::kInternal);
+  EXPECT_EQ(1, internal_id);
+
+  // Canceling internal callback with FrameCallbackType::kInternal should cancel
+  // internal without affecting web callback.
+  Controller().CancelFrameCallback(internal_id, FrameCallbackType::kInternal);
+
+  // Servicing scripted animations should run only the web callback.
+  PageAnimator::ServiceScriptedAnimations(base::TimeTicks(),
+                                          {{Controller(), false}});
+  EXPECT_EQ(1u, observer.Order().size());
+  EXPECT_EQ(1, observer.Order()[0]);
+}
+
+TEST_F(ScriptedAnimationControllerTest,
+       TestCancelWebCallbackDoesNotCancelInternalCallback) {
+  TaskOrderObserver observer;
+
+  int web_id = Controller().RegisterFrameCallback(
+      MakeGarbageCollected<RunTaskCallback>(observer.CreateTask(1)),
+      FrameCallbackType::kWebExposed);
+  Controller().RegisterFrameCallback(
+      MakeGarbageCollected<RunTaskCallback>(observer.CreateTask(2)),
+      FrameCallbackType::kInternal);
+
+  // CancelFrameCallback with kWebExposed cancels web callback without
+  // affecting internal callback.
+  Controller().CancelFrameCallback(web_id, FrameCallbackType::kWebExposed);
+
+  // Servicing scripted animations should run only the internal callback.
+  PageAnimator::ServiceScriptedAnimations(base::TimeTicks(),
+                                          {{Controller(), false}});
+  EXPECT_EQ(1u, observer.Order().size());
+  EXPECT_EQ(2, observer.Order()[0]);
+}
+
 }  // namespace blink
diff --git a/third_party/blink/renderer/core/frame/document_loading_rendering_test.cc b/third_party/blink/renderer/core/frame/document_loading_rendering_test.cc
index 786c949..223d8dc4 100644
--- a/third_party/blink/renderer/core/frame/document_loading_rendering_test.cc
+++ b/third_party/blink/renderer/core/frame/document_loading_rendering_test.cc
@@ -343,7 +343,8 @@
 
   // Frame while the child frame still has pending sheets.
   auto* frame1_callback = MakeGarbageCollected<CheckRafCallback>();
-  child_frame->contentDocument()->RequestAnimationFrame(frame1_callback);
+  child_frame->contentDocument()->RequestAnimationFrame(
+      frame1_callback, FrameCallbackType::kWebExposed);
   auto frame1 = Compositor().BeginFrame();
   EXPECT_FALSE(frame1_callback->WasCalled());
   EXPECT_TRUE(frame1.Contains(SimCanvas::kRect, "red"));
@@ -356,7 +357,8 @@
 
   // Frame with all lifecycle updates enabled.
   auto* frame2_callback = MakeGarbageCollected<CheckRafCallback>();
-  child_frame->contentDocument()->RequestAnimationFrame(frame2_callback);
+  child_frame->contentDocument()->RequestAnimationFrame(
+      frame2_callback, FrameCallbackType::kWebExposed);
   auto frame2 = Compositor().BeginFrame();
   EXPECT_TRUE(frame1_callback->WasCalled());
   EXPECT_TRUE(frame2_callback->WasCalled());
Loading diff…

Original Bug Report

The reporter's bug is still restricted on the tracker. Chrome de-restricts security bugs ~30–90 days after the fix ships; a later run will backfill it here.