Platform Views on Android require API 29 when using Vulkan.
~~I'm not sure if I did the CI changes right, want to run it here and see. The added test needs to run on a device or emulator with API 29 or above and API 28 or below to be valid.~~
Makes the following changes to CI:
- Runs the emulator tests on presubmit, but only if `shell/platform/android/**` or `lib/ui/**` or `.ci.yaml` changes.
- Changes the emulator tests to be a regular engine_v2 test that spawns an x86 build. Unfortunately, the older API level emulators are only available for x86, not x86_64, and you cannot run the binary we create unless it matches the ABI.
- Runs the new test on both API 28 and API 34.
Fixes https://github.com/flutter/flutter/issues/132984
/cc @johnmccutchan fyi
Runs all FlutterMenuPlugin tests in an AutoReleasepoolTest, which
ensures all allocations are cleaned up.
Also extracts out some hackery into the FlutterMenuPluginTest fixture to
ensure that NSApplication instantiates everything necessary to do menu
bar manipulation.
Also replaces the use of OCMock in FlutterMenuPluginTest.mm with a fake
FakePluginRegistrar class. This avoids unnecessary use of OCMock in the
tests, which has been responsible for flakiness in some tests, in
particular where the mock is used across threads. This test was not
problematic, but the fake makes the tests more readable.
Also fixes linter warnings about using NSLocalizedString for user-facing
strings.
Issue: https://github.com/flutter/flutter/issues/104789
Issue: https://github.com/flutter/flutter/issues/127441
Issue: https://github.com/flutter/flutter/issues/124840
Introduce weak_nsobject from chromium.
There are some usages of weak_ptr wrapping Objective-C ids, weak_ptr is not really designed for ids and such usages are blocking the arc migration.
This PR mostly copies the weak_nsobject from chromium, at the same hash that we copied the ARC/MRC compatible scoped_nsobject: fd625125b8
To match how we used weak_ptr for those ids, I made some changes to the weak_nsobject:
- WeakNSObjects needs to be generated by a WeakNSObjectFactory. The WeakNSObjectFactory is owned by the objc class and acts as the generator of the WeakNSObjects. All the WeakNSObjects' derefing thread should be the same of the WeakNSObjectFactory's creation thread.
- chromuim's WeakNSObjects can be detached from the thread and re-attached to a new thread. To match our weak_ptr behavior, I changed WeakNSObjects to be only accessed from a single thread, the same as weak_ptr
This PR also moves the FlutterEngine to use WeakNSObject and updated related classes.
part of https://github.com/flutter/flutter/issues/137801
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This PR hides the system highlights in iOS 16 (except when scribble is enabled).
Note that auto correction highlight is still drawn by flutter, so it still works.
I don't think we need to CP this, since it's iOS 16 only, and iOS 17 is already out.
## Why not use system highlight?
Unlike iOS 17, the iOS 16 system highlight only respect the width provided by `firstRectForRange`, but not the height. I have audited all sizing related APIs in UITextInput (doc [here](https://developer.apple.com/documentation/uikit/uitextinput#1653155)), specifically, `firstRect(for:)`, `caretRect(for:)` and `selectionRects(for:)`, and they all return the correct height.
## About scribble
The initial implementation of `firstRectForRange` (that returns the first selection rect) was introduced for the scribble feature on iPad (code [here](1d3165a31c (diff-4c7b102c0690b8ec5e2212b079f5d69fe3f816c84e47ce94bc7bc89312f39e40R1487-R1505))).
It turns out that a non-zero rect is required for scribble's advanced feature to work (e.g. inserting a space with a vertical bar). So we can't apply this fix for scribble.
*List which issues are fixed by this PR. You must list at least one issue.*
Fixes https://github.com/flutter/flutter/issues/136802
*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
After close examination of the UIKit's default string tokenizer, when querying the line enclosing the end of doc position **in forward direction**, we should return nil (regardless whether the position is forward or backward affinity).
This aligns with the [API doc](https://developer.apple.com/documentation/uikit/uitextinputtokenizer/1614464-rangeenclosingposition?language=objc):
> If the text position is at a text-unit boundary, it is considered enclosed only if the next position in the given direction is entirely enclosed.
Will cherry pick this soon. Otherwise it will be less and less important as users upgrade to iOS 17.1.
### Why my previous workaround also works?
It turns out my previous workaround PR https://github.com/flutter/engine/pull/46591 works only because our misuse of text affinity in our text input. Specifically, when adding text affinity support, we only added it to `FlutterTextPosition`, but not `FlutterTextRange`. So when getting the beginning/end position from the range, we assign arbitrary affinities.
*List which issues are fixed by this PR. You must list at least one issue.*
https://github.com/flutter/engine/pull/46591
*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This reverts commit af107ceba5d188019df780e0be192dd7092ea35a. It is no
longer needed as we have resolved the flickering issue in the
ImageReader backend.
The difference between this PR and the original is the change to the `AndroidManifest.xml`. For context, right before the original PR landed, [a change to upgrade the target sdk](https://github.com/flutter/engine/pull/47683) in the manifest from 31 to 33 landed. It also removed the lint complaining about using an old target sdk version from the baseline lint file. So this change upgrading the sdk, but not the target sdk in the manifest, triggered the lint again.
For the rest of the PR, [see the description of the original PR](https://github.com/flutter/engine/pull/47609).
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Reverts flutter/engine#47825
Initiated by: zanderso
This change reverts the following previous change:
Original Description:
Looks like this was proactively added in https://github.com/flutter/engine/pull/20496, but never wired up to anything on any platform. It is also unused in framework and customer code; we never exposed this on e.g. MediaQuery.
Related framework PR: https://github.com/flutter/flutter/pull/138103 (Checks will fail until that PR is submitted).
## Description
This moves the state update to only happen on realizing the window instead of at initialization time, based on [the comment from](https://github.com/flutter/flutter/issues/137262#issuecomment-1792020246) @robert-ancell .
## Related Issues
- https://github.com/flutter/flutter/issues/137262
## Tests
- I tried to add tests, but it doesn't seem possible to create a view without an actual display connected (and making a mock of it defeats the purpose of the test). I'm happy to be proven wrong, though.
Reverts flutter/engine#47609
Initiated by: zanderso
This change reverts the following previous change:
Original Description:
~**This should not land until https://github.com/flutter/buildroot/pull/790 (re)lands, and I swap the buildroot url back to the latest commit.**~ ~Reland of PR to update buildroot at https://github.com/flutter/buildroot/pull/792. ~ <- landed, and changed the buildroot commit to the latest in DEPS
Upgrades to android api 34
Also:
1. Upgrades to java 17 in DEPS/ci, because the linter now requires it.
2. Stops running some roboelectric tests on android apis 16-18, because Roboelectric indicated those versions were unsupported and we don't support them either.
3. Applies the four trivial new suggestions from the newer linter.
4. Updates the baseline lint to include the new non-trivial lint ([fixed in a different PR](https://github.com/flutter/engine/pull/47817/files)).
5. Changes some instances where we were hardcoding android apis as numbers (e.g. `sdk = 16') to use version codes (see the [comment below](https://github.com/flutter/engine/pull/47609#issuecomment-1800308658)).
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Internally we have a lint that surfaces this as a warning. Googlers, please refer to go/al-rule/NarrowingConversion.
Related: b/309552840
When we do:
```
coords.toolMajor = (float) (double) coordsList.get(3) * density;
```
`coordsList.get(3)` is casted to a `double`, then a `float`, before the multiplication happens.
I don't think this is intentional. The intention of the code here seems to be:
- Cast to a `double`: `coordsList` is a `List<Object>` so the cast narrows the value
- Cast to a `float`: To fit the resulting value into [`coords.toolMajor`](https://developer.android.com/reference/android/view/MotionEvent.PointerCoords#toolMajor), which is a `float`.
As such, add parenthesis to address this.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Factors out an RAII-based class that can be used to capture std::cout, std::cerr, or technically any other std::ostream, though that's unlikely to be useful.
This makes the logic reusable but more importantly, ensures the capture is cleaned up at the end of the test.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This PR contains no functional changes but improves existing unit tests to reduce the number of warnings output when the tests are ran:
1. Replaced `ON_CALL` with `EXPECT_CALL` for expected method calls
2. Added some missing mocks
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
The new `BUILD.gn` files in the Engine tree can't go under
`build/secondary` because Skia still has its own, and they'd be selected
first. So, this PR puts the new `BUILD.gn` files under `flutter/skia`.
## Description
Before we deprecate the `RawKeyEvent` code, it needs to provide parity in functionality. One thing that is missing is the `eventSource` field from the `RawKeyEventDataAndroid` class, which provides the device type for the event.
See https://developer.android.com/reference/android/view/InputDevice#SOURCE_KEYBOARD for an example.
This PR implements that support, and sets the source to `KeyEventDeviceType.keyboard` for platforms that don't provide this information. The main thing it does is add the enum `KeyEventDeviceType`, and a new field `KeyData.deviceType`.
## Related Issues
- https://github.com/flutter/flutter/issues/136419
## Tests
- Updated tests to also read/write/verify this property.
As part of eliminating the Flutter buildroot
(https://github.com/flutter/flutter/issues/67373), we are moving all
third-party dependencies from //third_party to //flutter/third_party.
Once all third-party dependencies have been migrated, tooling and config
will be moved and the buildroot will be eliminated altogether.
No tests changed because there is no semantic change to this PR. This is
simply relocating a dependency.
The tests for FlutterCompositor are not useful. The current tests test two things:
1. That the mocks we set up behave the way we set them up to behave.
2. That the implementation of FlutterCompositor is the current implementation of FlutterCompositor.
As an example, consider FlutterCompositorTest.TestPresent: 70317028f2/shell/platform/darwin/macos/framework/Source/FlutterCompositorTest.mm (L107-L137)
Ostensibly, this test verifies that the onPresent callback configured in our fake FlutterViewProvider implementation is called when FlutterCompositor::Present() is called.
However, taking a look at the mocking setup:
70317028f2/shell/platform/darwin/macos/framework/Source/FlutterCompositorTest.mm (L47-L85)
We do the following:
1. Mock out FlutterSurfaceManager such that when a surface is requested, we hand back a mock surface. A little gross since we're relying on some knowledge of implementation details of the compositor, but let's take this as reasonable for now.
2. We mock out `FlutterSurface asFlutterMetalTexture` to return a mock texture. Again, we're getting a bit deep into implementation details that the test shouldn't know about, but let's assume this gets us somewhere.
3. We mock out `FlutterSurfaceManager present:notify:` to actually call the `onPresent` callback if it's passed in.
In effect, we're testing that:
1. We configured our mock for `FlutterSurfaceManager present:notify:` to call onPresent.
2. That `FlutterCompositor::Present` actually calls `FlutterSurfaceManager present:notify:` despite that being a simple implementation detail of that call.
This removes these tests. I have filed the following issue to track refactoring this class for testability and adding tests: https://github.com/flutter/flutter/issues/137648
Encountered these tests as part of deflaking and cleaning up memory allocations throughout the macOS desktop tests.
Issue: https://github.com/flutter/flutter/issues/137648
Issue: https://github.com/flutter/flutter/issues/104789
Issue: https://github.com/flutter/flutter/issues/127441
Issue: https://github.com/flutter/flutter/issues/124840
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
As part of eliminating the Flutter buildroot
(https://github.com/flutter/flutter/issues/67373), we are moving all
third-party dependencies from //third_party to //flutter/third_party.
Once all third-party dependencies have been migrated, tooling and config
will be moved and the buildroot will be eliminated altogether.
No tests changed because there is no semantic change to this PR. This is
simply relocating a dependency.
When we enter hybrid composition mode we 'pause' the default RenderSurface (implemented by SurfaceView or TextureView) and swap to an ImageReader based RenderSurface.
When we return from hybrid composition mode we recreate and re-initialize the real RenderSurface as if it was being used for the first time.
This broke Platform Views in an internal app b/306122497 because we would incorrectly tell the texture to attach when it was never detached.
This CL changes the protocol so that when we return from hybrid composition mode we only swap the RenderSurface and do not re-create it. This avoids doing a bunch of unnecessary work and fixes the logic error of re-attaching textures that were never detached.
*Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.*
*List which issues are fixed by this PR. You must list at least one issue.*
*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Adds a gtest test fixture mixin and convenience class that instantiates an NSAutoreleasePool at the beginning of each test and flushes it at the end; this allows Objective-C tests using ARC to free allocations at the end of each test.
Adds a subclass for the macOS accessibility bridge tests that instantiates and re-uses an NSWindow* across any tests that use it. This is because instantiating, closing, and immediately collecting an NSWindow results in a crash.
Prior to this patch, tests started failing (on my machine) around the 855th iteration, and issued the following warning on the 101st iteration:
```
2023-10-26 13:02:45.390829-0700 flutter_desktop_darwin_unittests[40837:1509026] [Window] WARNING: NSWindow has detected an excessive live window count of 101. Window 0x1423 of class 'NSWindow' created after passing the threshold of 100. This window is not necessarily the cause, and this warning will only be shown once per window class. (
0 AppKit 0x0000000192820d28 -[NSWindow _setWindowNumber:] + 684
1 AppKit 0x00000001933050e4 _NXCreateWindow + 284
2 AppKit 0x0000000192901ae0 -[NSWindow _commonAwake] + 672
3 AppKit 0x000000019281ff00 -[NSWindow _commonInitFrame:styleMask:backing:defer:] + 972
4 AppKit 0x000000019281f798 -[NSWindow _initContent:styleMask:backing:defer:contentView:] + 796
5 AppKit 0x000000019281f470 -[NSWindow initWithContentRect:styleMask:backing:defer:] + 48
6 flutter_desktop_darwin_unittests 0x0000000100001e3c _ZN7flutter7testing89AccessibilityBridgeMacTest_SendsAccessibilityCreateNotificationToWindowOfFlutterView_Test8TestBodyEv + 328
```
See: http://www.openradar.me/FB13291861
Issue: https://github.com/flutter/flutter/issues/104789
Issue: https://github.com/flutter/flutter/issues/127441
Issue: https://github.com/flutter/flutter/issues/124840
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Reverts flutter/engine#46171
Initiated by: jonahwilliams
This change reverts the following previous change:
Original Description:
The "rasterizer screenshot" VM service API uses the Skia software backend, even when running with impeller. At best, this leads to confusing half working screenshots that are missing images.
See https://github.com/flutter/flutter/issues/135052
We should disable this functionality when running with impeller. Perhaps this could be reworked to use picture toImage instead of the offscreen so that it uses the "real" backend regardless of the platform it is running on.
Fixes https://github.com/flutter/flutter/issues/135052
This refactors how high contrast is implemented on Windows:
1. Added a test to verify accessibility features are updated when a view is created. This prevents staleness issues as the Windows embedder isn't notified of accessibility changes while in headless mode.
1. Moved high contrast mode detection to `WindowsProcTable` from `FlutterWindow` to remove engine to view to window plumbing.
1. `FlutterWindow` and `FlutterWindowsEngine` now share their `WindowsProcTable` (which is used for mocking and polyfilling win32 APIs) to reduce redundant dynamic loading.
This pull request contains no functional changes.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
The "rasterizer screenshot" VM service API uses the Skia software backend, even when running with impeller. At best, this leads to confusing half working screenshots that are missing images.
See https://github.com/flutter/flutter/issues/135052
We should disable this functionality when running with impeller. Perhaps this could be reworked to use picture toImage instead of the offscreen so that it uses the "real" backend regardless of the platform it is running on.
Fixes https://github.com/flutter/flutter/issues/135052
