Skip to content

Commit 3606d52

Browse files
fractalwrenchfractalwrench
committed
refactor: simplify bytecode instrumentation implementation
1 parent b0572d4 commit 3606d52

28 files changed

Lines changed: 153 additions & 627 deletions

‎embrace-bytecode-instrumentation-tests/src/test/java/io/embrace/gradle/plugin/instrumentation/InstrumentedBytecodeTestCases.kt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,19 +32,19 @@ import okhttp3.OkHttpClient
3232
import org.objectweb.asm.ClassVisitor
3333

3434
private val onClickFactory: ClassVisitorFactory = { visitor ->
35-
OnClickClassAdapter(ASM_API_VERSION, visitor) {}
35+
OnClickClassAdapter(ASM_API_VERSION, visitor)
3636
}
3737

3838
private val onLongClickFactory: ClassVisitorFactory = { visitor ->
39-
OnLongClickClassAdapter(ASM_API_VERSION, visitor) {}
39+
OnLongClickClassAdapter(ASM_API_VERSION, visitor)
4040
}
4141

4242
private val webviewFactory: ClassVisitorFactory = { visitor ->
43-
WebViewClientClassAdapter(ASM_API_VERSION, visitor) {}
43+
WebViewClientClassAdapter(ASM_API_VERSION, visitor)
4444
}
4545

4646
private val okHttpFactory: ClassVisitorFactory = { visitor ->
47-
OkHttpClassAdapter(ASM_API_VERSION, visitor) {}
47+
OkHttpClassAdapter(ASM_API_VERSION, visitor)
4848
}
4949

5050
/**

‎embrace-gradle-plugin/src/main/java/io/embrace/android/gradle/plugin/instrumentation/EmbraceClassVisitorFactory.kt‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package io.embrace.android.gradle.plugin.instrumentation
33
import com.android.build.api.instrumentation.AsmClassVisitorFactory
44
import com.android.build.api.instrumentation.ClassContext
55
import com.android.build.api.instrumentation.ClassData
6-
import io.embrace.android.gradle.plugin.Logger
76
import org.objectweb.asm.ClassVisitor
87

98
/**
@@ -25,9 +24,7 @@ abstract class EmbraceClassVisitorFactory : AsmClassVisitorFactory<BytecodeInstr
2524
nextClassVisitor,
2625
instrumentationContext,
2726
parameters
28-
) {
29-
logger.info(it())
30-
}
27+
)
3128
}
3229

3330
override fun isInstrumentable(classData: ClassData): Boolean {
@@ -46,5 +43,3 @@ abstract class EmbraceClassVisitorFactory : AsmClassVisitorFactory<BytecodeInstr
4643
return true
4744
}
4845
}
49-
50-
private val logger = Logger(EmbraceClassVisitorFactory::class.java)

‎embrace-gradle-plugin/src/main/java/io/embrace/android/gradle/plugin/instrumentation/EmbraceClassVisitorFactoryDelegate.kt‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@ internal fun createClassVisitorImpl(
1515
classContext: ClassContext,
1616
nextClassVisitor: ClassVisitor,
1717
instrumentationContext: InstrumentationContext,
18-
parameters: Property<BytecodeInstrumentationParams>,
19-
logger: (() -> String) -> Unit,
18+
parameters: Property<BytecodeInstrumentationParams>
2019
): ClassVisitor {
2120
val api = instrumentationContext.apiVersion.get()
2221
var visitor = nextClassVisitor
@@ -34,25 +33,20 @@ internal fun createClassVisitorImpl(
3433
if (parameters.get().shouldInstrumentFirebaseMessaging.get() &&
3534
FirebaseMessagingServiceClassAdapter.accept(classContext)
3635
) {
37-
visitor = FirebaseMessagingServiceClassAdapter(api, visitor, logger)
38-
logger { "Added FirebaseMessagingServiceClassAdapter for $className." }
36+
visitor = FirebaseMessagingServiceClassAdapter(api, visitor)
3937
}
4038

4139
if (parameters.get().shouldInstrumentWebview.get() && WebViewClientClassAdapter.accept(classContext)) {
42-
visitor = WebViewClientClassAdapter(api, visitor, logger)
43-
logger { "Added WebViewClientClassAdapter for $className." }
40+
visitor = WebViewClientClassAdapter(api, visitor)
4441
}
4542
if (parameters.get().shouldInstrumentOkHttp.get() && OkHttpClassAdapter.accept(classContext)) {
46-
visitor = OkHttpClassAdapter(api, visitor, logger)
47-
logger { "Added OkHttpClassAdapter for $className." }
43+
visitor = OkHttpClassAdapter(api, visitor)
4844
}
4945
if (parameters.get().shouldInstrumentOnLongClick.get() && OnLongClickClassAdapter.accept(classContext)) {
50-
visitor = OnLongClickClassAdapter(api, visitor, logger)
51-
logger { "Added OnLongClickClassAdapter for $className." }
46+
visitor = OnLongClickClassAdapter(api, visitor)
5247
}
5348
if (parameters.get().shouldInstrumentOnClick.get() && OnClickClassAdapter.accept(classContext)) {
54-
visitor = OnClickClassAdapter(api, visitor, logger)
55-
logger { "Added OnClickClassAdapter for $className." }
49+
visitor = OnClickClassAdapter(api, visitor)
5650
}
5751
return visitor
5852
}

‎embrace-gradle-plugin/src/main/java/io/embrace/android/gradle/plugin/instrumentation/visitor/BytecodeMethodInsertionParams.kt‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package io.embrace.android.gradle.plugin.instrumentation.visitor
2+
3+
/**
4+
* The parameters that should be used to insert a function call into the body of a method.
5+
*/
6+
internal data class BytecodeMethodInsertionParams(
7+
8+
/**
9+
* The fully qualified class name containing the method to be instrumented.
10+
*/
11+
val owner: String,
12+
13+
/**
14+
* The function name that will be instrumented.
15+
*/
16+
val name: String,
17+
18+
/**
19+
* The type signature of the function to be instrumented.
20+
*/
21+
val descriptor: String,
22+
23+
/**
24+
* The starting index of the local variable on the operand stack. We assume that by default the bytecode
25+
* entrypoint will take the current object as its first parameter, then all other parameters.
26+
*
27+
* For example, an instrumentation entrypoint for a View.OnClickListener would have the following
28+
* signature: instrumentedMethodName(android.view.View.OnClickListener thiz, android.view.View view).
29+
*
30+
* In future we can revisit this decision, given that the current object is not usually used.
31+
*/
32+
val startVarIndex: Int = 0,
33+
)

