Revert "Add declarative AttributePart syntax parsing and functionality"

This reverts commit 5a66ab6. Reason for revert: use-of-uninitialized-value detected https://ci.chromium.org/ui/p/chromium/builders/ci/WebKit%20Linux%20MSAN/23141/overview Original change's description: > Add declarative AttributePart syntax parsing and functionality > > This adds the ability to parse declarative AttributePart syntax: > > <div id={{}} class={{}}> > > The above snippet will now produce two "automatic" AttributeParts, > which do not show up in getParts() output, but which can be updated > while cloning via: > > root.clone({attributeValues: ['id_value', 'class_value']}); > > See [1] for more details. > > [1] https://docs.google.com/document/d/1z0YCj06-LFTWJlR_ayhtCOV2O4cJ5W1fHZbfckFsOdU/edit > > Bug: 1453291 > Change-Id: Icda4adbd3ce415999ff62e5a758124bd124188cb > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4930249 > Reviewed-by: Joey Arhar <jarhar@chromium.org> > Commit-Queue: Mason Freed <masonf@chromium.org> > Auto-Submit: Mason Freed <masonf@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1209190} Bug: 1453291 Change-Id: Ibd496fd34d0772a3bcad51585046c7733cc6f2cd No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4936691 Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Auto-Submit: Keishi Hattori <keishi@chromium.org> Cr-Commit-Position: refs/heads/main@{#1209253}
chromium · Oct 13, 2023 · 3cc3a37 · 3cc3a37
1 parent a2cbf5c
commit 3cc3a37
Show file tree

Hide file tree

Showing 19 changed files with 81 additions and 222 deletions.
diff --git a/third_party/blink/renderer/bindings/generated_in_core.gni b/third_party/blink/renderer/bindings/generated_in_core.gni
@@ -303,8 +303,6 @@ generated_dictionary_sources_in_core = [
   "$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_parse_from_string_options.h",
   "$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_part_init.cc",
   "$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_part_init.h",
-  "$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_part_root_clone_options.cc",
-  "$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_part_root_clone_options.h",
   "$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_pending_beacon_options.cc",
   "$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_pending_beacon_options.h",
   "$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_performance_entry_filter_options.cc",

diff --git a/third_party/blink/renderer/core/dom/attribute_part.cc b/third_party/blink/renderer/core/dom/attribute_part.cc
@@ -12,42 +12,27 @@ namespace blink {
 // static
 AttributePart* AttributePart::Create(PartRootUnion* root_union,
                                      Node* node,
-                                     AtomicString local_name,
+                                     String local_name,
                                      bool automatic,
-                                     const PartInit* init,
-                                     ExceptionState& exception_state) {
-  Element* element = DynamicTo<Element>(node);
-  if (!element) {
-    exception_state.ThrowDOMException(
-        DOMExceptionCode::kInvalidStateError,
-        "An AttributePart must be constructed on an Element.");
-    return nullptr;
-  }
+                                     const PartInit* init) {
   return MakeGarbageCollected<AttributePart>(
-      *PartRoot::GetPartRootFromUnion(root_union), *element, local_name,
-      automatic, init);
+      *PartRoot::GetPartRootFromUnion(root_union), *node, local_name, automatic,
+      init);
 }
 
 AttributePart::AttributePart(PartRoot& root,
-                             Element& element,
-                             AtomicString local_name,
+                             Node& node,
+                             String local_name,
                              bool automatic,
                              const Vector<String> metadata)
-    : NodePart(root, element, !automatic, metadata),
+    : NodePart(root, node, !automatic, metadata),
       local_name_(local_name),
       automatic_(automatic) {}
 
 Part* AttributePart::ClonePart(NodeCloningData& data, Node& node_clone) const {
   DCHECK(IsValid());
-  Element& element_clone = To<Element>(node_clone);
-  Part* new_part =
-      MakeGarbageCollected<AttributePart>(data.CurrentPartRoot(), element_clone,
-                                          local_name_, automatic_, metadata());
-  absl::optional<AtomicString> attribute_value = data.NextAttributeValue();
-  if (attribute_value) {
-    element_clone.setAttribute(local_name_, *attribute_value);
-  }
-  return new_part;
+  return MakeGarbageCollected<AttributePart>(
+      data.CurrentPartRoot(), node_clone, local_name_, automatic_, metadata());
 }
 
 }  // namespace blink
