Repackaging of classes with members having package dependent visibility RELNOTES: R8 will start to repackage classes, if possible, where the class or members have package dependent visibility. Bug: b/250671873 Change-Id: Icb492d737aeed5aea7260abaae089c6eef44f1f5

diff --git a/src/main/java/com/android/tools/r8/repackaging/Repackaging.java b/src/main/java/com/android/tools/r8/repackaging/Repackaging.java
index f292409..d3aee17 100644
--- a/src/main/java/com/android/tools/r8/repackaging/Repackaging.java
+++ b/src/main/java/com/android/tools/r8/repackaging/Repackaging.java

@@ -440,7 +440,7 @@
                   + simpleName.substring(outerClassSimpleName.length());
           repackagedDexType = repackagedDexType.withSimpleName(newSimpleName, dexItemFactory);
         } else {
-          assert false
+          assert options.disableInnerClassSeparatorValidationWhenRepackaging
               : "Unexpected name for inner class: "
                   + clazz.getType().toSourceString()
                   + " (outer class: "

diff --git a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
index a8058ac..9e0db7a 100644
--- a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
+++ b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java

@@ -64,7 +64,6 @@
 import com.android.tools.r8.utils.ListUtils;
 import com.android.tools.r8.utils.PredicateSet;
 import com.android.tools.r8.utils.ThreadUtils;
-import com.android.tools.r8.utils.TraversalContinuation;
 import com.android.tools.r8.utils.Visibility;
 import com.android.tools.r8.utils.WorkList;
 import com.android.tools.r8.utils.collections.ProgramMethodSet;
@@ -1089,22 +1088,11 @@
     if (!options().isRepackagingEnabled()) {
       return false;
     }
-    if (!keepInfo.getInfo(clazz).isRepackagingAllowed(clazz, options())) {
+    if (!keepInfo.getInfo(clazz).isRepackagingAllowed(options())) {
       return false;
     }
     SeedMapper applyMappingSeedMapper = appView.getApplyMappingSeedMapper();
-    if (applyMappingSeedMapper != null && applyMappingSeedMapper.hasMapping(clazz.type)) {
-      return false;
-    }
-    return clazz
-        .traverseProgramMembers(
-            member -> {
-              if (keepInfo.getInfo(member).isRepackagingAllowed(member, options())) {
-                return TraversalContinuation.doContinue();
-              }
-              return TraversalContinuation.doBreak();
-            })
-        .shouldContinue();
+    return applyMappingSeedMapper == null || !applyMappingSeedMapper.hasMapping(clazz.type);
   }
 
   public boolean isPinned(DexReference reference) {

diff --git a/src/main/java/com/android/tools/r8/shaking/KeepClassInfo.java b/src/main/java/com/android/tools/r8/shaking/KeepClassInfo.java
index 9785028..f504792 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepClassInfo.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepClassInfo.java

@@ -7,7 +7,6 @@
 import com.android.tools.r8.graph.DexItemFactory;
 import com.android.tools.r8.graph.DexProgramClass;
 import com.android.tools.r8.graph.DexType;
-import com.android.tools.r8.graph.ProgramDefinition;
 import com.android.tools.r8.utils.InternalOptions;
 import java.util.function.Function;
 
@@ -15,10 +14,10 @@
 public final class KeepClassInfo extends KeepInfo<KeepClassInfo.Builder, KeepClassInfo> {
 
   // Requires all aspects of a class to be kept.
-  private static final KeepClassInfo TOP = new Builder().makeTop().build();
+  private static final KeepClassInfo TOP = new Builder().makeTop().disallowRepackaging().build();
 
   // Requires no aspects of a class to be kept.
-  private static final KeepClassInfo BOTTOM = new Builder().makeBottom().build();
+  private static final KeepClassInfo BOTTOM = new Builder().makeBottom().allowRepackaging().build();
 
   public static KeepClassInfo top() {
     return TOP;
@@ -32,10 +31,12 @@
     return bottom().joiner();
   }
 
+  private final boolean allowRepackaging;
   private final boolean checkEnumUnboxed;
 
   private KeepClassInfo(Builder builder) {
     super(builder);
+    this.allowRepackaging = builder.isRepackagingAllowed();
     this.checkEnumUnboxed = builder.isCheckEnumUnboxedEnabled();
   }
 
@@ -57,13 +58,18 @@
     return new Joiner(this);
   }
 
-  @Override
-  public boolean isRepackagingAllowed(
-      ProgramDefinition definition, GlobalKeepInfoConfiguration configuration) {
-    return configuration.isRepackagingEnabled()
-        && internalIsRepackagingAllowed()
-        && (definition.getAccessFlags().isPublic()
-            || !internalIsAccessModificationRequiredForRepackaging());
+  /**
+   * True if an item may be repackaged.
+   *
+   * <p>This method requires knowledge of the global configuration as that can override the concrete
+   * value on a given item.
+   */
+  public boolean isRepackagingAllowed(GlobalKeepInfoConfiguration configuration) {
+    return configuration.isRepackagingEnabled() && internalIsRepackagingAllowed();
+  }
+
+  boolean internalIsRepackagingAllowed() {
+    return allowRepackaging;
   }
 
   public boolean isKotlinMetadataRemovalAllowed(
@@ -98,6 +104,7 @@
 
   public static class Builder extends KeepInfo.Builder<Builder, KeepClassInfo> {
 
+    private boolean allowRepackaging;
     private boolean checkEnumUnboxed;
 
     private Builder() {
@@ -106,6 +113,7 @@
 
     private Builder(KeepClassInfo original) {
       super(original);
+      allowRepackaging = original.internalIsRepackagingAllowed();
       checkEnumUnboxed = original.internalIsCheckEnumUnboxedEnabled();
     }
 
@@ -120,6 +128,23 @@
       return self();
     }
 
+    public boolean isRepackagingAllowed() {
+      return allowRepackaging;
+    }
+
+    private Builder setAllowRepackaging(boolean allowRepackaging) {
+      this.allowRepackaging = allowRepackaging;
+      return self();
+    }
+
+    public Builder allowRepackaging() {
+      return setAllowRepackaging(true);
+    }
+
+    public Builder disallowRepackaging() {
+      return setAllowRepackaging(false);
+    }
+
     public Builder setCheckEnumUnboxed() {
       return setCheckEnumUnboxed(true);
     }
@@ -151,6 +176,7 @@
     @Override
     boolean internalIsEqualTo(KeepClassInfo other) {
       return super.internalIsEqualTo(other)
+          && isRepackagingAllowed() == other.internalIsRepackagingAllowed()
           && isCheckEnumUnboxedEnabled() == other.internalIsCheckEnumUnboxedEnabled();
     }
 
@@ -161,12 +187,12 @@
 
     @Override
     public Builder makeTop() {
-      return super.makeTop().unsetCheckEnumUnboxed();
+      return super.makeTop().unsetCheckEnumUnboxed().disallowRepackaging();
     }
 
     @Override
     public Builder makeBottom() {
-      return super.makeBottom().unsetCheckEnumUnboxed();
+      return super.makeBottom().unsetCheckEnumUnboxed().allowRepackaging();
     }
   }
 
@@ -181,6 +207,11 @@
       return self();
     }
 
+    public Joiner disallowRepackaging() {
+      builder.disallowRepackaging();
+      return self();
+    }
+
     @Override
     public Joiner asClassJoiner() {
       return this;
@@ -190,7 +221,8 @@
     public Joiner merge(Joiner joiner) {
       // Should be extended to merge the fields of this class in case any are added.
       return super.merge(joiner)
-          .applyIf(joiner.builder.isCheckEnumUnboxedEnabled(), Joiner::setCheckEnumUnboxed);
+          .applyIf(joiner.builder.isCheckEnumUnboxedEnabled(), Joiner::setCheckEnumUnboxed)
+          .applyIf(!joiner.builder.isRepackagingAllowed(), Joiner::disallowRepackaging);
     }
 
     @Override

diff --git a/src/main/java/com/android/tools/r8/shaking/KeepInfo.java b/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
index d9708ce..092f4ea 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepInfo.java

@@ -6,7 +6,6 @@
 import com.android.tools.r8.graph.AppView;
 import com.android.tools.r8.graph.DexReference;
 import com.android.tools.r8.graph.EnclosingMethodAttribute;
-import com.android.tools.r8.graph.ProgramDefinition;
 import com.android.tools.r8.shaking.KeepInfo.Builder;
 import com.android.tools.r8.shaking.KeepReason.ReflectiveUseFrom;
 import com.android.tools.r8.utils.InternalOptions;
@@ -22,31 +21,25 @@
   private final boolean allowAnnotationRemoval;
   private final boolean allowMinification;
   private final boolean allowOptimization;
-  private final boolean allowRepackaging;
   private final boolean allowShrinking;
   private final boolean allowSignatureRemoval;
   private final boolean checkDiscarded;
-  private final boolean requireAccessModificationForRepackaging;
 
   private KeepInfo(
       boolean allowAccessModification,
       boolean allowAnnotationRemoval,
       boolean allowMinification,
       boolean allowOptimization,
-      boolean allowRepackaging,
       boolean allowShrinking,
       boolean allowSignatureRemoval,
-      boolean checkDiscarded,
-      boolean requireAccessModificationForRepackaging) {
+      boolean checkDiscarded) {
     this.allowAccessModification = allowAccessModification;
     this.allowAnnotationRemoval = allowAnnotationRemoval;
     this.allowMinification = allowMinification;
     this.allowOptimization = allowOptimization;
-    this.allowRepackaging = allowRepackaging;
     this.allowShrinking = allowShrinking;
     this.allowSignatureRemoval = allowSignatureRemoval;
     this.checkDiscarded = checkDiscarded;
-    this.requireAccessModificationForRepackaging = requireAccessModificationForRepackaging;
   }
 
   KeepInfo(B builder) {
@@ -55,11 +48,9 @@
         builder.isAnnotationRemovalAllowed(),
         builder.isMinificationAllowed(),
         builder.isOptimizationAllowed(),
-        builder.isRepackagingAllowed(),
         builder.isShrinkingAllowed(),
         builder.isSignatureRemovalAllowed(),
-        builder.isCheckDiscardedEnabled(),
-        builder.isAccessModificationRequiredForRepackaging());
+        builder.isCheckDiscardedEnabled());
   }
 
   public static Joiner<?, ?, ?> newEmptyJoinerFor(DexReference reference) {
@@ -164,23 +155,6 @@
   }
 
   /**
-   * True if an item may be repackaged.
-   *
-   * <p>This method requires knowledge of the global configuration as that can override the concrete
-   * value on a given item.
-   */
-  public abstract boolean isRepackagingAllowed(
-      ProgramDefinition definition, GlobalKeepInfoConfiguration configuration);
-
-  boolean internalIsRepackagingAllowed() {
-    return allowRepackaging;
-  }
-
-  boolean internalIsAccessModificationRequiredForRepackaging() {
-    return requireAccessModificationForRepackaging;
-  }
-
-  /**
    * True if an item may have its access flags modified.
    *
    * <p>This method requires knowledge of the global access modification as that will override the
@@ -242,7 +216,6 @@
         && (allowAnnotationRemoval || !other.internalIsAnnotationRemovalAllowed())
         && (allowMinification || !other.internalIsMinificationAllowed())
         && (allowOptimization || !other.internalIsOptimizationAllowed())
-        && (allowRepackaging || !other.internalIsRepackagingAllowed())
         && (allowShrinking || !other.internalIsShrinkingAllowed())
         && (allowSignatureRemoval || !other.internalIsSignatureRemovalAllowed())
         && (!checkDiscarded || other.internalIsCheckDiscardedEnabled());
@@ -265,12 +238,10 @@
     private boolean allowAccessModification;
     private boolean allowAnnotationRemoval;
     private boolean allowMinification;
-    private boolean allowRepackaging;
     private boolean allowOptimization;
     private boolean allowShrinking;
     private boolean allowSignatureRemoval;
     private boolean checkDiscarded;
-    private boolean requireAccessModificationForRepackaging;
 
     Builder() {
       // Default initialized. Use should be followed by makeTop/makeBottom.
@@ -282,12 +253,9 @@
       allowAnnotationRemoval = original.internalIsAnnotationRemovalAllowed();
       allowMinification = original.internalIsMinificationAllowed();
       allowOptimization = original.internalIsOptimizationAllowed();
-      allowRepackaging = original.internalIsRepackagingAllowed();
       allowShrinking = original.internalIsShrinkingAllowed();
       allowSignatureRemoval = original.internalIsSignatureRemovalAllowed();
       checkDiscarded = original.internalIsCheckDiscardedEnabled();
-      requireAccessModificationForRepackaging =
-          original.internalIsAccessModificationRequiredForRepackaging();
     }
 
     B makeTop() {
@@ -295,11 +263,9 @@
       disallowAnnotationRemoval();
       disallowMinification();
       disallowOptimization();
-      disallowRepackaging();
       disallowShrinking();
       disallowSignatureRemoval();
       unsetCheckDiscarded();
-      requireAccessModificationForRepackaging();
       return self();
     }
 
@@ -308,11 +274,9 @@
       allowAnnotationRemoval();
       allowMinification();
       allowOptimization();
-      allowRepackaging();
       allowShrinking();
       allowSignatureRemoval();
       unsetCheckDiscarded();
-      unsetRequireAccessModificationForRepackaging();
       return self();
     }
 
@@ -336,16 +300,9 @@
           && isAnnotationRemovalAllowed() == other.internalIsAnnotationRemovalAllowed()
           && isMinificationAllowed() == other.internalIsMinificationAllowed()
           && isOptimizationAllowed() == other.internalIsOptimizationAllowed()
-          && isRepackagingAllowed() == other.internalIsRepackagingAllowed()
           && isShrinkingAllowed() == other.internalIsShrinkingAllowed()
           && isSignatureRemovalAllowed() == other.internalIsSignatureRemovalAllowed()
-          && isCheckDiscardedEnabled() == other.internalIsCheckDiscardedEnabled()
-          && isAccessModificationRequiredForRepackaging()
-              == other.internalIsAccessModificationRequiredForRepackaging();
-    }
-
-    public boolean isAccessModificationRequiredForRepackaging() {
-      return requireAccessModificationForRepackaging;
+          && isCheckDiscardedEnabled() == other.internalIsCheckDiscardedEnabled();
     }
 
     public boolean isAccessModificationAllowed() {
@@ -368,10 +325,6 @@
       return allowOptimization;
     }
 
-    public boolean isRepackagingAllowed() {
-      return allowRepackaging;
-    }
-
     public boolean isShrinkingAllowed() {
       return allowShrinking;
     }
@@ -393,19 +346,6 @@
       return setAllowMinification(false);
     }
 
-    public B setAllowRepackaging(boolean allowRepackaging) {
-      this.allowRepackaging = allowRepackaging;
-      return self();
-    }
-
-    public B allowRepackaging() {
-      return setAllowRepackaging(true);
-    }
-
-    public B disallowRepackaging() {
-      return setAllowRepackaging(false);
-    }
-
     public B setAllowOptimization(boolean allowOptimization) {
       this.allowOptimization = allowOptimization;
       return self();
@@ -445,20 +385,6 @@
       return setCheckDiscarded(false);
     }
 
-    public B setRequireAccessModificationForRepackaging(
-        boolean requireAccessModificationForRepackaging) {
-      this.requireAccessModificationForRepackaging = requireAccessModificationForRepackaging;
-      return self();
-    }
-
-    public B requireAccessModificationForRepackaging() {
-      return setRequireAccessModificationForRepackaging(true);
-    }
-
-    public B unsetRequireAccessModificationForRepackaging() {
-      return setRequireAccessModificationForRepackaging(false);
-    }
-
     public B setAllowAccessModification(boolean allowAccessModification) {
       this.allowAccessModification = allowAccessModification;
       return self();
@@ -608,11 +534,6 @@
       return self();
     }
 
-    public J disallowRepackaging() {
-      builder.disallowRepackaging();
-      return self();
-    }
-
     public J disallowOptimization() {
       builder.disallowOptimization();
       return self();
@@ -633,24 +554,15 @@
       return self();
     }
 
-    public J requireAccessModificationForRepackaging() {
-      builder.requireAccessModificationForRepackaging();
-      return self();
-    }
-
     public J merge(J joiner) {
       Builder<B, K> builder = joiner.builder;
       applyIf(!builder.isAccessModificationAllowed(), Joiner::disallowAccessModification);
       applyIf(!builder.isAnnotationRemovalAllowed(), Joiner::disallowAnnotationRemoval);
       applyIf(!builder.isMinificationAllowed(), Joiner::disallowMinification);
       applyIf(!builder.isOptimizationAllowed(), Joiner::disallowOptimization);
-      applyIf(!builder.isRepackagingAllowed(), Joiner::disallowRepackaging);
       applyIf(!builder.isShrinkingAllowed(), Joiner::disallowShrinking);
       applyIf(!builder.isSignatureRemovalAllowed(), Joiner::disallowSignatureRemoval);
       applyIf(builder.isCheckDiscardedEnabled(), Joiner::setCheckDiscarded);
-      applyIf(
-          builder.isAccessModificationRequiredForRepackaging(),
-          Joiner::requireAccessModificationForRepackaging);
       reasons.addAll(joiner.reasons);
       rules.addAll(joiner.rules);
       return self();

diff --git a/src/main/java/com/android/tools/r8/shaking/KeepInfoCollection.java b/src/main/java/com/android/tools/r8/shaking/KeepInfoCollection.java
index 62529ed..c78e39a 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepInfoCollection.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepInfoCollection.java

@@ -140,6 +140,10 @@
     throw new Unreachable();
   }
 
+  public final KeepClassInfo getInfo(DexProgramClass clazz) {
+    return getClassInfo(clazz);
+  }
+
   public final KeepInfo<?, ?> getInfo(ProgramDefinition definition) {
     if (definition.isProgramClass()) {
       return getClassInfo(definition.asProgramClass());
@@ -279,8 +283,7 @@
             assert newType == type
                 || !info.isPinned(options)
                 || info.isMinificationAllowed(options)
-                || info.isRepackagingAllowed(
-                    definitions.definitionFor(newType).asProgramClass(), options);
+                || info.isRepackagingAllowed(options);
             KeepClassInfo previous = newClassInfo.put(newType, info);
             assert previous == null;
           });
@@ -461,29 +464,6 @@
       joinMethod(method, KeepInfo.Joiner::top);
     }
 
-    public void unsetRequireAllowAccessModificationForRepackaging(ProgramDefinition definition) {
-      if (definition.isProgramClass()) {
-        DexProgramClass clazz = definition.asProgramClass();
-        KeepClassInfo info = getClassInfo(clazz);
-        keepClassInfo.put(
-            clazz.getType(), info.builder().unsetRequireAccessModificationForRepackaging().build());
-      } else if (definition.isProgramMethod()) {
-        ProgramMethod method = definition.asProgramMethod();
-        KeepMethodInfo info = getMethodInfo(method);
-        keepMethodInfo.put(
-            method.getReference(),
-            info.builder().unsetRequireAccessModificationForRepackaging().build());
-      } else if (definition.isProgramField()) {
-        ProgramField field = definition.asProgramField();
-        KeepFieldInfo info = getFieldInfo(field);
-        keepFieldInfo.put(
-            field.getReference(),
-            info.builder().unsetRequireAccessModificationForRepackaging().build());
-      } else {
-        throw new Unreachable();
-      }
-    }
-
     public void joinField(ProgramField field, Consumer<? super KeepFieldInfo.Joiner> fn) {
       KeepFieldInfo info = getFieldInfo(field);
       if (info.isTop()) {

diff --git a/src/main/java/com/android/tools/r8/shaking/KeepMemberInfo.java b/src/main/java/com/android/tools/r8/shaking/KeepMemberInfo.java
index 2225e14..e77c27e 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepMemberInfo.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepMemberInfo.java

@@ -4,7 +4,6 @@
 package com.android.tools.r8.shaking;
 
 import com.android.tools.r8.graph.DexProgramClass;
-import com.android.tools.r8.graph.ProgramDefinition;
 import com.android.tools.r8.shaking.KeepInfo.Builder;
 
 /** Immutable keep requirements for a member. */
@@ -15,14 +14,6 @@
     super(builder);
   }
 
-  @Override
-  public boolean isRepackagingAllowed(
-      ProgramDefinition definition, GlobalKeepInfoConfiguration configuration) {
-    return configuration.isRepackagingEnabled()
-        && (definition.getAccessFlags().isPublic()
-            || !internalIsAccessModificationRequiredForRepackaging());
-  }
-
   public boolean isKotlinMetadataRemovalAllowed(
       DexProgramClass holder, GlobalKeepInfoConfiguration configuration) {
     // Checking the holder for missing kotlin information relies on the holder being processed

diff --git a/src/main/java/com/android/tools/r8/shaking/RootSetUtils.java b/src/main/java/com/android/tools/r8/shaking/RootSetUtils.java
index 3b46564..cc39429 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetUtils.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetUtils.java

@@ -1135,6 +1135,7 @@
         dependentMinimumKeepInfo
             .getOrCreateMinimumKeepInfoFor(preconditionEvent, clazz.getReference())
             .disallowMinification()
+            .asClassJoiner()
             .disallowRepackaging();
       }
     }
