fix($compile): workaround a GC bug in Chrome < 50

[dcherman]

In the version of V8 used in Chrome < 50, the parent of template nodes for
transclude: "element" directives would be improperly garbage collected
despite still having been referenced via parentNode.

No tests are included with this change since this is not a deterministic bug and would require triggering a GC sweep which would involve running the test suite with v8 natives enabled.

Fixes #14041

[dcherman]

For reasons mentioned in the original message, no tests were included, however I cannot reproduce the bug anymore with this patch.

[gkalpak]

Crazy how you tracked this down 😃

[lgalfaso]

Is there a bug opened in Chrome that tracks this? If there is one, can this information be added as a comment in the code.

[dcherman]

@lgalfaso So before making this PR I tried to find any issue or commit on the Chromium, Blink, or v8 bug trackers/repos that I could reference in a comment or the commit message, but I couldn't find anything. I think chances are high that it's a commit in v8 that fixed the actual bug since it's GC related, but no message that has the word GC, Garbage, or DOM stands out as being related to this.

Easiest way to figure it out would end up being to git-bisect v8 and re-run the test case to find the commit where it gets fixed, but I don't have a Chromium/v8 build environment and it may not be worth the effort.

[lgalfaso]

Hey, what I was looking for is more basic:

• In the comment, it claims that this bug only affects Chrome < 50. Is this
  fixed in Chrome 50?
• If yes, done!
• If no, open a bug in Chrome with this example and add a reference in the
  code to the bug. If someone looks at the code, they will know why it is
  there and when it is safe to delete it

On Tuesday, March 22, 2016, dherman [email protected] wrote:

@lgalfaso https://github.com/lgalfaso So before making this PR I tried
to find any issue or commit on the Chromium, Blink, or v8 bug
trackers/repos that I could reference in a comment or the commit message,
but I couldn't find anything. I think chances are high that it's a commit
in v8 that fixed the actual bug since it's GC related, but no message that
has the word GC, Garbage, or DOM stands out as being related to this.

Easiest way to figure it out would end up being to git-bisect v8 and
re-run the test case to find the commit where it gets fixed, but I don't
have a Chromium/v8 build environment and it may not be worth the effort.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#14286 (comment)

[gkalpak]

@lgalfaso, there is "empirical evidence" that it's fixed in Chrome 50 (but no hard proof 😛).

[lgalfaso]

@gkalpak empirical evidence is good enough :)

[dcherman]

Yep, I tested the original un-patched reproducer in both Chrome 50 and 51 (current canary). I cannot reproduce in either version, so somewhere between v8 versions 4.9.385.28 and 5.0.71, there was a commit that fixed this problem.

[Narretz]

Thanks @dcherman!

[gkalpak]

@Narretz, shouldn't this also be backported to v1.5.x (so it's included in the upcoming release) ?

[Narretz]

Absolutely yes. Will do
Am 23.03.2016 21:26 schrieb "Georgios Kalpakas" [email protected]:

@Narretz https://github.com/Narretz, shouldn't this also be backported
to v1.5.x (so it's included in the upcoming release) ?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#14286 (comment)