diff --git a/third_party/blink/renderer/core/dom/attribute_part.h b/third_party/blink/renderer/core/dom/attribute_part.h
@@ -14,8 +14,6 @@
 
 namespace blink {
 
-class ExceptionState;
-
 // Implementation of the AttributePart class, which is part of the DOM Parts
 // API. A AttributePart stores a reference to a single |Node| in the DOM tree.
 class CORE_EXPORT AttributePart : public NodePart {
@@ -24,24 +22,23 @@ class CORE_EXPORT AttributePart : public NodePart {
  public:
   static AttributePart* Create(PartRootUnion* root_union,
                                Node* node,
-                               AtomicString local_name,
+                               String local_name,
                                bool automatic,
-                               const PartInit* init,
-                               ExceptionState& exception_state);
+                               const PartInit* init);
   AttributePart(PartRoot& root,
-                Element& element,
-                AtomicString local_name,
+                Node& node,
+                String local_name,
                 bool automatic,
                 const PartInit* init)
       : AttributePart(root,
-                      element,
+                      node,
                       local_name,
                       automatic,
                       init && init->hasMetadata() ? init->metadata()
                                                   : Vector<String>()) {}
   AttributePart(PartRoot& root,
-                Element& element,
-                AtomicString local_name,
+                Node& node,
+                String local_name,
                 bool automatic,
                 const Vector<String> metadata);
   AttributePart(const AttributePart&) = delete;
@@ -51,11 +48,11 @@ class CORE_EXPORT AttributePart : public NodePart {
   bool IncludeInPartsList() const override { return !automatic_; }
 
   // AttributePart API
-  AtomicString localName() const { return local_name_; }
+  String localName() const { return local_name_; }
   bool automatic() const { return automatic_; }
 
  private:
-  AtomicString local_name_;
+  String local_name_;
   bool automatic_;
 };
 

diff --git a/third_party/blink/renderer/core/dom/attribute_part.idl b/third_party/blink/renderer/core/dom/attribute_part.idl
@@ -7,7 +7,7 @@
 
 [RuntimeEnabled=DOMPartsAPI,Exposed=Window]
 interface AttributePart : NodePart {
-  [RaisesException] constructor(PartRoot root, Element element, DOMString localName, boolean automatic, optional PartInit init = {});
+  constructor(PartRoot root, Element element, DOMString localName, boolean automatic, optional PartInit init = {});
   // For now, HTML only, don't deal with prefix or namespaceURI.
   readonly attribute DOMString localName;
   readonly attribute boolean automatic;

diff --git a/third_party/blink/renderer/core/dom/child_node_part.cc b/third_party/blink/renderer/core/dom/child_node_part.cc
@@ -53,8 +53,7 @@ void ChildNodePart::disconnect() {
   Part::disconnect();
 }
 
-PartRootUnion* ChildNodePart::clone(PartRootCloneOptions* options,
-                                    ExceptionState& exception_state) {
+PartRootUnion* ChildNodePart::clone(ExceptionState& exception_state) {
   // Since we're only cloning a part of the tree, not including this
   // ChildNodePart's `root`, we use a temporary DocumentFragment and its
   // PartRoot during the clone.
@@ -69,7 +68,6 @@ PartRootUnion* ChildNodePart::clone(PartRootCloneOptions* options,
   auto& document = GetDocument();
   auto* fragment = To<DocumentFragment>(DocumentFragment::Create(document));
   NodeCloningData data{CloneOption::kPreserveDOMParts};
-  data.SetPartRootCloneOptions(options);
   auto& fragment_part_root = fragment->getPartRoot();
   data.PushPartRoot(fragment_part_root);
   ContainerNode* new_parent = To<ContainerNode>(

diff --git a/third_party/blink/renderer/core/dom/child_node_part.h b/third_party/blink/renderer/core/dom/child_node_part.h
@@ -5,6 +5,7 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_DOM_CHILD_NODE_PART_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_DOM_CHILD_NODE_PART_H_
 
+#include "third_party/blink/renderer/bindings/core/v8/v8_part_init.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_union_node_string_trustedscript.h"
 #include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/core/dom/container_node.h"
@@ -18,9 +19,6 @@
 
 namespace blink {
 
-class PartInit;
-class PartRootCloneOptions;
-
 // Implementation of the ChildNodePart class, which is part of the DOM Parts
 // API. A ChildNodePart stores a reference to a range of nodes within the
 // children of a single parent |Node| in the DOM tree.
@@ -64,10 +62,7 @@ class CORE_EXPORT ChildNodePart : public Part, public PartRoot {
 
   // ChildNodePart API
   void disconnect() override;
-  PartRootUnion* clone(ExceptionState& exception_state) {
-    return clone(nullptr, exception_state);
-  }
-  PartRootUnion* clone(PartRootCloneOptions*, ExceptionState&);
+  PartRootUnion* clone(ExceptionState& exception_state);
   ContainerNode* rootContainer() const override;
   ContainerNode* parentNode() const { return previous_sibling_->parentNode(); }
   Node* previousSibling() const { return previous_sibling_; }

diff --git a/third_party/blink/renderer/core/dom/document_part_root.cc b/third_party/blink/renderer/core/dom/document_part_root.cc
@@ -24,11 +24,9 @@ void DocumentPartRoot::Trace(Visitor* visitor) const {
   PartRoot::Trace(visitor);
 }
 
-PartRootUnion* DocumentPartRoot::clone(PartRootCloneOptions* options,
-                                       ExceptionState&) {
+PartRootUnion* DocumentPartRoot::clone(ExceptionState&) {
   NodeCloningData data{CloneOption::kIncludeDescendants,
                        CloneOption::kPreserveDOMParts};
-  data.SetPartRootCloneOptions(options);
   Node* clone = rootContainer()->Clone(rootContainer()->GetDocument(), data,
                                        /*append_to*/ nullptr);
   // http://crbug.com/1467847: clone may be null and can be hit by clusterfuzz.

diff --git a/third_party/blink/renderer/core/dom/document_part_root.h b/third_party/blink/renderer/core/dom/document_part_root.h
@@ -13,8 +13,6 @@
 
 namespace blink {
 
-class PartRootCloneOptions;
-
 // Implementation of the DocumentPartRoot class, which is part of the DOM Parts
 // API. A DocumentPartRoot holds the parts for a Document or DocumentFragment.
 // A Document always owns one DocumentPartRoot.
@@ -41,10 +39,7 @@ class CORE_EXPORT DocumentPartRoot : public ScriptWrappable, public PartRoot {
   void Trace(Visitor*) const override;
 
   // PartRoot API
-  PartRootUnion* clone(ExceptionState& exception_state) {
-    return clone(nullptr, exception_state);
-  }
-  PartRootUnion* clone(PartRootCloneOptions*, ExceptionState&);
+  PartRootUnion* clone(ExceptionState&);
   ContainerNode* rootContainer() const override { return root_container_; }
 
  protected:

diff --git a/third_party/blink/renderer/core/dom/node_cloning_data.h b/third_party/blink/renderer/core/dom/node_cloning_data.h
@@ -6,7 +6,6 @@
 #define THIRD_PARTY_BLINK_RENDERER_CORE_DOM_NODE_CLONING_DATA_H_
 
 #include "base/containers/enum_set.h"
-#include "third_party/blink/renderer/bindings/core/v8/v8_part_root_clone_options.h"
 #include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/core/dom/child_node_part.h"
 #include "third_party/blink/renderer/core/dom/node.h"
@@ -16,8 +15,6 @@
 #include "third_party/blink/renderer/platform/heap/collection_support/heap_vector.h"
 #include "third_party/blink/renderer/platform/heap/garbage_collected.h"
 #include "third_party/blink/renderer/platform/heap/member.h"
-#include "third_party/blink/renderer/platform/wtf/text/atomic_string.h"
-#include "third_party/blink/renderer/platform/wtf/wtf_size_t.h"
 
 namespace blink {
 
@@ -70,28 +67,9 @@ class CORE_EXPORT NodeCloningData final {
     return *cloned_part_root_stack_.back();
   }
 
-  void SetPartRootCloneOptions(PartRootCloneOptions* options) {
-    if (options && options->hasAttributeValues()) {
-      attribute_values_.clear();
-      for (String value : options->attributeValues()) {
-        attribute_values_.push_back(AtomicString(value));
-      }
-      current_attribute_index_ = 0;
-    }
-  }
-
-  absl::optional<AtomicString> NextAttributeValue() {
-    if (current_attribute_index_ >= attribute_values_.size()) {
-      return absl::nullopt;
-    }
-    return attribute_values_[current_attribute_index_++];
-  }
-
  private:
   CloneOptionSet clone_options_;
   HeapVector<Member<PartRoot>> cloned_part_root_stack_;
-  VectorOf<AtomicString> attribute_values_;
-  wtf_size_t current_attribute_index_;
 };
 
 }  // namespace blink

diff --git a/third_party/blink/renderer/core/dom/part_root.idl b/third_party/blink/renderer/core/dom/part_root.idl
@@ -18,17 +18,13 @@ interface mixin PartRootMixin {
   // document tree is cloned. In the case of a ChildPartRoot, only the children
   // between `previous_node` and `next_node` are included, inclusive. The
   // `clone()` method returns the cloned PartRoot.
-  [RaisesException] PartRoot clone(optional PartRootCloneOptions options);
+  [RaisesException] PartRoot clone();
   // Return the root container for the PartRoot, which is the Document or
   // DocumentFragment for a DocumentPartRoot, or the parent ContainerNode of
   // a ChildNodePart.
   readonly attribute Node rootContainer;
 };
 
-dictionary PartRootCloneOptions {
-  sequence<DOMString> attributeValues;
-};
-
 DocumentPartRoot includes PartRootMixin;
 ChildNodePart includes PartRootMixin;
 

diff --git a/third_party/blink/renderer/core/html/parser/atomic_html_token.h b/third_party/blink/renderer/core/html/parser/atomic_html_token.h
@@ -42,7 +42,6 @@
 #include "third_party/blink/renderer/platform/runtime_enabled_features.h"
 #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
 #include "third_party/blink/renderer/platform/wtf/hash_set.h"
-#include "third_party/blink/renderer/platform/wtf/text/atomic_string.h"
 #include "third_party/blink/renderer/platform/wtf/text/atomic_string_hash.h"
 
 namespace blink {
@@ -206,9 +205,9 @@ class CORE_EXPORT AtomicHTMLToken {
     return dom_part_data_->metadata_;
   }
 
-  DOMPartsNeeded GetDOMPartsNeeded() {
-    DCHECK_EQ(type_, HTMLToken::kStartTag);
-    return dom_parts_needed_;
+  bool NeedsNodePart() const {
+    DCHECK(type_ == HTMLToken::kStartTag);
+    return needs_node_part_;
   }
 
   explicit AtomicHTMLToken(HTMLToken& token)
@@ -227,7 +226,7 @@ class CORE_EXPORT AtomicHTMLToken {
       case HTMLToken::kEndOfFile:
         break;
       case HTMLToken::kStartTag:
-        dom_parts_needed_ = token.GetDOMPartsNeeded();
+        needs_node_part_ = token.NeedsNodePart();
         [[fallthrough]];
       case HTMLToken::kEndTag: {
         self_closing_ = token.SelfClosing();
@@ -329,7 +328,7 @@ class CORE_EXPORT AtomicHTMLToken {
 
   // For DOM Parts
   std::unique_ptr<DOMPartData> dom_part_data_;
-  DOMPartsNeeded dom_parts_needed_;
+  bool needs_node_part_{false};
 
   // For StartTag and EndTag
   bool self_closing_ = false;