Build Resource Infinite Loop

[jridgewell]

We currently have a hidden infinite loop:

When building <amp-lightbox>, we append each child to a new container element. This causes a us to call attachedCallback on that child, which adds it the list of resources we need to track.

When we add that element to our list of resources, we add it to a list of unbuilt elements (this is our first problem, but we'll come back to it). When we add to the unbuilt elements list, we try to build any unbuilt resources.

And here's the infinite loop. Notice that we're still trying to build our <amp-lightbox>. Which means the <amp-lightbox> is still "unbuilt", and it's still in that list of unbuilt elements we're should try to build. So after adding that child element to the list of unbuilts, we'll loop through all of our unbuilts to build them. And lo and behold, one of them is the <amp-lightbox>. So we'll build it again, causing us to append all children to a container, causing the attachedCallback, causing the child it to be added to our unbuilts again. And again. And again. And again.

Easy Fix

Splice the current element before trying to build it. This is not a good solution.

Better Fix

Use a currentTryingToBuildOurUnbuilts mutex. When we enter #buildReadyResources_, check if it's true. If it is, return early. If not, set it to true, then do the loop. After the loop, set it to false.

Why's this better? We'll avoid a huge call stack. Let it unwind after the attachedCallback until we get back to the build loop. Because we're not caching the unbuilt's length, we'll eventually get to the newly added unbuilt element.

---

Remember that first problem I mentioned above? It's that we sometimes wrap our children in a container. But those children may have already been built. But since we fire another attachedCallback, we'll add them to the unbuilts again, and try to build again. It's cool to add them to the list again, but we should make sure they're really unbuilt before trying to #build them again.

[dvoytenko]

@jridgewell @mkhatib Makes sense and let's proceed with the fix. One thing: if the element is already built - we certainly don't need to build it again. But adding it to the "pending" set today shouldn't really cause any problems since CustomElement always ensures that it builds only once. Again - it's certainly better not to even try to build it in this case.

One case that still would fall through the cracks is rebuild of cloned nodes. I noticed it happening the other day in lightbox as well. This happens because a cloned node will have a new instance of custom element's ElementProto and not aware that it's already been built. I filed a new bug for this.

[mkhatib]

What are cloned nodes? These are other than the re-parenting of a node? (Just saw the bug #3365)