fix(YouTube): Simplify litho filtering patch (#4910)

extensions/youtube/src/main/java/app/revanced/extension/youtube/patches/components/LithoFilterPatch.java

@@ -87,6 +87,10 @@ public final class LithoFilterPatch {
      * the buffer is saved to a ThreadLocal so each calling thread does not interfere with other threads.
      */
     private static final ThreadLocal<ByteBuffer> bufferThreadLocal = new ThreadLocal<>();
+    /**
+     * Results of calling {@link #filter(String, StringBuilder)}.
+     */
+    private static final ThreadLocal<Boolean> filterResult = new ThreadLocal<>();
 
     static {
         for (Filter filter : filters) {
@@ -140,11 +144,22 @@ public final class LithoFilterPatch {
         }
     }
 
+    /**
+     * Injection point.
+     */
+    public static boolean shouldFilter() {
+        Boolean shouldFilter = filterResult.get();
+        return shouldFilter != null && shouldFilter;
+    }
+
     /**
      * Injection point.  Called off the main thread, and commonly called by multiple threads at the same time.
      */
-    @SuppressWarnings("unused")
+    public static void filter(@Nullable String lithoIdentifier, StringBuilder pathBuilder) {
-    public static boolean filter(@Nullable String lithoIdentifier, @NonNull StringBuilder pathBuilder) {
+        filterResult.set(handleFiltering(lithoIdentifier, pathBuilder));
+    }
+
+    private static boolean handleFiltering(@Nullable String lithoIdentifier, StringBuilder pathBuilder) {
         try {
             if (pathBuilder.length() == 0) {
                 return false;

patches/src/main/kotlin/app/revanced/patches/youtube/layout/returnyoutubedislike/Fingerprints.kt

@@ -5,18 +5,6 @@ import app.revanced.util.literal
 import com.android.tools.smali.dexlib2.AccessFlags
 import com.android.tools.smali.dexlib2.Opcode
 
-internal val conversionContextFingerprint = fingerprint {
-    returns("Ljava/lang/String;")
-    parameters()
-    strings(
-        ", widthConstraint=",
-        ", heightConstraint=",
-        ", templateLoggerFactory=",
-        ", rootDisposableContainer=",
-        "ConversionContext{containerInternal=",
-    )
-}
-
 internal val dislikeFingerprint = fingerprint {
     returns("V")
     strings("like/dislike")

patches/src/main/kotlin/app/revanced/patches/youtube/layout/returnyoutubedislike/ReturnYouTubeDislikePatch.kt

@@ -18,6 +18,7 @@ import app.revanced.patches.youtube.misc.playservice.versionCheckPatch
 import app.revanced.patches.youtube.misc.settings.addSettingPreference
 import app.revanced.patches.youtube.misc.settings.newIntent
 import app.revanced.patches.youtube.misc.settings.settingsPatch
+import app.revanced.patches.youtube.shared.conversionContextFingerprintToString
 import app.revanced.patches.youtube.shared.rollingNumberTextViewAnimationUpdateFingerprint
 import app.revanced.patches.youtube.video.videoid.hookPlayerResponseVideoId
 import app.revanced.patches.youtube.video.videoid.hookVideoId
@@ -113,11 +114,11 @@ val returnYouTubeDislikePatch = bytecodePatch(
         // This hook handles all situations, as it's where the created Spans are stored and later reused.
         // Find the field name of the conversion context.
         val conversionContextField = textComponentConstructorFingerprint.originalClassDef.fields.find {
-            it.type == conversionContextFingerprint.originalClassDef.type
+            it.type == conversionContextFingerprintToString.originalClassDef.type
         } ?: throw PatchException("Could not find conversion context field")
 
         textComponentLookupFingerprint.match(textComponentConstructorFingerprint.originalClassDef)
-        textComponentLookupFingerprint.method.apply {
+            .method.apply {
             // Find the instruction for creating the text data object.
             val textDataClassType = textComponentDataFingerprint.originalClassDef.type
 
@@ -165,7 +166,7 @@ val returnYouTubeDislikePatch = bytecodePatch(
                     iget-object v$tempRegister, v$tempRegister, $conversionContextField
                     invoke-static { v$tempRegister, v$charSequenceRegister }, $EXTENSION_CLASS_DESCRIPTOR->onLithoTextLoaded(Ljava/lang/Object;Ljava/lang/CharSequence;)Ljava/lang/CharSequence;
                     move-result-object v$charSequenceRegister
-                    """,
+                """
             )
         }
 
@@ -201,11 +202,9 @@ val returnYouTubeDislikePatch = bytecodePatch(
             val charSequenceFieldReference =
                 getInstruction<ReferenceInstruction>(dislikesIndex).reference
 
-            val registerCount = implementation!!.registerCount
+            val conversionContextRegister = implementation!!.registerCount - parameters.size + 1
 
-            // This register is being overwritten, so it is free to use.
+            val freeRegister = findFreeRegister(insertIndex, charSequenceInstanceRegister, conversionContextRegister)
-            val freeRegister = registerCount - 1
-            val conversionContextRegister = registerCount - parameters.size + 1
 
             addInstructions(
                 insertIndex,

patches/src/main/kotlin/app/revanced/patches/youtube/misc/litho/filter/Fingerprints.kt

@@ -5,10 +5,6 @@ import app.revanced.util.literal
 import com.android.tools.smali.dexlib2.AccessFlags
 import com.android.tools.smali.dexlib2.Opcode
 
-/**
- * In 19.17 and earlier, this resolves to the same method as [readComponentIdentifierFingerprint].
- * In 19.18+ this resolves to a different method.
- */
 internal val componentContextParserFingerprint = fingerprint {
     strings(
         "TreeNode result must be set.",
@@ -17,11 +13,21 @@ internal val componentContextParserFingerprint = fingerprint {
     )
 }
 
+/**
+ * Resolves to the class found in [componentContextParserFingerprint].
+ * When patching 19.16 this fingerprint matches the same method as [componentContextParserFingerprint].
+ */
+internal val componentContextSubParserFingerprint = fingerprint {
+    strings(
+        "Number of bits must be positive"
+    )
+}
+
 internal val lithoFilterFingerprint = fingerprint {
     accessFlags(AccessFlags.STATIC, AccessFlags.CONSTRUCTOR)
     returns("V")
     custom { _, classDef ->
-        classDef.endsWith("LithoFilterPatch;")
+        classDef.endsWith("/LithoFilterPatch;")
     }
 }
 
@@ -37,18 +43,6 @@ internal val protobufBufferReferenceFingerprint = fingerprint {
     )
 }
 
-/**
-* In 19.17 and earlier, this resolves to the same method as [componentContextParserFingerprint].
-* In 19.18+ this resolves to a different method.
-*/
-internal val readComponentIdentifierFingerprint = fingerprint {
-    strings("Number of bits must be positive")
-}
-
-internal val elementConfigFingerprint = fingerprint {
-    strings(" enableDroppedFrameLogging", " elementDepthInTree")
-}
-
 internal val emptyComponentFingerprint = fingerprint {
     accessFlags(AccessFlags.PRIVATE, AccessFlags.CONSTRUCTOR)
     parameters()

patches/src/main/kotlin/app/revanced/patches/youtube/misc/litho/filter/LithoFilterPatch.kt

@@ -7,30 +7,24 @@ import app.revanced.patcher.extensions.InstructionExtensions.addInstructions
 import app.revanced.patcher.extensions.InstructionExtensions.getInstruction
 import app.revanced.patcher.extensions.InstructionExtensions.removeInstructions
 import app.revanced.patcher.extensions.InstructionExtensions.replaceInstruction
-import app.revanced.patcher.patch.PatchException
 import app.revanced.patcher.patch.bytecodePatch
-import app.revanced.patcher.util.proxy.mutableTypes.MutableField.Companion.toMutable
-import app.revanced.patches.youtube.layout.returnyoutubedislike.conversionContextFingerprint
 import app.revanced.patches.youtube.misc.extension.sharedExtensionPatch
-import app.revanced.patches.youtube.misc.playservice.is_19_18_or_greater
 import app.revanced.patches.youtube.misc.playservice.is_19_25_or_greater
 import app.revanced.patches.youtube.misc.playservice.is_20_05_or_greater
 import app.revanced.patches.youtube.misc.playservice.versionCheckPatch
+import app.revanced.patches.youtube.shared.conversionContextFingerprintToString
 import app.revanced.util.addInstructionsAtControlFlowLabel
 import app.revanced.util.findFreeRegister
 import app.revanced.util.findInstructionIndicesReversedOrThrow
 import app.revanced.util.getReference
 import app.revanced.util.indexOfFirstInstructionOrThrow
-import app.revanced.util.indexOfFirstInstructionReversed
 import app.revanced.util.indexOfFirstInstructionReversedOrThrow
 import com.android.tools.smali.dexlib2.AccessFlags
 import com.android.tools.smali.dexlib2.Opcode
-import com.android.tools.smali.dexlib2.iface.instruction.FiveRegisterInstruction
 import com.android.tools.smali.dexlib2.iface.instruction.OneRegisterInstruction
 import com.android.tools.smali.dexlib2.iface.instruction.ReferenceInstruction
 import com.android.tools.smali.dexlib2.iface.reference.FieldReference
 import com.android.tools.smali.dexlib2.iface.reference.MethodReference
-import com.android.tools.smali.dexlib2.immutable.ImmutableField
 
 lateinit var addLithoFilter: (String) -> Unit
     private set
@@ -65,42 +59,27 @@ val lithoFilterPatch = bytecodePatch(
      * The following pseudocode shows how this patch works:
      *
      * class SomeOtherClass {
-     *    // Called before ComponentContextParser.readComponentIdentifier(...) method.
+     *    // Called before ComponentContextParser.parseComponent() method.
      *    public void someOtherMethod(ByteBuffer byteBuffer) {
      *        ExtensionClass.setProtoBuffer(byteBuffer); // Inserted by this patch.
      *        ...
      *   }
      * }
      *
-     * When patching 19.16:
-     *
      * class ComponentContextParser {
-     *    public Component readComponentIdentifier(...) {
+     *    public Component parseComponent() {
      *        ...
-     *        if (extensionClass.filter(identifier, pathBuilder)) { // Inserted by this patch.
+     *
+     *        // Checks if the component should be filtered.
+     *        // Sets a thread local with the filtering result.
+     *        extensionClass.filter(identifier, pathBuilder);  // Inserted by this patch.
+     *
+     *        ...
+     *
+     *        if (extensionClass.shouldFilter()) {  // Inserted by this patch.
      *            return emptyComponent;
      *        }
-     *        return originalUnpatchedComponent;
+     *        return originalUnpatchedComponent; // Original code.
-     *    }
-     * }
-     *
-     * When patching 19.18 and later:
-     *
-     * class ComponentContextParser {
-     *    public ComponentIdentifierObj readComponentIdentifier(...) {
-     *        ...
-     *        if (extensionClass.filter(identifier, pathBuilder)) { // Inserted by this patch.
-     *            this.patch_isFiltered = true;
-     *        }
-     *        ...
-     *    }
-     *
-     *    public Component parseBytesToComponentContext(...) {
-     *        ...
-     *        if (this.patch_isFiltered) { // Inserted by this patch.
-     *            return emptyComponent;
-     *        }
-     *        return originalUnpatchedComponent;
      *    }
      * }
      */
@@ -115,7 +94,7 @@ val lithoFilterPatch = bytecodePatch(
                     2,
                     """
                         new-instance v1, $classDescriptor
-                        invoke-direct {v1}, $classDescriptor-><init>()V
+                        invoke-direct { v1 }, $classDescriptor-><init>()V
                         const/16 v2, ${filterCount++}
                         aput-object v1, v0, v2
                     """,
@@ -134,135 +113,96 @@ val lithoFilterPatch = bytecodePatch(
 
         // region Hook the method that parses bytes into a ComponentContext.
 
+        // Allow the method to run to completion, and override the
+        // return value with an empty component if it should be filtered.
+        // It is important to allow the original code to always run to completion,
+        // otherwise memory leaks and poor app performance can occur.
+        //
+        // The extension filtering result needs to be saved off somewhere, but cannot
+        // save to a class field since the target class is called by multiple threads.
+        // It would be great if there was a way to change the register count of the
+        // method implementation and save the result to a high register to later use
+        // in the method, but there is no simple way to do that.
+        // Instead save the extension filter result to a thread local and check the
+        // filtering result at each method return index.
+        // String field for the litho identifier.
+        componentContextParserFingerprint.method.apply {
+            val conversionContextClass = conversionContextFingerprintToString.originalClassDef
+
+            val conversionContextIdentifierField = componentContextSubParserFingerprint.match(
+                componentContextParserFingerprint.originalClassDef
+            ).let {
+                // Identifier field is loaded just before the string declaration.
+                val index = it.method.indexOfFirstInstructionReversedOrThrow(
+                    it.stringMatches!!.first().index
+                ) {
+                    val reference = getReference<FieldReference>()
+                    reference?.definingClass == conversionContextClass.type
+                            && reference.type == "Ljava/lang/String;"
+                }
+                it.method.getInstruction<ReferenceInstruction>(index).getReference<FieldReference>()
+            }
+
+            // StringBuilder field for the litho path.
+            val conversionContextPathBuilderField = conversionContextClass.fields
+                .single { field -> field.type == "Ljava/lang/StringBuilder;" }
+
+            val conversionContextResultIndex = indexOfFirstInstructionOrThrow {
+                val reference = getReference<MethodReference>()
+                reference?.returnType == conversionContextClass.type
+            } + 1
+
+            val conversionContextResultRegister = getInstruction<OneRegisterInstruction>(
+                conversionContextResultIndex
+            ).registerA
+
+            val identifierRegister = findFreeRegister(
+                conversionContextResultIndex, conversionContextResultRegister
+            )
+            val stringBuilderRegister = findFreeRegister(
+                conversionContextResultIndex, conversionContextResultRegister, identifierRegister
+            )
+
+            // Check if the component should be filtered, and save the result to a thread local.
+            addInstructionsAtControlFlowLabel(
+                conversionContextResultIndex + 1,
+                """
+                    iget-object v$identifierRegister, v$conversionContextResultRegister, $conversionContextIdentifierField
+                    iget-object v$stringBuilderRegister, v$conversionContextResultRegister, $conversionContextPathBuilderField
+                    invoke-static { v$identifierRegister, v$stringBuilderRegister }, $EXTENSION_CLASS_DESCRIPTOR->filter(Ljava/lang/String;Ljava/lang/StringBuilder;)V
+                """
+            )
+
             // Get the only static method in the class.
-        val builderMethodDescriptor = emptyComponentFingerprint.classDef.methods.first { method ->
+            val builderMethodDescriptor = emptyComponentFingerprint.classDef.methods.single {
-            AccessFlags.STATIC.isSet(method.accessFlags)
+                method -> AccessFlags.STATIC.isSet(method.accessFlags)
             }
             // Only one field.
             val emptyComponentField = classBy { classDef ->
-            builderMethodDescriptor.returnType == classDef.type
+                classDef.type == builderMethodDescriptor.returnType
             }!!.immutableClass.fields.single()
 
-        // Add a field to store the result of the filtering. This allows checking the field
+            // Check at each return value if the component is filtered,
-        // just before returning so the original code always runs the same when filtering occurs.
+            // and return an empty component if filtering is needed.
-        val lithoFilterResultField = ImmutableField(
+            findInstructionIndicesReversedOrThrow(Opcode.RETURN_OBJECT).forEach { returnIndex ->
-            componentContextParserFingerprint.classDef.type,
+                val freeRegister = findFreeRegister(returnIndex)
-            "patch_isFiltered",
-            "Z",
-            AccessFlags.PRIVATE.value,
-            null,
-            null,
-            null,
-        ).toMutable()
-        componentContextParserFingerprint.classDef.fields.add(lithoFilterResultField)
 
-        // Returns an empty component instead of the original component.
+                addInstructionsAtControlFlowLabel(
-        fun returnEmptyComponentInstructions(free: Int): String = """
+                    returnIndex,
-            move-object/from16 v$free, p0
+                    """
-            iget-boolean v$free, v$free, $lithoFilterResultField
+                        invoke-static { }, $EXTENSION_CLASS_DESCRIPTOR->shouldFilter()Z
-            if-eqz v$free, :unfiltered
+                        move-result v$freeRegister
+                        if-eqz v$freeRegister, :unfiltered
         
-            move-object/from16 v$free, p1
+                        move-object/from16 v$freeRegister, p1
-            invoke-static { v$free }, $builderMethodDescriptor
+                        invoke-static { v$freeRegister }, $builderMethodDescriptor
-            move-result-object v$free
+                        move-result-object v$freeRegister
-            iget-object v$free, v$free, $emptyComponentField
+                        iget-object v$freeRegister, v$freeRegister, $emptyComponentField
-            return-object v$free
+                        return-object v$freeRegister
         
                         :unfiltered
                         nop    
                     """
-
-        componentContextParserFingerprint.method.apply {
-            // 19.18 and later require patching 2 methods instead of one.
-            // Otherwise the modifications done here are the same for all targets.
-            if (is_19_18_or_greater) {
-                findInstructionIndicesReversedOrThrow(Opcode.RETURN_OBJECT).forEach { index ->
-                    val free = findFreeRegister(index)
-
-                    addInstructionsAtControlFlowLabel(
-                        index,
-                        returnEmptyComponentInstructions(free)
-                    )
-                }
-            }
-        }
-
-        // endregion
-
-        // region Read component then store the result.
-
-        readComponentIdentifierFingerprint.method.apply {
-            val returnIndex = indexOfFirstInstructionReversedOrThrow(Opcode.RETURN_OBJECT)
-            if (indexOfFirstInstructionReversed(returnIndex - 1, Opcode.RETURN_OBJECT) >= 0) {
-                throw PatchException("Found multiple return indexes") // Patch needs an update.
-            }
-
-            val elementConfigClass = elementConfigFingerprint.originalClassDef
-            val elementConfigClassType = elementConfigClass.type
-            val elementConfigIndex = indexOfFirstInstructionReversedOrThrow(returnIndex) {
-                val reference = getReference<MethodReference>()
-                reference?.definingClass == elementConfigClassType
-            }
-            val elementConfigStringBuilderField = elementConfigClass.fields.single { field ->
-                field.type == "Ljava/lang/StringBuilder;"
-            }
-
-            // Identifier is saved to a field just before the string builder.
-            val putStringBuilderIndex = indexOfFirstInstructionOrThrow {
-                val reference = getReference<FieldReference>()
-                opcode == Opcode.IPUT_OBJECT &&
-                        reference?.definingClass == elementConfigClassType &&
-                        reference.type == "Ljava/lang/StringBuilder;"
-            }
-            val elementConfigIdentifierField = getInstruction<ReferenceInstruction>(
-                indexOfFirstInstructionReversedOrThrow(putStringBuilderIndex) {
-                    val reference = getReference<FieldReference>()
-                    opcode == Opcode.IPUT_OBJECT &&
-                            reference?.definingClass == elementConfigClassType &&
-                            reference.type == "Ljava/lang/String;"
-                }
-            ).getReference<FieldReference>()
-
-            // Could use some of these free registers multiple times, but this is inserting at a
-            // return instruction so there is always multiple 4-bit registers available.
-            val elementConfigRegister = getInstruction<FiveRegisterInstruction>(elementConfigIndex).registerC
-            val identifierRegister = findFreeRegister(returnIndex, elementConfigRegister)
-            val stringBuilderRegister = findFreeRegister(returnIndex, elementConfigRegister, identifierRegister)
-            val thisRegister = findFreeRegister(returnIndex, elementConfigRegister, identifierRegister, stringBuilderRegister)
-            val freeRegister = findFreeRegister(returnIndex, elementConfigRegister, identifierRegister, stringBuilderRegister, thisRegister)
-
-            val invokeFilterInstructions = """
-                iget-object v$identifierRegister, v$elementConfigRegister, $elementConfigIdentifierField
-                iget-object v$stringBuilderRegister, v$elementConfigRegister, $elementConfigStringBuilderField
-                invoke-static { v$identifierRegister, v$stringBuilderRegister }, $EXTENSION_CLASS_DESCRIPTOR->filter(Ljava/lang/String;Ljava/lang/StringBuilder;)Z
-                move-result v$freeRegister
-                move-object/from16 v$thisRegister, p0
-                iput-boolean v$freeRegister, v$thisRegister, $lithoFilterResultField
-            """
-
-            if (is_19_18_or_greater) {
-                addInstructionsAtControlFlowLabel(
-                    returnIndex,
-                    invokeFilterInstructions
-                )
-            } else {
-                val elementConfigMethod = conversionContextFingerprint.originalClassDef.methods
-                    .single { method ->
-                        !AccessFlags.STATIC.isSet(method.accessFlags) && method.returnType == elementConfigClassType
-                    }
-
-                addInstructionsAtControlFlowLabel(
-                    returnIndex,
-                    """
-                        # Element config is a method on a parameter.
-                        move-object/from16 v$elementConfigRegister, p2 
-                        invoke-virtual { v$elementConfigRegister }, $elementConfigMethod
-                        move-result-object v$elementConfigRegister
-                        
-                        $invokeFilterInstructions
-
-                        ${returnEmptyComponentInstructions(freeRegister)}
-                    """
                 )
             }
         }

patches/src/main/kotlin/app/revanced/patches/youtube/shared/Fingerprints.kt

@@ -4,6 +4,21 @@ import app.revanced.patcher.fingerprint
 import com.android.tools.smali.dexlib2.AccessFlags
 import com.android.tools.smali.dexlib2.Opcode
 
+internal val conversionContextFingerprintToString = fingerprint {
+    parameters()
+    strings(
+        "ConversionContext{containerInternal=",
+        ", widthConstraint=",
+        ", heightConstraint=",
+        ", templateLoggerFactory=",
+        ", rootDisposableContainer=",
+        ", identifierProperty="
+    )
+    custom { method, _ ->
+        method.name == "toString"
+    }
+}
+
 internal val autoRepeatFingerprint = fingerprint {
     accessFlags(AccessFlags.PUBLIC, AccessFlags.FINAL)
     returns("V")