Titanium JIRA Archive
Titanium SDK/CLI (TIMOB)

[TIMOB-24893] Windows: onreadystatechange should not be called on the UI thread

GitHub Issuen/a
TypeBug
PriorityHigh
StatusClosed
ResolutionFixed
Resolution Date2017-08-25T12:40:42.000+0000
Affected Version/sRelease 6.2.0
Fix Version/sRelease 6.2.0
ComponentsWindows
Labelsregression
ReporterEwan Harris
AssigneeKota Iguchi
Created2017-06-26T14:15:52.000+0000
Updated2017-08-28T15:20:41.000+0000

Description

Description

**The code below is a boiled down version of how LiveView makes http requests so it is crucial for this to work** The following code below appears to freeze the application, I believe this is because HTTPClient is running things such as onreadystatechange on the UI thread which is being blocked by the while loop, the following diff appears to fix the issue but I am not sure whether the changes were required https://gist.github.com/ewanharris/f45cf4347759247a3965e02161342fde
var win = Ti.UI.createWindow(),
    btn = Ti.UI.createButton({
        title: 'HTTP GET'
    });
btn.addEventListener('click', function () {
    var request = Ti.Network.createHTTPClient({
        onload: function () {
            alert('onload:\n\treadyState: ' + request.readyState);
        },
        onerror: function (e) {
            alert('onerror:\n\treadyState: ' + request.readyState + '\n\terror: ' + e.error);
        }
    }),
        timeout = new Date().getTime() + 5000,
        host = 'http://www.google.com/',
        done = false;
    request.cache = false;
    request.open('GET', host);
    request.send();
    while (!done) {
        if (request.readyState === 4 || request.status === 404) {
            done = true;
            alert('success');
        } else if ((timeout - (new Date()).getTime()) <= 0) {
            done = true;
            alert('failed');
        }
    }
});
win.add(btn);
win.open();

Steps to reproduce

Add the above code to an existing app.js and build for Windows

Click the HTTP GET button

Actual

App appears to freeze (button stays in clicked position), failed alert appears

Expected

App should not appear to freeze, no failed alert should show

Comments

  1. Ewan Harris 2017-07-27

    Assigning this to 6.2.0 as it is a regression and breaks liveview
  2. Kota Iguchi 2017-08-11

    Basically we need to ensure all JavaScript callbacks including onload and onerror should be called from UI thread especially on Windows 10 Store app, otherwise the app will crash. That's what TIMOB-24671 originally meant. For instance even following simple app crashes when you don't use RunOnUIThread. I would think that waiting readyState in while loop was basically a bad idea, but for a quick workaround I would suggest just removing RunOnUIThread from onreadystate for now, and deprecate listening onreadystate event on Windows Store app?
       var win = Ti.UI.createWindow(),
           btn = Ti.UI.createButton({
               title: 'HTTP GET'
           });
       btn.addEventListener('click', function () {
           var request = Ti.Network.createHTTPClient({
               onload: function () {
                   alert('onload:\n\treadyState: ' + request.readyState);
               },
               onerror: function (e) {
                   alert('onerror:\n\treadyState: ' + request.readyState + '\n\terror: ' + e.error);
               }
           }),
               timeout = new Date().getTime() + 5000,
               host = 'http://www.theresnosuchdomain.com/',
               done = false;
           request.cache = false;
           request.open('GET', host);
           request.send();
       });
       win.add(btn);
       win.open();
       
  3. Kota Iguchi 2017-08-11

    Can't we just update the LiveView to use setTimeout instead of waiting in while loop? We can use Ti.Platform.name == 'windows' to ensure this change affects only for Windows.
           if (Ti.Platform.name == 'windows' ) {
               setTimeout(function () {
                   if (request.readyState === 4 || request.status === 404) {
                       alert('success');
                   } else if ((timeout - (new Date()).getTime()) <= 0) {
                       alert('failed');
                   }
               }, timeout);
           }
       
  4. Ewan Harris 2017-08-14

    [~kiguchi] While I do agree that we could and should change LiveView I think we'd still need to revert the changes as we can't break usage in a minor. Also I'm not sure whether we're planning to release Studio alongside 6.2.0 which would be required if we made the changes in LiveView
  5. Kota Iguchi 2017-08-15

    https://github.com/appcelerator/titanium_mobile_windows/pull/1075
  6. Ewan Harris 2017-08-25

    6_2_X backport: https://github.com/appcelerator/titanium_mobile_windows/pull/1086
  7. Ewan Harris 2017-08-28

    Verified in 6.2.0.v20170825165823. Changes are not in master (7.0.0) yet though, will hold on closing til it is
  8. Ewan Harris 2017-08-28

    Aaaand verified in 7.0.0.v20170828071347, closing ticket

JSON Source