[web] remove obsolete object caches; simplify native object management #40862
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(this is attempt 2; details below)
Remove obsolete object caches and introduce a simpler way to manage native objects:
SynchronousSkiaObjectCache
.native_memory.dart
that's smaller and simpler thanskia_object_cache.dart
.UniqueRef
a reference with a unique Dart object owner.CountedRef
a ref-counted reference with multiple Dart object owners.FinalizationRegistry
) as a back-up.FinalizationRegistry
. All browsers support it now.SkParagraph
cache that predates the introduction ofParagraph.dispose
.CkParagraph
in terms ofUniqueRef
.CkImage
in terms ofCountedRef
; deleteSkiaObjectBox
.This PR does not migrate all objects from the old
skia_object_cache.dart
tonative_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 flutter/flutter#120921, but not all of them (more details in flutter/flutter#120921 (comment))
About attempt 2
This is the second attempt to land this. The first attempt (#40617) was reverted (#40861).
The issue with the first attempt was in the expression
SkObjectFinalizationRegistry((UniqueRef<Object> uniq) {...}.toJS)
, which as of right now is not supported by dart2js. This second iteration usesjs_util.callConstructor
to invoke the constructor. I'm not sure why it didn't trigger in our tests, but I suspect the difference is in how dart2js handles code between pre-rewritten SDK and post-rewritten SDK.