‎embrace-gradle-plugin/src/main/java/io/embrace/android/gradle/plugin/instrumentation/visitor/FirebaseMessagingServiceClassAdapter.kt‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,34 +11,38 @@ import org.objectweb.asm.MethodVisitor
1111
class FirebaseMessagingServiceClassAdapter(
1212
api: Int,
1313
internal val nextClassVisitor: ClassVisitor?,
14-
private val logger: (() -> String) -> Unit
1514
) : ClassVisitor(api, nextClassVisitor) {
1615

1716
companion object : ClassVisitFilter {
1817
private const val CLASS_NAME = "com.google.firebase.messaging.FirebaseMessagingService"
1918
private const val METHOD_NAME = "onMessageReceived"
2019
private const val METHOD_DESC = "(Lcom/google/firebase/messaging/RemoteMessage;)V"
2120

22-
@Suppress("UnstableApiUsage")
2321
override fun accept(classContext: ClassContext): Boolean {
24-
if (classContext.currentClassData.superClasses.contains(CLASS_NAME)) {
25-
return true
26-
}
27-
return false
22+
return classContext.currentClassData.superClasses.contains(CLASS_NAME)
2823
}
2924
}
25+
3026
override fun visitMethod(
3127
access: Int,
3228
name: String,
3329
desc: String,
3430
signature: String?,
35-
exceptions: Array<String>?
31+
exceptions: Array<String>?,
3632
): MethodVisitor? {
3733
val nextMethodVisitor = super.visitMethod(access, name, desc, signature, exceptions)
3834

3935
return if (METHOD_NAME == name && METHOD_DESC == desc) {
40-
logger { "FirebaseMessagingServiceClassAdapter: instrumented method $name $desc" }
41-
FirebaseMessagingServiceMethodAdapter(api, nextMethodVisitor)
36+
InstrumentationTargetMethodVisitor(
37+
api = api,
38+
methodVisitor = nextMethodVisitor,
39+
params = BytecodeMethodInsertionParams(
40+
owner = "io/embrace/android/embracesdk/fcm/swazzle/callback/com/android/fcm/FirebaseSwazzledHooks",
41+
name = "_onMessageReceived",
42+
descriptor = "(Lcom/google/firebase/messaging/RemoteMessage;)V",
43+
startVarIndex = 1,
44+
)
45+
)
4246
} else {
4347
nextMethodVisitor
4448
}

‎embrace-gradle-plugin/src/main/java/io/embrace/android/gradle/plugin/instrumentation/visitor/FirebaseMessagingServiceMethodAdapter.kt‎

Lines changed: 0 additions & 29 deletions
This file was deleted.

‎embrace-gradle-plugin/src/main/java/io/embrace/android/gradle/plugin/instrumentation/visitor/InstrumentationTargetMethodVisitor.kt‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package io.embrace.android.gradle.plugin.instrumentation.visitor
2+
3+
import org.objectweb.asm.MethodVisitor
4+
import org.objectweb.asm.Opcodes
5+
import org.objectweb.asm.Type
6+
7+
/**
8+
* Visits a method that should be rewritten to include an API call to instrumentation
9+
* and inserts a call to a static method at the very start.
10+
*/
11+
internal class InstrumentationTargetMethodVisitor(
12+
api: Int,
13+
methodVisitor: MethodVisitor?,
14+
private val params: BytecodeMethodInsertionParams,
15+
) : MethodVisitor(api, methodVisitor) {
16+
17+
override fun visitCode() {
18+
// count how many parameters are in the method descriptor
19+
val paramCount = Type.getArgumentTypes(params.descriptor).size
20+
21+
// load local variables and push onto the operand stack
22+
repeat(paramCount) {
23+
visitVarInsn(Opcodes.ALOAD, it + params.startVarIndex)
24+
}
25+
26+
// invoke the target method
27+
visitMethodInsn(
28+
Opcodes.INVOKESTATIC,
29+
params.owner,
30+
params.name,
31+
params.descriptor,
32+
false
33+
)
34+
35+
// call super last to reduce chance of interference with other bytecode instrumentation
36+
super.visitCode()
37+
}
38+
}

‎embrace-gradle-plugin/src/main/java/io/embrace/android/gradle/plugin/instrumentation/visitor/OkHttpClassAdapter.kt‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import org.objectweb.asm.MethodVisitor
1010
class OkHttpClassAdapter(
1111
api: Int,
1212
internal val nextClassVisitor: ClassVisitor?,
13-
private val logger: (() -> String) -> Unit
1413
) : ClassVisitor(api, nextClassVisitor) {
1514

1615
companion object : ClassVisitFilter {
@@ -28,13 +27,20 @@ class OkHttpClassAdapter(
2827
name: String,
2928
desc: String,
3029
signature: String?,
31-
exceptions: Array<String>?
30+
exceptions: Array<String>?,
3231
): MethodVisitor? {
3332
val nextMethodVisitor = super.visitMethod(access, name, desc, signature, exceptions)
3433

3534
return if (METHOD_NAME_BUILD == name && METHOD_DESC_BUILD == desc) {
36-
logger { "OkHttpClassAdapter: instrumented method $name $desc" }
37-
OkHttpMethodAdapter(api, nextMethodVisitor)
35+
InstrumentationTargetMethodVisitor(
36+
api = api,
37+
methodVisitor = nextMethodVisitor,
38+
params = BytecodeMethodInsertionParams(
39+
owner = "io/embrace/android/embracesdk/okhttp3/swazzle/callback/okhttp3/OkHttpClient\$Builder",
40+
name = "_preBuild",
41+
descriptor = "(Lokhttp3/OkHttpClient\$Builder;)V",
42+
)
43+
)
3844
} else {
3945
nextMethodVisitor
4046
}

‎embrace-gradle-plugin/src/main/java/io/embrace/android/gradle/plugin/instrumentation/visitor/OkHttpMethodAdapter.kt‎

Lines changed: 0 additions & 31 deletions
This file was deleted.

‎embrace-gradle-plugin/src/main/java/io/embrace/android/gradle/plugin/instrumentation/visitor/OnClickClassAdapter.kt‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import org.objectweb.asm.MethodVisitor
1111
class OnClickClassAdapter(
1212
api: Int,
1313
internal val nextClassVisitor: ClassVisitor?,
14-
private val logger: (() -> String) -> Unit
1514
) : ClassVisitor(api, nextClassVisitor) {
1615

1716
companion object : ClassVisitFilter {
@@ -26,15 +25,21 @@ class OnClickClassAdapter(
2625
name: String,
2726
desc: String,
2827
signature: String?,
29-
exceptions: Array<String>?
28+
exceptions: Array<String>?,
3029
): MethodVisitor? {
3130
val nextMethodVisitor = super.visitMethod(access, name, desc, signature, exceptions)
3231

3332
return if (METHOD_NAME == name && METHOD_DESC == desc && !isStatic(access)) {
34-
logger { "OnClickClassAdapter: instrumented method $name $desc" }
35-
OnClickMethodAdapter(api, nextMethodVisitor)
33+
InstrumentationTargetMethodVisitor(
34+
api = api,
35+
methodVisitor = nextMethodVisitor,
36+
params = BytecodeMethodInsertionParams(
37+
owner = "io/embrace/android/embracesdk/ViewSwazzledHooks\$OnClickListener",
38+
name = "_preOnClick",
39+
descriptor = "(Landroid/view/View\$OnClickListener;Landroid/view/View;)V",
40+
)
41+
)
3642
} else if (METHOD_DESC == desc && isStatic(access) && isSynthetic(access)) {
37-
logger { "OnClickClassAdapter: instrumented synthetic method $name $desc" }
3843
OnClickStaticMethodAdapter(api, nextMethodVisitor)
3944
} else {
4045
nextMethodVisitor

0 commit comments

Comments
 (0)