From 7710041f98212312f5810c831adca08157348975 Mon Sep 17 00:00:00 2001
From: Morten Stenshorne <[email protected]>
Date: Wed, 10 Dec 2025 08:53:37 -0800
Subject: [PATCH] [RouteMatching] Avoid recursion.

If another route was added in a route event handler, we'd end up
invoking RouteMap::UpdateMatchStatus() while already inside it. This is
bad. Fixing it by making a local copy of the routes and then walking
that set in UpdateMatchStatus() to prevent the set from being modified
(rehashed) while iterating it might seem compelling, but recursion here
doesn't seem good for correctness and code maintainability anyway.

Instead, fire route events as a separate step after having updated the
match status for all routes. Also do NOT trigger UpdateMatchStatus()
when adding new routes from <script>. Just update the match status with
no events being fired. Be sure to trigger a lifecycle update if anything
has changed, though.

<script type="routemap"> may be going away, in favor of the new @route
rule. See https://drafts.csswg.org/css-navigation-1/#at-route , and as
part of that task we should integrate route matching nicely into the
document lifecycle machinery.

Also add a DCHECK that fails on recursion.

Bug: 467297219
Change-Id: I604d3b29bf4a1cabf83a5c13f32c1dc01dbe84e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7246274
Commit-Queue: Morten Stenshorne <[email protected]>
Reviewed-by: Noam Rosenthal <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1556793}
---

diff --git a/third_party/blink/renderer/core/route_matching/route.cc b/third_party/blink/renderer/core/route_matching/route.cc
index 07987d6..bff1cbc 100644
--- a/third_party/blink/renderer/core/route_matching/route.cc
+++ b/third_party/blink/renderer/core/route_matching/route.cc
@@ -9,7 +9,6 @@
 #include "third_party/blink/renderer/core/dom/document.h"
 #include "third_party/blink/renderer/core/event_target_names.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
-#include "third_party/blink/renderer/core/route_matching/route_event.h"
 #include "third_party/blink/renderer/core/url_pattern/url_pattern.h"
 #include "third_party/blink/renderer/platform/weborigin/kurl.h"
 
@@ -78,10 +77,6 @@
   }
 
   matches_at_ = matches_at;
-  AtomicString type(matches_at_ ? "activate" : "deactivate");
-  auto* event = MakeGarbageCollected<RouteEvent>(type);
-  event->SetTarget(this);
-  DispatchEvent(*event);
   return true;
 }
 
diff --git a/third_party/blink/renderer/core/route_matching/route_map.cc b/third_party/blink/renderer/core/route_matching/route_map.cc
index 9e0c514..338e41e 100644
--- a/third_party/blink/renderer/core/route_matching/route_map.cc
+++ b/third_party/blink/renderer/core/route_matching/route_map.cc
@@ -4,11 +4,13 @@
 
 #include "third_party/blink/renderer/core/route_matching/route_map.h"
 
+#include "base/auto_reset.h"
 #include "base/check_is_test.h"
 #include "third_party/blink/renderer/core/css/style_engine.h"
 #include "third_party/blink/renderer/core/dom/document.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
 #include "third_party/blink/renderer/core/route_matching/route.h"
+#include "third_party/blink/renderer/core/route_matching/route_event.h"
 #include "third_party/blink/renderer/core/url_pattern/url_pattern.h"
 #include "third_party/blink/renderer/core/url_pattern/url_pattern_utils.h"
 #include "third_party/blink/renderer/platform/json/json_parser.h"
@@ -18,20 +20,6 @@
 
 namespace {
 
-RouteMap::ParseResult AddPatternToRoute(const Document& document,
-                                        Route& route,
-                                        const JSONValue& value) {
-  base::expected<URLPattern*, String> pattern =
-      ParseURLPatternFromJSON(document.GetExecutionContext()->GetIsolate(),
-                              value, document.Url(), IGNORE_EXCEPTION);
-  if (pattern.has_value()) {
-    DCHECK(*pattern);
-    route.AddPattern(*pattern);
-    return RouteMap::ParseResult(RouteMap::ParseResult::kSuccess);
-  }
-  return RouteMap::ParseResult(RouteMap::ParseResult::kSyntaxError,
-                               pattern.error());
-}
 
 }  // anonymous namespace
 
@@ -80,12 +68,6 @@
 
 RouteMap::ParseResult RouteMap::ParseAndApplyRoutes(
     const String& route_map_text) {
-  RouteMap::ParseResult result = ParseRoutes(route_map_text);
-  UpdateActiveRoutes();
-  return result;
-}
-
-RouteMap::ParseResult RouteMap::ParseRoutes(const String& route_map_text) {
   constexpr char kPattern[] = "pattern";
   std::unique_ptr<JSONValue> value = ParseJSON(route_map_text);
   // TODO(crbug.com/436805487): Error reporting needs to be specced. Should we
@@ -136,8 +118,7 @@
                              "Missing pattern in route entry");
         }
         for (const JSONValue& pattern : *patterns) {
-          ParseResult result =
-              AddPatternToRoute(GetDocument(), *route, pattern);
+          ParseResult result = AddPatternToRoute(*route, pattern);
           if (!result.IsSuccess()) {
             return result;
           }
@@ -149,7 +130,7 @@
           return ParseResult(ParseResult::kTypeError,
                              "Missing pattern in route entry");
         }
