From 447ecc2155002b8804cfe54d3a2e8320fd04862d Mon Sep 17 00:00:00 2001
From: Timothy Flynn <trflynn89@pm.me>
Date: Thu, 1 Jul 2021 12:27:32 -0400
Subject: [PATCH] LibWeb: Maintain a map of child-to-parent nodes in OOPWV DOM
 Inspector

Currently, each time parent_index() is invoked, two depth-first searches
are incurred to find the node's parent and grandparent. This becomes
particularly expensive, for example, when trying to scroll through a
large <ul> list.

Instead, upon creation, traverse the DOM JSON and create a map of child
nodes to their parent. Then those two lookups become hash map lookups
rather than a DFS traversal.
---
 .../Libraries/LibWeb/DOMTreeJSONModel.cpp     | 36 ++++++++-----------
 Userland/Libraries/LibWeb/DOMTreeJSONModel.h  | 17 +++++----
 2 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/Userland/Libraries/LibWeb/DOMTreeJSONModel.cpp b/Userland/Libraries/LibWeb/DOMTreeJSONModel.cpp
index 47e90f7e079..78b8dbf81df 100644
--- a/Userland/Libraries/LibWeb/DOMTreeJSONModel.cpp
+++ b/Userland/Libraries/LibWeb/DOMTreeJSONModel.cpp
@@ -18,6 +18,8 @@ DOMTreeJSONModel::DOMTreeJSONModel(JsonObject dom_tree)
     m_document_icon.set_bitmap_for_size(16, Gfx::Bitmap::load_from_file("/res/icons/16x16/filetype-html.png"));
     m_element_icon.set_bitmap_for_size(16, Gfx::Bitmap::load_from_file("/res/icons/16x16/inspector-object.png"));
     m_text_icon.set_bitmap_for_size(16, Gfx::Bitmap::load_from_file("/res/icons/16x16/filetype-unknown.png"));
+
+    map_dom_nodes_to_parent(nullptr, &m_dom_tree);
 }
 
 DOMTreeJSONModel::~DOMTreeJSONModel()
@@ -47,20 +49,18 @@ GUI::ModelIndex DOMTreeJSONModel::parent_index(const GUI::ModelIndex& index) con
         return {};
 
     auto const& node = *static_cast<JsonObject const*>(index.internal_data());
-    auto node_internal_id = get_internal_id(node);
 
-    auto const* parent_node = find_parent_of_child_with_internal_id(node_internal_id);
+    auto const* parent_node = get_parent(node);
     if (!parent_node)
         return {};
 
     // If the parent is the root document, we know it has index 0, 0
-    auto parent_node_internal_id = get_internal_id(*parent_node);
-    if (parent_node_internal_id == get_internal_id(m_dom_tree)) {
+    if (parent_node == &m_dom_tree) {
         return create_index(0, 0, parent_node);
     }
 
     // Otherwise, we need to find the grandparent, to find the index of parent within that
-    auto const* grandparent_node = find_parent_of_child_with_internal_id(parent_node_internal_id);
+    auto const* grandparent_node = get_parent(*parent_node);
     VERIFY(grandparent_node);
 
     auto const* grandparent_children = get_children(*grandparent_node);
@@ -69,7 +69,7 @@ GUI::ModelIndex DOMTreeJSONModel::parent_index(const GUI::ModelIndex& index) con
 
     for (size_t grandparent_child_index = 0; grandparent_child_index < grandparent_children->size(); ++grandparent_child_index) {
         auto const& child = grandparent_children->at(grandparent_child_index).as_object();
-        if (get_internal_id(child) == parent_node_internal_id)
+        if (&child == parent_node)
             return create_index(grandparent_child_index, 0, parent_node);
     }
 
@@ -157,24 +157,18 @@ void DOMTreeJSONModel::update()
     did_update();
 }
 
-JsonObject const* DOMTreeJSONModel::find_parent_of_child_with_internal_id(JsonObject const& node, size_t internal_id) const
+void DOMTreeJSONModel::map_dom_nodes_to_parent(JsonObject const* parent, JsonObject const* node)
 {
-    auto const* children = get_children(node);
+    m_dom_node_to_parent_map.set(node, parent);
+
+    auto const* children = get_children(*node);
     if (!children)
-        return nullptr;
+        return;
 
-    for (size_t i = 0; i < children->size(); ++i) {
-        auto const& child = children->at(i).as_object();
-
-        auto child_internal_id = get_internal_id(child);
-        if (child_internal_id == internal_id)
-            return &node;
-
-        if (auto const* maybe_node = find_parent_of_child_with_internal_id(child, internal_id); maybe_node)
-            return maybe_node;
-    }
-
-    return nullptr;
+    children->for_each([&](auto const& child) {
+        auto const& child_node = child.as_object();
+        map_dom_nodes_to_parent(node, &child_node);
+    });
 }
 
 }
diff --git a/Userland/Libraries/LibWeb/DOMTreeJSONModel.h b/Userland/Libraries/LibWeb/DOMTreeJSONModel.h
index 9f209938cfa..90f6cfe862f 100644
--- a/Userland/Libraries/LibWeb/DOMTreeJSONModel.h
+++ b/Userland/Libraries/LibWeb/DOMTreeJSONModel.h
@@ -7,6 +7,7 @@
 
 #pragma once
 
+#include <AK/HashMap.h>
 #include <AK/JsonObject.h>
 #include <LibGUI/Model.h>
 #include <LibWeb/Forward.h>
@@ -36,16 +37,11 @@ public:
 private:
     explicit DOMTreeJSONModel(JsonObject);
 
-    ALWAYS_INLINE JsonObject const* find_parent_of_child_with_internal_id(size_t internal_id) const
+    ALWAYS_INLINE JsonObject const* get_parent(const JsonObject& o) const
     {
-        return find_parent_of_child_with_internal_id(m_dom_tree, internal_id);
-    }
-
-    JsonObject const* find_parent_of_child_with_internal_id(JsonObject const&, size_t) const;
-
-    ALWAYS_INLINE static size_t get_internal_id(const JsonObject& o)
-    {
-        return o.get("internal_id").as_u32();
+        auto parent_node = m_dom_node_to_parent_map.get(&o);
+        VERIFY(parent_node.has_value());
+        return *parent_node;
     }
 
     ALWAYS_INLINE static JsonArray const* get_children(const JsonObject& o)
@@ -55,10 +51,13 @@ private:
         return nullptr;
     }
 
+    void map_dom_nodes_to_parent(JsonObject const* parent, JsonObject const* child);
+
     GUI::Icon m_document_icon;
     GUI::Icon m_element_icon;
     GUI::Icon m_text_icon;
     JsonObject m_dom_tree;
+    HashMap<JsonObject const*, JsonObject const*> m_dom_node_to_parent_map;
 };
 
 }