-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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-20320]: Refactor requestPermissions #7948
Conversation
Add generic permissions request handling for Android Marshmallow see example usage in tipermissions module https://github.com/stgrosshh/tipermissions
Change to strong references to avoid some issues with async UI creation Provide infos about requested and denied permissions in callback result
} | ||
} | ||
|
||
private static ConcurrentHashMap<Integer,PermissionContextData> callbackDataByPermission = new ConcurrentHashMap<Integer, PermissionContextData>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this cause any memory leaks on Activities (Windows)? Might need to check when using this, when the Window is closed, does the Activity get released by doing Heap dumps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a check and there was no leak on WindowProxy by opening 10 windows and requesting (and denying) permissions using the code below:-
(function openWindow(n) {
var win = Ti.UI.createWindow({
backgroundColor : "#aaaaaa",
layout : "vertical"
});
win.title = "Window # " + n;
var label = Ti.UI.createLabel({
text : "Window # " + n,
color : "#ffffff",
top : 20
});
win.add(label);
label = null;
var openButton = Ti.UI.createButton({
title : "Open",
color : "#ffffff",
top : 20,
_n: n
});
openButton.addEventListener('click', function onOpenByButton(evt) {
openWindow(evt.source._n + 1);
});
win.add(openButton);
openButton = null;
var closeButton = Ti.UI.createButton({
title : "Close",
color : "#ffffff",
top : 20
});
closeButton.addEventListener('click', function onCloseByButton(evt) {
evt.source.getParent().close();
});
win.add(closeButton);
win.open();
Ti.Media.requestCameraPermissions(function(e) {
if (e.success === true) {
Ti.API.log("Success");
} else {
Ti.API.log("Access denied, error: " + e.error);
}
});
win = null;
})(1);
Code is based on https://jira.appcelerator.org/browse/TIMOB-19891.
No leaks seen when hprof was inspected for WindowProxy.
Everything looks good and seems to be working but would help greatly if there was a test app.js provided to test all the permissions. |
Functionally tested with code from jira:- var cameraPermission = "android.permission.CAMERA";
var storagePermission = "android.permission.READ_EXTERNAL_STORAGE";
var hasCameraPerm = Ti.Android.hasPermission(cameraPermission);
var hasStoragePerm = Ti.Android.hasPermission(storagePermission);
var permissionsToRequest = [];
if (!hasCameraPerm) {
permissionsToRequest.push(cameraPermission);
}
if (!hasStoragePerm) {
permissionsToRequest.push(storagePermission);
}
if (permissionsToRequest.length > 0) {
Ti.Android.requestPermissions(permissionsToRequest, function(e) {
if (e.success) {
Ti.API.info("SUCCESS");
} else {
Ti.API.info("ERROR: " + e.error);
}
});
} Code reviewed and functionally tested to be working well. 👍 |
@jonasbjurhult Thanks for your comment! Could you show an example code of what you mean? |
JIRA: https://jira.appcelerator.org/browse/TIMOB-20320