Fix assignedNodes({flatten:true}) to return fallback content in document tree

third_party/WebKit/Source/core/html/HTMLSlotElement.cpp

 /*
  * Copyright (C) 2015 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 59 matching lines @@
     const AssignedNodesOptions& options) {
   UpdateDistribution();
   if (options.hasFlatten() && options.flatten())
     return GetDistributedNodesForBinding();
   return assigned_nodes_;
 }
 
 const HeapVector<Member<Node>>
 HTMLSlotElement::GetDistributedNodesForBinding() {
   DCHECK(!NeedsDistributionRecalc());
+  return GetDistributedNodesWithFallback();
+}
+
+const HeapVector<Member<Node>>
+HTMLSlotElement::GetDistributedNodesWithFallback() const {

[hayato, 2017/05/16 07:50:07]

The concept of DistributedNodes includes Fallback elements. So this name would
be confusing.

I rather name this just 'GetDistributedNodes'.

Couldn't we unify this with GetDistributedNodesForBinding?
Something is wrong if the only difference between them is DCHECK().
That implies that we should use DCHECK() for both cases.

Could you pursue to unify them?

   if (SupportsDistribution())
     return distributed_nodes_;
-
-  // If a slot does not support distribution, its m_distributedNodes should not
+  // If a slot does not support distribution, its distributed_nodes_ should not
   // be used.  Instead, calculate distribution manually here. This happens only
   // in a slot in non-shadow trees, so its assigned nodes are always empty.
   HeapVector<Member<Node>> distributed_nodes;
   Node* child = NodeTraversal::FirstChild(*this);
   while (child) {
     if (!child->IsSlotable()) {
       child = NodeTraversal::NextSkippingChildren(*child, this);
       continue;
     }
     if (isHTMLSlotElement(child)) {
@@ skipping 31 matching lines @@
 }
 
 void HTMLSlotElement::AppendDistributedNode(Node& node) {
   size_t size = distributed_nodes_.size();
   distributed_nodes_.push_back(&node);
   distributed_indices_.Set(&node, size);
 }
 
 void HTMLSlotElement::AppendDistributedNodesFrom(const HTMLSlotElement& other) {
   size_t index = distributed_nodes_.size();
-  distributed_nodes_.AppendVector(other.distributed_nodes_);
-  for (const auto& node : other.distributed_nodes_)
+
+  const HeapVector<Member<Node>>& other_distributed_nodes =
+      other.GetDistributedNodesWithFallback();
+  distributed_nodes_.AppendVector(other_distributed_nodes);
+
+  for (const auto& node : other_distributed_nodes)
     distributed_indices_.Set(node.Get(), index++);
 }
 
 void HTMLSlotElement::ClearDistribution() {
   // TODO(hayato): Figure out when to call
   // lazyReattachDistributedNodesIfNeeded()
   assigned_nodes_.clear();
   distributed_nodes_.clear();
   distributed_indices_.clear();
 }
@@ skipping 229 matching lines @@
 
 DEFINE_TRACE(HTMLSlotElement) {
   visitor->Trace(assigned_nodes_);
   visitor->Trace(distributed_nodes_);
   visitor->Trace(old_distributed_nodes_);
   visitor->Trace(distributed_indices_);
   HTMLElement::Trace(visitor);
 }
 
 }  // namespace blink