OMS: extract verifyIdmap from createIdmap

For clarity, split IIdmap2::createIdmap into two separate functions:

  - IIdmap2::verifyIdmap [check if an existing idmap file is OK to use]
  - IIdmap2::createIdmap [unconditionally (re)create an idmap file]

Teach the IdmapManager to call verifyIdmap and to proceed with
createIdmap only if actually needed.

Test: atest OverlayDeviceTests OverlayHostTests
Change-Id: I9f6f1192011fcb094adffeca1eb3f709520bbd24

4 files changed, 24 insertions, 16 deletions

diff --git a/cmds/idmap2/idmap2d/Idmap2Service.cpp b/cmds/idmap2/idmap2d/Idmap2Service.cpp
index cf72cb94da2c..86b00f1d6e95 100644
--- a/cmds/idmap2/idmap2d/Idmap2Service.cpp
+++ b/cmds/idmap2/idmap2d/Idmap2Service.cpp
@@ -78,6 +78,18 @@ Status Idmap2Service::removeIdmap(const std::string& overlay_apk_path,
   }
 }
 
+Status Idmap2Service::verifyIdmap(const std::string& overlay_apk_path,
+                                  int32_t user_id ATTRIBUTE_UNUSED, bool* _aidl_return) {
+  assert(_aidl_return);
+  const std::string idmap_path = Idmap::CanonicalIdmapPathFor(kIdmapCacheDir, overlay_apk_path);
+  std::ifstream fin(idmap_path);
+  const std::unique_ptr<const IdmapHeader> header = IdmapHeader::FromBinaryStream(fin);
+  fin.close();
+  std::stringstream dev_null;
+  *_aidl_return = header && header->IsUpToDate(dev_null);
+  return ok();
+}
+
 Status Idmap2Service::createIdmap(const std::string& target_apk_path,
                                   const std::string& overlay_apk_path, int32_t user_id,
                                   std::unique_ptr<std::string>* _aidl_return) {
@@ -90,17 +102,6 @@ Status Idmap2Service::createIdmap(const std::string& target_apk_path,
 
   _aidl_return->reset(nullptr);
 
-  const std::string idmap_path = Idmap::CanonicalIdmapPathFor(kIdmapCacheDir, overlay_apk_path);
-  std::ifstream fin(idmap_path);
-  const std::unique_ptr<const IdmapHeader> header = IdmapHeader::FromBinaryStream(fin);
-  fin.close();
-  // do not reuse error stream from IsUpToDate below, or error messages will be
-  // polluted with irrelevant data
-  std::stringstream dev_null;
-  if (header && header->IsUpToDate(dev_null)) {
-    return ok();
-  }
-
   const std::unique_ptr<const ApkAssets> target_apk = ApkAssets::Load(target_apk_path);
   if (!target_apk) {
     return error("failed to load apk " + target_apk_path);
@@ -119,6 +120,7 @@ Status Idmap2Service::createIdmap(const std::string& target_apk_path,
   }
 
   umask(0133);  // u=rw,g=r,o=r
+  const std::string idmap_path = Idmap::CanonicalIdmapPathFor(kIdmapCacheDir, overlay_apk_path);
   std::ofstream fout(idmap_path);
   if (fout.fail()) {
     return error("failed to open idmap path " + idmap_path);
diff --git a/cmds/idmap2/idmap2d/Idmap2Service.h b/cmds/idmap2/idmap2d/Idmap2Service.h
index 2b32042d6aa3..4e5abc9069f4 100644
--- a/cmds/idmap2/idmap2d/Idmap2Service.h
+++ b/cmds/idmap2/idmap2d/Idmap2Service.h
@@ -39,6 +39,9 @@ class Idmap2Service : public BinderService<Idmap2Service>, public BnIdmap2 {
   binder::Status removeIdmap(const std::string& overlay_apk_path, int32_t user_id,
                              bool* _aidl_return);
 
+  binder::Status verifyIdmap(const std::string& overlay_apk_path, int32_t user_id,
+                             bool* _aidl_return);
+
   binder::Status createIdmap(const std::string& target_apk_path,
                              const std::string& overlay_apk_path, int32_t user_id,
                              std::unique_ptr<std::string>* _aidl_return);
diff --git a/cmds/idmap2/idmap2d/aidl/android/os/IIdmap2.aidl b/cmds/idmap2/idmap2d/aidl/android/os/IIdmap2.aidl
index 5d196101a7a6..d475417a0935 100644
--- a/cmds/idmap2/idmap2d/aidl/android/os/IIdmap2.aidl
+++ b/cmds/idmap2/idmap2d/aidl/android/os/IIdmap2.aidl
@@ -22,6 +22,7 @@ package android.os;
 interface IIdmap2 {
   @utf8InCpp String getIdmapPath(@utf8InCpp String overlayApkPath, int userId);
   boolean removeIdmap(@utf8InCpp String overlayApkPath, int userId);
+  boolean verifyIdmap(@utf8InCpp String overlayApkPath, int userId);
   @nullable @utf8InCpp String createIdmap(@utf8InCpp String targetApkPath,
                                           @utf8InCpp String overlayApkPath, int userId);
 }
diff --git a/services/core/java/com/android/server/om/IdmapManager.java b/services/core/java/com/android/server/om/IdmapManager.java
index 6d5982708f4c..16143d3ae9e0 100644
--- a/services/core/java/com/android/server/om/IdmapManager.java
+++ b/services/core/java/com/android/server/om/IdmapManager.java
@@ -60,7 +60,6 @@ class IdmapManager {
 
     boolean createIdmap(@NonNull final PackageInfo targetPackage,
             @NonNull final PackageInfo overlayPackage, int userId) {
-        // unused userId: see comment in OverlayManagerServiceImpl.removeIdmapIfPossible
         if (DEBUG) {
             Slog.d(TAG, "create idmap for " + targetPackage.packageName + " and "
                     + overlayPackage.packageName);
@@ -70,16 +69,19 @@ class IdmapManager {
         final String overlayPath = overlayPackage.applicationInfo.getBaseCodePath();
         try {
             if (FEATURE_FLAG_IDMAP2) {
-                mIdmap2Service.createIdmap(targetPath, overlayPath, userId);
+                if (mIdmap2Service.verifyIdmap(overlayPath, userId)) {
+                    return true;
+                }
+                return mIdmap2Service.createIdmap(targetPath, overlayPath, userId) != null;
             } else {
                 mInstaller.idmap(targetPath, overlayPath, sharedGid);
+                return true;
             }
         } catch (Exception e) {
             Slog.w(TAG, "failed to generate idmap for " + targetPath + " and "
                     + overlayPath + ": " + e.getMessage());
             return false;
         }
-        return true;
     }
 
     boolean removeIdmap(@NonNull final OverlayInfo oi, final int userId) {
@@ -88,15 +90,15 @@ class IdmapManager {
         }
         try {
             if (FEATURE_FLAG_IDMAP2) {
-                mIdmap2Service.removeIdmap(oi.baseCodePath, userId);
+                return mIdmap2Service.removeIdmap(oi.baseCodePath, userId);
             } else {
                 mInstaller.removeIdmap(oi.baseCodePath);
+                return true;
             }
         } catch (Exception e) {
             Slog.w(TAG, "failed to remove idmap for " + oi.baseCodePath + ": " + e.getMessage());
             return false;
         }
-        return true;
     }
 
     boolean idmapExists(@NonNull final OverlayInfo oi) {