Make custom elements work in HTML imports

third_party/WebKit/Source/core/dom/custom/CustomElementUpgradeSorter.cpp

Index: third_party/WebKit/Source/core/dom/custom/CustomElementUpgradeSorter.cpp
diff --git a/third_party/WebKit/Source/core/dom/custom/CustomElementUpgradeSorter.cpp b/third_party/WebKit/Source/core/dom/custom/CustomElementUpgradeSorter.cpp
index 34f9e28e5ea741d3eb6a526ae982debb3d29d0fa..2401593c44455eb57dfcac980d5971b8c2abd685 100644
--- a/third_party/WebKit/Source/core/dom/custom/CustomElementUpgradeSorter.cpp
+++ b/third_party/WebKit/Source/core/dom/custom/CustomElementUpgradeSorter.cpp
@@ -8,6 +8,10 @@
 #include "core/dom/ElementTraversal.h"
 #include "core/dom/Node.h"
 #include "core/dom/shadow/ShadowRoot.h"
+#include "core/html/HTMLLinkElement.h"
+#include "core/html/imports/HTMLImportChild.h"
+#include "core/html/imports/HTMLImportTreeRoot.h"
+#include "core/html/imports/HTMLImportsController.h"
 
 namespace blink {
 
@@ -17,23 +21,59 @@ CustomElementUpgradeSorter::CustomElementUpgradeSorter()
 {
 }
 
+static HTMLLinkElement* GetParentLinkElement(Document* d)

[dominicc (has gone to gerrit), 2016/08/22 08:07:36]

Get -> get

Does HTML imports use this nomenclature of "parent link element"?

Can't you do this with document->importLoader()->firstImport()->link() and avoid
adding stuff to HTMLImportTreeRoot an O(n) search?

[kochi, 2016/08/22 12:02:00]

Renamed the function to getLinkElement().
Thanks!  I did not notice that path.  It worked.

+{
+    DCHECK(d);
+    HTMLImportsController* c = d->importsController();
+    if (!c)
+        return nullptr;
+
+    HTMLImportTreeRoot* r = c->root();
+    DCHECK(r);
+
+    HTMLImportChild* ch = r->find(*d);
+    if (!ch)
+        return nullptr;
+
+    // Get HTMLLinkElement <link rel="import">
+    return ch->link();
+}
+
+CustomElementUpgradeSorter::AddResult CustomElementUpgradeSorter::addToParentChildMap(
+    Node* parent,
+    Node* child)
+{
+    ParentChildMap::iterator it = m_parentChildMap->find(parent);
+    if (it != m_parentChildMap->end()) {
+        it->value.add(child);
+        // The entry for the parent exists; so must its parents.
+        return kParentAlreadyExistsInMap;
+    }
+    ParentChildMap::AddResult result =
+        m_parentChildMap->add(parent, HeapHashSet<Member<Node>>());
+    result.storedValue->value.add(child);
+    return kParentAddedToMap;
+}
+
 void CustomElementUpgradeSorter::add(Element* element)
 {
     m_elements->add(element);
 
-    for (Node* n = element, *parent = n->parentOrShadowHostNode();
-        parent;
-        n = parent, parent = parent->parentOrShadowHostNode()) {
-
-        ParentChildMap::iterator it = m_parentChildMap->find(parent);
-        if (it == m_parentChildMap->end()) {
-            ParentChildMap::AddResult result =
-                m_parentChildMap->add(parent, HeapHashSet<Member<Node>>());
-            result.storedValue->value.add(n);
-        } else {
-            it->value.add(n);
-            // The entry for the parent exists; so must its parents.
+    for (Node* n = element, *parent = n->parentOrShadowHostNode(); parent;) {

[dominicc (has gone to gerrit), 2016/08/22 08:07:36]

Think about what you've done here when the document element of an import is a
custom element. It won't work because the whole body of the for loop is skipped;
you've broadened the concept of 'parent' for imports but not applied it
consistently.

You need to pull 'parent' retrieval out into a function and call it consistently
for this, and below for line 67+.

[dominicc (has gone to gerrit), 2016/08/22 08:10:55]

Hmm, actually, maybe this is fine, because this will step into the document.
Maybe just add a comment about why the initializer of the for loop is special.

[kochi, 2016/08/22 12:02:00]

Added some comments.

+        if (addToParentChildMap(parent, n) == kParentAlreadyExistsInMap)
             break;
+
+        n = parent;
+        parent = parent->parentOrShadowHostNode();
+
+        if (!parent && n->isDocumentNode()) {
+            Document* d = toDocument(n);
+            Element* link = GetParentLinkElement(d);
+            if (!link || addToParentChildMap(link, n) == kParentAlreadyExistsInMap)
+                break;
+
+            n = link;
+            parent = link->parentOrShadowHostNode();
         }
     }
 }