Revert "[WebLayer] Update Fragment.getActivity() implementation."

This reverts commit d3f64670.

Reason for revert: Broke several chrome_junit_tests
See https://ci.chromium.org/p/chromium/builders/ci/Android%20arm%20Builder%20%28dbg%29/53156
Sample broken chrome_junit_tests: ConfirmSyncDataStateMachineDelegateTest

Original change's description:
> [WebLayer] Update Fragment.getActivity() implementation.
>
> This CL updates the bytecode modifications we perform on
> Fragment.getActivity() to have it call ContextUtils.activityFromContext(
> getContext()), and modifies compile_java.py to detect compilation errors
> related to this change and print a message pointing you to documentation
> about the bytecode modifications.
>
> A compilation error will now print:
> ../...../PasswordEntryEditor.java:144: error: cannot find symbol
>                 getActivity().getSupportFragmentManager(),
>                              ^
>   symbol:   method getSupportFragmentManager()
>   location: class Activity
>             Expecting a FragmentActivity? See docs/.......md
>
> Bug: 1123216
> Change-Id: I04016e30fba46c7d5c592e9ab2c5e5fa4c4cfde1
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2508352
> Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
> Reviewed-by: Andrew Grieve <agrieve@chromium.org>
> Reviewed-by: Natalie Chouinard <chouinard@chromium.org>
> Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#828643}

TBR=twellington@chromium.org,agrieve@chromium.org,bsazonov@chromium.org,rmcelrath@chromium.org,chouinard@chromium.org

Change-Id: Iee2c469e87e9067bdf7ea2c1744398c473328711
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1123216
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2547121Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828748}

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

