From c4f0b0099a414c51ecd6d9bc4787c0e5e9d5b04a Mon Sep 17 00:00:00 2001 From: Yo Chiang Date: Tue, 5 Jan 2021 18:32:43 +0800 Subject: [PATCH] Refactor DynamicSystemInstallationService notifications Right now we use two flags to track the internal states of DSU notifications, mJustCancelledByUser and mKeepNotification. This makes the notification posting / dismissing logic very complicated, as any method can potentially modify the flags. This change refactors the logic around postStatus() and resetTaskAndStop() and remove the two flags. After this change, notification posting logic are consolidated within postStatus() and dismissing logic are consolidated within resetTaskAndStop(). Bug: 165471299 Test: Start a DSU installation and test "cancel" & "discard" Change-Id: I59548553e0e92399d322c301011dfd270e7bb0e9 --- .../DynamicSystemInstallationService.java | 52 ++++++------------- 1 file changed, 17 insertions(+), 35 deletions(-) diff --git a/packages/DynamicSystemInstallationService/src/com/android/dynsystem/DynamicSystemInstallationService.java b/packages/DynamicSystemInstallationService/src/com/android/dynsystem/DynamicSystemInstallationService.java index c1dca5df1b2f7..16a946dc7bc66 100644 --- a/packages/DynamicSystemInstallationService/src/com/android/dynsystem/DynamicSystemInstallationService.java +++ b/packages/DynamicSystemInstallationService/src/com/android/dynsystem/DynamicSystemInstallationService.java @@ -138,9 +138,6 @@ public class DynamicSystemInstallationService extends Service private long mCurrentPartitionSize; private long mCurrentPartitionInstalledSize; - private boolean mJustCancelledByUser; - private boolean mKeepNotification; - // This is for testing only now private boolean mEnableWhenCompleted; @@ -174,11 +171,6 @@ public class DynamicSystemInstallationService extends Service if (cache != null) { cache.flush(); } - - if (!mKeepNotification) { - // Cancel the persistent notification. - mNM.cancel(NOTIFICATION_ID); - } } @Override @@ -231,9 +223,11 @@ public class DynamicSystemInstallationService extends Service return; } + boolean removeNotification = false; switch (result) { case RESULT_CANCELLED: postStatus(STATUS_NOT_STARTED, CAUSE_INSTALL_CANCELLED, null); + removeNotification = true; break; case RESULT_ERROR_IO: @@ -251,7 +245,7 @@ public class DynamicSystemInstallationService extends Service } // if it's not successful, reset the task and stop self. - resetTaskAndStop(); + resetTaskAndStop(removeNotification); } private void executeInstallCommand(Intent intent) { @@ -302,12 +296,12 @@ public class DynamicSystemInstallationService extends Service return; } - stopForeground(true); - mJustCancelledByUser = true; - if (mInstallTask.cancel(false)) { - // Will stopSelf() in onResult() + // onResult() would call resetTaskAndStop() upon task completion. Log.d(TAG, "Cancel request filed successfully"); + // Dismiss the notification as soon as possible as DynamicSystemManager.remove() may + // block. + stopForeground(STOP_FOREGROUND_REMOVE); } else { Log.e(TAG, "Trying to cancel installation while it's already completed."); } @@ -322,8 +316,7 @@ public class DynamicSystemInstallationService extends Service if (!isDynamicSystemInstalled() && (getStatus() != STATUS_READY)) { Log.e(TAG, "Trying to discard AOT while there is no complete installation"); // Stop foreground state and dismiss stale notification. - stopForeground(STOP_FOREGROUND_REMOVE); - resetTaskAndStop(); + resetTaskAndStop(true); return; } @@ -331,8 +324,8 @@ public class DynamicSystemInstallationService extends Service getString(R.string.toast_dynsystem_discarded), Toast.LENGTH_LONG).show(); - resetTaskAndStop(); postStatus(STATUS_NOT_STARTED, CAUSE_INSTALL_CANCELLED, null); + resetTaskAndStop(true); mDynSystem.remove(); } @@ -412,12 +405,13 @@ public class DynamicSystemInstallationService extends Service } private void resetTaskAndStop() { - mInstallTask = null; + resetTaskAndStop(/* removeNotification= */ false); + } - new Handler().postDelayed(() -> { - stopForeground(STOP_FOREGROUND_DETACH); - stopSelf(); - }, 50); + private void resetTaskAndStop(boolean removeNotification) { + mInstallTask = null; + stopForeground(removeNotification ? STOP_FOREGROUND_REMOVE : STOP_FOREGROUND_DETACH); + stopSelf(); } private void prepareNotification() { @@ -525,7 +519,7 @@ public class DynamicSystemInstallationService extends Service private void postStatus(int status, int cause, Throwable detail) { String statusString; String causeString; - mKeepNotification = false; + boolean notifyOnNotificationBar = true; switch (status) { case STATUS_NOT_STARTED: @@ -551,18 +545,16 @@ public class DynamicSystemInstallationService extends Service break; case CAUSE_INSTALL_CANCELLED: causeString = "INSTALL_CANCELLED"; + notifyOnNotificationBar = false; break; case CAUSE_ERROR_IO: causeString = "ERROR_IO"; - mKeepNotification = true; break; case CAUSE_ERROR_INVALID_URL: causeString = "ERROR_INVALID_URL"; - mKeepNotification = true; break; case CAUSE_ERROR_EXCEPTION: causeString = "ERROR_EXCEPTION"; - mKeepNotification = true; break; default: causeString = "CAUSE_NOT_SPECIFIED"; @@ -571,16 +563,6 @@ public class DynamicSystemInstallationService extends Service Log.d(TAG, "status=" + statusString + ", cause=" + causeString + ", detail=" + detail); - boolean notifyOnNotificationBar = true; - - if (status == STATUS_NOT_STARTED - && cause == CAUSE_INSTALL_CANCELLED - && mJustCancelledByUser) { - // if task is cancelled by user, do not notify them - notifyOnNotificationBar = false; - mJustCancelledByUser = false; - } - if (notifyOnNotificationBar) { mNM.notify(NOTIFICATION_ID, buildNotification(status, cause, detail)); }