[BUGFIX release] [Fixes #15492] WorkAround WebKit/JSC JIT WorkARound #15502
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stefanpenner
force-pushed
the
jit-fun
branch
2 times, most recently
from
581dd95
to
a676ef2
Compare
|
found we still set null explicitly here: https://github.com/emberjs/ember.js/blob/master/packages/ember-runtime/lib/mixins/array.js#L641 |
stefanpenner
force-pushed
the
jit-fun
branch
2 times, most recently
from
c759eaa
to
89b311b
Compare
krisselden
reviewed
Jul 14, 2017
packages/ember-metal/lib/meta.js
Outdated
| @@ -518,7 +518,7 @@ if (HAS_NATIVE_WEAKMAP) { | |||
| // stop if we find a `null` value, since | |||
There was a problem hiding this comment.
this comment seems incorrect
| @@ -638,7 +638,7 @@ const ArrayMixin = Mixin.create(Enumerable, { | |||
| function EachProxy(content) { | |||
| this._content = content; | |||
| this._keys = undefined; | |||
| this.__ember_meta__ = null; | |||
| this.__ember_meta__ = undefined; | |||
There was a problem hiding this comment.
I think we can strip it once we drop support for old browsers entirely. For now, keeping the shape stable sorta important?
There was a problem hiding this comment.
why not then call meta(this) instead?
There was a problem hiding this comment.
I wanted to avoid making any significant changes for bugfix we intend to backport without proper measurement.
But I can gladly make the change
packages/ember-metal/lib/meta.js
Outdated
| @@ -509,7 +509,7 @@ if (HAS_NATIVE_WEAKMAP) { | |||
| peekMeta = function WeakMap_peekParentMeta(obj) { | |||
| let pointer = obj; | |||
| let meta; | |||
| while (pointer !== undefined && pointer !== null) { | |||
| while (pointer !== undefined && null !== pointer) { | |||
There was a problem hiding this comment.
what is this for? this seems unrelated
stefanpenner
force-pushed
the
jit-fun
branch
2 times, most recently
from
f5c2548
to
83ea077
Compare
Affected version: 10.1.1 (12603.2.4) Unaffected version: Safari 11.0, WebKit 12604.1.29 Which suggests the issue has been addressed upstream. --- The following commit 986710f#diff-7e13eecefe753df1d82ce67b32bc4366R566 remove an implicit toBoolean conversion in WeakMap_peekParentMeta which apparently trips up Webkit… Simply reversing the expression addressed the issue. failed: `if (meta === null || meta !== undefined) { … }` passed: `if (meta !== undefined || meta === null) { … }` We also, confirmed that reverting the one line in question to `meta === null || !meta) {` fixed Safari. This PR removes the need for the an unused feature which avoids the expression entirely. @krisselden / @stefanpenner
krisselden
approved these changes
Jul 14, 2017
backspace
added a commit
to travis-ci/travis-web
that referenced
this pull request
Jul 14, 2017
Thanks to @krisselden and @stefanpenner for fixing emberjs/ember.js#15502.
backspace
added a commit
to travis-ci/travis-web
that referenced
this pull request
Jul 17, 2017
Thanks to @krisselden and @stefanpenner for fixing emberjs/ember.js#15502.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Affected version: Version 10.0.3 (12602.4.8)
Unaffected versions: Safari 11.0, WebKit 12604.1.29
Which suggests the issue has been addressed upstream.
The following commit 986710f#diff-7e13eecefe753df1d82ce67b32bc4366R566 remove an implicit toBoolean conversion in WeakMap_peekParentMeta which apparently trips up Webkit…
Simply reversing the expression addressed the issue.
failed:
if (meta === null || meta !== undefined) { … }passed:
if (meta !== undefined || meta === null) { … }We also, confirmed that reverting the one line in question to
meta === null || !meta) {fixed Safari.This PR removes the need for the an unused feature which avoids the expression entirely.
@krisselden / @stefanpenner