-
Notifications
You must be signed in to change notification settings - Fork 46
Determine whether broker app opts out from battery optimization, Fixes AB#3428706 #2819
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
Conversation
|
❌ Work item link check failed. Description does not contain AB#{ID}. Click here to Learn more. |
c7f78aa to
0a6d351
Compare
|
✅ Work item link check complete. Description contains link AB#3428706 to an Azure Boards work item. |
|
✅ Work item link check complete. Description contains link AB#3428706 to an Azure Boards work item. |
0a6d351 to
88c676c
Compare
ea0f533 to
146a034
Compare
146a034 to
683d985
Compare
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.
Pull Request Overview
This PR adds functionality to determine whether a broker app has opted out from battery optimization, addressing work item AB#3428706. The implementation includes an in-memory cache to persist battery optimization status checks across multiple calls for the same package.
Key Changes:
- Added
isAppOptedOutFromBatteryOptimization()method with caching and exception handling - Introduced
ConcurrentHashMapto cache battery optimization status per package name - Removed API level checks from
getPowerOptimizationSettings()and@RequiresApiannotation fromisIgnoringBatteryOptimizations()
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| common/src/main/java/com/microsoft/identity/common/adal/internal/PowerManagerWrapper.java | Adds new public method to check battery optimization status with caching, removes API level guards from existing methods |
| changelog.txt | Documents the new feature as a MINOR change |
Comments suppressed due to low confidence (1)
common/src/main/java/com/microsoft/identity/common/adal/internal/PowerManagerWrapper.java:131
- Severity: High – API level check removed without replacement.
Issue: The diff shows removal of the API level check that prevented calling isIgnoringBatteryOptimizations() on devices below API 23 (Android M). The method PowerManager.isIgnoringBatteryOptimizations() was introduced in API 23 and will cause a NoSuchMethodError or crash on older devices.
Impact: This will cause crashes on devices running Android versions below API 23 (Marshmallow) when getPowerOptimizationSettings() is called.
Recommendation: Restore the API level check that was removed:
public String getPowerOptimizationSettings(@NonNull final Context context){
try {
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) {
return UNKNOWN_STATUS;
}
final PowerManager powerManager = ((PowerManager) context.getSystemService(Context.POWER_SERVICE));
if (powerManager.isIgnoringBatteryOptimizations(context.getPackageName())){
return "OptOut";
} else {
return "";
}
} catch (final Exception e){
// Swallow all exception!
return UNKNOWN_STATUS;
}
} public String getPowerOptimizationSettings(@NonNull final Context context){
try {
final PowerManager powerManager = ((PowerManager) context.getSystemService(Context.POWER_SERVICE));
if (powerManager.isIgnoringBatteryOptimizations(context.getPackageName())){
return "OptOut";
} else {
return "";
}
} catch (final Exception e){
// Swallow all exception!
return UNKNOWN_STATUS;
}
}
common/src/main/java/com/microsoft/identity/common/adal/internal/PowerManagerWrapper.java
Outdated
Show resolved
Hide resolved
common/src/main/java/com/microsoft/identity/common/adal/internal/PowerManagerWrapper.java
Outdated
Show resolved
Hide resolved
common/src/main/java/com/microsoft/identity/common/adal/internal/PowerManagerWrapper.java
Show resolved
Hide resolved
common/src/main/java/com/microsoft/identity/common/adal/internal/PowerManagerWrapper.java
Show resolved
Hide resolved
common/src/main/java/com/microsoft/identity/common/adal/internal/PowerManagerWrapper.java
Outdated
Show resolved
Hide resolved
common/src/main/java/com/microsoft/identity/common/adal/internal/PowerManagerWrapper.java
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
c146180 to
530ab3b
Compare
common/src/main/java/com/microsoft/identity/common/adal/internal/PowerManagerWrapper.java
Outdated
Show resolved
Hide resolved
common/src/main/java/com/microsoft/identity/common/adal/internal/PowerManagerWrapper.java
Show resolved
Hide resolved
442c646 to
753412e
Compare
common/src/main/java/com/microsoft/identity/common/adal/internal/PowerManagerWrapper.java
Outdated
Show resolved
Hide resolved
common/src/main/java/com/microsoft/identity/common/internal/DeviceDozeModeStatus.kt
Show resolved
Hide resolved
common/src/main/java/com/microsoft/identity/common/adal/internal/PowerManagerWrapper.java
Show resolved
Hide resolved
common/src/main/java/com/microsoft/identity/common/adal/internal/BatteryOptimizationStatus.kt
Outdated
Show resolved
Hide resolved
mohitc1
left a comment
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.
![]()
AB#3428706
The value would also be cached in memory (since it should rarely change).
The caller can invoke this method right after it gets the package name via BrokerDiscoveryClient.getActiveBroker().
(NOTE: OneAuth would also need to invoke getDeviceIdleMode(), which will return the device status.
These two values shall be emitted together.
For that method, given that that value fluctuates based on usage, we are not going to cache it)