Version 1.4.35

Cherry-pick:
Do not publicize indirectly kept methods.
CL: https://r8-review.googlesource.com/33820

Cherry-pick:
Reproduce b/123575857: dependent rules with access modifier not honored.
CL: https://r8-review.googlesource.com/33660

Cherry-pick:
Fix root set membership check in publicizer.
CL: https://r8-review.googlesource.com/33641

Bug: 123575857, 121005865, 121295633
Change-Id: Id7813363a86575fc042c954ae406aee96674b1f7
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 4da9b6e..89efb56 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -363,9 +363,12 @@
         timing.end();
       }
 
+      assert appView.appInfo().hasLiveness();
+
       if (options.getProguardConfiguration().isAccessModificationAllowed()) {
         GraphLense publicizedLense =
-            ClassAndMemberPublicizer.run(executorService, timing, application, appView, rootSet);
+            ClassAndMemberPublicizer.run(
+                executorService, timing, application, appView.withLiveness(), rootSet);
         if (publicizedLense != appView.graphLense()) {
           appView.setGraphLense(publicizedLense);
           // We can now remove visibility bridges. Note that we do not need to update the
@@ -476,8 +479,6 @@
             new EnumOrdinalMapCollector(appViewWithLiveness, options).run());
       }
 
-      assert appView.appInfo().hasLiveness();
-
       timing.begin("Create IR");
       Set<DexCallSite> desugaredCallSites;
       CfgPrinter printer = options.printCfg ? new CfgPrinter() : null;
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index ee26dd8..558540a 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
 
   // This field is accessed from release scripts using simple pattern matching.
   // Therefore, changing this field could break our release scripts.
-  public static final String LABEL = "1.4.34";
+  public static final String LABEL = "1.4.35";
 
   private Version() {
   }
diff --git a/src/main/java/com/android/tools/r8/optimize/ClassAndMemberPublicizer.java b/src/main/java/com/android/tools/r8/optimize/ClassAndMemberPublicizer.java
index b59e3e9..3b3c125 100644
--- a/src/main/java/com/android/tools/r8/optimize/ClassAndMemberPublicizer.java
+++ b/src/main/java/com/android/tools/r8/optimize/ClassAndMemberPublicizer.java
@@ -12,6 +12,7 @@
 import com.android.tools.r8.graph.MethodAccessFlags;
 import com.android.tools.r8.ir.optimize.MethodPoolCollection;
 import com.android.tools.r8.optimize.PublicizerLense.PublicizedLenseBuilder;
+import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
 import com.android.tools.r8.shaking.RootSetBuilder.RootSet;
 import com.android.tools.r8.utils.Timing;
 import java.util.LinkedHashSet;
