Skip to content

Commit

Permalink
Reland "[WebLayer] Change the return type of Fragment.getActivity() t…
Browse files Browse the repository at this point in the history
…o Activity."

This is a reland of ae52b2a with fixes
for robolectric tests, which run on the JVM which appears to have
stricter verification checks.

Original change's description:
> [WebLayer] Change the return type of Fragment.getActivity() to Activity.
>
> This CL adds the ability to specify a script in an android_aar_prebuilt
> rule that can modify prebuilt jar files, and adds a script to change
> Fragment.getActivity() and Fragment.requireActivity() to return an
> Activity instead of a FragmentActivity.
>
> This is the first CL in a chain that will allow us to remove the fake
> activity we create when embedding Fragments that cross classloader
> boundaries.
>
> Bug: 1123216
> Change-Id: I4b9d3ca5f9c3a4d86e08d64f49d601c08fca9a70
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2432413
> Reviewed-by: Theresa  <twellington@chromium.org>
> Reviewed-by: Andrew Grieve <agrieve@chromium.org>
> Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#823582}

Bug: 1123216
Change-Id: I83ed0c47cda12e4a71ed90cc0bce4cde2a07782d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2521343
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826944}
  • Loading branch information
robbiemc authored and Commit Bot committed Nov 12, 2020
1 parent 87d504a commit 360e54d
Show file tree
Hide file tree
Showing 15 changed files with 340 additions and 28 deletions.
1 change: 1 addition & 0 deletions PRESUBMIT.py
Expand Up @@ -1306,6 +1306,7 @@
'build/android/gyp/apkbuilder.pydeps',
'build/android/gyp/assert_static_initializers.pydeps',
'build/android/gyp/bytecode_processor.pydeps',
'build/android/gyp/bytecode_rewriter.pydeps',
'build/android/gyp/compile_java.pydeps',
'build/android/gyp/compile_resources.pydeps',
'build/android/gyp/copy_ex.pydeps',
Expand Down
14 changes: 14 additions & 0 deletions build/android/bytecode/BUILD.gn
Expand Up @@ -18,3 +18,17 @@ java_binary("bytecode_processor") {
wrapper_script_name = "helper/bytecode_processor"
enable_bytecode_checks = false
}

java_binary("fragment_activity_replacer") {
sources = [
"java/org/chromium/bytecode/ByteCodeRewriter.java",
"java/org/chromium/bytecode/FragmentActivityReplacer.java",
]
main_class = "org.chromium.bytecode.FragmentActivityReplacer"
deps = [
"//third_party/android_deps:org_ow2_asm_asm_commons_java",
"//third_party/android_deps:org_ow2_asm_asm_java",
"//third_party/android_deps:org_ow2_asm_asm_util_java",
]
wrapper_script_name = "helper/fragment_activity_replacer"
}
@@ -0,0 +1,91 @@
// 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.bytecode;

import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassVisitor;
import org.objectweb.asm.ClassWriter;

import java.io.BufferedInputStream;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.util.zip.ZipEntry;
import java.util.zip.ZipInputStream;
import java.util.zip.ZipOutputStream;

/**
* Base class for scripts that perform bytecode modifications on a jar file.
*/
public abstract class ByteCodeRewriter {
private static final String CLASS_FILE_SUFFIX = ".class";

public void rewrite(File inputJar, File outputJar) throws IOException {
if (!inputJar.exists()) {
throw new FileNotFoundException("Input jar not found: " + inputJar.getPath());
}
try (InputStream inputStream = new BufferedInputStream(new FileInputStream(inputJar))) {
try (OutputStream outputStream = new FileOutputStream(outputJar)) {
processZip(inputStream, outputStream);
}
}
}

/** Returns true if the class at the given path in the archive should be rewritten. */
protected abstract boolean shouldRewriteClass(String classPath);

/**
* Returns the ClassVisitor that should be used to modify the bytecode of class at the given
* path in the archive.
*/
protected abstract ClassVisitor getClassVisitorForClass(
String classPath, ClassVisitor delegate);

private void processZip(InputStream inputStream, OutputStream outputStream) {
try (ZipOutputStream zipOutputStream = new ZipOutputStream(outputStream)) {
ZipInputStream zipInputStream = new ZipInputStream(inputStream);
ZipEntry entry;
while ((entry = zipInputStream.getNextEntry()) != null) {
ByteArrayOutputStream buffer = new ByteArrayOutputStream();
boolean handled = processClassEntry(entry, zipInputStream, buffer);
if (handled) {
ZipEntry newEntry = new ZipEntry(entry.getName());
zipOutputStream.putNextEntry(newEntry);
zipOutputStream.write(buffer.toByteArray(), 0, buffer.size());
} else {
zipOutputStream.putNextEntry(entry);
zipInputStream.transferTo(zipOutputStream);
}
}
} catch (IOException e) {
throw new RuntimeException(e);
}
}

private boolean processClassEntry(
ZipEntry entry, InputStream inputStream, OutputStream outputStream) {
if (!entry.getName().endsWith(CLASS_FILE_SUFFIX) || !shouldRewriteClass(entry.getName())) {
return false;
}
try {
ClassReader reader = new ClassReader(inputStream);
ClassWriter writer = new ClassWriter(reader, ClassWriter.COMPUTE_FRAMES);
ClassVisitor classVisitor = getClassVisitorForClass(entry.getName(), writer);
reader.accept(classVisitor, ClassReader.EXPAND_FRAMES);

writer.visitEnd();
byte[] classData = writer.toByteArray();
outputStream.write(classData, 0, classData.length);
return true;
} catch (IOException e) {
throw new RuntimeException(e);
}
}
}
@@ -0,0 +1,119 @@
// 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.bytecode;

