[web] fix SkFinalizationRegistry for dart2js (attempt 4)

[yjbanov]

(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 flutter/flutter#120921, but not all of them (more details in flutter/flutter#120921 (comment))

About attempt 4

More info about the revert of attempt 3 in #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):

The repro provided in flutter/flutter#123204 dropped from 110ms/frame to 10ms/frame.

[flutter-dashboard]

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #40938 at sha eb662fc

[eyebrowsoffire]

I mentioned this offline, but perhaps it would be a bit less fragile to do the following:

• Make this variable nullable
• Guard the construction of this object by browserSupportsFinalizationRegistry, otherwise return null
• Have any uses of this object (i.e. register below) do a null check instead of browserSupportsFinalizationRegistry

That feels a little less fragile, because right now if someone accidentally touches this variable without checking browserSupportsFinalizationRegistry first they could cause things to explode. A nullable variable would let the static type system save you from accidentally doing that.

[ditman]

Is this _finalizationRegistry ever intended to be used from outside the NativeMemoryFinalizationRegistry facade though? If it's only used from the facade, I think this is safe enough as is (famous last words? :P)

[yjbanov]

Happy to fix it as a follow-up. For now there's no risk of this variable being touched by accident, because it's private. But in general, I like the idea of explicit lazy initialization.

[eyebrowsoffire]

Also I wonder if it would be worthwhile to emit a warning to console if FinalizationRegistry is not supported?

[ditman]

Also I wonder if it would be worthwhile to emit a warning to console if FinalizationRegistry is not supported?

I think this is a warning worth adding, especially because FinalizationRegistry is fairly recent on iOS Safari (14.5 according to MDN).

[ditman]

RSLGTM, good luck! (Please create an issue with whatever fast-follows you want to do on top of this PR, like Jackson's null-safety suggestion or adding the warning)

[eyebrowsoffire]

My comments are not blocking, we can always follow up if needed.

[yjbanov]

Also I wonder if it would be worthwhile to emit a warning to console if FinalizationRegistry is not supported?

That's a good idea. I'll do it as a follow-up. I realized that we should have similar warnings when FinalizationRegistry detects objects that were not disposed of, which I can add as a feature of UniqueRef and CountedRef.

[yjbanov]

Flutter Gold stuff is a know Impeller flakines, so merging manually.