@@ -1638,14 +1639,14 @@
         dependentMinimumKeepInfo
             .getOrCreateMinimumKeepInfoFor(preconditionEvent, item.getReference())
             .disallowMinification()
-            .disallowRepackaging();
+            .applyIf(item.isProgramClass(), joiner -> joiner.asClassJoiner().disallowRepackaging());
         context.markAsUsed();
       }
 
       if (appView.options().isRepackagingEnabled() && !modifiers.allowsObfuscation) {
         dependentMinimumKeepInfo
             .getOrCreateMinimumKeepInfoFor(preconditionEvent, item.getReference())
-            .disallowRepackaging();
+            .applyIf(item.isProgramClass(), joiner -> joiner.asClassJoiner().disallowRepackaging());
         context.markAsUsed();
       }
 
@@ -1664,10 +1665,6 @@
                 .addRule(keepRule)
                 .disallowShrinking();
         context.markAsUsed();
-
-        if (item.getAccessFlags().isPackagePrivateOrProtected()) {
-          minimumKeepInfoForItem.requireAccessModificationForRepackaging();
-        }
       }
 
       if (modifiers.includeDescriptorClasses) {
@@ -1981,7 +1978,8 @@
       getDependentMinimumKeepInfo()
           .getOrCreateUnconditionalMinimumKeepInfoFor(definition.getReference())
           .disallowMinification()
-          .disallowRepackaging();
+          .applyIf(
+              definition.isProgramClass(), joiner -> joiner.asClassJoiner().disallowRepackaging());
     }
 
     public boolean verifyKeptFieldsAreAccessedAndLive(AppView<AppInfoWithLiveness> appView) {

diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 68c055f..9af4f06 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java

@@ -804,6 +804,7 @@
   public boolean reportMissingClassesInEnclosingMethodAttribute = false;
   public boolean reportMissingClassesInInnerClassAttributes = false;
   public boolean disableGenericSignatureValidation = false;
+  public boolean disableInnerClassSeparatorValidationWhenRepackaging = false;
 
   // EXPERIMENTAL flag to get behaviour as close to Proguard as possible.
   public boolean forceProguardCompatibility = false;

diff --git a/src/test/java/com/android/tools/r8/ir/optimize/reflection/InnerClassNameTestRunner.java b/src/test/java/com/android/tools/r8/ir/optimize/reflection/InnerClassNameTestRunner.java
index 8b33acb..374fb98 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/reflection/InnerClassNameTestRunner.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/reflection/InnerClassNameTestRunner.java

@@ -225,12 +225,16 @@
     R8TestCompileResult r8CompileResult =
         testForR8(parameters.getBackend())
             .addKeepMainRule(MAIN_CLASS)
-            .addKeepRules("-keep,allowobfuscation class * { *; }")
-            .addKeepRules("-keepattributes InnerClasses,EnclosingMethod")
+            .addKeepAllClassesRuleWithAllowObfuscation()
+            .addKeepAttributeInnerClassesAndEnclosingMethod()
             .addProgramClassFileData(InnerClassNameTestDump.dump(config, parameters))
             .allowDiagnosticInfoMessages(hasMalformedInnerClassAttribute())
             .minification(minify)
-            .addOptionsModification(InternalOptions::disableNameReflectionOptimization)
+            .addOptionsModification(
+                options -> {
+                  options.disableInnerClassSeparatorValidationWhenRepackaging = true;
+                  options.disableNameReflectionOptimization();
+                })
             .setMinApi(parameters.getApiLevel())
             .compile()
             .apply(this::checkWarningsAboutMalformedAttribute);

diff --git a/src/test/java/com/android/tools/r8/naming/PackageNamingTest.java b/src/test/java/com/android/tools/r8/naming/PackageNamingTest.java
index 487498f..fa9de18 100644
--- a/src/test/java/com/android/tools/r8/naming/PackageNamingTest.java
+++ b/src/test/java/com/android/tools/r8/naming/PackageNamingTest.java

@@ -356,13 +356,11 @@
         ba.getDexProgramClass().getType().getPackageName(),
         bb.getDexProgramClass().getType().getPackageName());
 
