Reland "[WebLayer] Fix FragmentActivityReplacer with SupportLifecycleFragmentImpl"

This is a fixed reland of a3349311

In the original CL, I removed the code that made Fragment.getActivity()
non-final, as our approach no longer relied on overriding it. However,
the PromptDialogDelegate interface in the hats20 library, which other
Fragment subclasses in that library implement, defines an
`Activity getActivity()` method. Fragment.getActivity() is considered
the implementation of this method from a typechecking perspective, but
javac still generates a getActivity implementation that calls the parent
class's. This didn't cause issues before because the two methods
returned different types, but now that Fragment.getActivity() returns an
Activity, the bytecode verifier thinks the javac-generated getActivity
from PromptDialogDelegate is extending Fragment's implementation, which
it can't do because it's final. Making Fragment.getActivity() non-final
again fixes the issue.

Original change's description:
> [WebLayer] Fix FragmentActivityReplacer with SupportLifecycleFragmentImpl
>
> This fixes a comilation error due to FragmentActivityReplacer's
> interaction with SupportLifecycleFragmentImpl.
>
> Bug: 1152898, 1123216
> Change-Id: I81de8100f7ddd08310f8a36c8f1c9c62e82c0469
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2566455
> Reviewed-by: Andrew Grieve <agrieve@chromium.org>
> Reviewed-by: Sam Maier <smaier@chromium.org>
> Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#832426}

Bug: 1152898
Bug: 1123216
Change-Id: Ibf923570bbb27d5ffb0a337775aac449b59d5986
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2569950
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833457}

build/android/bytecode/java/org/chromium/bytecode/FragmentActivityReplacer.java

