Fire PlatformViewController FlutterView callbacks #13015
Conversation
|
@amirh this still needs a test before it can land but if you want to review the basic approach in the meantime feel free. |
|
This is ready for review. The unit test I added is not good, it's just an interaction test. To really test the behavior I would have need to have significantly refactored the |
| // TODO(mattcarroll): add javadocs | ||
| @UiThread | ||
| public static native boolean nativeGetIsSoftwareRenderingEnabled(); | ||
| public native boolean nativeGetIsSoftwareRenderingEnabled(); |
There was a problem hiding this comment.
Was this meant to be part of this change?
There was a problem hiding this comment.
I see now.
This seems to indicate that most of these methods shouldn't be static then, right?
shell/platform/android/test/io/flutter/embedding/android/FlutterViewTest.java
Outdated
Show resolved
Hide resolved
I agree. I conservatively just instanced the one thing I needed to for this test case but I think it would make sense to move all of them. These are really depending on some mutable state (the native library being loaded) so I don't think they're really immutable/"static" to begin with. |
Fixes a bug where `PlatformViewController` was not being notified of `FlutterView` attachment changes. Unfortunately the test here is just an interaction test. I refactored some of the underlying code so that FlutterEngine could be initialized without through missing native library errors, but even with that variance there isn't any obvious way to test the real behavior here. I'd need to significantly refactor FlutterEngine and PlatformViewsController to write a more effective test here.
4 participants
Fixes a bug where
PlatformViewControllerwas not being notified ofFlutterViewattachment changes.Related to flutter/flutter#41851.