import org.objectweb.asm.ClassVisitor;
import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.commons.MethodRemapper;
import org.objectweb.asm.commons.Remapper;

import java.io.File;
import java.io.IOException;

/**
* Java application that modifies Fragment.getActivity() to return an Activity instead of a
* FragmentActivity, and updates any existing getActivity() calls to reference the updated method.
*
* 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 REQUIRE_ACTIVITY_METHOD_NAME = "requireActivity";
private static final String OLD_METHOD_DESCRIPTOR =
"()Landroidx/fragment/app/FragmentActivity;";
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
if (args.length != 2) {
System.err.println("Expected 2 arguments: [input.jar] [output.jar]");
System.exit(1);
}

FragmentActivityReplacer rewriter = new FragmentActivityReplacer();
rewriter.rewrite(new File(args[0]), new File(args[1]));
}

@Override
protected boolean shouldRewriteClass(String classPath) {
return true;
}

@Override
protected ClassVisitor getClassVisitorForClass(String classPath, ClassVisitor delegate) {
ClassVisitor getActivityReplacer = new GetActivityReplacer(delegate);
if (classPath.equals(FRAGMENT_CLASS_PATH)) {
return new FragmentClassVisitor(getActivityReplacer);
}
return getActivityReplacer;
}

/** Updates any Fragment.getActivity/requireActivity() calls to call the replaced method. */
private static class GetActivityReplacer extends ClassVisitor {
private GetActivityReplacer(ClassVisitor baseVisitor) {
super(Opcodes.ASM7, baseVisitor);
}

@Override
public MethodVisitor visitMethod(
int access, String name, String descriptor, String signature, String[] exceptions) {
MethodVisitor base = super.visitMethod(access, name, descriptor, signature, exceptions);
return new MethodVisitor(Opcodes.ASM7, base) {
@Override
public void visitMethodInsn(int opcode, String owner, String name,
String descriptor, boolean isInterface) {
if ((opcode == Opcodes.INVOKEVIRTUAL || opcode == Opcodes.INVOKESPECIAL)
&& descriptor.equals(OLD_METHOD_DESCRIPTOR)
&& (name.equals(GET_ACTIVITY_METHOD_NAME)
|| name.equals(REQUIRE_ACTIVITY_METHOD_NAME))) {
super.visitMethodInsn(
opcode, owner, name, NEW_METHOD_DESCRIPTOR, isInterface);
} else {
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
}
}
};
}
}

