dart:js_interop and package:js will start conflicting. Eventually, we
want people to only use dart:js_interop, so this CL refactors code to do
that.
Unblocks https://dart-review.googlesource.com/c/sdk/+/294130/8 and
prevents confusing shadowing of dart:js_interop annotations like we do
today.
## Pre-launch Checklist
- [X] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [X] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [X] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [X] I signed the [CLA].
- [Mentioned CL that is unblocked] I listed at least one issue that this
PR fixes in the description above.
- [X] I updated/added relevant documentation (doc comments with `///`).
- [Need test-exemption] I added new tests to check the change I am
making, or this PR is [test-exempt].
- [X] All existing and new tests are passing.
These keep turning up, so I did a little bash-fu to find them all.
```bash
grep -lL "#pragma once" $(grep -lL "#ifndef .*_H_" $(find . | grep "\.h$")) | cut -c 3-
```
No change in functionality. I've just renamed the TU's to replace `display_list_` to `dl_`. I am sure there are chances to make the naming better. For instance, the op receiver still being named a dispatcher.
Remove unnecessary instrumentation from picture. `UniqueRef`
automatically instruments all allocations, disposals, and garbage
collections, which covers `CkPicture`.
Does two things:
- Exposes a string about the GPU model on `impeller::Context`
- passes that string on to Skia gold when we add a test image.
This should help reduce noise/flakiness in golden images.
With this PR we no longer need to hold DisplayLists in GPUObject wrappers and they can be disposed instantly instead of queueing on the Unref thread.
This will definitely be a win for Impeller as none of the objects used in a frame now require queueing, but the performance impact on apps running on top of skia is less clear if they depend on a lot of images inside their DisplayLists that still need to be queued to be freed. After getting further in the work, it looks like only decoded images need to use the protected DlImage wrappers and most of those should survive many frames before they are disposed. That should hopefully leave very few unrefs happening per frame.
~There are 3 unit tests in `shell_unittests.cc` and `embedder_metal_unittests.mm` that are now GSKIP'd as they now invoke code that needs a fully initialized UIDartState in order to protect their images. I will look into fixing the tests and/or making the code they invoke provide protection without relying on UIDartState.~ (This looks to be fixed in the latest commit by simply not creating DlImageGPUs all over the source base and simply catching only those that end up in UI data structures. There is actually existing code in one of the modules that feeds ui.Image with an answer to wrap the image in a DlImageGPU if it has a skia image anyway, so most of these additional uses of DlImageGPU that were having trouble getting the Skia unref queue just didn't need it anyway.)
Previous attempt was here: https://github.com/flutter/engine/pull/40846
I was including un-rewritten source files, which caused breakage. Now we run out `ui_web` files through the sdk_rewriter script before putting them into sky_engine
Migrate Paint API to `UniqueRef`. This includes `Paint`, `ImageFilter` (and all subtypes), `ColorFilter` (and all subtypes).
Also fix the following memory leaks:
* `CkPaint` is frequently used by layers where a one-off paint object is created, used, and immediately dropped. `CkPaint` now has a `dispose` method, and all one-off usages now dispose of the paint after they are done.
* `CkColorFilter.initRawImageFilter` was leaking the `SkColorFilter` created by `_initRawColorFilter` inside the expression.
* `CkManagedSkImageFilterConvertible.imageFilter` now takes a closure, which allows the implementation decide on the lifetime of the `SkImageFilter` vended to the caller. Because `CkColorFilter` is a const class it cannot store C++ instances inside its own fields, so it creates a temporary `SkImageFilter` class to be used by the caller and then it needs to delete it. Now it does.
Fixes https://github.com/flutter/flutter/issues/124269
Manually verified this is safe on an iPhone 11.
We're not creating/encoding command buffers in this path so it's ok.
AFAICT the test I added would fail if we started doing that because it
doesn't provide any real command buffer interfaces. Most of the code
here is related to tests.
(this is attempt 4; details below)
Remove obsolete object caches and introduce a simpler way to manage
native objects:
* Remove the unused `SynchronousSkiaObjectCache`.
* Introduce new library `native_memory.dart` that's smaller and simpler
than `skia_object_cache.dart`.
* Introduce two types of native object references:
* `UniqueRef` a reference with a unique Dart object owner.
* `CountedRef` a ref-counted reference with multiple Dart object owners.
* All native references use GC (via `FinalizationRegistry`) as a
back-up.
* The new library removes everything related to object resurrection that
was needed only in browsers that didn't support `FinalizationRegistry`.
All browsers support it now.
* Remove the ad hoc `SkParagraph` cache that predates the introduction
of `Paragraph.dispose`.
* Rewrite `CkParagraph` in terms of `UniqueRef`.
* Rewrite `CkImage` in terms of `CountedRef`; delete `SkiaObjectBox`.
This PR does not migrate all objects from the old
`skia_object_cache.dart` to `native_memory.dart`. That would be too big
of a change. The migration can be done in multiple smaller PRs.
This also removes a few unnecessary relayouts observed in
https://github.com/flutter/flutter/issues/120921, but not all of them
(more details in
https://github.com/flutter/flutter/issues/120921#issuecomment-1481958762)
## About attempt 4
More info about the revert of attempt 3 in
https://github.com/flutter/engine/pull/40937.
In this attempt I check that the browser supports `FinalizationRegistry`
before registering the object. This will allow the code to run in older
browsers, but there will be no protection from memory leaks when the app
fails to dispose of the respective objects.
## Benchmarks
Now that this landed in flutter/flutter I have some benchmark numbers
from the devicelab. The `text_out_of_picture_bounds` benchmark dropped
by 3-4x (lower is better):
<img width="358" alt="Screenshot 2023-04-04 at 6 13 06 PM"
src="https://user-images.githubusercontent.com/211513/229956170-a5399ed3-c779-4af0-babb-ea40440f96ff.png">
The repro provided in https://github.com/flutter/flutter/issues/123204
dropped from 110ms/frame to 10ms/frame.
(this is attempt 3; details below)
Remove obsolete object caches and introduce a simpler way to manage
native objects:
* Remove the unused `SynchronousSkiaObjectCache`.
* Introduce new library `native_memory.dart` that's smaller and simpler
than `skia_object_cache.dart`.
* Introduce two types of native object references:
* `UniqueRef` a reference with a unique Dart object owner.
* `CountedRef` a ref-counted reference with multiple Dart object owners.
* All native references use GC (via `FinalizationRegistry`) as a
back-up.
* The new library removes everything related to object resurrection that
was needed only in browsers that didn't support `FinalizationRegistry`.
All browsers support it now.
* Remove the ad hoc `SkParagraph` cache that predates the introduction
of `Paragraph.dispose`.
* Rewrite `CkParagraph` in terms of `UniqueRef`.
* Rewrite `CkImage` in terms of `CountedRef`; delete `SkiaObjectBox`.
This PR does not migrate all objects from the old
`skia_object_cache.dart` to `native_memory.dart`. That would be too big
of a change. The migration can be done in multiple smaller PRs.
This also removes a few unnecessary relayouts observed in
https://github.com/flutter/flutter/issues/120921, but not all of them
(more details in
https://github.com/flutter/flutter/issues/120921#issuecomment-1481958762)
## About attempt 3
More about [attempt 2
here](https://github.com/flutter/engine/pull/40862).
In this attempt 3 I'm replacing the `factory` with a top-level function.
Reverts flutter/engine#40862
Google Testing is failing on
```
The compiler crashed: root:🎯_engine::SkObjectFinalizationRegistry::@methods::|staticInteropFactoryStub is already bound to Reference to dart:_engine::SkObjectFinalizationRegistry::@methods::|staticInteropFactoryStub, trying to bind to Reference to SkObjectFinalizationRegistry.|staticInteropFactoryStub with node SkObjectFinalizationRegistry.|staticInteropFactoryStub (Procedure:1207727)
```
