[BUGFIX release] Fix for #13071 V8 inlining bug

[raido]

I have made changes based on a comment here - https://bugs.chromium.org/p/v8/issues/detail?id=4839#c9

I will prefix the commit once it is proven that this solves the problem.

[workmanw]

As mentioned on #13071, as far as I can tell, this does successfully work around the V8 bug.

Given the nature of this issue, I would advocate this be merged and a new version of ember 2.4 be cut. I'm not sure, but it might be worth also back porting for 2.3 users given that 2.3 was the first version that exposed this V8 bug.

Also since it's not at all obvious why the code is this way, it might make sense to add a quick code comment referencing either this PR or issue #13071. Maybe that would help to avoid this regressing in the future. I'm not sure what ember's code standards are in this area, that might be overkill. Just a thought, maybe an ember core member could weigh in on the value of that.

[rwjblue]

I'd be fine with a small comment, but I also think that Chrome should fix it :stuck_out_tongue_closed_eyes.

I completely agree that this should be backported to 2.4.x, as such the commit should be prefixed with [BUGFIX release].

[raido]

Prefixed and added issue links as comments.

[rwjblue]

Once green I'll pull into release branch and y'all can use it from there. I'd love it if folks could test that on the release channel before doing an official 2.4.3 just to get a bit more testing time into it and really confirm things are fixed...

[workmanw]

@rwjblue Hehe. I am building Chrome from source presently to test their fix. (https://codereview.chromium.org/1811693002).

My main concern is that we unfortunately have enterprise customers who lock their users into arbitrary versions of Chrome. One customer only permits their employees to use Chrome 42 . Obviously ember can't support every version of Chrome forever. But having an ember side workaround for a period of time should help smooth out the impact of this issue.

I have no idea why enterprises insist on locking their users into versions of Chrome. I've asked this repeatedly and gotten no good answers :(.

[raido]

@rwjblue Sure, i will test release branch once it's merged.

[stefanpenner]

@raido / @workmanw good job guys!

[jakobkummerow]

Just to note: the Chrome-side fix will show up in Canary in the next 1-2 days, and then in versions 49 and 50 over the next 1-3 weeks or so (whenever Stable/Beta channels get refreshed).
Of course if you want to support older revisions (as workmanw noted), you have no choice but to also deploy the ember-side workaround.

Running outdated browser versions (of any browser!) is indeed a pretty bad idea, especially in an enterprise environment where security presumably matters. All outdated browsers have security bugs (as well as other bugs, like this one) that simply won't get fixed.

[jaro-sevcik]

Indeed, good job; thank you for your hard work on the repro! Glad to hear the suggested workaround worked.

This kind of bugs is really hard to discover for us because such bugs do not result in crashes, so we do not see them in statistics. This particular one was especially tricky because it relies on deoptimization triggering at a very specific point for a very specific reason (I spent embarrassing amount of time debugging this).

[stefanpenner]

glad ember can be a smoke test for you guys :)

[rwjblue]

Released in v2.4.3.

[stefanpenner]

@rwjblue <3