Fix split install issue
Extract the optimization and resubmit it to https://github.com/microg/GmsCore/pull/2799
- Download notifications for split packages should be dismissible.
- After a split package is downloaded, clicking the notification should trigger installation (to handle cases where the install prompt can't appear due to a locked screen).
- Prevent multiple notifications for the same split package download.
- Remove notifications when the network is disconnected or the download fails.
- Implement resumable downloads to save data usage.
- Ensure device synchronization to avoid missing split package information.
- Support account elevation to prevent failure in accessing split package data.
Thanks for these improvements. I think resumable downloads, and only downloading one split install package at a time, really make sense. However, I would prefer a different implementation approach: the way you are doing it is that the packages are first downloaded to a temporary directory, and then copied to the install session from this directory. (A previous iteration of the code also had this behavior, but we changed it to avoid occupying 2× the space of the installed package while installing it.)
I think it should be possible to instead resume downloads into the pending session. To do this:
- We need a registry of session IDs, so we know which download corresponds to which session.
- Sessions must not be discarded when download fails.
- We don't need temporary files anymore.
Do you see any reason that speaks against implementing it like this?
We temporarily store the downloaded files locally to avoid repeated downloads in case of issues, which could lead to unnecessary data usage for users. Regarding your suggestion of resuming directly into the install session — isn’t there a limitation on how many active sessions the system allows? That’s part of the reason we initially chose to manage downloads outside the session system.
Regarding your suggestion of resuming directly into the install session — isn’t there a limitation on how many active sessions the system allows?
I don't know if there is such a limit. What do you expect the limit to be? What happens if it is exceeded? Do you expect to open many sessions, i.e. many partial and failed downloads? This would also mean a large storage consumption in either implementation, so it's not really desirable in either implementation.
Regarding your suggestion of resuming directly into the install session — isn’t there a limitation on how many active sessions the system allows?
I don't know if there is such a limit. What do you expect the limit to be? What happens if it is exceeded? Do you expect to open many sessions, i.e. many partial and failed downloads? This would also mean a large storage consumption in either implementation, so it's not really desirable in either implementation.
This is just an extreme case because Marvin initially raised concerns about data usage with me. That’s why I chose this approach. Regarding your suggestion of tracking through sessions, wouldn’t the sessions be lost if the app is killed?
Regarding your suggestion of tracking through sessions, wouldn’t the sessions be lost if the app is killed?
The session will even be persisted across multiple boots, please refer to: https://developer.android.com/reference/android/content/pm/PackageInstaller#createSession(android.content.pm.PackageInstaller.SessionParams)
Regarding your suggestion of tracking through sessions, wouldn’t the sessions be lost if the app is killed?
The session will even be persisted across multiple boots, please refer to: https://developer.android.com/reference/android/content/pm/PackageInstaller#createSession(android.content.pm.PackageInstaller.SessionParams)
So do we need to store the sessions locally to facilitate reuse later? If we use this method, how can we track the split package download progress, and how can we implement resumable downloads?
Regarding your suggestion of tracking through sessions, wouldn’t the sessions be lost if the app is killed?
The session will even be persisted across multiple boots, please refer to: https://developer.android.com/reference/android/content/pm/PackageInstaller#createSession(android.content.pm.PackageInstaller.SessionParams)
I think your solution still cannot meet the requirements for resumable downloads in special cases.
Yes, as I mentioned, we could use a registry of session IDs, so we know which download corresponds to which session. Since we are session owner, we might also be able to use: https://developer.android.com/reference/kotlin/android/content/pm/PackageInstaller?hl=en#getStagedSessions() and compare the package name of the session.
I suppose we might need to store how many bytes we wrote to the session, if it's not possible to ask the session that, though it might be feasible to use: https://developer.android.com/reference/kotlin/android/content/pm/PackageInstaller.Session?hl=en#openRead(kotlin.String)
What special cases do you have in mind?
Yes, as I mentioned, we could use a registry of session IDs, so we know which download corresponds to which session. Since we are session owner, we might also be able to use: https://developer.android.com/reference/kotlin/android/content/pm/PackageInstaller?hl=en#getStagedSessions() and compare the package name of the session.
I suppose we might need to store how many bytes we wrote to the session, if it's not possible to ask the session that, though it might be feasible to use: https://developer.android.com/reference/kotlin/android/content/pm/PackageInstaller.Session?hl=en#openRead(kotlin.String)
What special cases do you have in mind?
I tried using your method, but I kept getting the following error when calling openSession: java.lang.SecurityException: Caller has no access to session 2068134291. @fynngodau
@DaVinci9196 Can you provide me with the code where this issue occurs? That will make it much easier for me to find out why that could be the case.
@DaVinci9196 Can you provide me with the code where this issue occurs? That will make it much easier for me to find out why that could be the case.
Install.txt I modified Install.kt to Install.txt and uploaded it。 The core code is as follows. An error occurs when calling openSession
val cacheSessionId = SessionStorage.getInstance(this).getSessionId(key)
sessionId = if (cacheSessionId == -1) {
packageInstaller.createSession(params)
} else {
cacheSessionId
}
for (info in packageInstaller.allSessions) {
Log.d(TAG, "id=${info.sessionId}, createdBy=${info.installerPackageName}")
}
SessionStorage.getInstance(this).saveSessionId(key, sessionId)
session = packageInstaller.openSession(sessionId)
@DaVinci9196 Can you provide me with the code where this issue occurs? That will make it much easier for me to find out why that could be the case.
Install.txt I modified Install.kt to Install.txt and uploaded it。 The core code is as follows. An error occurs when calling openSession
val cacheSessionId = SessionStorage.getInstance(this).getSessionId(key) sessionId = if (cacheSessionId == -1) { packageInstaller.createSession(params) } else { cacheSessionId } for (info in packageInstaller.allSessions) { Log.d(TAG, "id=${info.sessionId}, createdBy=${info.installerPackageName}") } SessionStorage.getInstance(this).saveSessionId(key, sessionId) session = packageInstaller.openSession(sessionId)
@fynngodau Can you take a look? This is quite urgent.
@DaVinci9196 I could not reproduce your problem, but I also do not have access to the SessionStorage class that you implemented. Hence I instead tried the following code (replacing lines 134–135 of Install.kt):
sessionId = packageInstaller.createSession(params)
session = packageInstaller.openSession(sessionId)
packageInstaller.updateSessionAppLabel(sessionId, packageName)
session.close()
Log.d(TAG, "session closed")
Log.d(TAG, packageInstaller.mySessions.joinToString(",") { sessionInfo -> sessionInfo.sessionId.toString() + " of package " + sessionInfo.appLabel })
session = packageInstaller.openSession(sessionId)
Log.d(TAG, "session reopened")
This worked without problems:
-
mySessionsreturned the currently pending session - I could access its metadata (this could be a good alternative to your
SessionStorageimplementation, as I would expectmySessionsto always be in sync with whether the system has discarded the session due to timeout (after a day or so per docs)) - I was able to reopen the session (but I did not try with closing and reopening the app)
Does the above code already produce a problem for you?
I noticed that you are never closing the session. I would suggest adding this to the finally block in your version of the code, maybe it helps.
@DaVinci9196 I could not reproduce your problem, but I also do not have access to the
SessionStorageclass that you implemented. Hence I instead tried the following code (replacing lines 134–135 ofInstall.kt):sessionId = packageInstaller.createSession(params) session = packageInstaller.openSession(sessionId) packageInstaller.updateSessionAppLabel(sessionId, packageName) session.close() Log.d(TAG, "session closed") Log.d(TAG, packageInstaller.mySessions.joinToString(",") { sessionInfo -> sessionInfo.sessionId.toString() + " of package " + sessionInfo.appLabel }) session = packageInstaller.openSession(sessionId) Log.d(TAG, "session reopened")This worked without problems:
mySessionsreturned the currently pending session- I could access its metadata (this could be a good alternative to your
SessionStorageimplementation, as I would expectmySessionsto always be in sync with whether the system has discarded the session due to timeout (after a day or so per docs))- I was able to reopen the session (but I did not try with closing and reopening the app)
Does the above code already produce a problem for you?
I noticed that you are never closing the session. I would suggest adding this to the
finallyblock in your version of the code, maybe it helps.
This code cannot run on my phone and will report an error @fynngodau
and @fynngodau I see the following code in the Delivery.kt class. What will happen if these codes are not included?
pf=2&pf=3&pf=4&pf=5&pf=7&pf=8&pf=9&pf=10&da=4&bda=4&bf=4&fdcf=1&fdcf=2
I have found that &da=4 will cause problems when PayPal switches to voice. Can this be removed? @fynngodau
@DaVinci9196 I tried with just the code fragment from https://github.com/microg/GmsCore/pull/2898#issuecomment-3003730353 in a minimal demo app on multiple devices, including P60 Pro with EMUI 14 and Mate X5 with Harmony OS 4 and it worked as expected and reopened the session on all of them. What device did you use for this?
Looking at the code, I believe the issue might be a concurrency issue. Note that installPackagesInternal is a suspendable function and does not have any mutexes or anything preventing it from being invoked more than once. This means that a second call to installPackagesInternal will fail if it happens for the same package as one that is already ongoing.
@DaVinci9196 I tried with just the code fragment from #2898 (comment) in a minimal demo app on multiple devices, including P60 Pro with EMUI 14 and Mate X5 with Harmony OS 4 and it worked as expected and reopened the session on all of them. What device did you use for this?
Looking at the code, I believe the issue might be a concurrency issue. Note that
installPackagesInternalis a suspendable function and does not have any mutexes or anything preventing it from being invoked more than once. This means that a second call toinstallPackagesInternalwill fail if it happens for the same package as one that is already ongoing.
I have modified it according to this comment #2898 (comment), you can test it with the latest code
I have found that &da=4 will cause problems when PayPal switches to voice. Can this be removed?
My answer is: nobody really knows what these options mean. I see no regressions caused by removing it so I consider it ok.
0.3.9 here we come!
LGTM, Thanks @DaVinci9196 and @fynngodau for the work!