/**
* 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) {
super(Opcodes.ASM7, baseVisitor);
}

@Override
public MethodVisitor visitMethod(
int access, String name, String descriptor, String signature, String[] exceptions) {
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);
} else {
base = 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;
}
});
}
}
}
37 changes: 37 additions & 0 deletions build/android/gyp/bytecode_rewriter.py
@@ -0,0 +1,37 @@
#!/usr/bin/env python
# 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.
"""Wrapper script around ByteCodeRewriter subclass scripts."""

import argparse
import sys

from util import build_utils


def main(argv):
argv = build_utils.ExpandFileArgs(argv[1:])
parser = argparse.ArgumentParser()
build_utils.AddDepfileOption(parser)
parser.add_argument('--script',
required=True,
help='Path to the java binary wrapper script.')
parser.add_argument('--classpath', action='append', nargs='+')
parser.add_argument('--input-jar', required=True)
parser.add_argument('--output-jar', required=True)
args = parser.parse_args(argv)

classpath = build_utils.ParseGnList(args.classpath)
build_utils.WriteDepfile(args.depfile, args.output_jar, inputs=classpath)

classpath.append(args.input_jar)
cmd = [
args.script, '--classpath', ':'.join(classpath), args.input_jar,
args.output_jar
]
build_utils.CheckOutput(cmd, print_stdout=True)


if __name__ == '__main__':
sys.exit(main(sys.argv))
6 changes: 6 additions & 0 deletions build/android/gyp/bytecode_rewriter.pydeps
@@ -0,0 +1,6 @@
# Generated by running:
# build/print_python_deps.py --root build/android/gyp --output build/android/gyp/bytecode_rewriter.pydeps build/android/gyp/bytecode_rewriter.py
../../gn_helpers.py
bytecode_rewriter.py
util/__init__.py
util/build_utils.py
67 changes: 53 additions & 14 deletions build/config/android/internal_rules.gni
Expand Up @@ -3982,8 +3982,56 @@ if (enable_java_templates) {
}
} # _has_sources

# TODO(crbug.com/1123216): Implement bytecode_rewriter_target.
not_needed(invoker, [ "bytecode_rewriter_target" ])
if (_is_prebuilt || _build_device_jar || _build_host_jar) {
_unprocessed_jar_deps = _full_classpath_deps
if (_has_sources) {
_unprocessed_jar_deps += [ ":$_compile_java_target" ]
}
}

if (defined(invoker.bytecode_rewriter_target)) {
assert(_build_host_jar || _build_device_jar,
"A host or device jar must be created to use bytecode rewriting")

_rewritten_jar =
string_replace(_unprocessed_jar_path, ".jar", "_rewritten.jar")
_rewritten_jar_target_name = "${target_name}__rewritten"
_rewriter_path = root_build_dir + "/bin/helper/" +
get_label_info(invoker.bytecode_rewriter_target, "name")
_rebased_build_config = rebase_path(_build_config, root_build_dir)
action_with_pydeps(_rewritten_jar_target_name) {
script = "//build/android/gyp/bytecode_rewriter.py"
inputs = [
_rewriter_path,
_build_config,
_unprocessed_jar_path,
]
outputs = [ _rewritten_jar ]
depfile = "$target_gen_dir/$target_name.d"
args = [
"--depfile",
rebase_path(depfile, root_build_dir),
"--script",
rebase_path(_rewriter_path, root_build_dir),
"--classpath",
"@FileArg($_rebased_build_config:deps_info:javac_full_classpath)",
"--classpath",
"@FileArg($_rebased_build_config:android:sdk_jars)",
"--input-jar",
rebase_path(_unprocessed_jar_path, root_build_dir),
"--output-jar",
rebase_path(_rewritten_jar, root_build_dir),
]
deps = _unprocessed_jar_deps + [
":$_build_config_target_name",
invoker.bytecode_rewriter_target,
]
}

_unprocessed_jar_deps = []
_unprocessed_jar_deps = [ ":$_rewritten_jar_target_name" ]
_unprocessed_jar_path = _rewritten_jar
}

if (_is_prebuilt) {
generate_interface_jar(_header_target_name) {
Expand All @@ -3998,10 +4046,7 @@ if (enable_java_templates) {
# target. If we can change compile & desugar steps to use direct
# interface classpath rather than full interface classpath, then this
# could just be _non_java_deps.
deps = _classpath_deps
if (_has_sources) {
deps += [ ":$_compile_java_target" ]
}
deps = _unprocessed_jar_deps
}
_public_deps += [ ":$_header_target_name" ]
}
Expand All @@ -4017,10 +4062,7 @@ if (enable_java_templates) {
build_config = _build_config
build_config_dep = ":$_build_config_target_name"
input_jar_path = _unprocessed_jar_path
jar_deps = _non_java_deps
if (_has_sources) {
jar_deps += [ ":$_compile_java_target" ]
}
jar_deps = _unprocessed_jar_deps
if (_build_host_jar) {
host_jar_path = _host_processed_jar_path
}
Expand Down Expand Up @@ -4063,10 +4105,7 @@ if (enable_java_templates) {
_bytecode_checks_target = "${target_name}__validate_classpath"
bytecode_processor(_bytecode_checks_target) {
forward_variables_from(invoker, [ "missing_classes_allowlist" ])
deps = _full_classpath_deps
if (_has_sources) {
deps += [ ":$_compile_java_target" ]
}
deps = _unprocessed_jar_deps + [ ":$_build_config_target_name" ]
requires_android = _requires_android
target_label =
get_label_info(":${invoker.target_name}", "label_no_toolchain")
Expand Down
Expand Up @@ -4,7 +4,7 @@

package org.chromium.chrome.browser.firstrun;

import androidx.fragment.app.FragmentActivity;
import android.app.Activity;

/**
* This interface is implemented by FRE fragments.
Expand All @@ -25,7 +25,7 @@ default boolean interceptBackPressed() {
/**
* @see Fragment#getActivity().
*/
FragmentActivity getActivity();
Activity getActivity();

/**
* Set the a11y focus when the fragment is shown on the screen.
Expand Down

0 comments on commit 360e54d

Please sign in to comment.