@@ -22,13 +23,16 @@
 public final class ClassAndMemberPublicizer {
 
   private final DexApplication application;
-  private final AppView appView;
+  private final AppView<? extends AppInfoWithLiveness> appView;
   private final RootSet rootSet;
   private final MethodPoolCollection methodPoolCollection;
 
   private final PublicizedLenseBuilder lenseBuilder = PublicizerLense.createBuilder();
 
-  private ClassAndMemberPublicizer(DexApplication application, AppView appView, RootSet rootSet) {
+  private ClassAndMemberPublicizer(
+      DexApplication application,
+      AppView<? extends AppInfoWithLiveness> appView,
+      RootSet rootSet) {
     this.application = application;
     this.appView = appView;
     this.methodPoolCollection = new MethodPoolCollection(application);
@@ -45,7 +49,7 @@
       ExecutorService executorService,
       Timing timing,
       DexApplication application,
-      AppView appView,
+      AppView<? extends AppInfoWithLiveness> appView,
       RootSet rootSet)
       throws ExecutionException {
     return new ClassAndMemberPublicizer(application, appView, rootSet).run(executorService, timing);
@@ -107,7 +111,7 @@
 
     if (!accessFlags.isStatic()) {
       // If this method is mentioned in keep rules, do not transform (rule applications changed).
-      if (rootSet.noShrinking.containsKey(encodedMethod)) {
+      if (appView.appInfo().isPinned(encodedMethod.method)) {
         return false;
       }
 
@@ -121,7 +125,7 @@
       boolean wasSeen = methodPoolCollection.markIfNotSeen(holder, encodedMethod.method);
       if (wasSeen) {
         // We can't do anything further because even renaming is not allowed due to the keep rule.
-        if (rootSet.noObfuscation.contains(encodedMethod)) {
+        if (rootSet.noObfuscation.contains(encodedMethod.method)) {
           return false;
         }
         // TODO(b/111118390): Renaming will enable more private instance methods to be publicized.
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/NoRelaxationForSerializableTest.java b/src/test/java/com/android/tools/r8/accessrelaxation/NoRelaxationForSerializableTest.java
new file mode 100644
index 0000000..8fc4c6f
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/NoRelaxationForSerializableTest.java
@@ -0,0 +1,181 @@
+// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.accessrelaxation;
+
+import static java.lang.System.exit;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.junit.Assume.assumeTrue;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NeverMerge;
+import com.android.tools.r8.R8TestCompileResult;
+import com.android.tools.r8.naming.MemberNaming.MethodSignature;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.FileUtils;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.google.common.collect.ImmutableList;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.io.Serializable;
+import java.nio.file.Path;
+import java.util.List;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@NeverMerge
+class MySerializable implements Serializable {
+  transient int value;
+
+  MySerializable(int value) {
+    this.value = value;
+  }
+
+  @NeverInline
+  private void writeObject(ObjectOutputStream out) throws IOException {
+    System.out.println("Serializable::write");
+    out.writeInt(value);
+  }
+
+  @NeverInline
+  private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
+    System.out.println("Serializable::read");
+    value = in.readInt();
+  }
+}
+
+class NoRelaxationForSerializableTestRunner {
+
+  public static void main(String[] args) {
+    MySerializable instance = new MySerializable(8);
+    byte[] bytes = {};
+    try(ByteArrayOutputStream bas = new ByteArrayOutputStream();
+        ObjectOutputStream oos = new ObjectOutputStream(bas)) {
+      oos.writeObject(instance);
+      oos.flush();
+      bytes = bas.toByteArray();
+    } catch(IOException e) {
+      e.printStackTrace(System.err);
+      exit(1);
+    }
+    try(ByteArrayInputStream bis = new ByteArrayInputStream(bytes);
+        ObjectInputStream ois = new ObjectInputStream(bis)) {
+      MySerializable obj = (MySerializable) ois.readObject();
+      if (obj.value != 8) {
+        throw new AssertionError("Could not deserialize");
+      }
+      System.out.println(obj.value);
+    } catch(IOException | ClassNotFoundException e) {
+      e.printStackTrace(System.err);
+      exit(1);
+    }
+  }
+}
+
+@RunWith(Parameterized.class)
+public class NoRelaxationForSerializableTest extends AccessRelaxationTestBase {
+  private static final Class<?> MAIN = NoRelaxationForSerializableTestRunner.class;
+  private static final List<Class<?>> CLASSES = ImmutableList.of(
+      NeverMerge.class, NeverInline.class, MySerializable.class, MAIN);
+  private static final String KEEPMEMBER_RULES = StringUtils.lines(
+      "-keepclassmembers class * implements java.io.Serializable {",
+      "  private void writeObject(java.io.ObjectOutputStream);",
+      "  private void readObject(java.io.ObjectInputStream);",
+      "}"
+  );
+  private static final String EXPECTED_OUTPUT = StringUtils.lines(
+      "Serializable::write",
+      "Serializable::read",
+      "8"
+  );
+
+  private final boolean accessModification;
+  private Path configuration;
+
+  @Parameterized.Parameters(name = "Backend: {0}, access-modification: {1}")
+  public static List<Object[]> data() {
+    return buildParameters(Backend.values(), BooleanUtils.values());
+  }
+
+  public NoRelaxationForSerializableTest(Backend backend, boolean accessModification) {
+    super(backend);
+    this.accessModification = accessModification;
+  }
+
+  @Before
+  public void setUpConfiguration() throws Exception {
+    configuration = temp.newFile("pg.conf").toPath().toAbsolutePath();
+    FileUtils.writeTextFile(configuration, StringUtils.lines(
+        keepMainProguardConfiguration(MAIN),
+        accessModification ? "-allowaccessmodification" : ""
+    ));
+  }
+
+  @Test
+  public void testProguard_withKeepRules() throws Exception {
+    assumeTrue(backend == Backend.CF);
+    testForProguard()
+        .addProgramClasses(CLASSES)
+        .addKeepRuleFiles(configuration)
+        .addKeepRules(KEEPMEMBER_RULES)
+        .compile()
+        .run(MAIN)
+        .assertSuccessWithOutput(EXPECTED_OUTPUT)
+        .inspect(this::inspect);
+  }
+
+  @Test
+  public void testR8_withKeepRules() throws Exception {
+    R8TestCompileResult result = testForR8(backend)
+        .addProgramClasses(CLASSES)
+        .enableClassInliningAnnotations()
+        .enableInliningAnnotations()
+        .addKeepRuleFiles(configuration)
+        .addKeepRules(KEEPMEMBER_RULES)
+        .compile()
+        .inspect(this::inspect);
+    // TODO(b/117302947): Need to update ART binary.
+    if (backend == Backend.CF) {
+      result.run(MAIN).assertSuccessWithOutput(EXPECTED_OUTPUT);
+    }
+  }
+
+  @Test
+  public void testProguard_withoutKeepRules() throws Exception {
+    assumeTrue(backend == Backend.CF);
+    testForProguard()
+        .addProgramClasses(CLASSES)
+        .addKeepRuleFiles(configuration)
+        .compile()
+        .run(MAIN)
+        .assertFailureWithErrorThatMatches(containsString("Could not deserialize"));
+  }
+
+  @Test
+  public void testR8_withoutKeepRules() throws Exception {
+    R8TestCompileResult result = testForR8(backend)
+        .addProgramClasses(CLASSES)
+        .enableClassInliningAnnotations()
+        .enableInliningAnnotations()
+        .addKeepRuleFiles(configuration)
+        .compile();
+    // TODO(b/117302947): Need to update ART binary.
+    if (backend == Backend.CF) {
+      result.run(MAIN).assertFailureWithErrorThatMatches(containsString("Could not deserialize"));
+    }
+  }
+
+  private void inspect(CodeInspector inspector) {
+    assertNotPublic(inspector, MySerializable.class,
+        new MethodSignature("writeObject", "void", ImmutableList.of("java.io.ObjectOutputStream")));
+    assertNotPublic(inspector, MySerializable.class,
+        new MethodSignature("readObject", "void", ImmutableList.of("java.io.ObjectInputStream")));
+  }
+
+}
diff --git a/src/test/java/com/android/tools/r8/naming/NamingTestBase.java b/src/test/java/com/android/tools/r8/naming/NamingTestBase.java
index 4f4639a..7b6052a 100644
--- a/src/test/java/com/android/tools/r8/naming/NamingTestBase.java
+++ b/src/test/java/com/android/tools/r8/naming/NamingTestBase.java
@@ -9,7 +9,6 @@
 import com.android.tools.r8.graph.DexApplication;
 import com.android.tools.r8.graph.DexItemFactory;
 import com.android.tools.r8.graph.GraphLense;
-import com.android.tools.r8.optimize.ClassAndMemberPublicizer;
 import com.android.tools.r8.shaking.Enqueuer;
 import com.android.tools.r8.shaking.ProguardConfiguration;
 import com.android.tools.r8.shaking.RootSetBuilder;
@@ -74,12 +73,6 @@
     RootSet rootSet =
         new RootSetBuilder(appView, program, configuration.getRules(), options).run(executor);
 
-    if (options.getProguardConfiguration().isAccessModificationAllowed()) {
-      ClassAndMemberPublicizer.run(executor, timing, program, appView, rootSet);
-      rootSet =
-          new RootSetBuilder(appView, program, configuration.getRules(), options).run(executor);
-    }
-
     Enqueuer enqueuer = new Enqueuer(appView, options, null, options.forceProguardCompatibility);
     AppInfoWithSubtyping appInfo =
         enqueuer.traceApplication(rootSet, configuration.getDontWarnPatterns(), executor, timing);