Clean up STOPSHIP comments for role.

Bug: 132909319
Test: presubmit
Change-Id: I3cc812563515b08cfc79f97df1afb658b9c8a68b

9 files changed, 15 insertions, 12 deletions

diff --git a/src/com/android/packageinstaller/role/model/PreferredActivity.java b/src/com/android/packageinstaller/role/model/PreferredActivity.java
index f5004002..5062e9e0 100644
--- a/src/com/android/packageinstaller/role/model/PreferredActivity.java
+++ b/src/com/android/packageinstaller/role/model/PreferredActivity.java
@@ -72,8 +72,9 @@ public class PreferredActivity {
         PackageManager packageManager = context.getPackageManager();
         ComponentName packageActivity = mActivity.getQualifyingComponentForPackage(
                 packageName, context);
-        // TODO: STOPSHIP: Race condition, what if packageActivity became null? Just don't crash?
         if (packageActivity == null) {
+            // We might be running into some race condition here, but we can't do anything about it.
+            // This should be handled by a future reconciliation started by the package change.
             return;
         }
 
diff --git a/src/com/android/packageinstaller/role/model/RequiredContentProvider.java b/src/com/android/packageinstaller/role/model/RequiredContentProvider.java
index 76fd3944..69b5a64a 100644
--- a/src/com/android/packageinstaller/role/model/RequiredContentProvider.java
+++ b/src/com/android/packageinstaller/role/model/RequiredContentProvider.java
@@ -60,7 +60,7 @@ public class RequiredContentProvider extends RequiredComponent {
     @Nullable
     @Override
     protected String getComponentPermission(@NonNull ResolveInfo resolveInfo) {
-        // TODO: STOPSHIP: Which permission? Or both?
+        // TODO: Which permission? Or both?
         //return resolveInfo.providerInfo.readPermission;
         throw new UnsupportedOperationException();
     }
diff --git a/src/com/android/packageinstaller/role/model/Role.java b/src/com/android/packageinstaller/role/model/Role.java
index 615480ec..58cbf622 100644
--- a/src/com/android/packageinstaller/role/model/Role.java
+++ b/src/com/android/packageinstaller/role/model/Role.java
@@ -525,7 +525,9 @@ public class Role {
             return false;
         }
 
-        // TODO: STOPSHIP: Check for disabled packages?
+        if (!applicationInfo.enabled) {
+            return false;
+        }
 
         if (applicationInfo.isInstantApp()) {
             return false;
@@ -613,7 +615,10 @@ public class Role {
             appOp.revoke(packageName, context);
         }
 
-        // TODO: STOPSHIP: Revoke preferred activities?
+        // TODO: Revoke preferred activities? But this is unnecessary for most roles using it as
+        //  they have fallback holders. Moreover, clearing the preferred activity might result in
+        //  other system components listening to preferred activity change get notified for the
+        //  wrong thing when we are removing a exclusive role holder for adding another.
 
         if (mBehavior != null) {
             mBehavior.revoke(this, packageName, context);
diff --git a/src/com/android/packageinstaller/role/model/SmsRoleBehavior.java b/src/com/android/packageinstaller/role/model/SmsRoleBehavior.java
index 2cd2080a..606b95c8 100644
--- a/src/com/android/packageinstaller/role/model/SmsRoleBehavior.java
+++ b/src/com/android/packageinstaller/role/model/SmsRoleBehavior.java
@@ -66,8 +66,9 @@ public class SmsRoleBehavior implements RoleBehavior {
             return defaultPackageName;
         }
 
-        // TODO: STOPSHIP: This was the previous behavior, however this allows any third-party app
-        // to suddenly become the default SMS app and get the permissions.
+        // TODO(b/132916161): This was the previous behavior, however this may allow any third-party
+        //  app to suddenly become the default SMS app and get the permissions, if no system default
+        //  SMS app is available.
         List<String> qualifyingPackageNames = role.getQualifyingPackagesAsUser(
                 Process.myUserHandle(), context);
         return CollectionUtils.firstOrNull(qualifyingPackageNames);
diff --git a/src/com/android/packageinstaller/role/service/RoleControllerServiceImpl.java b/src/com/android/packageinstaller/role/service/RoleControllerServiceImpl.java
index 2c462845..3a0b1e01 100644
--- a/src/com/android/packageinstaller/role/service/RoleControllerServiceImpl.java
+++ b/src/com/android/packageinstaller/role/service/RoleControllerServiceImpl.java
@@ -45,7 +45,7 @@ public class RoleControllerServiceImpl extends RoleControllerService {
 
     private static final String LOG_TAG = RoleControllerServiceImpl.class.getSimpleName();
 
-    // TODO: STOPSHIP: Turn off debugging before we ship.
+    // STOPSHIP: Turn off debugging before we ship.
     private static final boolean DEBUG = true;
 
     private RoleManager mRoleManager;
@@ -53,6 +53,7 @@ public class RoleControllerServiceImpl extends RoleControllerService {
     @Override
     public void onCreate() {
         super.onCreate();
+
         mRoleManager = getSystemService(RoleManager.class);
     }
 
@@ -335,7 +336,6 @@ public class RoleControllerServiceImpl extends RoleControllerService {
         }
 
         if (applicationInfo != null) {
-            // TODO: STOPSHIP: Pass in appropriate arguments.
             role.revoke(packageName, dontKillApp, false, this);
         }
 
diff --git a/src/com/android/packageinstaller/role/ui/DefaultAppChildFragment.java b/src/com/android/packageinstaller/role/ui/DefaultAppChildFragment.java
index b64ea46f..675f35d8 100644
--- a/src/com/android/packageinstaller/role/ui/DefaultAppChildFragment.java
+++ b/src/com/android/packageinstaller/role/ui/DefaultAppChildFragment.java
@@ -217,7 +217,6 @@ public class DefaultAppChildFragment<PF extends PreferenceFragmentCompat
                 liveData.resetState();
                 break;
             case ManageRoleHolderStateLiveData.STATE_FAILURE:
-                // TODO: STOPSHIP: Notify user.
                 liveData.resetState();
                 break;
         }
diff --git a/src/com/android/packageinstaller/role/ui/RequestRoleActivity.java b/src/com/android/packageinstaller/role/ui/RequestRoleActivity.java
index 44d37400..8b56a221 100644
--- a/src/com/android/packageinstaller/role/ui/RequestRoleActivity.java
+++ b/src/com/android/packageinstaller/role/ui/RequestRoleActivity.java
@@ -171,7 +171,6 @@ public class RequestRoleActivity extends FragmentActivity {
             return;
         }
 
-        // TODO: STOPSHIP: Handle other form factors.
         if (savedInstanceState == null) {
             RequestRoleFragment fragment = RequestRoleFragment.newInstance(mRoleName, mPackageName);
             getSupportFragmentManager().beginTransaction()
diff --git a/src/com/android/packageinstaller/role/ui/RequestRoleFragment.java b/src/com/android/packageinstaller/role/ui/RequestRoleFragment.java
index a1af450a..55ea3dd1 100644
--- a/src/com/android/packageinstaller/role/ui/RequestRoleFragment.java
+++ b/src/com/android/packageinstaller/role/ui/RequestRoleFragment.java
@@ -327,7 +327,6 @@ public class RequestRoleFragment extends DialogFragment {
                 break;
             }
             case ManageRoleHolderStateLiveData.STATE_FAILURE:
-                // TODO: Notify user?
                 finish();
                 break;
         }
diff --git a/src/com/android/packageinstaller/role/ui/SpecialAppAccessChildFragment.java b/src/com/android/packageinstaller/role/ui/SpecialAppAccessChildFragment.java
index 0ff4a773..c34f0eb1 100644
--- a/src/com/android/packageinstaller/role/ui/SpecialAppAccessChildFragment.java
+++ b/src/com/android/packageinstaller/role/ui/SpecialAppAccessChildFragment.java
@@ -177,7 +177,6 @@ public class SpecialAppAccessChildFragment<PF extends PreferenceFragmentCompat
                 break;
             case ManageRoleHolderStateLiveData.STATE_FAILURE:
                 liveData.resetState();
-                // TODO: STOPSHIP: Notify user.
                 break;
         }
     }