Add the ability to disconnect DOM Parts

This turns out to be important, at least in the case of writing
tests. `disconnect()` is a one-way operation; there's no way to
re-connect a part to its part root.

This CL also adds the constraint that NodePart can't have child parts.
It therefore adds exceptions to the constructors for NodePart and
ChildNodePart in the case that a NodePart is supplied.

Bug: 1453291
Change-Id: Ied398c417b1e105c77a95c3836ee2d09902df7c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4617275
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1159094}

third_party/blink/renderer/core/dom/child_node_part.cc

@@ -6,6 +6,22 @@
 
 namespace blink {
 
+// static
+ChildNodePart* ChildNodePart::Create(PartRoot* root,
+                                     Node* previous_sibling,
+                                     Node* next_sibling,
+                                     const NodePartInit* init,
+                                     ExceptionState& exception_state) {
+  if (!root->SupportsContainedParts()) {
+    exception_state.ThrowDOMException(
+        DOMExceptionCode::kNotSupportedError,
+        "The provided PartRoot does not support contained parts");
+    return nullptr;
+  }
+  return MakeGarbageCollected<ChildNodePart>(*root, *previous_sibling,
+                                             *next_sibling, init);
+}
+
 void ChildNodePart::Trace(Visitor* visitor) const {
   visitor->Trace(previous_sibling_);
   visitor->Trace(next_sibling_);

third_party/blink/renderer/core/dom/child_node_part.h

@@ -29,10 +29,8 @@ class CORE_EXPORT ChildNodePart : public Part {
   static ChildNodePart* Create(PartRoot* root,
                                Node* previous_sibling,
                                Node* next_sibling,
-                               const NodePartInit* init) {
-    return MakeGarbageCollected<ChildNodePart>(*root, *previous_sibling,
-                                               *next_sibling, init);
-  }
+                               const NodePartInit* init,
+                               ExceptionState& exception_state);
   // TODO(crbug.com/1453291): Handle the init parameter.
   ChildNodePart(PartRoot& root,
                 Node& previous_sibling,
@@ -47,6 +45,7 @@ class CORE_EXPORT ChildNodePart : public Part {
   void Trace(Visitor* visitor) const override;
   Node* RelevantNode() const override;
   String ToString() const override;
+  bool SupportsContainedParts() const override { return true; }
 
   // ChildNodePart API
   Node* previousSibling() const { return previous_sibling_; }

third_party/blink/renderer/core/dom/child_node_part.idl

@@ -7,7 +7,7 @@
 
 [RuntimeEnabled=DOMPartsAPI,Exposed=Window]
 interface ChildNodePart : Part {
-  constructor(PartRoot root, Node previousSibling, Node nextSibling, optional NodePartInit init = {});
+  [RaisesException] constructor(PartRoot root, Node previousSibling, Node nextSibling, optional NodePartInit init = {});
   readonly attribute Node previousSibling;
   readonly attribute Node nextSibling;
   readonly attribute FrozenArray<Node> children;

third_party/blink/renderer/core/dom/document_part_root.h

@@ -23,6 +23,8 @@ class CORE_EXPORT DocumentPartRoot : public PartRoot {
   ~DocumentPartRoot() override = default;
 
   Document* GetDocument() const override { return document_; }
+  bool SupportsContainedParts() const override { return true; }
+
   String ToString() const override { return "DocumentPartRoot"; }
   void Trace(Visitor*) const override;
 

third_party/blink/renderer/core/dom/node_part.cc

@@ -6,6 +6,20 @@
 
 namespace blink {
 
+// static
+NodePart* NodePart::Create(PartRoot* root,
+                           Node* node,
+                           const NodePartInit* init,
+                           ExceptionState& exception_state) {
+  if (!root->SupportsContainedParts()) {
+    exception_state.ThrowDOMException(
+        DOMExceptionCode::kNotSupportedError,
+        "The provided PartRoot does not support contained parts");
+    return nullptr;
+  }
+  return MakeGarbageCollected<NodePart>(*root, node, init);
+}
+
 void NodePart::Trace(Visitor* visitor) const {
   visitor->Trace(node_);
   Part::Trace(visitor);

third_party/blink/renderer/core/dom/node_part.h

@@ -14,6 +14,8 @@
 
 namespace blink {
 
+class ExceptionState;
+
 // Implementation of the NodePart class, which is part of the DOM Parts API.
 // A NodePart stores a reference to a single |Node| in the DOM tree.
 class CORE_EXPORT NodePart : public Part {
@@ -22,9 +24,8 @@ class CORE_EXPORT NodePart : public Part {
  public:
   static NodePart* Create(PartRoot* root,
                           Node* node,
-                          const NodePartInit* init) {
-    return MakeGarbageCollected<NodePart>(*root, node, init);
-  }
+                          const NodePartInit* init,
+                          ExceptionState& exception_state);
   // TODO(crbug.com/1453291): Handle the init parameter.
   NodePart(PartRoot& root, Node* node, const NodePartInit* init)
       : Part(root), node_(node) {}

third_party/blink/renderer/core/dom/node_part.idl

@@ -7,7 +7,7 @@
 
 [RuntimeEnabled=DOMPartsAPI,Exposed=Window]
 interface NodePart : Part {
-  constructor(PartRoot root, Node node, optional NodePartInit init = {});
+  [RaisesException] constructor(PartRoot root, Node node, optional NodePartInit init = {});
   readonly attribute Node node;
 };
 

third_party/blink/renderer/core/dom/part.cc

@@ -9,6 +9,7 @@
 namespace blink {
 
 Part::Part(PartRoot& root) : root_(root) {
+  CHECK(root.SupportsContainedParts());
   root.AddPart(*this);
 }
 
@@ -17,4 +18,12 @@ void Part::Trace(Visitor* visitor) const {
   PartRoot::Trace(visitor);
 }
 
+void Part::disconnect() {
+  if (!root_) {
+    return;
+  }
+  root_->RemovePart(*this);
+  root_ = nullptr;
+}
+
 }  // namespace blink

third_party/blink/renderer/core/dom/part.h

@@ -31,9 +31,10 @@ class CORE_EXPORT Part : public PartRoot {
   virtual Node* RelevantNode() const = 0;
 
   // Part API
-  PartRoot& root() const { return *root_; }
+  PartRoot* root() const { return root_; }
   // TODO(1453291) Populate metadata_.
   Vector<String>& metadata() { return metadata_; }
+  void disconnect();
 
  protected:
   explicit Part(PartRoot& root);

third_party/blink/renderer/core/dom/part.idl

@@ -9,4 +9,5 @@
 interface Part : PartRoot {
   readonly attribute PartRoot root;
   readonly attribute FrozenArray<DOMString> metadata;
+  void disconnect();
 };

third_party/blink/renderer/core/dom/part_root.cc

@@ -19,10 +19,18 @@ void PartRoot::Trace(Visitor* visitor) const {
 }
 
 void PartRoot::AddPart(Part& new_part) {
+  CHECK(!parts_unordered_.Contains(&new_part));
   parts_unordered_.push_back(new_part);
   cached_parts_list_dirty_ = true;
 }
 
+void PartRoot::RemovePart(Part& part) {
+  auto index = parts_unordered_.Find(&part);
+  CHECK_NE(index, kNotFound);
+  parts_unordered_.EraseAt(index);
+  cached_parts_list_dirty_ = true;
+}
+
 namespace {
 
 Node* LowestCommonAncestor(Node* a,
@@ -128,7 +136,7 @@ DocumentPartRoot* PartRoot::GetDocumentPartRoot() {
   PartRoot* root = this;
   while (!root->IsDocumentPartRoot()) {
     CHECK(root->IsPart());
-    root = &static_cast<Part*>(root)->root();
+    root = static_cast<Part*>(root)->root();
   }
   return static_cast<DocumentPartRoot*>(root);
 }

third_party/blink/renderer/core/dom/part_root.h

@@ -30,7 +30,13 @@ class CORE_EXPORT PartRoot : public ScriptWrappable {
 
   // Adds a new part to this PartRoot's collection of maintained parts.
   void AddPart(Part& new_part);
+  void RemovePart(Part& part);
   virtual String ToString() const = 0;
+  // Both DocumentPartRoot and ChildNodePart can have contained parts, while
+  // NodePart cannot. However, due to the class hierarchy, NodePart is a
+  // PartRoot, so this method is used to detect which PartRoots can actually
+  // have contained parts.
+  virtual bool SupportsContainedParts() const { return false; }
 
   // PartRoot API
   HeapVector<Member<Part>> getParts();

third_party/blink/web_tests/external/wpt/dom/parts/basic-dom-part-objects.tentative.html

@@ -21,22 +21,29 @@ <h1 id="name">First<?child-node-part name?>Middle<?/child-node-part?>Last</h1>
 const target = document.getElementById('target');
 assert_true(target.children.length >= 3);
 
-test(() => {
+function addCleanup(t, part) {
+  t.add_cleanup(() => part.disconnect());
+  return part;
+}
+
+test((t) => {
   const root = document.getPartRoot();
   assert_true(root instanceof DocumentPartRoot);
   assert_true(root instanceof PartRoot);
   const parts = root.getParts();
   assert_equals(parts.length,0,'getParts() should start out empty');
 
-  const nodePart = new NodePart(root,target);
+  const nodePart = addCleanup(t,new NodePart(root,target));
   assert_true(nodePart instanceof NodePart);
   assert_equals(nodePart.node,target);
   assert_equals(nodePart.root,root);
   assert_equals(root.getParts().length,1,'getParts() for the root should now have this nodePart');
   assert_equals(root.getParts()[0],nodePart);
   assert_equals(parts.length,0,'Return value of getParts() is not live');
 
-  const childNodePart = new ChildNodePart(root,target.children[0], target.children[2]);
+  assert_throws_dom("NotSupportedError",() => new NodePart(nodePart,target.children[0]),'Constructing a Part with a NodePart as the PartRoot should throw');
+
+  const childNodePart = addCleanup(t,new ChildNodePart(root,target.children[0], target.children[2]));
   assert_true(childNodePart instanceof ChildNodePart);
   assert_true(childNodePart instanceof Part);
   assert_equals(childNodePart.root,root);
@@ -47,14 +54,24 @@ <h1 id="name">First<?child-node-part name?>Middle<?/child-node-part?>Last</h1>
   assert_equals(root.getParts()[1],childNodePart);
 
   const nodeBefore = target.previousSibling || target.parentNode;
-  const nodePartBefore = new NodePart(root,nodeBefore);
+  const nodePartBefore = addCleanup(t,new NodePart(root,nodeBefore));
   assert_equals(root.getParts().length,3,'getParts() for the root should now have this nodePart');
   assert_array_equals(root.getParts(),[nodePartBefore,nodePart,childNodePart],'getParts() should return nodes in tree order');
 
-  const nodePart2 = new NodePart(childNodePart,target.children[2]);
+  const nodePart2 = addCleanup(t,new NodePart(childNodePart,target.children[2]));
   assert_equals(nodePart2.root,childNodePart);
   assert_equals(root.getParts().length,3,'getParts() for the root DocumentPartRoot shouldn\'t change');
-  assert_equals(childNodePart.getParts().length,1);
-  assert_equals(childNodePart.getParts()[0],nodePart2);
+  assert_array_equals(childNodePart.getParts(),[nodePart2]);
+
+  nodePart2.disconnect();
+  assert_equals(nodePart2.root,null);
+  assert_equals(nodePart2.node,target.children[2],'node should still be connected');
+  assert_equals(childNodePart.getParts().length,0,'calling disconnect() should remove the part from root.getParts()');
+  assert_equals(root.getParts().length,3,'getParts() for the root DocumentPartRoot still shouldn\'t change');
+  nodePart2.disconnect(); // Calling twice should be ok.
+
+  childNodePart.disconnect();
+  assert_equals(childNodePart.root,null);
+  assert_array_equals(root.getParts(),[nodePartBefore,nodePart]);
 }, 'Basic imperative DOM Parts object construction');
 </script>

third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt

@@ -6756,6 +6756,7 @@ interface Part : PartRoot
     getter metadata
     getter root
     method constructor
+    method disconnect
 interface PartRoot
     attribute @@toStringTag
     method clone