-    // We cannot repackage c or d since these have package-private members and a
-    // keep,allowobfuscation. For us to be able to repackage them, we have to use
-    // -allowaccesmodification.
+    // We can repackage c and d since these have no external package-private references.
     List<String> klasses = ImmutableList.of("naming101.c", "naming101.d");
     for (String klass : klasses) {
       ClassSubject k = inspector.clazz(klass);
-      assertNotEquals("naming101.a", k.getDexProgramClass().getType().getPackageName());
+      assertEquals("naming101.a", k.getDexProgramClass().getType().getPackageName());
     }
 
     // All other classes can be repackaged to naming101.a, but naming101.a.a exists to make a name

diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageProtectedInSamePackageTest.java b/src/test/java/com/android/tools/r8/repackage/RepackageProtectedInSamePackageTest.java
index a6c9ea3..8152688 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageProtectedInSamePackageTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageProtectedInSamePackageTest.java

@@ -45,7 +45,7 @@
         .addKeepMainRule(Main.class)
         .addDontWarn(RepackageProtectedInSamePackageTest.class, NeverClassInline.class)
         .run(parameters.getRuntime(), Main.class, typeName(RepackageForKeepClassMembers.class))
-        .inspect(inspector -> inspect(inspector, true))
+        .inspect(this::inspect)
         .assertSuccessWithOutputLines(EXPECTED);
   }
 
