The iOS golden scenario tests are supposed to run on Skia and Impeller, but are actually running on Skia twice.
#### Issue 1: script-side
Regression from https://github.com/flutter/engine/pull/46329.
Currently, the tests log "Running simulator tests with Impeller" but then you see that it's not:
```
Running simulator tests with Impeller
...
2024-09-06 13:38:07.692810-0700 Scenarios[73857:470266] [IMPORTANT:flutter/shell/common/shell.cc(456)] [Action Required] The application opted out of Impeller by either using the --no-enable-impeller flag or FLTEnableImpeller=false plist flag. This option is going to go away in an upcoming Flutter release. Remove the explicit opt-out. If you need to opt-out, report a bug describing the issue.
```
https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8737514337106072353/+/u/test:_Scenario_App_Integration_Tests/stdout
`INFOPLIST_FILE="Scenarios/Info_Impeller.plist"` isn't passed into the xcodebuild any more. I confirmed #46329 caused this by inserting a comment in the middle of the `xcodebuild` command, which caused the last argument after the comment to not be passed in.
#### Issue 2: test-side
Regression from https://github.com/flutter/engine/pull/45093.
[The logic deciding whether to use Skia or Impeller goldens is incorrect](1da5dd68fd/testing/scenario_app/ios/Scenarios/ScenariosUITests/GoldenTestManager.m (L84-L90)) since `FLTEnableImpeller` is set in the app Info.plist, not the UI test, so the bundle should be "dev.flutter.Scenarios". That means even if the script was passing in the right Info.plist, the tests would fail since they would have compared the Skia screenshots.
#### This PR
1. Once https://github.com/flutter/engine/pull/55016 merges (edit: done), these tests will no longer be running with the software renderer. Remove all the test skips so they all run on Impeller.
2. Add the missing Impeller golden screenshots.
3. Update the default Info.plist to use Impeller, so if another script mishap happens, it will default to testing Impeller twice, and not Skia. Add a Skia Info.plist variant instead.
4. Update the test logic to check the right bundle ID Info.plist to decide whether to compare against the Skia or Impeller screenshots. Prefer Impeller so if another test-side mishap happens, it will also default to testing Impeller and not Skia. It will only use the Skia goldens if the bool is set in the Info.plist, and that bool is NO (not just if it's missing).
5. All this made the now-default Impeller tests pass, but when passing in `INFOPLIST_FILE="Scenarios/Info_Skia.plist"` the app wasn't launching, with the error:
> Scenarios encountered an error (Failed to install or launch the test runner. (Underlying Error: Simulator device returned an error for the requested operation. Failed to create promise. (Underlying Error: Failed to create app extension placeholder for /Users/chrome-bot/Library/Developer/Xcode/DerivedData/Scenarios-aypjgouuxctxctfazxalyegcximf/Build/Products/Debug-iphonesimulator/Scenarios.app/PlugIns/ScenariosShare.appex. Failed to create promise. (Underlying Error: Failed to set placeholder attributes dev.flutter.Scenarios.ScenariosShare. Failed to create promise. (Underlying Error: extensionDictionary must be set in placeholder attributes for an app extension placeholder. Invalid placeholder attributes.)))))
This is identical to the error in https://github.com/flutter/engine/pull/53717. I finally realized it was because the ScenariosShare app extension requires specific extension keys in its plist, which weren't present in the app Info.plist at `Scenarios/Info_Skia.plist`. So I changed the `INFOPLIST_FILE` path to `$(TARGET_NAME)/Info_Skia.plist` so it would resolve to target-specific copies like `Scenarios/ScenariosShare/Info_Skia.plist`. That meant I had to add a few more copies where they didn't exist in the target path.
Dependent on https://github.com/flutter/engine/pull/55016 landing.
Fixes https://github.com/flutter/flutter/issues/131888
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Fixes https://github.com/flutter/flutter/issues/154549
Each overlay surface gets its own content context and each cc gets its own transient buffer. So not reseting the overlay surfaces causes a memory leak. The overlay surfaces would just continue to allocate into the same buffer which would allocate endlessly
This is a re-land of https://github.com/flutter/engine/pull/55006, except that we special case the navigation channel to immediately dispatch its message if the isolate is not yet running.
This preserves the existing behavior relied upon by several iOS add2app tests, as well as the still used embedder v1 - and potentially undicovered future embedders.
Reverts: flutter/engine#55006
Initiated by: jonahwilliams
Reason for reverting: still failling mac module test
Original PR Author: jonahwilliams
Reviewed By: {jason-simmons}
This change reverts the following previous change:
Reland of https://github.com/flutter/engine/pull/54975
Fixes the initial route behavior for iOS. Previously the initial route setting would _always_ be posted as a task, which after merging the threads would fire after isolate creation, thus it would not actually update the initial route setting. Fixed Engine constructor so that it reads the initial route from the settings.
Reland of https://github.com/flutter/engine/pull/54975
Fixes the initial route behavior for iOS. Previously the initial route setting would _always_ be posted as a task, which after merging the threads would fire after isolate creation, thus it would not actually update the initial route setting. Fixed Engine constructor so that it reads the initial route from the settings.
The Dart analyzer will soon be changed so that if the `default` clause
of a `switch` statement is determined to be unreachable by the
exhaustiveness checker, a new warning of type
`unreachable_switch_default` will be issued. This parallels the behavior
of the existing `unreachable_switch_case` warning, which is issued
whenever a `case` clause of a `switch` statement is determined to be
unreachable.
Before adding the new warning to the analyzer, code in the engine needs
to first be updated to eliminate these unreachable `default` clauses, so
that the warning won't cause builds to break.
For more information, see
https://github.com/dart-lang/sdk/issues/54575.
Wean the DlOpReceiver interface and implementations off of using the SkScalar, Sk[I]Rect, and SkPoint objects in favor of our own DL/Impeller versions.
The start of an ongoing effort to eventually compartmentalize all use of Skia interfaces into a single backend rendering module that is one of 2 semi-pluggable renderers.
Changed the word "there" to "their" in Flutter GPU doc... which I think is correct, but I could be wrong. Thanks!
- [] I listed at least one issue that this PR fixes in the description above.
- [] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Reverts: flutter/engine#54975
Initiated by: jtmcdole
Reason for reverting: I believe this change caused flutter/flutter to break in an engine roll. It was one of two changes - the other being a webui change.
Tests broken:
module_test_ios: testDualCold
router test:
```
[2024-09-05 17:43:20.837343] [STDOUT] stderr: [ +135 ms] VMServiceFlutterDriver: Connected to Flutter application.
[2024-09-05 17:43:20.841927] [STDOUT] stdout: [ +4 ms] 00:00 [32m+0[0m:
Original PR Author: jonahwilliams
Reviewed By: {jason-simmons, johnmccutchan}
This change reverts the following previous change:
If we use runNowOrPostTask on platform channel responses, then we may not wake up the dart message loop. If nothing else wakes it up, then the embedder may hang on platform channel responses.
This fixes several google3 integration tests when run in merged thread mode.
When duplicates are found in the input lists, log them to stderr. For the case of duplicates across list, we print the whole list, sorted, rather than three separate lists, as it makes it simpler to spot the duplicate, at which point the file will be easy to spot in either `create_ios_framework.py` or `create_macos_framework.py`.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
There are three categories of binaries produced as part of the framework artifacts:
* Those that use APIs that require entitlements and must be code-signed; e.g. gen_snapshot
* Those that do not use APIs that require entitlements and must be code-signed; e.g. Flutter.framework dylib.
* Those that do not need to be code-signed; e.g. Flutter.dSYM symbols.
Until now, our signing infrastructure has assumed that all mach-o binaries in the artifacts we produce require a signature. dSYM files are not required to be codesigned, although the xcframework containing them are, and as such they cannot be removed or tampered with.
The framework code-signing tests in `dev/bots/suite_runners/run_verify_binaries_codesigned_tests.dart` are only run on post-submit on release branches, and thus, this issue was not uncovered until the first release after all the dSYM work landed. Those tests were updated in https://github.com/flutter/flutter/pull/154591. This updates the framework and artifact archive generation code to also explicitly exclude those files from signing.
Issue: https://github.com/flutter/flutter/issues/154571
Related: https://github.com/flutter/flutter/issues/116493
Related: https://github.com/flutter/flutter/issues/153532
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
If we use runNowOrPostTask on platform channel responses, then we may not wake up the dart message loop. If nothing else wakes it up, then the embedder may hang on platform channel responses.
This fixes several google3 integration tests when run in merged thread mode.
In [some cases][1], text editing utilities re-focus the `<input />` element during a blur event. This causes an unusual sequence of `focusin` and `focusout` events, leading to the engine sending unintended events.
Consider the following HTML code:
```html
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title></title>
</head>
<body>
<div id="container">
<input type="" value="1" id="input1">
<input type="" value="2" id="input2">
<input type="" value="3" id="input3">
</div>
<script>
container.addEventListener('focusin', (ev) => {
console.log('focusin: focus was gained by', ev.target);
});
container.addEventListener('focusout', (ev) => {
console.log('focusout: focus is leaving', ev.target, 'and it will go to', ev.relatedTarget);
});
</script>
</body>
</html>
```
Clicking input1, then input2, then input3 produces the following console logs:
```
// Input1 is clicked
focusin: focus was gained by <input type value=â"1" id=â"input1">â
// Input2 is clicked
focusout: focus is leaving <input type value=â"1" id=â"input1">â and it will go to <input type value=â"2" id=â"input2">â
focusin: focus was gained by <input type value=â"2" id=â"input2">â
// Input3 is clicked
focusout: focus is leaving <input type value=â"2" id=â"input2">â and it will go to <input type value=â"3" id=â"input3">â
focusin: focus was gained by <input type value=â"3" id=â"input3">â
```
Now, let's add a blur handler that changes focus:
```html
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title></title>
</head>
<body>
<div id="container">
<input type="" value="1" id="input1">
<input type="" value="2" id="input2">
<input type="" value="3" id="input3">
</div>
<script>
container.addEventListener('focusin', (ev) => {
console.log('focusin: focus was gained by', ev.target);
});
container.addEventListener('focusout', (ev) => {
console.log('focusout: focus is leaving', ev.target, 'and it will go to', ev.relatedTarget);
});
input2.addEventListener('blur', (ev) => {
input2.focus();
});
</script>
</body>
</html>
```
The log sequence changes and gives the wrong impression that no dom element has focus:
```
// Input1 is clicked
focusin: focus was gained by <input type value=â"1" id=â"input1">â
// Input2 is clicked
focusout: focus is leaving <input type value=â"1" id=â"input1">â and it will go to <input type value=â"2" id=â"input2">â
focusin: focus was gained by <input type value=â"2" id=â"input2">â
// Input3 is clicked, but the handler kicks in and instead of the following line being a focusout, it results in a focusin call first.
focusin: focus was gained by <input type value=â"2" id=â"input2">â
focusout: focus is leaving <input type value=â"2" id=â"input2">â and it will go to null
```
In addition to that, during `focusout` processing, `activeElement` typically points to `<body />`. However, if an element is focused during a `blur` event, `activeElement` points to that focused element. Although, undocumented it can be verified with:
```html
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title></title>
</head>
<body>
<div id="container">
<input type="" value="1" id="input1">
<input type="" value="2" id="input2">
<input type="" value="3" id="input3">
</div>
<script>
container.addEventListener('focusin', (ev) => {
console.log('focusin: was gained by', ev.target);
});
container.addEventListener('focusout', (ev) => {
console.log('document.hasFocus()', document.hasFocus());
console.log('document.activeElement', document.activeElement);
console.log('focusout: focus is leaving', ev.target, 'and it will go to', ev.relatedTarget);
});
input2.addEventListener('blur', (ev) => {
input2.focus();
});
</script>
</body>
</html>
```
We leverage these behaviors to ignore `focusout` events when the document has focus but `activeElement` is not `<body />`.
https://github.com/flutter/flutter/issues/153022
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Fixes https://github.com/flutter/flutter/issues/144070
There is a mechanism by which you can access a DisplayListBuilder as if it were a DlOpReceiver for dispatching one DisplayList into another.
This mechanism also resembles the legacy way in which one would write graphics snippets prior to the creation of the DlCanvas interface and so we have dozens of old unit tests which were written to test the Builder class by filling it with commands in the Receiver/Dispatcher format. This type of access is obsolete and maintaining the ability for arbitrary code to talk to a Builder in that manner is getting in the way of future work.
This PR rewrites over 100 of such unit tests to use the standard DlCanvas-style interface to record operations into a Builder, leaving only a dozen or so cases of internal state tests that still inject ops directly to test the internal state keeping.
