Fix: fix Node.childNodes didn't update when nodes changed.

bridge/CMakeLists.txt

@@ -286,6 +286,7 @@ if ($ENV{WEBF_JS_ENGINE} MATCHES "quickjs")
     core/dom/events/event_target_impl.cc
     core/binding_object.cc
     core/dom/node.cc
+    core/dom/node_list.cc
     core/dom/node_traversal.cc
     core/dom/live_node_list_base.cc
     core/dom/character_data.cc
@@ -323,7 +324,6 @@ if ($ENV{WEBF_JS_ENGINE} MATCHES "quickjs")
     core/events/keyboard_event.cc
     core/events/promise_rejection_event.cc
     core/html/parser/html_parser.cc
-    core/html/legacy/html_collection.cc
     core/html/html_element.cc
     core/html/html_div_element.cc
     core/html/html_head_element.cc
@@ -338,6 +338,7 @@ if ($ENV{WEBF_JS_ENGINE} MATCHES "quickjs")
     core/html/html_link_element.cc
     core/html/html_unknown_element.cc
     core/html/image.cc
+    core/html/html_collection.cc
     core/html/canvas/html_canvas_element.cc
     core/html/canvas/canvas_rendering_context.cc
     core/html/canvas/canvas_rendering_context_2d.cc
@@ -444,6 +445,7 @@ if ($ENV{WEBF_JS_ENGINE} MATCHES "quickjs")
     out/qjs_scroll_to_options.cc
     out/qjs_html_element.cc
     out/qjs_html_all_collection.cc
+    out/qjs_html_collection.cc
     out/qjs_html_anchor_element.cc
     out/qjs_html_div_element.cc
     out/qjs_html_head_element.cc

bridge/bindings/qjs/binding_initializer.cc

@@ -38,6 +38,7 @@
 #include "qjs_html_body_element.h"
 #include "qjs_html_button_element.h"
 #include "qjs_html_canvas_element.h"
+#include "qjs_html_collection.h"
 #include "qjs_html_div_element.h"
 #include "qjs_html_element.h"
 #include "qjs_html_form_element.h"