-        ParseResult result = AddPatternToRoute(GetDocument(), *route, *pattern);
+        ParseResult result = AddPatternToRoute(*route, *pattern);
         if (!result.IsSuccess()) {
           return result;
         }
@@ -188,15 +169,30 @@
 }
 
 void RouteMap::UpdateActiveRoutes() {
+#if DCHECK_IS_ON()
+  DCHECK(!is_updating_active_routes_);
+  base::AutoReset<bool> is_updating(&is_updating_active_routes_, true);
+#endif
+
+  HeapVector<Member<Route>> routes_needing_event;
   bool changed = false;
   for (const auto& entry : routes_) {
     Route& route = *entry.value;
-    changed = route.UpdateMatchStatus(previous_url_, next_url_) || changed;
+    changed |= UpdateMatchStatus(route, &routes_needing_event);
   }
   for (const auto& entry : anonymous_routes_) {
     Route& route = *entry.value;
-    changed = route.UpdateMatchStatus(previous_url_, next_url_) || changed;
+    changed |= UpdateMatchStatus(route, &routes_needing_event);
   }
+
+  for (Route* route : routes_needing_event) {
+    bool matches_at = route->Matches(NavigationPreposition::kAt);
+    AtomicString type(matches_at ? "activate" : "deactivate");
+    auto* event = MakeGarbageCollected<RouteEvent>(type);
+    event->SetTarget(route);
+    route->DispatchEvent(*event);
+  }
+
   if (changed) {
     GetDocument().GetStyleEngine().NavigationsMayHaveChanged();
   }
@@ -220,4 +216,36 @@
   }
 }
 
+RouteMap::ParseResult RouteMap::AddPatternToRoute(Route& route,
+                                                  const JSONValue& value) {
+  base::expected<URLPattern*, String> pattern =
+      ParseURLPatternFromJSON(GetDocument().GetExecutionContext()->GetIsolate(),
+                              value, GetDocument().Url(), IGNORE_EXCEPTION);
+  if (pattern.has_value()) {
+    DCHECK(*pattern);
+    route.AddPattern(*pattern);
+    // TODO(crbug.com/436805487): If we actually end up keeping support for
+    // <script type="routemap">, we're missing events here.
+    if (route.UpdateMatchStatus(previous_url_, next_url_)) {
+      GetDocument().GetStyleEngine().NavigationsMayHaveChanged();
+    }
+    return RouteMap::ParseResult(RouteMap::ParseResult::kSuccess);
+  }
+  return RouteMap::ParseResult(RouteMap::ParseResult::kSyntaxError,
+                               pattern.error());
+}
+
+bool RouteMap::UpdateMatchStatus(
+    Route& route,
+    HeapVector<Member<Route>>* routes_needing_event) {
+  bool matched_at = route.Matches(NavigationPreposition::kAt);
+  if (!route.UpdateMatchStatus(previous_url_, next_url_)) {
+    return false;
+  }
+  if (matched_at != route.Matches(NavigationPreposition::kAt)) {
+    routes_needing_event->push_back(&route);
+  }
+  return true;
+}
+
 }  // namespace blink
diff --git a/third_party/blink/renderer/core/route_matching/route_map.h b/third_party/blink/renderer/core/route_matching/route_map.h
index 6023f3e..843ea1d 100644
--- a/third_party/blink/renderer/core/route_matching/route_map.h

diff --git a/third_party/blink/web_tests/wpt_internal/route/crashtests/add-route-on-activate.html b/third_party/blink/web_tests/wpt_internal/route/crashtests/add-route-on-activate.html
new file mode 100644
index 0000000..454b55d
--- /dev/null
+++ b/third_party/blink/web_tests/wpt_internal/route/crashtests/add-route-on-activate.html
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<link rel="help" href="https://issues.chromium.org/issues/467297219">
+<script type="routemap">
+  {
+    "routes": [
+      {
+        "name": "initial-route",
+        "pattern": { "pathname": "/activate-me" }
+      }
+    ]
+  }
+</script>
+
+PASS
+
+<script>
+  const DYNAMIC_JSON = JSON.stringify({
+    "routes": [
+      {
+        "name": "dynamic-route",
+        "pattern": { "pathname": "/dynamic-path" }
+      }
+    ]
+  });
+
+  const route = document.routeMap.get('initial-route');
+  route.addEventListener('activate', (e) => {
+    const script = document.createElement('script');
+    script.type = "routemap";
+    script.textContent = DYNAMIC_JSON;
+    document.head.appendChild(script);
+  });
+  history.pushState({}, '', '/activate-me');
+</script>