Skip to content

fix: install button not appearing on android 8 #3333

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.

Sign up for GitHub

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

Open
germanamv wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

fix: install button not appearing on android 8 #3333

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 17 additions & 13 deletions app/src/main/java/app/revanced/manager/patcher/worker/PatcherWorker.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,29 +117,33 @@ class PatcherWorker(
}

try {
// This does not always show up for some reason.
createNotificationChannel() // Safe to call multiple times

@kitadai31 kitadai31 May 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Creating the notification channel here should be unnecessary, as it has already been done within the createNotification() method above that is called from getForegroundInfo().

Also, if you have really confirmed that creating a notification channel twice here fixes the problem, use the same method within createNotification().
The ID of the notification channel created by the new method is different from the one that is actually being used.
This change will make a duplicate notification channel.

@TUSHAR91316 TUSHAR91316 Aug 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The channel ID "patcher_channel" used here is hardcoded and differs from the channel ID used in createNotification(). If a channel is needed before setForeground, we should reuse the existing channel ID / method to avoid creating a duplicate notification channel.

setForeground(getForegroundInfo())
} catch (e: Exception) {
Log.d(tag, "Failed to set foreground info:", e)
Log.e(tag, "Failed to set foreground info:", e)
// On Android 8, if this fails, the job is likely doomed
return Result.failure()
}

val wakeLock: PowerManager.WakeLock =
(applicationContext.getSystemService(Context.POWER_SERVICE) as PowerManager)
.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, "$tag::Patcher")
.apply {
acquire(10 * 60 * 1000L)
Log.d(tag, "Acquired wakelock.")
}

@TUSHAR91316 TUSHAR91316 Aug 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Removing the WakeLock might cause the patching process to stop prematurely when the screen turns off (see PR #2147). WorkManager does not guarantee a CPU wakelock for CoroutineWorker.


val args = workerRepository.claimInput(this)

return try {
runPatcher(args)
} finally {
wakeLock.release()
runPatcher(args) // WorkManager holds its own WakeLock internally
Comment on lines +120 to +131

@oSumAtrIX oSumAtrIX May 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
createNotificationChannel() // Safe to call multiple times
setForeground(getForegroundInfo())
} catch (e: Exception) {
Log.d(tag, "Failed to set foreground info:", e)
Log.e(tag, "Failed to set foreground info:", e)
// On Android 8, if this fails, the job is likely doomed
return Result.failure()
}
val wakeLock: PowerManager.WakeLock =
(applicationContext.getSystemService(Context.POWER_SERVICE) as PowerManager)
.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, "$tag::Patcher")
.apply {
acquire(10 * 60 * 1000L)
Log.d(tag, "Acquired wakelock.")
}
val args = workerRepository.claimInput(this)
return try {
runPatcher(args)
} finally {
wakeLock.release()
runPatcher(args) // WorkManager holds its own WakeLock internally
createNotificationChannel() // Safe to call multiple times.
setForeground(getForegroundInfo())
} catch (e: Exception) {
Log.e(tag, "Failed to set foreground info:", e)
// On Android 8, if this fails, the job is likely doomed.
return Result.failure()
}
val args = workerRepository.claimInput(this)
return try {
runPatcher(args) // WorkManager holds its own WakeLock internally.

@germanamv germanamv May 5, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

What is the change here? The code looks exactly the same.

@Axelen123 Axelen123 May 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah the suggestion does look kinda weird.

@kitadai31 kitadai31 May 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

iirc, WorkManager doesn't holds WakeLock
Try turning off the screen while patching without WakeLock
It should stop patching while the screen is off

@Axelen123 Axelen123 May 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I assumed the statement in the comment was true. Is it documented anywhere?

@kitadai31 kitadai31 May 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See conversation in #2147
It is not documented, but some reports can be found on StackOverflow

} catch (e: Exception) {
Log.e(tag, "Patcher encountered an error", e)
Result.failure()

@TUSHAR91316 TUSHAR91316 Aug 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Returning Result.failure() here will abort the entire patching operation if setForeground() throws. We should log the exception and allow patching to proceed instead of failing the job.

}
}

private fun createNotificationChannel() {
val channel = NotificationChannel(
"patcher_channel", "Patcher Service",
NotificationManager.IMPORTANCE_LOW
)
val manager = applicationContext.getSystemService(NotificationManager::class.java)
manager.createNotificationChannel(channel)
}

private suspend fun runPatcher(args: Args): Result {
val patchedApk = fs.tempDir.resolve("patched.apk")

Expand Down