@@ -61,7 +61,7 @@
         .addKeepMainRule(Main.class)
         .enableNeverClassInliningAnnotations()
         .run(parameters.getRuntime(), Main.class)
-        .inspect(inspector -> inspect(inspector, false))
+        .inspect(this::inspect)
         .assertSuccessWithOutputLines(EXPECTED);
   }
 
@@ -82,16 +82,10 @@
         .transform();
   }
 
-  private void inspect(CodeInspector inspector, boolean isProguard) {
+  private void inspect(CodeInspector inspector) {
     ClassSubject clazz = inspector.clazz(RepackageForKeepClassMembers.class);
     assertThat(clazz, isPresent());
-    // TODO(b/250671873): We should be able to repackage the Sub class since the only reference
-    //  to Sub.class is in the same package and we have allowobfuscation.
-    if (isProguard) {
-      assertThat(RepackageForKeepClassMembers.class, isRepackaged(inspector));
-    } else {
-      assertThat(RepackageForKeepClassMembers.class, isNotRepackaged(inspector));
-    }
+    assertThat(RepackageForKeepClassMembers.class, isRepackaged(inspector));
     assertThat(clazz.uniqueFieldWithOriginalName("hashCodeCache"), isPresentAndNotRenamed());
     assertThat(clazz.uniqueMethodWithOriginalName("calculateHashCode"), isPresentAndNotRenamed());
   }

diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageTest.java b/src/test/java/com/android/tools/r8/repackage/RepackageTest.java
index 179845d..9742b6a 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageTest.java

@@ -163,18 +163,16 @@
 
     // 3.A, 3.B, 3.C) Accessing a kept method is OK.
     markShouldAlwaysBeEligible.accept(AccessPublicKeptMethodOnReachableClass.class);
-    markEligibleWithAllowAccessModification.accept(
-        AccessPackagePrivateKeptMethodOnReachableClassDirect.class);
-    markEligibleWithAllowAccessModification.accept(
-        AccessPackagePrivateKeptMethodOnReachableClassIndirect.class);
+    markShouldAlwaysBeEligible.accept(AccessPackagePrivateKeptMethodOnReachableClassDirect.class);
+    markShouldAlwaysBeEligible.accept(AccessPackagePrivateKeptMethodOnReachableClassIndirect.class);
 
     // 4) -keepclassmembers,allowobfuscation class ReachableClassWithKeptMethod { <methods>; }
 
     // 4.A, 4.B, 4.C) Accessing a kept method is OK.
     markShouldAlwaysBeEligible.accept(AccessPublicKeptMethodAllowRenamingOnReachableClass.class);
-    markEligibleWithAllowAccessModification.accept(
+    markShouldAlwaysBeEligible.accept(
         AccessPackagePrivateKeptMethodAllowRenamingOnReachableClassDirect.class);
-    markEligibleWithAllowAccessModification.accept(
+    markShouldAlwaysBeEligible.accept(
         AccessPackagePrivateKeptMethodAllowRenamingOnReachableClassIndirect.class);
 
     // 5) No keep rule.

diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageWithKeepPackagePrivateTest.java b/src/test/java/com/android/tools/r8/repackage/RepackageWithKeepPackagePrivateTest.java
index 8020e78..fda2011 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageWithKeepPackagePrivateTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageWithKeepPackagePrivateTest.java

@@ -10,6 +10,7 @@
 
 import com.android.tools.r8.R8TestCompileResult;
 import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.ToolHelper.DexVm.Version;
 import com.android.tools.r8.utils.BooleanUtils;
 import com.android.tools.r8.utils.codeinspector.CodeInspector;
 import com.google.common.collect.ImmutableList;
@@ -62,7 +63,12 @@
         .compile()
         .addRunClasspathFiles(compileResult.writeToZip())
         .run(parameters.getRuntime(), TestClass.class)
-        .assertSuccessWithOutputLines("Hello world!");
+        .assertFailureWithErrorThatThrowsIf(hasIllegalAccessError(), IllegalAccessError.class)
+        .assertSuccessWithOutputLinesIf(!hasIllegalAccessError(), "Hello world!");
+  }
+
+  private boolean hasIllegalAccessError() {
+    return !allowAccessModification && !parameters.isDexRuntimeVersion(Version.V8_1_0);
   }
 
   private List<String> getKeepRules() {
@@ -76,8 +82,8 @@
   }
 
   private void inspect(CodeInspector inspector) {
-    assertThat(A.class, isRepackagedIf(inspector, allowAccessModification));
-    assertThat(B.class, isRepackagedIf(inspector, allowAccessModification));
+    assertThat(A.class, isRepackaged(inspector));
+    assertThat(B.class, isRepackaged(inspector));
   }
 
   public static class TestClass {