Prevent dom-repeat's forwardHostProp from retaining HTMLElement this

[sigurdschneider]

TL;DR: The function __ensureTemplatized in elements/dom-repeat.js
caused the templatizeInstanceClass to retain a reference to the
HTMLElement (i.e. the DomRepeat object) that triggered creation of the
templatizeInstanceClass (see templatize.js:566).

The function __ensureTemplatized in elements/dom-repeat.js contained
an arrow function definition for the MutationObserver and the
function for forwardHostProp in it; the arrow function definition
forces context allocation of this so it can accesses it later. This
introduces false sharing with forwardHostProp: that function refers
to the same context, and transitively also to the this of
__ensureTemplatized, although it neither references nor accesses it.
Note that forwardHostProp will get it's own this variable on each
invocation). This results in the HTMLElement that originally created
the __ctor to be retained (i.e. not cleaned up by the GC). The
retainment path can be seen in the following heap snapshot:
https://imgur.com/a/javUpRx

In the screenshot, the tab on the top shows the detached HTML element
(the HTMLElement that created the __ctor, which I suspect is really
a DomRepeat object inheriting from HTMLElement) and the bottom shows
the retainment chain. The retainment chain is to be read as follows:
Line 1: this is a property of a context. Line 2: that context is
the context of the function forwardHostProp. Line 3: That function
is stored in a property called forwardHostProp of an Object. Line 4:
that Object is stored in a property __templatizeOptions in an object
with constructor TemplateLegacy. Line 5: That TemplateLegacy object
is the __proto__ of an object constructed with constructor klass.
Line 6: That object, in turn, is the __proto__ of an object
constructed with klass$jscomp$1. Line 7: that object is a property
called __templatizeInstance on am HTMLElement object. Line 8: that
HTMLElement is an internal property of a ShadowRoot object, and so on.

This CL fixes this problem by moving the arrow function into its own
method __waitForTemplate. This causes the context of
__waitForTemplate to get a context-allocated this, but prevents
the context of __ensureTemplatized from having a context-allocated
this. Unfortunately, it is hard to verify this fix, because
inspecting the function object for forwardHostProp shows all these
context collapsed into 'Local'. Basically, the only way to verify a
fix for an issue like this atm. is to get a reproducible way to
produce a heap snapshot that exhibits the problem, and then verifying
that the problem is not present anymore after applying the fix.
In my repro (navigating between to pages of Gerrit code review tool)
this got rid of about 37 detached HTMLElements.

Reference Issue

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.