@@ -20,11 +20,15 @@ import java.io.IOException;
  * See crbug.com/1144345 for more context.
  */
 public class FragmentActivityReplacer extends ByteCodeRewriter {
+    private static final String FRAGMENT_CLASS_PATH = "androidx/fragment/app/Fragment.class";
+    private static final String FRAGMENT_ACTIVITY_INTERNAL_CLASS_NAME =
+            "androidx/fragment/app/FragmentActivity";
+    private static final String ACTIVITY_INTERNAL_CLASS_NAME = "android/app/Activity";
     private static final String GET_ACTIVITY_METHOD_NAME = "getActivity";
-    private static final String NEW_METHOD_DESCRIPTOR = "()Landroid/app/Activity;";
+    private static final String REQUIRE_ACTIVITY_METHOD_NAME = "requireActivity";
     private static final String OLD_METHOD_DESCRIPTOR =
             "()Landroidx/fragment/app/FragmentActivity;";
-    private static final String REQUIRE_ACTIVITY_METHOD_NAME = "requireActivity";
+    private static final String NEW_METHOD_DESCRIPTOR = "()Landroid/app/Activity;";
 
     public static void main(String[] args) throws IOException {
         // Invoke this script using //build/android/gyp/bytecode_processor.py
@@ -45,7 +49,7 @@ 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")) {
+        if (classPath.equals(FRAGMENT_CLASS_PATH)) {
             return new FragmentClassVisitor(getActivityReplacer);
         }
         return getActivityReplacer;
@@ -80,7 +84,8 @@ public class FragmentActivityReplacer extends ByteCodeRewriter {
     }
 
     /**
-     * Updates the implementation of Fragment.getActivity() and Fragment.requireActivity().
+     * Makes Fragment.getActivity() and Fragment.requireActivity() non-final, and changes their
+     * return types to Activity.
      */
     private static class FragmentClassVisitor extends ClassVisitor {
         private FragmentClassVisitor(ClassVisitor baseVisitor) {
@@ -90,36 +95,25 @@ public class FragmentActivityReplacer extends ByteCodeRewriter {
         @Override
         public MethodVisitor visitMethod(
                 int access, String name, String descriptor, String signature, String[] exceptions) {
-            // Replace FragmentActivity with Fragment in requireActivity().
-            if (name.equals(REQUIRE_ACTIVITY_METHOD_NAME)) {
-                return new MethodRemapper(super.visitMethod(access & ~Opcodes.ACC_FINAL, name,
-                                                  NEW_METHOD_DESCRIPTOR, null, exceptions),
-                        new Remapper() {
-                            @Override
-                            public String mapType(String internalName) {
-                                if (internalName.equals("androidx/fragment/app/FragmentActivity")) {
-                                    return "android/app/Activity";
-                                }
-                                return internalName;
-                            }
-                        });
-            }
-
-            // Replace getActivity() with ContextUtils.activityFromContext(getContext()).
-            if (name.equals(GET_ACTIVITY_METHOD_NAME)) {
-                MethodVisitor visitor = super.visitMethod(
+            MethodVisitor base;
+            // Update the descriptor of getActivity/requireActivity, and make them non-final.
+            if (name.equals(GET_ACTIVITY_METHOD_NAME)
+                    || name.equals(REQUIRE_ACTIVITY_METHOD_NAME)) {
+                base = super.visitMethod(
                         access & ~Opcodes.ACC_FINAL, name, NEW_METHOD_DESCRIPTOR, null, exceptions);
-                visitor.visitVarInsn(Opcodes.ALOAD, 0);
-                visitor.visitMethodInsn(Opcodes.INVOKEVIRTUAL, "androidx/fragment/app/Fragment",
-                        "getContext", "()Landroid/content/Context;", false);
-                visitor.visitMethodInsn(Opcodes.INVOKESTATIC, "org/chromium/utils/ContextUtils",
-                        "activityFromContext", "(Landroid/content/Context;)Landroid/app/Activity;",
-                        false);
-                visitor.visitInsn(Opcodes.ARETURN);
-                return null;
+            } else {
+                base = super.visitMethod(access, name, descriptor, signature, exceptions);
             }
 
-            return super.visitMethod(access, name, descriptor, signature, exceptions);
+            return new MethodRemapper(base, new Remapper() {
+                @Override
+                public String mapType(String internalName) {
+                    if (internalName.equals(FRAGMENT_ACTIVITY_INTERNAL_CLASS_NAME)) {
+                        return ACTIVITY_INTERNAL_CLASS_NAME;
+                    }
+                    return internalName;
+                }
+            });
         }
     }
 }

build/android/gyp/compile_java.py

@@ -207,8 +207,6 @@ def ProcessJavacOutput(output):
       r'(Note: .* uses? unchecked or unsafe operations.)$')
   recompile_re = re.compile(r'(Note: Recompile with -Xlint:.* for details.)$')
 
-  activity_re = re.compile(r'^(?P<prefix>\s*location: )class Activity$')
-
   warning_color = ['full_message', colorama.Fore.YELLOW + colorama.Style.DIM]
   error_color = ['full_message', colorama.Fore.MAGENTA + colorama.Style.BRIGHT]
   marker_color = ['marker', colorama.Fore.BLUE + colorama.Style.BRIGHT]
@@ -224,13 +222,6 @@ def ProcessJavacOutput(output):
     return not (deprecated_re.match(line) or unchecked_re.match(line)
                 or recompile_re.match(line))
 
-  def Elaborate(line):
-    if activity_re.match(line):
-      prefix = ' ' * activity_re.match(line).end('prefix')
-      return '{}\n{}Expecting a FragmentActivity? See {}'.format(
-          line, prefix, 'docs/ui/android/bytecode_rewriting.md')
-    return line
-
   def ApplyColors(line):
     if warning_re.match(line):
       line = Colorize(line, warning_re, warning_color)
@@ -240,9 +231,7 @@ def ProcessJavacOutput(output):
       line = Colorize(line, marker_re, marker_color)
     return line
 
-  lines = (l for l in output.split('\n') if ApplyFilters(l))
-  lines = (ApplyColors(Elaborate(l)) for l in lines)
-  return '\n'.join(lines)
+  return '\n'.join(map(ApplyColors, filter(ApplyFilters, output.split('\n'))))
 
 
 def CheckErrorproneStderrWarning(jar_path, expected_warning_regex,

docs/ui/android/bytecode_rewriting.md

-# Bytecode Rewriting
-
-## TL;DR
-
-We modify the return type of AndroidX's `Fragment.getActivity()` method from `FragmentActivity`
-to `Activity` to more easily add Fragments from multiple ClassLoaders into the same Fragment tree.
-
-## Why?
-
-In Java, two instances of the same class loaded from two different ClassLoaders aren't compatible
-with each other at runtime. Because AndroidX libraries are bundled with each APK that use them, and
-different APKs are loaded with different ClassLoaders, AndroidX classes from one APK cannot be used
-with the same class from another APK. This causes problems for Fragment-based UIs in WebLayer, where
-the implementation is in a different ClassLoader than the embedding app, so its Fragments cannot be
-added to the embedding app's Fragment tree.
-
-Note that this issue doesn't apply to Framework or standard library classes. Java ClassLoaders form
-a tree, and if a ClassLoader can't find a particular class, it delegates to its parent.
-The leaf ClassLoader used to load an app is responsible for loading the app's class files, while one
-of its parents will load system-level classes. Because AndroidX classes get loaded by the
-app-specific ClassLoader, different apps will load mutually incompatible versions, but a class like
-Activity, which gets loaded from a parent ClassLoader, *will* be compatible between APKs at runtime,
-because it ends up gets loaded from a common ClassLoader.
-
-To get around this incompatibility, we can create a RemoteFragment that lives in the embedding app,
-and a RemoteFragmentImpl that lives in another APK. The RemoteFragment can be added to the original
-Fragment tree, and will forward all Fragment lifecycle events over an AIDL interface to
-RemoteFragmentImpl. The fake Fragment in the secondary APK (RemoteFragmentImpl) can create a
-FragmentController, which allows it to become the host of its own Fragment tree, and any UIs from
-the secondary ClassLoaded can be added to this new Fragment tree that's been essentially grafted
-onto the original.
-
-This mostly works, but runs into issues when Fragments call `Fragment.getActivity()`, which they do
-a lot. The getActivity implementation takes the Activity given to the FragmentController constructor
-(via FragmentHostCallback), and casts it to a FragmentActivity before returning it. The original
-Activity will typically be a FragmentActivity from the embedding app's ClassLoader, which means that
-due to the aforementioned issues, this cast will fail when run in the secondary ClassLoader's
-Fragment class because even though the Activity is a FragmentActivity, it's from the wrong
-ClassLoader.
-
-To fix this second issue, we modify the bytecode of `Fragment.getActivity()` in the AndroidX
-prebuilt .aar files to return a plain Activity instead of a FragmentActivity. This allows us to
-continue calling getActivity() as normal. Note that this does mean FragmentActivity-specific methods
-can no longer be used in Fragments, but there were no uses of them in Chromium that couldn't be
-trivially removed as of late 2020.
-
-## How does it work?
-
-The bytecode rewriting happens at build time by
-[FragmentActivityReplacer](https://source.chromium.org/chromium/chromium/src/+/master:build/android/bytecode/java/org/chromium/bytecode/FragmentActivityReplacer.java),
-which is specified as a bytecode rewriter via the `bytecode_rewriter_target` rule.  Compilation errors
-related to this should get detected by
-[compile_java.py](https://source.chromium.org/chromium/chromium/src/+/master:build/android/gyp/compile_java.py),
-and print a message pointing users here, which is likely why you're reading this :)
-
-## How does this affect my code?
-
-The goal is for these changes to be as transparent as possible; most code shouldn't run into issues.
-However, if there's no way around calling a FragmentActivity method in your code, **and your
-Fragment is in //chrome**, you could cast the Activity to a FragmentActivity as AndroidX used to do.
-If your Fragment is in //components, FragmentActivity methods will likely not work directly, and may
-need to be forwarded to an implementation in the original ClassLoader somehow.
-
-The more important thing to note is that in a multi-ClassLoader world, `getActivity()` and
-`getContext()` will typically return two different objects, so we need to be more careful about which
-method we call, particularly for code in //components. `getActivity()` will return the Activity from
-the original ClassLoader, and should be used to for calls like `.finish*()`, `.setTitle(), and
-`.startActivity()` (which live in Activity anyway). When loading resources, you should default to
-calling `getContext()`, as resources usually come from the same ClassLoader as the Fragment, and the
-Context should be configured to load them correctly.
-
-As a rule of thumb, prefer `getContext()` to `getActivity()`, unless you need to operate on the
-Activity itself, or you know the resource or setting you need belongs to the original Activity.

third_party/android_deps/BUILD.gn

@@ -364,10 +364,7 @@ android_aar_prebuilt("androidx_fragment_fragment_java") {
     ":androidx_viewpager_viewpager_java",
   ]
   resource_overlay = true
-  deps += [
-    "//third_party/android_deps/local_modifications/androidx_fragment_fragment:androidx_fragment_fragment_prebuilt_java",
-    "//third_party/android_deps/utils:java",
-  ]
+  deps += [ "//third_party/android_deps/local_modifications/androidx_fragment_fragment:androidx_fragment_fragment_prebuilt_java" ]
 
   # Omit this file since we use our own copy, included above.
   # We can remove this once we migrate to AndroidX master for all libraries.

third_party/android_deps/buildSrc/src/main/groovy/BuildConfigGenerator.groovy

@@ -346,10 +346,7 @@ class BuildConfigGenerator extends DefaultTask {
                 break
             case 'androidx_fragment_fragment':
                 sb.append("""\
-                |  deps += [
-                |    "//third_party/android_deps/utils:java",
-                |    "//third_party/android_deps/local_modifications/androidx_fragment_fragment:androidx_fragment_fragment_prebuilt_java",
-                |  ]
+                |  deps += [ "//third_party/android_deps/local_modifications/androidx_fragment_fragment:androidx_fragment_fragment_prebuilt_java" ]
                 |  # Omit this file since we use our own copy, included above.
                 |  # We can remove this once we migrate to AndroidX master for all libraries.
                 |  jar_excluded_patterns = [

third_party/android_deps/utils/BUILD.gn

-# Copyright 2020 The Chromium Authors. All rights reserved.
-# Use of this source code is governed by a BSD-style license that can be
-# found in the LICENSE file.
-
-import("//build/config/android/rules.gni")
-
-android_library("java") {
-  sources = [ "java/org/chromium/utils/ContextUtils.java" ]
-  deps = [ "//third_party/android_deps:androidx_annotation_annotation_java" ]
-}

third_party/android_deps/utils/java/org/chromium/utils/ContextUtils.java

-// Copyright 2020 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-package org.chromium.utils;
-
-import android.app.Activity;
-import android.content.Context;
-import android.content.ContextWrapper;
-
-import androidx.annotation.Nullable;
-
-/**
- * Contains a helper method needed by the AndroidX Fragment library due to bytecode modification we
- * perform at build time.
- */
-public class ContextUtils {
-    private ContextUtils() {}
-
-    /**
-     * Extract the {@link Activity} if the given {@link Context} either is or wraps one.
-     *
-     * Copied from //base/android/java/src/org/chromium/base/ContextUtils.java
-     *
-     * @param context The context to check.
-     * @return Extracted activity if it exists, otherwise null.
-     */
-    public static @Nullable Activity activityFromContext(@Nullable Context context) {
-        // Only retrieves the base context if the supplied context is a ContextWrapper but not an
-        // Activity, because Activity is a subclass of ContextWrapper.
-        while (context instanceof ContextWrapper) {
-            if (context instanceof Activity) return (Activity) context;
-
-            context = ((ContextWrapper) context).getBaseContext();
-        }
-
-        return null;
-    }
-}