Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

getOwner(this) is sometimes returning undefined after ember-2.14.0-beta2 #15492

Closed
psbanka opened this issue Jul 11, 2017 · 10 comments
Closed

getOwner(this) is sometimes returning undefined after ember-2.14.0-beta2 #15492

psbanka opened this issue Jul 11, 2017 · 10 comments
Labels
Bug

Comments

@psbanka
Copy link

psbanka commented Jul 11, 2017

This is related to:

intercom/ember-href-to#81
and
https://github.com/Vestorly/torii/issues/349#issuecomment-302141236
and perhaps
#15468

It appears that sometimes getOwner(this) returns undefined, and we are having to work around it with const owner = getOwner(this) || this.container
@psbanka psbanka mentioned this issue Jul 11, 2017
Open
@rwjblue
Copy link
Member

rwjblue commented Jul 12, 2017

Is there a reproduction?

@krisselden
Copy link
Contributor

reproduction https://github.com/simonihmig/ember-2-14-safari

@krisselden
Copy link
Contributor

I've not see a reproduction on a production build.

@krisselden
Copy link
Contributor

I need to know if this is a problem with production builds and if anyone has a reproduction in a production build.

@krisselden
Copy link
Contributor

people have confirmed this affects production builds via error logs

@krisselden
Copy link
Contributor

I still would like a production build reproduction, but I've also confirmed with the existing reproduction that WebKit nightly is not affected by this.

@krisselden
Copy link
Contributor

Bisected to this 44e04ba then checked the cherry picked commit 986710f against its parent.

It seems to be this improved JIT but the JIT is deopting at some late point and not returning undefined from peekMeta

stefanpenner added a commit that referenced this issue Jul 14, 2017
@stefanpenner
[BUGFIX release] [Fixes #15492] WorkAround WebKit/JSC JIT WorkARound
ad67673 
Affected versions: Version 10.0.3 (12602.4.8)
Unaffected version: r219450 (WebKit Nightly)

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 krisselden changed the title getOwner(this) is sometimes returning undefined after ember-2.13.0-beta2 getOwner(this) is sometimes returning undefined after ember-2.14.0-beta2 Jul 14, 2017
stefanpenner added a commit that referenced this issue Jul 14, 2017
@stefanpenner
[BUGFIX release] [Fixes #15492] WorkAround WebKit/JSC JIT WorkARound
581dd95 
Affected versions: Version 10.0.3 (12602.4.8)
Unaffected version: r219450 (WebKit Nightly)

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
Copy link
Contributor

This versions of Safari I've checked are Version 10.1.1 (12603.2.4) which has the problem and the WebKit Nightly Revision 219450 which is fixed.

stefanpenner added a commit that referenced this issue Jul 14, 2017
@stefanpenner
[BUGFIX release] [Fixes #15492] WorkAround WebKit/JSC JIT WorkARound
a676ef2 
Affected versions: Version 10.0.3 (12602.4.8)
Unaffected version: r219450 (WebKit Nightly)

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
Copy link
Contributor

@stefanpenner just checked Release 35 (Safari 11.0, WebKit 12604.1.29) it is fixed there too.

stefanpenner added a commit that referenced this issue Jul 14, 2017
@stefanpenner
[BUGFIX release] [Fixes #15492] WorkAround WebKit/JSC JIT WorkARound
c759eaa 
Affected versions: Version 10.0.3 (12602.4.8)
Unaffected version: r219450 (WebKit Nightly)

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
stefanpenner added a commit that referenced this issue Jul 14, 2017
@stefanpenner
[BUGFIX release] [Fixes #15492] WorkAround WebKit/JSC JIT WorkARound
89b311b 
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
stefanpenner added a commit that referenced this issue Jul 14, 2017
@stefanpenner
[BUGFIX release] [Fixes #15492] WorkAround WebKit/JSC JIT WorkARound
f5c2548 
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
stefanpenner added a commit that referenced this issue Jul 14, 2017
@stefanpenner
[BUGFIX release] [Fixes #15492] WorkAround WebKit/JSC JIT WorkARound
83ea077 
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
stefanpenner added a commit that referenced this issue Jul 14, 2017
@stefanpenner
Merge pull request #15502 from emberjs/jit-fun
2adb0fd 
[BUGFIX release] [Fixes #15492] WorkAround WebKit/JSC JIT WorkARound
stefanpenner added a commit that referenced this issue Jul 14, 2017
@stefanpenner
[BUGFIX release] [Fixes #15492] WorkAround WebKit/JSC JIT WorkARound
d783db7 
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
stefanpenner added a commit that referenced this issue Jul 14, 2017
@stefanpenner
[BUGFIX release] [Fixes #15492] WorkAround WebKit/JSC JIT WorkARound
5b03e17 
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
@stefanpenner
Copy link
Member

stefanpenner commented Jul 14, 2017 •
edited
Loading

master: #15502 (green + merged)
beta: #15504 (running)
release: #15505 (running)

cc @rwjblue

locks added a commit that referenced this issue Jul 14, 2017
@locks
Merge pull request #15504 from emberjs/jit-fun-beta
1322689 
backport to BETA [BUGFIX release] [Fixes #15492] WorkAround WebKit/JSC JIT WorkARound
locks added a commit that referenced this issue Jul 14, 2017
@locks
Merge pull request #15505 from emberjs/jit-fun-release
0299c70 
release backport [BUGFIX release] [Fixes #15492] WorkAround WebKit/JSC JIT WorkARound
@stefanpenner stefanpenner mentioned this issue Jul 14, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@stefanpenner @rwjblue @krisselden @psbanka