@@ -21,6 +21,7 @@ import java.io.IOException;
  */
 public class FragmentActivityReplacer extends ByteCodeRewriter {
     private static final String GET_ACTIVITY_METHOD_NAME = "getActivity";
+    private static final String GET_LIFECYCLE_ACTIVITY_METHOD_NAME = "getLifecycleActivity";
     private static final String NEW_METHOD_DESCRIPTOR = "()Landroid/app/Activity;";
     private static final String OLD_METHOD_DESCRIPTOR =
             "()Landroidx/fragment/app/FragmentActivity;";
@@ -44,16 +45,23 @@ public class FragmentActivityReplacer extends ByteCodeRewriter {
 
     @Override
     protected ClassVisitor getClassVisitorForClass(String classPath, ClassVisitor delegate) {
-        ClassVisitor getActivityReplacer = new GetActivityReplacer(delegate);
-        if (classPath.equals("androidx/fragment/app/Fragment.class")) {
-            return new FragmentClassVisitor(getActivityReplacer);
+        ClassVisitor invocationVisitor = new InvocationReplacer(delegate);
+        switch (classPath) {
+            case "androidx/fragment/app/Fragment.class":
+                return new FragmentClassVisitor(invocationVisitor);
+            case "com/google/android/gms/common/api/internal/SupportLifecycleFragmentImpl.class":
+                return new SupportLifecycleFragmentImplClassVisitor(invocationVisitor);
+            default:
+                return invocationVisitor;
         }
-        return getActivityReplacer;
     }
 
-    /** Updates any Fragment.getActivity/requireActivity() calls to call the replaced method. */
-    private static class GetActivityReplacer extends ClassVisitor {
-        private GetActivityReplacer(ClassVisitor baseVisitor) {
+    /**
+     * Updates any Fragment.getActivity/requireActivity() or getLifecycleActivity() calls to call
+     * the replaced method.
+     */
+    private static class InvocationReplacer extends ClassVisitor {
+        private InvocationReplacer(ClassVisitor baseVisitor) {
             super(Opcodes.ASM7, baseVisitor);
         }
 
@@ -68,7 +76,8 @@ public class FragmentActivityReplacer extends ByteCodeRewriter {
                     if ((opcode == Opcodes.INVOKEVIRTUAL || opcode == Opcodes.INVOKESPECIAL)
                             && descriptor.equals(OLD_METHOD_DESCRIPTOR)
                             && (name.equals(GET_ACTIVITY_METHOD_NAME)
-                                    || name.equals(REQUIRE_ACTIVITY_METHOD_NAME))) {
+                                    || name.equals(REQUIRE_ACTIVITY_METHOD_NAME)
+                                    || name.equals(GET_LIFECYCLE_ACTIVITY_METHOD_NAME))) {
                         super.visitMethodInsn(
                                 opcode, owner, name, NEW_METHOD_DESCRIPTOR, isInterface);
                     } else {
@@ -92,8 +101,17 @@ public class FragmentActivityReplacer extends ByteCodeRewriter {
                 int access, String name, String descriptor, String signature, String[] exceptions) {
             // Update the descriptor of getActivity() and requireActivity().
             MethodVisitor baseVisitor;
-            if (name.equals(GET_ACTIVITY_METHOD_NAME)
-                    || name.equals(REQUIRE_ACTIVITY_METHOD_NAME)) {
+            if (descriptor.equals(OLD_METHOD_DESCRIPTOR)
+                    && (name.equals(GET_ACTIVITY_METHOD_NAME)
+                            || name.equals(REQUIRE_ACTIVITY_METHOD_NAME))) {
+                // Some Fragments in a Clank library implement an interface that defines an
+                // `Activity getActivity()` method. Fragment.getActivity() is considered its
+                // implementation from a typechecking perspective, but javac still generates a
+                // getActivity() method in these Fragments that call Fragment.getActivity(). This
+                // isn't an issue when the methods return different types, but after changing
+                // Fragment.getActivity() to return an Activity, this generated implementation is
+                // now overriding Fragment's, which it can't do because Fragment.getActivity() is
+                // final. We make it non-final here to avoid this issue.
                 baseVisitor = super.visitMethod(
                         access & ~Opcodes.ACC_FINAL, name, NEW_METHOD_DESCRIPTOR, null, exceptions);
             } else {
@@ -101,7 +119,7 @@ public class FragmentActivityReplacer extends ByteCodeRewriter {
             }
 
             // Replace getActivity() with `return ContextUtils.activityFromContext(getContext());`
-            if (name.equals(GET_ACTIVITY_METHOD_NAME)) {
+            if (name.equals(GET_ACTIVITY_METHOD_NAME) && descriptor.equals(OLD_METHOD_DESCRIPTOR)) {
                 baseVisitor.visitVarInsn(Opcodes.ALOAD, 0);
                 baseVisitor.visitMethodInsn(Opcodes.INVOKEVIRTUAL, "androidx/fragment/app/Fragment",
                         "getContext", "()Landroid/content/Context;", false);
@@ -123,4 +141,34 @@ public class FragmentActivityReplacer extends ByteCodeRewriter {
             });
         }
     }
+
+    /**
+     * Update SupportLifecycleFragmentImpl.getLifecycleActivity().
+     */
+    private static class SupportLifecycleFragmentImplClassVisitor extends ClassVisitor {
+        private SupportLifecycleFragmentImplClassVisitor(ClassVisitor baseVisitor) {
+            super(Opcodes.ASM7, baseVisitor);
+        }
+
+        @Override
+        public MethodVisitor visitMethod(
+                int access, String name, String descriptor, String signature, String[] exceptions) {
+            // SupportLifecycleFragmentImpl has two getActivity methods:
+            //   1. public FragmentActivity getLifecycleActivity():
+            //      This is what you'll see in the source. This delegates to Fragment.getActivity().
+            //   2. public Activity getLifecycleActivity():
+            //      This is generated because the class implements LifecycleFragment, which
+            //      declares this method, and delegates to #1.
+            //
+            // Here we change the return type of #1 and delete #2.
+            if (name.equals(GET_LIFECYCLE_ACTIVITY_METHOD_NAME)) {
+                if (descriptor.equals(OLD_METHOD_DESCRIPTOR)) {
+                    return super.visitMethod(
+                            access, name, NEW_METHOD_DESCRIPTOR, signature, exceptions);
+                }
+                return null;
+            }
+            return super.visitMethod(access, name, descriptor, signature, exceptions);
+        }
+    }
 }