[TIMOB-26226] iOS: Remove Kroll-Thread

[hansemannn]

JIRA: https://jira.appcelerator.org/browse/TIMOB-26226

[build]

	Messages
📖	👍 Hey!, You deleted more code than you added. That's awesome!
📖	💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

[sgtcoolguy]

Are you certain there's no thread contention here any more?

[hansemannn]

I think it is a left-over from the initial kroll-thread deprecation. We used NSLock context-locks in some other places to guard race conditions, but these should not even occur with main thread enabled. I'll write up some setTimeout penetration tests to validate this behavior.

[hansemannn]

This seems to work fine so far (git-diffing where the initial locking came from):

var win = Ti.UI.createWindow({
    backgroundColor: '#fff'
});

var btn = Ti.UI.createButton({
    title: 'Trigger'
});

btn.addEventListener('click', function() {
    setInterval(function () {
        Ti.API.info('INTERVAL');
    }, 1000);
    setTimeout(function () {
        Ti.API.info('1');
        setTimeout(function() {
            Ti.API.info('LAST');
        }, 500);
    }, 50);
    setTimeout(function () {
        Ti.API.info('2');
    }, 100);
    setTimeout(function () {
        Ti.API.info('3');
    }, 150);
    setTimeout(function () {
        Ti.API.info('4');
    }, 200);
    setTimeout(function () {
        Ti.API.info('5');
   }, 250);
});

win.add(btn);
win.open();

[sgtcoolguy]

CR looks good to me.

[sgtcoolguy]

Maybe my lack of Obj-C expertise is shining through here, but shouldn't the static variable be declared/initialized somewhere else? Does this only set it to 0 the very first time? Or will every timer id == 0?

[hansemannn]

It actually increases correctly, but you are right, it should rather be instance-scoped like here, although it doesn't really affect it here.

[sgtcoolguy]

@hansemannn I assume there's also some CLI code related to passing along the main thread/kroll thread value to the build? We should probably rip that out (and/or spit out an error/warning if they are explicitly trying to use Kroll thread which we no longer support).

[hansemannn]

@sgtcoolguy I updated the CLI to spit out a warning if the kroll-thread is explicitely used via tiapp.xml. Before merging this, we should really think about Android as well, as it seems it was never really deprecated in code there and was not even used by default, although running Ti SDK 7+.

@jquick-axway Are we save to run on the main-thread on Android?

[jquick-axway]

Are we safe to run on the main-thread on Android?

Yes. We mainly test Android apps whose JS runtime is configured for the main thread already. I'm all for this change.

Edit:
Wait... we still need to solve the one issue on Android where setting the backgroundImage property to a URL will crash if running on the main UI thread. I have a fix for it from last year that I've been sitting on. I believe @Topener has written up a ticket on this.

[hansemannn]

Yep, it seems to be TIMOB-24605 which I think we can fix independently from this one.

[jquick-axway]

@hansemannn, I also remember the iOS activity indicator can animate while the main UI thread is blocked, but Android and Windows do not support this. Although, the best solution to this problem is to have "ti.worker" support on all platforms and avoid blocking the UI thread.

(Android does have a SurfaceView which renders on a separate thread, but its z-order handling is infamously buggy. Especially when there is more than 1 onscreen.)