Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Keep Mapbox.java when obfuscating code #15762

Merged
merged 1 commit into from
Oct 4, 2019
Merged

Keep Mapbox.java when obfuscating code #15762

merged 1 commit into from
Oct 4, 2019

Conversation

tobrun
Copy link
Member

@tobrun tobrun commented Oct 4, 2019

This is needed to allow looking up this file from JNI code.
Refs https://github.com/mapbox/mapbox-gl-native/pull/15712/files

@tobrun tobrun added the Android Mapbox Maps SDK for Android label Oct 4, 2019
@tobrun tobrun added this to the release-sangria milestone Oct 4, 2019
@tobrun tobrun requested review from pozdnyakov, alexshalamov and a team October 4, 2019 14:05
@tobrun tobrun self-assigned this Oct 4, 2019
Copy link
Contributor

@alexshalamov alexshalamov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

verified locally, release and debug build test app works on pixel 2

@@ -25,6 +26,7 @@
*/
@UiThread
@SuppressLint("StaticFieldLeak")
@Keep
Copy link
Member

@LukasPaczos LukasPaczos Oct 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping just the getAssetManager method that is referenced from JNI is enough in this context and would allow obfuscating all other java methods.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we also look up the class from JNI, so it needs to be kept as well

Copy link
Member

@LukasPaczos LukasPaczos Oct 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that if you keep at least one method/field from the class, the class name is not going to be obfuscated. This allows all other methods that are not accessed from native to be obfuscated then.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, interesting. It fails because if we're looking up a static method, JNI tries to verify the instance first with:
Caused by: java.lang.NoSuchMethodError: no static method "Lcom/mapbox/mapboxsdk/Mapbox;.hasInstance()Z"

The class is not obfuscated, but this internal, static hasInstance() method seems to be? Anyway 🍏

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we probably could fine tune this with using a specific proguard rule..

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants