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.

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

[TIMOB-23901] Fix IdleNotificationDeadline() duration #8411

Merged
merged 1 commit into from Sep 21, 2016

Conversation

garymathews
Copy link
Contributor

@garymathews garymathews commented Sep 21, 2016

  • This is the root cause of the splash screen issue, the nativeDispose() call hangs the KrollRuntimeThread which prevents the message handler from processing the MSG_INIT to re-initialise KrollRuntime
  • This is due to an incorrect IdleNotificationDeadline() duration
TEST CASE
  1. Create and launch any Titanium Android application
  2. Press the back button to close the app
  3. Launch the app again
EXPECTED
  • Application should resume
ACTUAL
  • Application hangs on splash screen

JIRA Ticket

@build
Copy link
Contributor

build commented Sep 21, 2016

Can one of the admins verify this patch?

@sgtcoolguy
Copy link
Contributor

ok to test

@sgtcoolguy
Copy link
Contributor

add to whitelist

@sgtcoolguy
Copy link
Contributor

looks good to me.

@sgtcoolguy sgtcoolguy added this to the 6.1.0 milestone Sep 21, 2016
@sgtcoolguy sgtcoolguy merged commit 58f836c into tidev:master Sep 21, 2016
@sgtcoolguy
Copy link
Contributor

@sgtcoolguy
Copy link
Contributor

@ashcoding @garymathews I guess maybe the old V8 integration had effectively the same bug?

It seemed unlikely, but maybe it's not?

These seem relevant:

They moved from:

while(!V8::IdleNotification()) {};

to:

while(!V8::IdleNotification(500)) {};
V8::LowMemoryNotification();

Though, I honestly don't think there's much difference there. The first is actually equivalent to:

while(!V8::IdleNotification(1000)) {};

I mean that code could possibly just loop forever and not return true - but I don't really know.

@ashcoding
Copy link
Contributor

@garymathews Tested this PR by building master branch and using Nexus 6 with Android 6.0 running. Still facing the issue. I'll add comments on the JIRA ticket to ask for QE to help test this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants