matomo-org / matomo-sdk-android

SDK for Android to measure your apps with Matomo. Works on Android phones, tablets, Fire TV sticks, and more!
BSD 3-Clause "New" or "Revised" License
387 stars 163 forks source link

Crash with targetSdkVersion 31 due to accessing WindowManager from ApplicationContext #323

Open Zhuinden opened 2 years ago

Zhuinden commented 2 years ago
E/ContextImpl: Tried to access visual service WindowManager from a non-visual Context: WindowManager should be accessed from Activity or other visual Context. Use an Activity or a Context created with Context#createWindowContext(int, Bundle), which are adjusted to the configuration and visual bounds of an area on screen.
    java.lang.IllegalAccessException: Tried to access visual service WindowManager from a non-visual Context
        at android.app.ContextImpl.getSystemService(ContextImpl.java:2059)
        at android.content.ContextWrapper.getSystemService(ContextWrapper.java:857)
        at pro.piwik.sdk.tools.DeviceHelper.getResolution(DeviceHelper.java:123)
        at pro.piwik.sdk.Tracker.<init>(Tracker.java:137)
        at pro.piwik.sdk.Piwik.newTracker(Piwik.java:59)
hannesa2 commented 1 year ago

This line doesn't exist DeviceHelper.java:123 This means the file got same changes and raise the question: does the issue still exists ?

Btw, when I see this

image

The question is valid, if we set minSDk to 19 and loose 0,41 % ? @d4rken

image
d4rken commented 1 year ago

The error is from here, not sure why the line numbers are off.

https://github.com/matomo-org/matomo-sdk-android/blob/7216a3227cd225abd2850ba9615f0d05194079ca/tracker/src/main/java/org/matomo/sdk/tools/DeviceHelper.java#L78-L85

The SDK inits on the app context, so we get the Display from a non-ui context.

I wouldn't be opposed to raising the minApi (do we do a major version bump for this?), but I don't think it would solve this.

Even with the legacy code removed )#335) we need the Display instance for:

        DisplayMetrics displayMetrics = new DisplayMetrics();
        display.getRealMetrics(displayMetrics);
        width = displayMetrics.widthPixels;
        height = displayMetrics.heightPixels;

Otherwise we'd have to get one from the dev ( trackDeviceScreen(activity: Activity)?) (and we'd have to be careful not to leak it). Could maybe be automated by hooking into the Activity lifecycle listeners via the Application instance, but we currently don't explicitly get the Application instance, just a Context.

https://github.com/matomo-org/matomo-sdk-android/blob/7216a3227cd225abd2850ba9615f0d05194079ca/tracker/src/main/java/org/matomo/sdk/Matomo.java#L40

Is there a way to do it correctly without a UI context? :thinking: