Skip to content

Commit

Permalink
ANDROID: moved setContentView to after permissions
Browse files Browse the repository at this point in the history
Calling setContentView between permission requests would cause some apps to hang. This problem was previously addressed by adding a shell override function for onRequestPermissionResult, which is no longer needed and is removed in this commit
  • Loading branch information
peterlew committed Nov 19, 2020
1 parent 1f656d1 commit d86a516
Showing 1 changed file with 4 additions and 14 deletions.
18 changes: 4 additions & 14 deletions loaders/android/bootstrap.java.in
Original file line number Diff line number Diff line change
Expand Up @@ -107,17 +107,6 @@ public class @SYS_APPNAME@ extends Activity implements @ANDROID_JAVA_IMPLEMENTS@
return true;
}

@IF_ANDROIDAPI_GT_22@
@Override
public void onRequestPermissionsResult(int requestCode, String[] permissions, int[] grantResults) {
super.onRequestPermissionsResult(requestCode, permissions, grantResults);
//Must do something with results data or app will hang
int rc = requestCode;
String p = permissions[0];
int gr = grantResults[0];
}
/* end of IF_ANDROIDAPI_GT_22 */

@Override
public void startActivityForResult(Intent intent, int cont) {
try {
Expand Down Expand Up @@ -185,12 +174,13 @@ public class @SYS_APPNAME@ extends Activity implements @ANDROID_JAVA_IMPLEMENTS@
nativeInstanceInit(getApplicationContext().getPackageCodePath().toString(), getFilesDir().toString());

mSensorManager = (SensorManager)getSystemService(Context.SENSOR_SERVICE);
checkOrRequestPermission(android.Manifest.permission.WRITE_EXTERNAL_STORAGE);
setContentView(mGLView); // MUST NOT run before nativeInstanceInit completed

// Additions needed by modules, e.g. gps
checkOrRequestPermission(android.Manifest.permission.WRITE_EXTERNAL_STORAGE);
// Additions and permissions needed by modules, e.g. gps
@ANDROID_JAVA_ONCREATE@

setContentView(mGLView); // MUST NOT run before nativeInstanceInit completed

// start EVENT_IDLE
if(idle_tmScheduleRate > 0) idle_tm.scheduleAtFixedRate(idle_task, 0, idle_tmScheduleRate);
}
Expand Down

17 comments on commit d86a516

@0-8-15
Copy link
Contributor

@0-8-15 0-8-15 commented on d86a516 Nov 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change based on established knowledge or guess work from trial&error?

Since I merged the change, apps only resume (graphic output) if reactivated within a few seconds. Otherwise they stay black/blank --- while still working, i.e. receive network packages, play sound etc. Just no rendering.

Edit: still, when resuming from a longer sleep, they take ages to display.

2nd edit: maybe still no win. At least once those ages are beyond my patience; however switching Windows does help after a while. -- I'm clueless.

@peterlew
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have an app that was hanging for 60 seconds with a white screen between asking for storage and asking for other permissions. Since this line was the only thing happening between asking for permissions, it made sense to try moving it, and it did fix my problem. I'm sorry if it's changed the behavior of yours, though. I'm guessing you have non-permission code running in your ANDROID_JAVA_ONCREATE? In that case, it may make sense for us to split up the permissions and other ONCREATE code and put this line between them.

@ddunsmuir
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I experienced the same issue as 0-8-15 with apps being called from other apps with an Intent and then when called a second time they would appear without the rendering. I also found that switching windows and then going back to the app brought it back up. My temporary solution has been to terminate the app and restart it fresh each time it is called, but fixing the underlying issue would be better. I don't know if it's from this commit but we could certainly test it.

@0-8-15
Copy link
Contributor

@0-8-15 0-8-15 commented on d86a516 Nov 23, 2020 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@0-8-15
Copy link
Contributor

@0-8-15 0-8-15 commented on d86a516 Nov 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I'm now in the guesswork stage wrt. those app hangs, I made a little change.

This seems to help so far. But take this with a grain of salt: the breakage seems random to me. A random "looks better" is IMHO not much of a promise.

Nevertheless: please try this patch and tell me if it helps or introduces other issue.

e5472fa

Edit: The above patch looks still OK to me, but does not nail it!

Please try 93485a1 in PR #387 - looks like a more logical fix and so far looks like it does actually deal with the hanging application.

@peterlew
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add to the list of possible related issues: Issue #295, where re-entering an app after leaving it using the back button leads to a frozen white or black screen.

@0-8-15, I tried 93485a1 and it doesn't work for my application - it starts up to a frozen white screen and never even gets to requesting permissions. Perhaps we can keep the check for (mGLView == null) but move setContentView(mGLView); to after @ANDROID_JAVA_ONCREATE@, would that help your application at all?

@0-8-15
Copy link
Contributor

@0-8-15 0-8-15 commented on d86a516 Nov 24, 2020 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@0-8-15
Copy link
Contributor

@0-8-15 0-8-15 commented on d86a516 Nov 24, 2020 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@0-8-15
Copy link
Contributor

@0-8-15 0-8-15 commented on d86a516 Nov 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still doubt that the permission check is actually the part which freezes. After all, my app needs several permissions too. No freeze here anymore.

In fact, I can no longer reproduce the freeze. It froze upon update to the version I currently use, once.

Since yesterday it works on three devices. Forced stop and restart: still comes up immediately.

There is however on difference between the code I posted yesterday and what I actually use:

71799c7

Sorry, I did not post that, as I believe(d) this would be debug/instrumentation with no useful effect. Please try this, maybe it helps?

@0-8-15
Copy link
Contributor

@0-8-15 0-8-15 commented on d86a516 Nov 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This are becoming more strange. I've been able again to reproduce the hang:

  1. Install fresh compiled app: works
  2. Close it
  3. Open it again -> white freeze
  4. Forced close (via system/appinfo)
  5. start again: works
  6. Close: works
  7. start again: works goto (6)...

A mystery

@0-8-15
Copy link
Contributor

@0-8-15 0-8-15 commented on d86a516 Nov 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of experiments later: This seems to nail it:

0ba99db

@peterlew
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@0-8-15, I updated my bootstrap.java.in to match yours from 0ba99db, and it works fine for my app. However, it doesn't fix the back button freezing problem of #295. Perhaps @ddunsmuir will get a chance to check whether it helps with his app's bug?

@0-8-15
Copy link
Contributor

@0-8-15 0-8-15 commented on d86a516 Nov 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@0-8-15, I updated my bootstrap.javaIn to match yours from 0ba99db, and it works fine for my app.

Great to hear that!

However, it doesn't fix the back button freezing problem of #295. Perhaps @ddunsmuir will get a chance to check whether it helps with his app's bug?

Try #398 - maybe it helps.

@ddunsmuir
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, using that same bootstrap.java.in resolved my issue as well!

@mgorges
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for testing @ddunsmuir and @peterlew - merged as 20b33a1 with credit to @0-8-15. Will do a code-review discussion tomorrow regarding some of the other items.

@peterlew
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been changed by 51dfeed to request all permissions from the manifest first, then do the initial setContentView, then run ONCREATE code. Hopefully that is a sensible solution for everyone.

@0-8-15
Copy link
Contributor

@0-8-15 0-8-15 commented on d86a516 Nov 28, 2020 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.