From 4fa8d897f83ab2ee66dc3505409a6e44936d7e4c Mon Sep 17 00:00:00 2001
From: Philip Pfaffe <[email protected]>
Date: Wed, 26 Feb 2025 09:16:42 -0800
Subject: [PATCH] Fix crash in DOM snapshot agent

When taking as snapshot, the DOM snapshot agent holds a pointer to a
hashmap on the stack. The hashmap is destroyed when the devtools session
detaches, so if that happens while taking a snapshot we run into a
crash.

Fixed: 395032416
Change-Id: I4b784cb3105f6a760ca5b1d3dcbe1ea254b3ad3a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6297444
Reviewed-by: Alex Rudenko <[email protected]>
Commit-Queue: Philip Pfaffe <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1425198}
---

diff --git a/third_party/blink/renderer/core/inspector/inspector_dom_snapshot_agent.cc b/third_party/blink/renderer/core/inspector/inspector_dom_snapshot_agent.cc
index 4207bc5..4b7b8c54 100644
--- a/third_party/blink/renderer/core/inspector/inspector_dom_snapshot_agent.cc
+++ b/third_party/blink/renderer/core/inspector/inspector_dom_snapshot_agent.cc
@@ -214,13 +214,13 @@
     CharacterData* character_data) {
   String origin_url = GetOriginUrl(character_data);
   if (origin_url)
-    origin_url_map_->insert(character_data->GetDomNodeId(), origin_url);
+    origin_url_map_->map.insert(character_data->GetDomNodeId(), origin_url);
 }
 
 void InspectorDOMSnapshotAgent::DidInsertDOMNode(Node* node) {
   String origin_url = GetOriginUrl(node);
   if (origin_url)
-    origin_url_map_->insert(node->GetDomNodeId(), origin_url);
+    origin_url_map_->map.insert(node->GetDomNodeId(), origin_url);
 }
 
 void InspectorDOMSnapshotAgent::EnableAndReset() {
@@ -264,8 +264,10 @@
   Document* document = inspected_frames_->Root()->GetDocument();
   if (!document)
     return protocol::Response::ServerError("Document is not available");
-  LegacyDOMSnapshotAgent legacySupport(dom_debugger_agent_,
-                                       origin_url_map_.get());
+  LegacyDOMSnapshotAgent legacySupport(
+      dom_debugger_agent_, origin_url_map_
+                               ? origin_url_map_->weak_ptr_factory.GetWeakPtr()
+                               : base::WeakPtr<OriginUrlMap>());
   return legacySupport.GetSnapshot(
       document, std::move(style_filter), std::move(include_event_listeners),
       std::move(include_paint_order), std::move(include_user_agent_shadow_tree),
@@ -517,16 +519,17 @@
       BuildArrayForElementAttributes(node));
   BuildLayoutTreeNode(node->GetLayoutObject(), node, index, contrast);
 
-  if (origin_url_map_ && origin_url_map_->Contains(backend_node_id)) {
-    String origin_url = origin_url_map_->at(backend_node_id);
+  if (origin_url_map_ && origin_url_map_->map.Contains(backend_node_id)) {
+    String origin_url = origin_url_map_->map.at(backend_node_id);
     // In common cases, it is implicit that a child node would have the same
     // origin url as its parent, so no need to mark twice.
     if (!node->parentNode()) {
       SetRare(nodes->getOriginURL(nullptr), index, std::move(origin_url));
     } else {
       DOMNodeId parent_id = node->parentNode()->GetDomNodeId();
-      auto it = origin_url_map_->find(parent_id);
-      String parent_url = it != origin_url_map_->end() ? it->value : String();
+      auto it = origin_url_map_->map.find(parent_id);
+      String parent_url =
+          it != origin_url_map_->map.end() ? it->value : String();
       if (parent_url != origin_url)
         SetRare(nodes->getOriginURL(nullptr), index, std::move(origin_url));
     }
diff --git a/third_party/blink/renderer/core/inspector/inspector_dom_snapshot_agent.h b/third_party/blink/renderer/core/inspector/inspector_dom_snapshot_agent.h
index 3c57106..8e1a146fa 100644
--- a/third_party/blink/renderer/core/inspector/inspector_dom_snapshot_agent.h
+++ b/third_party/blink/renderer/core/inspector/inspector_dom_snapshot_agent.h
@@ -5,6 +5,7 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_INSPECTOR_INSPECTOR_DOM_SNAPSHOT_AGENT_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_INSPECTOR_INSPECTOR_DOM_SNAPSHOT_AGENT_H_
 
+#include "base/memory/weak_ptr.h"
 #include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/core/css/css_property_names.h"
 #include "third_party/blink/renderer/core/inspector/inspector_base_agent.h"
@@ -25,6 +26,7 @@
 class InspectedFrames;
 class Node;
 class PaintLayer;
+struct OriginUrlMap;
 
 class CORE_EXPORT InspectorDOMSnapshotAgent final
     : public InspectorBaseAgent<protocol::DOMSnapshot::Metainfo> {
@@ -105,7 +107,6 @@
   static void VisitPaintLayer(PaintLayer*, PaintOrderMap* paint_order_map);
 
   using CSSPropertyFilter = Vector<const CSSProperty*>;
-  using OriginUrlMap = WTF::HashMap<DOMNodeId, String>;
 
   // State of current snapshot.
   std::unique_ptr<protocol::Array<protocol::DOMSnapshot::DOMNode>> dom_nodes_;
diff --git a/third_party/blink/renderer/core/inspector/legacy_dom_snapshot_agent.cc b/third_party/blink/renderer/core/inspector/legacy_dom_snapshot_agent.cc
index 472959e..def78f98 100644
--- a/third_party/blink/renderer/core/inspector/legacy_dom_snapshot_agent.cc
+++ b/third_party/blink/renderer/core/inspector/legacy_dom_snapshot_agent.cc
@@ -97,8 +97,8 @@
 
 LegacyDOMSnapshotAgent::LegacyDOMSnapshotAgent(
     InspectorDOMDebuggerAgent* dom_debugger_agent,
-    OriginUrlMap* origin_url_map)
-    : origin_url_map_(origin_url_map),
+    base::WeakPtr<OriginUrlMap> origin_url_map)
+    : origin_url_map_(std::move(origin_url_map)),
       dom_debugger_agent_(dom_debugger_agent) {}
 
 LegacyDOMSnapshotAgent::~LegacyDOMSnapshotAgent() = default;
@@ -186,16 +186,18 @@
           .setBackendNodeId(IdentifiersFactory::IntIdForNode(node))
           .build();
   if (origin_url_map_ &&
-      origin_url_map_->Contains(owned_value->getBackendNodeId())) {
-    String origin_url = origin_url_map_->at(owned_value->getBackendNodeId());
+      origin_url_map_->map.Contains(owned_value->getBackendNodeId())) {
+    String origin_url =
+        origin_url_map_->map.at(owned_value->getBackendNodeId());
     // In common cases, it is implicit that a child node would have the same
     // origin url as its parent, so no need to mark twice.
     if (!node->parentNode()) {
       owned_value->setOriginURL(std::move(origin_url));
     } else {
       DOMNodeId parent_id = node->parentNode()->GetDomNodeId();
-      auto it = origin_url_map_->find(parent_id);
-      String parent_url = it != origin_url_map_->end() ? it->value : String();
+      auto it = origin_url_map_->map.find(parent_id);
+      String parent_url =
+          it != origin_url_map_->map.end() ? it->value : String();
       if (parent_url != origin_url)
         owned_value->setOriginURL(std::move(origin_url));
     }
diff --git a/third_party/blink/renderer/core/inspector/legacy_dom_snapshot_agent.h b/third_party/blink/renderer/core/inspector/legacy_dom_snapshot_agent.h
index 13c764b1..96a2687 100644
--- a/third_party/blink/renderer/core/inspector/legacy_dom_snapshot_agent.h
+++ b/third_party/blink/renderer/core/inspector/legacy_dom_snapshot_agent.h
@@ -22,12 +22,17 @@
 class Node;
 class PaintLayer;
 
+struct OriginUrlMap {
+  WTF::HashMap<DOMNodeId, String> map;
+  base::WeakPtrFactory<OriginUrlMap> weak_ptr_factory{this};
+};
+
 class CORE_EXPORT LegacyDOMSnapshotAgent {
   STACK_ALLOCATED();
 
  public:
-  using OriginUrlMap = WTF::HashMap<DOMNodeId, String>;
-  LegacyDOMSnapshotAgent(InspectorDOMDebuggerAgent*, OriginUrlMap*);
+  LegacyDOMSnapshotAgent(InspectorDOMDebuggerAgent*,
+                         base::WeakPtr<OriginUrlMap>);
   LegacyDOMSnapshotAgent(const LegacyDOMSnapshotAgent&) = delete;
   LegacyDOMSnapshotAgent& operator=(const LegacyDOMSnapshotAgent&) = delete;
   ~LegacyDOMSnapshotAgent();
@@ -98,7 +103,7 @@
   PaintOrderMap* paint_order_map_ = nullptr;
   // Maps a backend node id to the url of the script (if any) that generates
   // the corresponding node.
-  OriginUrlMap* origin_url_map_;
+  base::WeakPtr<OriginUrlMap> origin_url_map_;
   using DocumentOrderMap = HeapHashMap<Member<Document>, int>;
   InspectorDOMDebuggerAgent* dom_debugger_agent_;
 };