@@ -156,7 +157,6 @@ void InstallBindings(ExecutingContext* context) {
   QJSInlineCssStyleDeclaration::Install(context);
   QJSComputedCssStyleDeclaration::Install(context);
   QJSBoundingClientRect::Install(context);
-  QJSHTMLAllCollection::Install(context);
   QJSScreen::Install(context);
   QJSBlob::Install(context);
   QJSTouch::Install(context);
@@ -167,6 +167,8 @@ void InstallBindings(ExecutingContext* context) {
   QJSPerformanceEntry::Install(context);
   QJSPerformanceMark::Install(context);
   QJSPerformanceMeasure::Install(context);
+  QJSHTMLCollection::Install(context);
+  QJSHTMLAllCollection::Install(context);
 
   // SVG
   QJSSVGElement::Install(context);

bridge/bindings/qjs/converter_impl.h

@@ -10,7 +10,6 @@
 #include "atomic_string.h"
 #include "bindings/qjs/union_base.h"
 #include "converter.h"
-#include "core/dom/document.h"
 #include "core/dom/events/event.h"
 #include "core/dom/events/event_target.h"
 #include "core/dom/node_list.h"

bridge/bindings/qjs/cppgc/garbage_collected.h

@@ -67,6 +67,15 @@ class MakeGarbageCollectedTrait {
   friend GarbageCollected<T>;
 };
 
+class GarbageCollectedMixin {
+ public:
+  /**
+   * This Trace method must be overriden by objects inheriting from
+   * GarbageCollectedMixin.
+   */
+  virtual void Trace(GCVisitor*) const {}
+};
+
 template <typename T, typename... Args>
 T* MakeGarbageCollected(Args&&... args) {
   static_assert(std::is_base_of<ScriptWrappable, T>::value,

bridge/bindings/qjs/script_wrappable.cc

@@ -220,10 +220,12 @@ void ScriptWrappable::InitializeQuickJSObject() {
         if (wrapper_type_info->string_property_getter_handler_ != nullptr) {
           JSValue return_value = wrapper_type_info->string_property_getter_handler_(ctx, obj, prop);
           if (!JS_IsNull(return_value)) {
-            desc->flags = JS_PROP_ENUMERABLE;
-            desc->value = return_value;
-            desc->getter = JS_NULL;
-            desc->setter = JS_NULL;
+            if (desc != nullptr) {
+              desc->flags = JS_PROP_ENUMERABLE;
+              desc->value = return_value;
+              desc->getter = JS_NULL;
+              desc->setter = JS_NULL;
+            }
             return true;
           }
         }
@@ -232,10 +234,12 @@ void ScriptWrappable::InitializeQuickJSObject() {
           uint32_t index = JS_AtomToUInt32(prop);
           JSValue return_value = wrapper_type_info->indexed_property_getter_handler_(ctx, obj, index);
           if (!JS_IsNull(return_value)) {
-            desc->flags = JS_PROP_ENUMERABLE;
-            desc->value = return_value;
-            desc->getter = JS_NULL;
-            desc->setter = JS_NULL;
+            if (desc != nullptr) {
+              desc->flags = JS_PROP_ENUMERABLE;
+              desc->value = return_value;
+              desc->getter = JS_NULL;
+              desc->setter = JS_NULL;
+            }
             return true;
           }
         }

bridge/bindings/qjs/wrapper_type_info.h

@@ -56,6 +56,7 @@ enum {
   JS_CLASS_BOUNDING_CLIENT_RECT,
   JS_CLASS_ELEMENT_ATTRIBUTES,
   JS_CLASS_HTML_ALL_COLLECTION,
+  JS_CLASS_HTML_COLLECTION,
   JS_CLASS_HTML_ELEMENT,
   JS_CLASS_WIDGET_ELEMENT,
   JS_CLASS_HTML_DIV_ELEMENT,

bridge/core/dom/child_node_list.cc

@@ -31,6 +31,16 @@ void ChildNodeList::NamedPropertyEnumerator(std::vector<AtomicString>& names, Ex
   }
 }
 
+void ChildNodeList::ChildrenChanged(const ContainerNode::ChildrenChange& change) {
+  if (change.IsChildInsertion()) {
+    collection_index_cache_.NodeInserted();
+  } else if (change.IsChildRemoval()) {
+    collection_index_cache_.NodeRemoved();
+  } else {
+    collection_index_cache_.Invalidate();
+  }
+}
+
 Node* ChildNodeList::TraverseForwardToOffset(unsigned offset, Node& current_node, unsigned& current_offset) const {
   assert(current_offset < offset);
   assert(OwnerNode().childNodes() == this);

bridge/core/dom/child_node_list.h

@@ -37,12 +37,12 @@
 
 #include "bindings/qjs/cppgc/gc_visitor.h"
 #include "core/dom/collection_index_cache.h"
-#include "core/dom/container_node.h"
 #include "core/dom/node_list.h"
 
 namespace webf {
 
 class ExceptionState;
+class ContainerNode;
 
 class ChildNodeList : public NodeList {
  public:
@@ -58,7 +58,11 @@ class ChildNodeList : public NodeList {
   void NamedPropertyEnumerator(std::vector<AtomicString>& names, ExceptionState& exception_state) override;
 
   // Non-DOM API.
-  void InvalidateCache() { collection_index_cache_.Invalidate(); }
+  void ChildrenChanged(const ContainerNode::ChildrenChange&);
+  void InvalidateCache() override {
+    collection_index_cache_.Invalidate();
+    NodeList::InvalidateCache();
+  }
   ContainerNode& OwnerNode() const { return *parent_.Get(); }
 
   ContainerNode& RootNode() const { return OwnerNode(); }

bridge/core/dom/collection_items_cache.h

@@ -47,7 +47,9 @@ class CollectionItemsCache : public CollectionIndexCache<Collection, NodeType> {
   ~CollectionItemsCache();
 
   void Trace(GCVisitor* visitor) const override {
-    visitor->TraceValue(cached_list_);
+    for (auto& item : cached_list_) {
+      visitor->TraceMember(item);
+    }
     Base::Trace(visitor);
   }
 
@@ -70,7 +72,10 @@ template <typename Collection, typename NodeType>
 void CollectionItemsCache<Collection, NodeType>::Invalidate() {
   Base::Invalidate();
   if (list_valid_) {
-    cached_list_.Shrink(0);
+    for (auto& item : cached_list_) {
+      item.Clear();
+    }
+    cached_list_.clear();
     list_valid_ = false;
   }
 }
@@ -95,7 +100,7 @@ unsigned CollectionItemsCache<Collection, NodeType>::NodeCount(const Collection&
 template <typename Collection, typename NodeType>
 inline NodeType* CollectionItemsCache<Collection, NodeType>::NodeAt(const Collection& collection, unsigned index) {
   if (list_valid_) {
-    DCHECK(this->IsCachedNodeCountValid());
+    assert(this->IsCachedNodeCountValid());
     return index < this->CachedNodeCount() ? cached_list_[index] : nullptr;
   }
   return Base::NodeAt(collection, index);

bridge/core/dom/container_node.cc

@@ -1,3 +1,26 @@
+/*
+ * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
+ *           (C) 1999 Antti Koivisto (koivisto@kde.org)
+ *           (C) 2001 Dirk Mueller (mueller@kde.org)
+ * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2013 Apple Inc. All rights
+ * reserved.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public License
+ * along with this library; see the file COPYING.LIB.  If not, write to
+ * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
+ */
+
 /*
  * Copyright (C) 2019-2022 The Kraken authors. All rights reserved.
  * Copyright (C) 2022-present The WebF authors. All rights reserved.
@@ -15,16 +38,8 @@
 namespace webf {
 
 // Legacy impls due to limited time, should remove this func in the future.
-std::vector<Element*> ContainerNode::Children() {
-  std::vector<Element*> elements;
-  uint32_t length = childNodes()->length();
-  for (int i = 0; i < length; i++) {
-    auto* element = DynamicTo<Element>(childNodes()->item(i, ASSERT_NO_EXCEPTION()));
-    if (element) {
-      elements.emplace_back(element);
-    }
-  }
-  return elements;
+HTMLCollection* ContainerNode::Children() {
+  return EnsureCachedCollection<HTMLCollection>(CollectionType::kNodeChildren);
 }
 
 unsigned ContainerNode::CountChildren() const {
@@ -154,6 +169,7 @@ Node* ContainerNode::InsertBefore(Node* new_child, Node* ref_child, ExceptionSta
   // 5. Insert node into parent before reference child.
   NodeVector post_insertion_notification_targets;
   { InsertNodeVector(targets, ref_child, AdoptAndInsertBefore(), &post_insertion_notification_targets); }
+  DidInsertNodeVector(targets, ref_child, post_insertion_notification_targets);
   return new_child;
 }
 
@@ -219,6 +235,7 @@ Node* ContainerNode::ReplaceChild(Node* new_child, Node* old_child, ExceptionSta
       InsertNodeVector(targets, nullptr, AdoptAndAppendChild(), &post_insertion_notification_targets);
     }
   }
+  DidInsertNodeVector(targets, next, post_insertion_notification_targets);
 
   // 16. Return child.
   return old_child;
@@ -250,6 +267,7 @@ Node* ContainerNode::RemoveChild(Node* old_child, ExceptionState& exception_stat
       RemoveBetween(prev, next, *child);
       NotifyNodeRemoved(*child);
     }
+    ChildrenChanged(ChildrenChange::ForRemoval(*child, prev, next, ChildrenChangeSource::kAPI));
   }
   return child;
 }
@@ -268,6 +286,7 @@ Node* ContainerNode::AppendChild(Node* new_child, ExceptionState& exception_stat
   NodeVector post_insertion_notification_targets;
   post_insertion_notification_targets.reserve(kInitialNodeVectorSize);
   { InsertNodeVector(targets, nullptr, AdoptAndAppendChild(), &post_insertion_notification_targets); }
+  DidInsertNodeVector(targets, nullptr, post_insertion_notification_targets);
   return new_child;
 }
 
@@ -331,14 +350,21 @@ void ContainerNode::RemoveChildren() {
   if (!first_child_)
     return;
 
+  bool has_element_child = false;
+
   while (Node* child = first_child_) {
+    if (child->IsElementNode()) {
+      has_element_child = true;
+    }
     RemoveBetween(nullptr, child->nextSibling(), *child);
     NotifyNodeRemoved(*child);
   }
 
-  auto* this_node = DynamicTo<ContainerNode>(this);
-  if (this_node)
-    EnsureNodeData().EnsureChildNodeList(*this_node)->InvalidateCache();
+  ChildrenChange change = {
+      .type = ChildrenChangeType::kAllChildrenRemoved,
+      .by_parser = ChildrenChangeSource::kAPI,
+      .affects_elements = has_element_child ? ChildrenChangeAffectsElements::kYes : ChildrenChangeAffectsElements::kNo};
+  ChildrenChanged(change);
 }
 
 void ContainerNode::CloneChildNodesFrom(const ContainerNode& node, CloneChildrenFlag flag) {
@@ -394,6 +420,15 @@ void ContainerNode::InsertNodeVector(const NodeVector& targets,
   }
 }
 
+void ContainerNode::DidInsertNodeVector(const webf::NodeVector& targets,
+                                        webf::Node* next,
+                                        const webf::NodeVector& post_insertion_notification_targets) {
+  Node* unchanged_previous = targets.size() > 0 ? targets[0]->previousSibling() : nullptr;
+  for (const auto& target_node : targets) {
+    ChildrenChanged(ChildrenChange::ForInsertion(*target_node, unchanged_previous, next, ChildrenChangeSource::kAPI));
+  }
+}
+
 void ContainerNode::InsertBeforeCommon(Node& next_child, Node& new_child) {
   // Use insertBefore if you need to handle reparenting (and want DOM mutation
   // events).
@@ -457,6 +492,42 @@ void ContainerNode::NotifyNodeRemoved(Node& root) {
   }
 }
 
+void ContainerNode::ChildrenChanged(const webf::ContainerNode::ChildrenChange& change) {
+  InvalidateNodeListCachesInAncestors(&change);
+}
+
+void ContainerNode::InvalidateNodeListCachesInAncestors(const webf::ContainerNode::ChildrenChange* change) {
+  // This is a performance optimization, NodeList cache invalidation is
+  // not necessary for a text change.
+  if (change && change->type == ChildrenChangeType::kTextChanged)
+    return;
+
+  if (HasNodeData()) {
+    if (NodeList* lists = Data()->NodeLists()) {
+      if (lists != nullptr && lists->IsChildNodeList()) {
+        auto* child_node_list = static_cast<ChildNodeList*>(lists);
+        if (change) {
+          child_node_list->ChildrenChanged(*change);
+        } else {
+          child_node_list->InvalidateCache();
+        }
+      }
+    }
+  }
+
+  // This is a performance optimization, NodeList cache invalidation is
+  // not necessary for non-element nodes.
+  if (change && change->affects_elements == ChildrenChangeAffectsElements::kNo)
+    return;
+
+  for (ContainerNode* node = this; node; node = node->parentNode()) {
+    NodeList* lists = node->childNodes();
+    if (lists->IsChildNodeList()) {
+      reinterpret_cast<ChildNodeList*>(lists)->InvalidateCache();
+    }
+  }
+}
+
 void ContainerNode::Trace(GCVisitor* visitor) const {
   visitor->TraceMember(first_child_);
   visitor->TraceMember(last_child_);