[7_4_X] fix(ios): Fix iOS 8/9 compatibility of timers on main thread, register exception handler #10428
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
hansemannn
reviewed
Nov 6, 2018
iphone/Classes/KrollContext.m
Outdated
| JSValue *callbackFunction = [callbackArgs objectAtIndex:0]; | ||
| [callbackArgs removeObjectAtIndex:0]; | ||
| double interval = [[callbackArgs objectAtIndex:0] toDouble] / 1000; | ||
| [callbackArgs removeObjectAtIndex:0]; | ||
| JSVirtualMachine *vm = callbackFunction.context.virtualMachine; |
There was a problem hiding this comment.
For SDK < 8, this should likely use the Ti prefixed JSCore calls, otherwise this could break apps not using JSCore so far.
There was a problem hiding this comment.
The new timer management is only enabled with JSCore
sgtcoolguy
force-pushed
the
timers-managed-7_4_X
branch
from
e3b94cb
to
fcefb32
Compare
sgtcoolguy
commented
Nov 8, 2018
iphone/Classes/KrollContext.m
Outdated
| return [timerIdentifier unsignedIntegerValue]; | ||
| } | ||
|
|
||
| - (void)callJsCallback:(NSTimer *_Nonnull)timer | ||
| { | ||
| NSArray<JSValue *> *args = [timer userInfo]; |
There was a problem hiding this comment.
I'm not sure if trying to optimize for the most common case of no additional arguments and setting just the callback as the user info and then checking type here would improve performance or not...
Tests:
Generated by 🚫 dangerJS |
sgtcoolguy
changed the title
…r exception handler - Re-work timers to try and do less ourselves around cleaning up single-shot timers. Keep weak references to timers, let API invalidate those single-shot timers - Handle unspecified interval/duration, set minimum to 1ms - Make setTimeout/setInterval/clearTimeout block signatures match expected types (even though we cheat and access JSContext currentArguments for varargs)
sgtcoolguy
force-pushed
the
timers-managed-7_4_X
branch
from
d5d9aa1
to
f07b13d
Compare
janvennemann
reviewed
Nov 8, 2018
iphone/Classes/KrollContext.m
Outdated
| @@ -1610,36 +1622,54 @@ - (void)dealloc | |||
|
|
|||
| - (void)invalidateAllTimers | |||
| { | |||
| for (NSNumber *timerIdentifier in self.timers.allKeys) { | |||
| for (NSNumber *timerIdentifier in self.timers) { | |||
There was a problem hiding this comment.
Is it save to iterate directly over the NSMapTable and remove entries?
sgtcoolguy
added a commit
that referenced
this pull request
Nov 8, 2018
… register exception handler (#10428) * fix(ios): Fix iOS 8/9 compatibility of timers on main thread, register exception handler - Re-work timers to try and do less ourselves around cleaning up single-shot timers. Keep weak references to timers, let API invalidate those single-shot timers - Handle unspecified interval/duration, set minimum to 1ms - Make setTimeout/setInterval/clearTimeout block signatures match expected types (even though we cheat and access JSContext currentArguments for varargs)
rlustemberg
pushed a commit
to inzdr/titanium_mobile
that referenced
this pull request
Nov 19, 2018
… register exception handler (tidev#10428) * fix(ios): Fix iOS 8/9 compatibility of timers on main thread, register exception handler - Re-work timers to try and do less ourselves around cleaning up single-shot timers. Keep weak references to timers, let API invalidate those single-shot timers - Handle unspecified interval/duration, set minimum to 1ms - Make setTimeout/setInterval/clearTimeout block signatures match expected types (even though we cheat and access JSContext currentArguments for varargs)
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.
JIRA: https://jira.appcelerator.org/browse/TIMOB-26391
Optional Description:
Backport of #10426