Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Fix Android T deprecated WindowManager INCORRECT_CONTEXT_USAGE #28774

Merged
merged 17 commits into from
Sep 23, 2021
Merged

Fix Android T deprecated WindowManager INCORRECT_CONTEXT_USAGE #28774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
39a4059
Fix Android T deprecated WindowManager getter call
GaryQian Sep 21, 2021
433a7f5
Imports
GaryQian Sep 21, 2021
b5a86a8
VsyncWaiter takes Display instead of WindowManager
GaryQian Sep 21, 2021
2428099
Version gate and take fps directly
GaryQian Sep 21, 2021
5bd268f
Loader test
GaryQian Sep 22, 2021
7f4ebd5
Test sdk differences
GaryQian Sep 22, 2021
baea4ca
Rename tests
GaryQian Sep 22, 2021
49aee59
Address comments
GaryQian Sep 22, 2021
1fc56c1
Verify getSystemService is not ccalled
GaryQian Sep 22, 2021
8cea69e
Missing import
GaryQian Sep 22, 2021
03f137e
Imports
GaryQian Sep 22, 2021
38af333
No mock static methods
GaryQian Sep 22, 2021
3115d91
Fix tests
GaryQian Sep 23, 2021
ae196c1
Tests
GaryQian Sep 23, 2021
d41270f
Float literal
GaryQian Sep 23, 2021
99c4a19
Cleanup
GaryQian Sep 23, 2021
8b9ccd1
Cleanup
GaryQian Sep 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,13 @@
import android.content.pm.ApplicationInfo;
import android.content.pm.PackageManager;
import android.content.res.AssetManager;
import android.hardware.display.DisplayManager;
import android.os.Build;
import android.os.Bundle;
import android.os.Handler;
import android.os.Looper;
import android.os.SystemClock;
import android.view.Display;
import android.view.WindowManager;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
Expand Down Expand Up @@ -106,7 +109,6 @@ public FlutterLoader(@NonNull FlutterJNI flutterJNI, @NonNull ExecutorService ex
private FlutterApplicationInfo flutterApplicationInfo;
private FlutterJNI flutterJNI;
private ExecutorService executorService;
private WindowManager windowManager;

private static class InitResult {
final String appStoragePath;
Expand Down Expand Up @@ -158,8 +160,19 @@ public void startInitialization(@NonNull Context applicationContext, @NonNull Se

initStartTimestampMillis = SystemClock.uptimeMillis();
flutterApplicationInfo = ApplicationInfoLoader.load(appContext);
VsyncWaiter.getInstance((WindowManager) appContext.getSystemService(Context.WINDOW_SERVICE))
.init();

float fps;
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
Copy link

@blasten blasten Sep 22, 2021

Choose a reason for hiding this comment

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

did it mean TIRAMISU ? https://source.corp.google.com/android/frameworks/base/core/java/android/os/Build.java;l=1137;bpv=0;bpt=1

Copy link
Contributor Author

@GaryQian GaryQian Sep 22, 2021

Choose a reason for hiding this comment

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

This check is specifically making sure the new code is used only for API 23+ which is when the display API was added. I guess it could be gated to only apply to T and above, I have no strong opinions either way. End goal is just to obtain the FPS float.

Copy link

@blasten blasten Sep 22, 2021

Choose a reason for hiding this comment

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

ah ok, that's fine too then

Copy link
Contributor Author

@GaryQian GaryQian Sep 22, 2021

Choose a reason for hiding this comment

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

Gating at 23 is more likely to have better test coverage as most of our testing is with API 30 and below (as of now), making this safer in the long run. I'll leave it as is.

Copy link
Contributor

@dnfield dnfield Nov 17, 2021

Choose a reason for hiding this comment

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

The display API was actually added in 17. I'd like to change this, is there any good reason not to?

Copy link
Contributor

@dnfield dnfield Nov 17, 2021

Choose a reason for hiding this comment

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

Or at least, all of the display code we're using :)

Copy link
Contributor

@dnfield dnfield Nov 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I'm changing this in #29800 - please add comments if it needs to be at 23 for some reason I'm missing.

Copy link
Contributor

@dnfield dnfield Nov 18, 2021

Choose a reason for hiding this comment

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

That should hav ebeen #29800....

Copy link
Contributor

@dnfield dnfield Nov 18, 2021

Choose a reason for hiding this comment

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

I see now - getSystemService was using the API 23 call instead of the one compatible with API 17. Fixing that upstream in my PR.

final DisplayManager dm = appContext.getSystemService(DisplayManager.class);
final Display primaryDisplay = dm.getDisplay(Display.DEFAULT_DISPLAY);
fps = primaryDisplay.getRefreshRate();
} else {
fps =
((WindowManager) appContext.getSystemService(Context.WINDOW_SERVICE))
.getDefaultDisplay()
.getRefreshRate();
}
VsyncWaiter.getInstance(fps).init();

// Use a background thread for initialization tasks that require disk access.
Callable<InitResult> initTask =
Expand Down
13 changes: 5 additions & 8 deletions shell/platform/android/io/flutter/view/VsyncWaiter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
package io.flutter.view;

import android.view.Choreographer;
import android.view.WindowManager;
import androidx.annotation.NonNull;
import io.flutter.embedding.engine.FlutterJNI;

Expand All @@ -14,14 +13,14 @@ public class VsyncWaiter {
private static VsyncWaiter instance;

@NonNull
public static VsyncWaiter getInstance(@NonNull WindowManager windowManager) {
public static VsyncWaiter getInstance(float fps) {
if (instance == null) {
instance = new VsyncWaiter(windowManager);
instance = new VsyncWaiter(fps);
}
return instance;
}

@NonNull private final WindowManager windowManager;
private final float fps;

private final FlutterJNI.AsyncWaitForVsyncDelegate asyncWaitForVsyncDelegate =
new FlutterJNI.AsyncWaitForVsyncDelegate() {
Expand All @@ -32,7 +31,6 @@ public void asyncWaitForVsync(long cookie) {
new Choreographer.FrameCallback() {
@Override
public void doFrame(long frameTimeNanos) {
float fps = windowManager.getDefaultDisplay().getRefreshRate();
long refreshPeriodNanos = (long) (1000000000.0 / fps);
FlutterJNI.nativeOnVsync(
frameTimeNanos, frameTimeNanos + refreshPeriodNanos, cookie);
Expand All @@ -41,15 +39,14 @@ public void doFrame(long frameTimeNanos) {
}
};

private VsyncWaiter(@NonNull WindowManager windowManager) {
this.windowManager = windowManager;
private VsyncWaiter(float fps) {
this.fps = fps;
}

public void init() {
FlutterJNI.setAsyncWaitForVsyncDelegate(asyncWaitForVsyncDelegate);

// TODO(mattcarroll): look into moving FPS reporting to a plugin
float fps = windowManager.getDefaultDisplay().getRefreshRate();
FlutterJNI.setRefreshRateFPS(fps);
}
}
17 changes: 17 additions & 0 deletions shell/platform/android/test/io/flutter/embedding/engine/loader/FlutterLoaderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@
import static org.mockito.Mockito.anyString;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.robolectric.Shadows.shadowOf;

import android.annotation.TargetApi;
import android.app.ActivityManager;
import android.content.Context;
import io.flutter.embedding.engine.FlutterJNI;
Expand Down Expand Up @@ -92,4 +95,18 @@ public void itUsesCorrectExecutorService() {
flutterLoader.startInitialization(RuntimeEnvironment.application);
verify(mockExecutorService, times(1)).submit(any(Callable.class));
}

@Test
@TargetApi(23)
@Config(sdk = 23)
public void itReportsFpsToVsyncWaiterAndroidM() {
FlutterJNI mockFlutterJNI = mock(FlutterJNI.class);
FlutterLoader flutterLoader = new FlutterLoader(mockFlutterJNI);

Context appContextSpy = spy(RuntimeEnvironment.application);

assertFalse(flutterLoader.initialized());
flutterLoader.startInitialization(appContextSpy);
verify(appContextSpy, never()).getSystemService(anyString());
}
}