From f66eab85a00d38210680b77d8ef85c66b5868566 Mon Sep 17 00:00:00 2001 From: Bryan Date: Wed, 16 Jun 2021 15:57:38 +0000 Subject: [PATCH] Changes to how we notify widget restore participants of new ids As part of widget restore, we need to notify participants (hosts & providers) of the mapping between ancestral and new widget ids. Prior to this change, this was done at the end of system restore and after each restore-at-install of unbundled apps. There were two issues with this: 1) We would try to notify not-yet-installed participants at the end of system restore and then *not* try again when they were installed. 2) We relied on PACKAGE_ADDED broadcasts to know which participants were installed, but these are delivered asynchronously and not guaranteed to be processed before the restoreFinished call. After this change, we: 1) Only notify participants that are actually installed 2) Use receipt of the PACKAGE_ADDED / CHANGED broadcast to trigger notification of any restore participants that are added or changed *after* system restore completes. 2 is safe because by the time PackageManager sends the package added / changed broadcasts, the app data restore has been completed, and the app has been killed if needed. Bug: 162057170 Test: Manual restore of Keep widget via cloud and d2d Change-Id: I51dec33d09b62faba6d7c7daacd718a3c0682f7d --- .../android/server/AppWidgetBackupBridge.java | 8 +- .../android/server/WidgetBackupProvider.java | 4 +- .../appwidget/AppWidgetServiceImpl.java | 145 +++++++++++------- .../restore/PerformUnifiedRestoreTask.java | 8 +- 4 files changed, 101 insertions(+), 64 deletions(-) diff --git a/core/java/com/android/server/AppWidgetBackupBridge.java b/core/java/com/android/server/AppWidgetBackupBridge.java index 7d82d355e3eb1..8e834a87784d9 100644 --- a/core/java/com/android/server/AppWidgetBackupBridge.java +++ b/core/java/com/android/server/AppWidgetBackupBridge.java @@ -47,9 +47,9 @@ public class AppWidgetBackupBridge { : null; } - public static void restoreStarting(int userId) { + public static void systemRestoreStarting(int userId) { if (sAppWidgetService != null) { - sAppWidgetService.restoreStarting(userId); + sAppWidgetService.systemRestoreStarting(userId); } } @@ -59,9 +59,9 @@ public class AppWidgetBackupBridge { } } - public static void restoreFinished(int userId) { + public static void systemRestoreFinished(int userId) { if (sAppWidgetService != null) { - sAppWidgetService.restoreFinished(userId); + sAppWidgetService.systemRestoreFinished(userId); } } } diff --git a/core/java/com/android/server/WidgetBackupProvider.java b/core/java/com/android/server/WidgetBackupProvider.java index a2efbdd6f2f86..5453c4de6986d 100644 --- a/core/java/com/android/server/WidgetBackupProvider.java +++ b/core/java/com/android/server/WidgetBackupProvider.java @@ -28,7 +28,7 @@ import java.util.List; public interface WidgetBackupProvider { public List getWidgetParticipants(int userId); public byte[] getWidgetState(String packageName, int userId); - public void restoreStarting(int userId); + public void systemRestoreStarting(int userId); public void restoreWidgetState(String packageName, byte[] restoredState, int userId); - public void restoreFinished(int userId); + public void systemRestoreFinished(int userId); } diff --git a/services/appwidget/java/com/android/server/appwidget/AppWidgetServiceImpl.java b/services/appwidget/java/com/android/server/appwidget/AppWidgetServiceImpl.java index acbf4875ae8d4..5aec6aa99c128 100644 --- a/services/appwidget/java/com/android/server/appwidget/AppWidgetServiceImpl.java +++ b/services/appwidget/java/com/android/server/appwidget/AppWidgetServiceImpl.java @@ -409,6 +409,8 @@ class AppWidgetServiceImpl extends IAppWidgetService.Stub implements WidgetBacku // If the set of providers has been modified, notify each active AppWidgetHost scheduleNotifyGroupHostsForProvidersChangedLocked(userId); + // Possibly notify any new components of widget id changes + mBackupRestoreController.widgetComponentsChanged(userId); } } } @@ -2471,8 +2473,8 @@ class AppWidgetServiceImpl extends IAppWidgetService.Stub implements WidgetBacku } @Override - public void restoreStarting(int userId) { - mBackupRestoreController.restoreStarting(userId); + public void systemRestoreStarting(int userId) { + mBackupRestoreController.systemRestoreStarting(userId); } @Override @@ -2481,8 +2483,8 @@ class AppWidgetServiceImpl extends IAppWidgetService.Stub implements WidgetBacku } @Override - public void restoreFinished(int userId) { - mBackupRestoreController.restoreFinished(userId); + public void systemRestoreFinished(int userId) { + mBackupRestoreController.systemRestoreFinished(userId); } @SuppressWarnings("deprecation") @@ -4272,6 +4274,9 @@ class AppWidgetServiceImpl extends IAppWidgetService.Stub implements WidgetBacku private final HashMap> mUpdatesByHost = new HashMap<>(); + @GuardedBy("mLock") + private boolean mHasSystemRestoreFinished; + public List getWidgetParticipants(int userId) { if (DEBUG) { Slog.i(TAG, "Getting widget participants for user: " + userId); @@ -4375,12 +4380,13 @@ class AppWidgetServiceImpl extends IAppWidgetService.Stub implements WidgetBacku return stream.toByteArray(); } - public void restoreStarting(int userId) { + public void systemRestoreStarting(int userId) { if (DEBUG) { - Slog.i(TAG, "Restore starting for user: " + userId); + Slog.i(TAG, "System restore starting for user: " + userId); } synchronized (mLock) { + mHasSystemRestoreFinished = false; // We're starting a new "system" restore operation, so any widget restore // state that we see from here on is intended to replace the current // widget configuration of any/all of the affected apps. @@ -4542,26 +4548,90 @@ class AppWidgetServiceImpl extends IAppWidgetService.Stub implements WidgetBacku } } - // Called once following the conclusion of a restore operation. This is when we + // Called once following the conclusion of a system restore operation. This is when we // send out updates to apps involved in widget-state restore telling them about - // the new widget ID space. - public void restoreFinished(int userId) { + // the new widget ID space. Apps that are not yet installed will be notifed when they are. + public void systemRestoreFinished(int userId) { if (DEBUG) { - Slog.i(TAG, "restoreFinished for " + userId); + Slog.i(TAG, "systemRestoreFinished for " + userId); + } + synchronized (mLock) { + mHasSystemRestoreFinished = true; + maybeSendWidgetRestoreBroadcastsLocked(userId); + } + } + + // Called when widget components (hosts or providers) are added or changed. If system + // restore has completed, we use this opportunity to tell the apps to update to the new + // widget ID space. If system restore is still in progress, we delay the updates until + // the end, to allow all participants to restore their state before updating widget IDs. + public void widgetComponentsChanged(int userId) { + synchronized (mLock) { + if (mHasSystemRestoreFinished) { + maybeSendWidgetRestoreBroadcastsLocked(userId); + } + } + } + + // Called following the conclusion of a restore operation and when widget components + // are added or changed. This is when we send out updates to apps involved in widget-state + // restore telling them about the new widget ID space. + @GuardedBy("mLock") + private void maybeSendWidgetRestoreBroadcastsLocked(int userId) { + if (DEBUG) { + Slog.i(TAG, "maybeSendWidgetRestoreBroadcasts for " + userId); } final UserHandle userHandle = new UserHandle(userId); - synchronized (mLock) { - // Build the providers' broadcasts and send them off - Set>> providerEntries - = mUpdatesByProvider.entrySet(); - for (Map.Entry> e : providerEntries) { - // For each provider there's a list of affected IDs - Provider provider = e.getKey(); + // Build the providers' broadcasts and send them off + Set>> providerEntries + = mUpdatesByProvider.entrySet(); + for (Map.Entry> e : providerEntries) { + // For each provider there's a list of affected IDs + Provider provider = e.getKey(); + if (provider.zombie) { + // Provider not installed, we can't send them broadcasts yet. + // We'll be called again when the provider is installed. + continue; + } + ArrayList updates = e.getValue(); + final int pending = countPendingUpdates(updates); + if (DEBUG) { + Slog.i(TAG, "Provider " + provider + " pending: " + pending); + } + if (pending > 0) { + int[] oldIds = new int[pending]; + int[] newIds = new int[pending]; + final int N = updates.size(); + int nextPending = 0; + for (int i = 0; i < N; i++) { + RestoreUpdateRecord r = updates.get(i); + if (!r.notified) { + r.notified = true; + oldIds[nextPending] = r.oldId; + newIds[nextPending] = r.newId; + nextPending++; + if (DEBUG) { + Slog.i(TAG, " " + r.oldId + " => " + r.newId); + } + } + } + sendWidgetRestoreBroadcastLocked( + AppWidgetManager.ACTION_APPWIDGET_RESTORED, + provider, null, oldIds, newIds, userHandle); + } + } + + // same thing per host + Set>> hostEntries + = mUpdatesByHost.entrySet(); + for (Map.Entry> e : hostEntries) { + Host host = e.getKey(); + if (host.id.uid != UNKNOWN_UID) { ArrayList updates = e.getValue(); final int pending = countPendingUpdates(updates); if (DEBUG) { - Slog.i(TAG, "Provider " + provider + " pending: " + pending); + Slog.i(TAG, "Host " + host + " pending: " + pending); } if (pending > 0) { int[] oldIds = new int[pending]; @@ -4581,43 +4651,8 @@ class AppWidgetServiceImpl extends IAppWidgetService.Stub implements WidgetBacku } } sendWidgetRestoreBroadcastLocked( - AppWidgetManager.ACTION_APPWIDGET_RESTORED, - provider, null, oldIds, newIds, userHandle); - } - } - - // same thing per host - Set>> hostEntries - = mUpdatesByHost.entrySet(); - for (Map.Entry> e : hostEntries) { - Host host = e.getKey(); - if (host.id.uid != UNKNOWN_UID) { - ArrayList updates = e.getValue(); - final int pending = countPendingUpdates(updates); - if (DEBUG) { - Slog.i(TAG, "Host " + host + " pending: " + pending); - } - if (pending > 0) { - int[] oldIds = new int[pending]; - int[] newIds = new int[pending]; - final int N = updates.size(); - int nextPending = 0; - for (int i = 0; i < N; i++) { - RestoreUpdateRecord r = updates.get(i); - if (!r.notified) { - r.notified = true; - oldIds[nextPending] = r.oldId; - newIds[nextPending] = r.newId; - nextPending++; - if (DEBUG) { - Slog.i(TAG, " " + r.oldId + " => " + r.newId); - } - } - } - sendWidgetRestoreBroadcastLocked( - AppWidgetManager.ACTION_APPWIDGET_HOST_RESTORED, - null, host, oldIds, newIds, userHandle); - } + AppWidgetManager.ACTION_APPWIDGET_HOST_RESTORED, + null, host, oldIds, newIds, userHandle); } } } diff --git a/services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java b/services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java index 261ebe69a15ea..f07bac8cd762c 100644 --- a/services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java +++ b/services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java @@ -381,7 +381,7 @@ public class PerformUnifiedRestoreTask implements BackupRestoreTask { // If we're starting a full-system restore, set up to begin widget ID remapping if (mIsSystemRestore) { - AppWidgetBackupBridge.restoreStarting(mUserId); + AppWidgetBackupBridge.systemRestoreStarting(mUserId); } try { @@ -1133,8 +1133,10 @@ public class PerformUnifiedRestoreTask implements BackupRestoreTask { restoreAgentTimeoutMillis); } - // Kick off any work that may be needed regarding app widget restores - AppWidgetBackupBridge.restoreFinished(mUserId); + if (mIsSystemRestore) { + // Kick off any work that may be needed regarding app widget restores + AppWidgetBackupBridge.systemRestoreFinished(mUserId); + } // If this was a full-system restore, record the ancestral // dataset information