Merge "Fix role granting flow."

5 files changed, 52 insertions, 36 deletions

diff --git a/core/java/android/app/role/RoleManager.java b/core/java/android/app/role/RoleManager.java
index ed27d9fe9fd4..ef86b01beabd 100644
--- a/core/java/android/app/role/RoleManager.java
+++ b/core/java/android/app/role/RoleManager.java
@@ -25,6 +25,7 @@ import android.annotation.SystemService;
 import android.content.Context;
 import android.content.Intent;
 import android.os.Binder;
+import android.os.Process;
 import android.os.RemoteException;
 import android.os.ServiceManager;
 import android.os.UserHandle;
@@ -189,7 +190,7 @@ public final class RoleManager {
     @RequiresPermission(Manifest.permission.MANAGE_ROLE_HOLDERS)
     @SystemApi
     public List<String> getRoleHolders(@NonNull String roleName) {
-        return getRoleHoldersAsUser(roleName, UserHandle.of(UserHandle.getCallingUserId()));
+        return getRoleHoldersAsUser(roleName, Process.myUserHandle());
     }
 
     /**
diff --git a/core/java/android/rolecontrollerservice/RoleControllerService.java b/core/java/android/rolecontrollerservice/RoleControllerService.java
index 44c45bb32acd..6eda504f24da 100644
--- a/core/java/android/rolecontrollerservice/RoleControllerService.java
+++ b/core/java/android/rolecontrollerservice/RoleControllerService.java
@@ -93,8 +93,8 @@ public abstract class RoleControllerService extends Service {
             @Override
             public void onGrantDefaultRoles(IRoleManagerCallback callback) {
                 Preconditions.checkNotNull(callback, "callback cannot be null");
-                RoleControllerService.this.onGrantDefaultRoles(
-                        new RoleManagerCallbackDelegate(callback));
+                RoleControllerService.this.onGrantDefaultRoles(new RoleManagerCallbackDelegate(
+                        callback));
             }
         };
     }
diff --git a/core/res/AndroidManifest.xml b/core/res/AndroidManifest.xml
index 533ce64c8493..20d315fe2ff0 100644
--- a/core/res/AndroidManifest.xml
+++ b/core/res/AndroidManifest.xml
@@ -2467,7 +2467,7 @@
          {@link android.content.pm.PackageManager#addPackageToPreferred}
          for details. -->
     <permission android:name="android.permission.SET_PREFERRED_APPLICATIONS"
-        android:protectionLevel="signature|verifier" />
+        android:protectionLevel="signature|installer|verifier" />
 
     <!-- Allows an application to receive the
          {@link android.content.Intent#ACTION_BOOT_COMPLETED} that is
diff --git a/services/core/java/com/android/server/role/RemoteRoleControllerService.java b/services/core/java/com/android/server/role/RemoteRoleControllerService.java
index 7d34270734df..538f4cf47364 100644
--- a/services/core/java/com/android/server/role/RemoteRoleControllerService.java
+++ b/services/core/java/com/android/server/role/RemoteRoleControllerService.java
@@ -219,7 +219,7 @@ public class RemoteRoleControllerService {
             private final IRoleManagerCallback mCallback;
 
             @NonNull
-            private final Runnable mTimeoutRunnable = () -> notifyCallback(false);
+            private final Runnable mTimeoutRunnable = this::notifyTimeout;
 
             private boolean mCallbackNotified;
 
@@ -244,6 +244,12 @@ public class RemoteRoleControllerService {
             }
 
             @WorkerThread
+            private void notifyTimeout() {
+                Slog.e(LOG_TAG, "Call timed out, calling onFailure()");
+                notifyCallback(false);
+            }
+
+            @WorkerThread
             private void notifyCallback(boolean success) {
                 if (mCallbackNotified) {
                     return;
diff --git a/services/core/java/com/android/server/role/RoleUserState.java b/services/core/java/com/android/server/role/RoleUserState.java
index becc9622dffb..68e737e8656a 100644
--- a/services/core/java/com/android/server/role/RoleUserState.java
+++ b/services/core/java/com/android/server/role/RoleUserState.java
@@ -67,13 +67,13 @@ public class RoleUserState {
     private final int mUserId;
 
     @GuardedBy("RoleManagerService.mLock")
-    private int mVersion = VERSION_UNDEFINED;
+    private int mVersion;
 
     /**
      * Maps role names to its holders' package names. The values should never be null.
      */
     @GuardedBy("RoleManagerService.mLock")
-    private ArrayMap<String, ArraySet<String>> mRoles = null;
+    private ArrayMap<String, ArraySet<String>> mRoles;
 
     @GuardedBy("RoleManagerService.mLock")
     private boolean mDestroyed;
@@ -146,6 +146,8 @@ public class RoleUserState {
         throwIfDestroyedLocked();
         ArraySet<String> roleHolders = mRoles.get(roleName);
         if (roleHolders == null) {
+            Slog.e(LOG_TAG, "Cannot add role holder for unknown role, role: " + roleName
+                    + ", package: " + packageName);
             return false;
         }
         roleHolders.add(packageName);
@@ -167,6 +169,8 @@ public class RoleUserState {
         throwIfDestroyedLocked();
         ArraySet<String> roleHolders = mRoles.get(roleName);
         if (roleHolders == null) {
+            Slog.e(LOG_TAG, "Cannot remove role holder for unknown role, role: " + roleName
+                    + ", package: " + packageName);
             return false;
         }
         roleHolders.remove(packageName);
@@ -194,10 +198,10 @@ public class RoleUserState {
 
     @WorkerThread
     private void writeSync(int version, @NonNull ArrayMap<String, ArraySet<String>> roles) {
-        AtomicFile destination = new AtomicFile(getFile(mUserId), "roles-" + mUserId);
+        AtomicFile atomicFile = new AtomicFile(getFile(mUserId), "roles-" + mUserId);
         FileOutputStream out = null;
         try {
-            out = destination.startWrite();
+            out = atomicFile.startWrite();
 
             XmlSerializer serializer = Xml.newSerializer();
             serializer.setOutput(out, StandardCharsets.UTF_8.name());
@@ -208,11 +212,12 @@ public class RoleUserState {
             serializeRoles(serializer, version, roles);
 
             serializer.endDocument();
-            destination.finishWrite(out);
-        } catch (Throwable t) {
-            // Any error while writing is fatal.
-            Slog.wtf(LOG_TAG, "Failed to write roles file, restoring backup", t);
-            destination.failWrite(out);
+            atomicFile.finishWrite(out);
+        } catch (IllegalArgumentException | IllegalStateException | IOException e) {
+            Slog.wtf(LOG_TAG, "Failed to write roles.xml, restoring backup", e);
+            if (out != null) {
+                atomicFile.failWrite(out);
+            }
         } finally {
             IoUtils.closeQuietly(out);
         }
@@ -251,37 +256,34 @@ public class RoleUserState {
     @GuardedBy("RoleManagerService.mLock")
     public void readSyncLocked() {
         if (mRoles != null) {
-            throw new IllegalStateException("This RoleUserState has already read the XML file");
-        }
-        File file = getFile(mUserId);
-        FileInputStream in;
-        try {
-            in = new AtomicFile(file).openRead();
-        } catch (FileNotFoundException e) {
-            Slog.i(LOG_TAG, "No roles file found");
-            return;
+            throw new IllegalStateException("This RoleUserState has already read the roles.xml");
         }
 
-        try {
+        File file = getFile(mUserId);
+        try (FileInputStream in = new AtomicFile(file).openRead()) {
             XmlPullParser parser = Xml.newPullParser();
             parser.setInput(in, null);
             parseXmlLocked(parser);
+        } catch (FileNotFoundException e) {
+            Slog.i(LOG_TAG, "roles.xml not found");
+            mRoles = new ArrayMap<>();
+            mVersion = VERSION_UNDEFINED;
         } catch (XmlPullParserException | IOException e) {
-            throw new IllegalStateException("Failed to parse roles file: " + file , e);
-        } finally {
-            IoUtils.closeQuietly(in);
+            throw new IllegalStateException("Failed to parse roles.xml: " + file, e);
         }
     }
 
     private void parseXmlLocked(@NonNull XmlPullParser parser) throws IOException,
             XmlPullParserException {
-        int outerDepth = parser.getDepth();
         int type;
+        int depth;
+        int innerDepth = parser.getDepth() + 1;
         while ((type = parser.next()) != XmlPullParser.END_DOCUMENT
-                && (type != XmlPullParser.END_TAG || parser.getDepth() > outerDepth)) {
-            if (type == XmlPullParser.END_TAG || type == XmlPullParser.TEXT) {
+                && ((depth = parser.getDepth()) >= innerDepth || type != XmlPullParser.END_TAG)) {
+            if (depth > innerDepth || type != XmlPullParser.START_TAG) {
                 continue;
             }
+
             if (parser.getName().equals(TAG_ROLES)) {
                 parseRolesLocked(parser);
                 return;
@@ -293,13 +295,16 @@ public class RoleUserState {
             XmlPullParserException {
         mVersion = Integer.parseInt(parser.getAttributeValue(null, ATTRIBUTE_VERSION));
         mRoles = new ArrayMap<>();
-        int outerDepth = parser.getDepth();
+
         int type;
+        int depth;
+        int innerDepth = parser.getDepth() + 1;
         while ((type = parser.next()) != XmlPullParser.END_DOCUMENT
-                && (type != XmlPullParser.END_TAG || parser.getDepth() > outerDepth)) {
-            if (type == XmlPullParser.END_TAG || type == XmlPullParser.TEXT) {
+                && ((depth = parser.getDepth()) >= innerDepth || type != XmlPullParser.END_TAG)) {
+            if (depth > innerDepth || type != XmlPullParser.START_TAG) {
                 continue;
             }
+
             if (parser.getName().equals(TAG_ROLE)) {
                 String roleName = parser.getAttributeValue(null, ATTRIBUTE_NAME);
                 ArraySet<String> roleHolders = parseRoleHoldersLocked(parser);
@@ -312,18 +317,22 @@ public class RoleUserState {
     private ArraySet<String> parseRoleHoldersLocked(@NonNull XmlPullParser parser)
             throws IOException, XmlPullParserException {
         ArraySet<String> roleHolders = new ArraySet<>();
-        int outerDepth = parser.getDepth();
+
         int type;
+        int depth;
+        int innerDepth = parser.getDepth() + 1;
         while ((type = parser.next()) != XmlPullParser.END_DOCUMENT
-                && (type != XmlPullParser.END_TAG || parser.getDepth() > outerDepth)) {
-            if (type == XmlPullParser.END_TAG || type == XmlPullParser.TEXT) {
+                && ((depth = parser.getDepth()) >= innerDepth || type != XmlPullParser.END_TAG)) {
+            if (depth > innerDepth || type != XmlPullParser.START_TAG) {
                 continue;
             }
+
             if (parser.getName().equals(TAG_HOLDER)) {
                 String roleHolder = parser.getAttributeValue(null, ATTRIBUTE_NAME);
                 roleHolders.add(roleHolder);
             }
         }
+
         return roleHolders;
     }