Reland "Remove usage of Dart_AllocateWithNativeFields" #16713

dnfield · 2020-02-20T21:04:56Z

See #16684 for revert

This relands the original change with the following additions:

Cache a persistent handle to the private constructor name to avoid spamming the timeline and creating unnecessarily duplicate strings.
Avoid the slower Dart_New path for high touch objects that we need every frame - Picture, EngineLayer, and Scene.

We should be able to extend this approach to other objects, but they'll require bigger refactors. I've started the work for Path, Image, and Paragraph and am convinced this approach is extensible. However, I'd like to land those in separate patches to avoid needing to revert these more critical parts if I introduce regressions there.

This expected to have a small regression on some of our performance benchmarks, but it should be much smaller than the original patch.

jason-simmons · 2020-02-20T21:12:36Z

lib/ui/compositing.dart

    assert(_debugPushLayer(layer));
    return layer;
  }

-  EngineLayer _pushTransform(Float64List matrix4) native 'SceneBuilder_pushTransform';
+  void _pushTransform(EngineLayer layer, Float64List matrix4) native 'SceneBuilder_pushTransform';


Can we adopt a convention to indicate that these are output parameters of the native method? (e.g. prefixing these parameter names with out_?)

How about just out iwthout the underscore? Snake case looks weird in Dart.

But that sounds fine to me.

out with no underscore SGTM

bkonyi

Dart API usage looks good to me.

bkonyi · 2020-02-20T21:21:44Z

lib/ui/painting.dart

+  // private constructor.
+  // TODO(dnfield): rework this to use ClaimNativeField - https://github.com/flutter/flutter/issues/50997
+  @pragma('vm:entry-point')
+  Path._() { _constructor(); }


Super nitty, but can't this be re-written as the following?

Path._() => _constructor();

I suppose - I'll update it. I'll have another patch coming where this will just become Path._();.

Actually - this matches the style of a few lines above. I'd rather just keep it consistent.

bkonyi · 2020-02-20T21:23:52Z

lib/ui/painting/picture_recorder.cc

-  fml::RefPtr<Picture> picture = Picture::Create(UIDartState::CreateGPUObject(
-      picture_recorder_.finishRecordingAsPicture()));
+  fml::RefPtr<Picture> picture =
+      Picture::Create(std::move(dart_picture),


My understanding of std::move is quite limited, but is this necessary here and elsewhere? This is simply a Dart_Handle which is just a pointer.

It's probably not necessary here then - I default to using move whenever possible at this point.

@chinmaygarde wdyt?

Here and elsewhere, it is redundant. clang-tidy will warn about this. I'd prefer not adding this unnecessarily as we already delete copy constructors on objects that we'd have folks rather not copy.

Done - I think I got them all.

bkonyi · 2020-02-20T21:24:59Z

third_party/tonic/dart_state.cc

+
+  private_constructor_name_.Clear();
+  Dart_EnterScope();
+  private_constructor_name_.Set(this, Dart_NewPersistentHandle(Dart_NewStringFromCString("_")));


Is there any reason why the string "_" can't be cached rather than creating it for each DartState?

This is tieing it to the lifetime of the isolate. We don't want to leak it if the isolate changes.

bkonyi · 2020-02-20T21:25:37Z

third_party/tonic/dart_wrappable.cc

@@ -15,27 +15,48 @@ DartWrappable::~DartWrappable() {
  TONIC_CHECK(!dart_wrapper_);
 }

+// TODO(dnfield): Delete this. https://github.com/flutter/flutter/issues/50997


Can we deprecate this so we ensure no one else starts using it?

I don't think anyone is directly using it, and I was assuming that if I deprecate it now it would cause build errors. I'm going to keep focusing on removing all references to it over the next week or so and just get it down. It's not something we typically add more usages of.

chinmaygarde

Sound direction but this API can be made foolproof against accidental leaks by introducing an RAII wrapper that claims the handle automatically and have that be owned by the the object itself instead of making the caller explicitly call ClaimDartHandle.

chinmaygarde · 2020-02-20T21:37:56Z

lib/ui/painting/picture_recorder.cc

-  fml::RefPtr<Picture> picture = Picture::Create(UIDartState::CreateGPUObject(
-      picture_recorder_.finishRecordingAsPicture()));
+  fml::RefPtr<Picture> picture =
+      Picture::Create(std::move(dart_picture),


Here and elsewhere, it is redundant. clang-tidy will warn about this. I'd prefer not adding this unnecessarily as we already delete copy constructors on objects that we'd have folks rather not copy.

chinmaygarde · 2020-02-20T21:39:55Z

third_party/tonic/dart_state.h

@@ -49,6 +49,11 @@ class DartState : public std::enable_shared_from_this<DartState> {
  Dart_Isolate isolate() { return isolate_; }
  void SetIsolate(Dart_Isolate isolate);

+  // TODO(dnfield): delete this https://github.com/flutter/flutter/issues/50997


I don't believe we are getting that from Dart. This workaround is the way to go for speed.

My thought is to repurpose the bug for tracking knocking this down - since it has all the context.

chinmaygarde · 2020-02-20T21:41:26Z

third_party/tonic/dart_wrappable.h

  Dart_Handle CreateDartWrapper(DartState* dart_state);
+  void ClaimDartHandle(Dart_Handle wrappable);


Prefer an RAII wrapper that takes a handle and claims it on collection. It is easy to forget claiming this handle and cause a leak.

You'll have to rewrite the ClaimDartHandle routine to ignore null wrappers.

How would the RAII wrapper work? ClaimDartHandle binds a C++ DartWrappable object to a Dart object. The binding lasts until the Dart object is garbage collected, at which point the C++ object is dereffed.

ClaimDartHandle is similar to the existing DartWrappable::AssociateWithDartWrapper method used during construction of Dart objects from C++. I'd recommend replacing AssociateWithDartWrapper with ClaimDartHandle. DartCallConstructor would then need to be changed to extract the Dart this reference and pass it to ClaimDartHandle.

Same question.

The DartWrappable will claim the handle on destruction currently, but we still need to tell it at some point about the Dart_Handle that it needs to track as its Dart peer.

Ah, I read "Claim" as "Collect" or "Destroy" instead of "claim as a peer".

If there's a better name I'm for it - Associate was already taken, and does something a bit different - and I didn't want to break it in the mean time.

Maybe SetDartPeer?

Or just keep the name AssociateWithDartWrapper and change it to take a Dart_Handle. The part of AssociateWithDartWrapper that grabs the Dart_Handle from the Dart_NativeArguments would then be moved into DartCallConstructor.

Ahhh ok, I see now. That SG.

Argh. I forgot I had the lable on this. I'll open a new PR with that rename applied.

chinmaygarde · 2020-02-20T21:43:26Z

third_party/tonic/dart_state.h

@@ -49,6 +49,11 @@ class DartState : public std::enable_shared_from_this<DartState> {
  Dart_Isolate isolate() { return isolate_; }
  void SetIsolate(Dart_Isolate isolate);

+  // TODO(dnfield): delete this https://github.com/flutter/flutter/issues/50997


Nit: TODO(issue name or URL): Description of why this is present and when to remove it.. Assignments are fluid and we can get those from the attached issue.

Style guide still says to use this format: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#avoid-checking-in-comments-that-ask-questions

Last time this came up, there wasn't much consensus around how to best handle it :\

We follow https://google.github.io/styleguide/cppguide.html#TODO_Comments for C++. Since you added both the issue and your name, I felt the third format more relevant as your ldap was redundant if the issue was linked and the comment could have done with more context on when to remove.

Alright, will update.

…er/engine#16713)

* 5b0cbbe Roll fuchsia/sdk/core/mac-amd64 from _jvYk... to WZgbp... (flutter/engine#16692) * cbb0ff8 Roll src/third_party/skia c5ff41f2976e..7dfb46e7f397 (20 commits) (flutter/engine#16691) * ab454ea Roll src/third_party/dart 0f141be8bd52..7469b87b042a (9 commits) (flutter/engine#16693) * 1b73784 fix param (flutter/engine#16694) * bdd2cf8 Roll src/third_party/skia 7dfb46e7f397..9baef3593c3c (3 commits) (flutter/engine#16696) * 8f0bbfa Roll src/third_party/skia 9baef3593c3c..ed1ff23c2768 (5 commits) (flutter/engine#16699) * f052d2e Roll fuchsia/sdk/core/linux-amd64 from VHyDa... to YPr0t... (flutter/engine#16701) * 3fe3325 Roll src/third_party/dart 7469b87b042a..e187e42593e8 (11 commits) (flutter/engine#16702) * f83092a Roll src/third_party/skia ed1ff23c2768..a5097354217b (1 commits) (flutter/engine#16703) * 6b3d439 Roll src/third_party/skia a5097354217b..2c2db2762809 (1 commits) (flutter/engine#16704) * fdabcad Enable lazy-async-stacks by-default in all modes (flutter/engine#16556) * 02aa865 Fix the newline on some keyboards (flutter/engine#16560) * cde7d47 Roll src/third_party/dart e187e42593e8..81d4cc6bc99a (3 commits) (flutter/engine#16705) * 14a2b03 Roll fuchsia/sdk/core/mac-amd64 from WZgbp... to 78ZcV... (flutter/engine#16706) * 7b0453e Roll src/third_party/skia 2c2db2762809..9d4e31d6cda5 (1 commits) (flutter/engine#16707) * 5cef6d0 Roll fuchsia/sdk/core/linux-amd64 from YPr0t... to CZTpy... (flutter/engine#16708) * 0ef67b5 opt out dart:ui from nnbd (flutter/engine#16473) * bc4a27f [shell tests] Integrate Vulkan with Shell Tests (flutter/engine#16621) * 2fdba52 Roll src/third_party/skia 9d4e31d6cda5..62076977a0b7 (11 commits) (flutter/engine#16710) * 969cfc1 Roll src/third_party/skia 62076977a0b7..1d589a578ca4 (6 commits) (flutter/engine#16712) * 8eb727e Flush the SkCanvas when submitting a frame in ShellTestPlatformViewVulkan::OffscreenSurface (flutter/engine#16717) * e44f99b Roll src/third_party/skia 1d589a578ca4..706851dc99d9 (2 commits) (flutter/engine#16714) * 60b27fd Reland "Remove usage of Dart_AllocateWithNativeFields" (flutter/engine#16713) * 7a50e4d Roll src/third_party/dart 81d4cc6bc99a..fd20c7b92bb8 (31 commits) (flutter/engine#16716) * 4aaafe0 Roll src/third_party/skia 706851dc99d9..df283d01cabb (3 commits) (flutter/engine#16719) * e18aba3 Refactor of ClaimDartHandle -> AssociateWithDartWrapper (flutter/engine#16720) * 66742b6 Roll src/third_party/skia df283d01cabb..3ffa7af75301 (1 commits) (flutter/engine#16722) * e0303b0 Roll src/third_party/dart fd20c7b92bb8..6ef9131d82c4 (7 commits) (flutter/engine#16723) * 0bd69f9 Roll src/third_party/skia 3ffa7af75301..77521d9e06e8 (2 commits) (flutter/engine#16725) * 3e69286 Roll fuchsia/sdk/core/mac-amd64 from 78ZcV... to iYYAH... (flutter/engine#16726) * 704396b Roll fuchsia/sdk/core/linux-amd64 from CZTpy... to -u-iU... (flutter/engine#16727) * 645d4e6 Roll src/third_party/dart 6ef9131d82c4..5829fc7829d5 (3 commits) (flutter/engine#16728) * b73dfed Roll src/third_party/skia 77521d9e06e8..392846665c40 (1 commits) (flutter/engine#16729) * 2724727 Roll src/third_party/skia 392846665c40..bf5cb0f539e7 (1 commits) (flutter/engine#16730) * ab0dd12 [web] Running safari tests on LUCI (flutter/engine#16715) * e5091a8 Enable Vulkan-related shell unittests on Fuchsia (flutter/engine#16718) * 930a80a Fix some compiler warnings in newer versions of Clang. (flutter/engine#16733) * 941aee3 [web] add comment to skipped safari test (flutter/engine#16737) * 136a057 [web] Rename LineMetrics.text to LineMetrics.displayText (flutter/engine#16734) * afe7968 [web] Paragraph.longestLine doesn't need to check for `isSingleLine` anymore (flutter/engine#16736) * c3f4c1a Migrate Path to AssociateWithDartWrapper (flutter/engine#16753) * d0c2418 Add support for Increase Contrast on iOS (flutter/engine#15343) * 6d2cbb2 Roll src/third_party/skia bf5cb0f539e7..46f5c5f08b61 (2 commits) (flutter/engine#16732) * f758eb9 Roll fuchsia/sdk/core/linux-amd64 from -u-iU... to 3rB22... (flutter/engine#16752) * a4a1f4f Roll fuchsia/sdk/core/mac-amd64 from iYYAH... to 5B5jq... (flutter/engine#16744) * 8caf6d1 Roll src/third_party/skia 46f5c5f08b61..9e8f60534464 (29 commits) (flutter/engine#16754) * 340f855 Roll fuchsia/sdk/core/mac-amd64 from 5B5jq... to g1vJn... (flutter/engine#16755) * 9a9abb7 Roll src/third_party/skia 9e8f60534464..d1c90e10f0ca (1 commits) (flutter/engine#16757) * 2e12cdc Roll src/third_party/skia d1c90e10f0ca..998066127e0d (1 commits) (flutter/engine#16759) * 566cfae Roll src/third_party/skia 998066127e0d..57bc977e124c (3 commits) (flutter/engine#16762) * 73fdff1 Roll fuchsia/sdk/core/linux-amd64 from 3rB22... to PGfiE... (flutter/engine#16763) * ecca175 Roll fuchsia/sdk/core/mac-amd64 from g1vJn... to mcI8X... (flutter/engine#16764) * 4d50517 Roll src/third_party/skia 57bc977e124c..cc5415a8ce36 (1 commits) (flutter/engine#16767) * 237ddb1 Roll src/third_party/skia cc5415a8ce36..1cec4d5e3d92 (2 commits) (flutter/engine#16769) * 8da64e0 Roll src/third_party/dart 5829fc7829d5..c75256299280 (43 commits) (flutter/engine#16770) * 0d87160 Roll src/third_party/skia 1cec4d5e3d92..7d252302268a (2 commits) (flutter/engine#16771) * 1aef7a4 Delete FlutterAppDelegate_Internal.h (flutter/engine#16772) * b87bb0a [web] Fix canvas leak when dpi changes. Evict from BitmapCanvas cache under… (flutter/engine#16721) * e0e24f5 Roll src/third_party/skia 7d252302268a..03b8ab225fd7 (8 commits) (flutter/engine#16773) * 38dc2ea [i18n] Deprecates fuchsia.timezone.Timezone (flutter/engine#16700) * 92abb22 Reland "Lift restriction that embedders may not trample the render thread OpenGL context in composition callbacks." (flutter/engine#16711) * 971122b [web] Reduce the usage of unnecessary lists in pointer binding (flutter/engine#16745) * d670059 [web] Respect maxLines when calculating boxes for a range (flutter/engine#16749) * 5496bb4 Roll src/third_party/skia 03b8ab225fd7..659cc1c90705 (4 commits) (flutter/engine#16774) * bad1fbe Roll src/third_party/dart c75256299280..73f6d15665a3 (9 commits) (flutter/engine#16776) * d2e2cc9 Roll fuchsia/sdk/core/mac-amd64 from mcI8X... to O6w2L... (flutter/engine#16777) * 888a62c Revert "Enable lazy-async-stacks by-default in all modes (#16556)" (flutter/engine#16781)

…er#16713)" This reverts commit 32905c9.

Reland

9394ba2

dnfield requested review from chinmaygarde, jason-simmons and bkonyi February 20, 2020 21:04

auto-assign bot requested a review from iskakaushik February 20, 2020 21:04

googlebot added the cla: yes label Feb 20, 2020

dnfield mentioned this pull request Feb 20, 2020

Avoid calling Dart_New in engine - it's slow. flutter/flutter#50997

Closed

jason-simmons reviewed Feb 20, 2020

View reviewed changes

bkonyi approved these changes Feb 20, 2020

View reviewed changes

out

54a30ae

chinmaygarde suggested changes Feb 20, 2020

View reviewed changes

unnecessary moves

58758c7

chinmaygarde approved these changes Feb 20, 2020

View reviewed changes

dnfield added 2 commits February 20, 2020 14:21

more

eb08fd5

issue nit

6d19e8c

dnfield added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Feb 20, 2020

format

e4051f9

fluttergithubbot merged commit 60b27fd into flutter:master Feb 21, 2020

dnfield mentioned this pull request Feb 21, 2020

Refactor of ClaimDartHandle -> AssociateWithDartWrapper #16720

Merged

engine-flutter-autoroll mentioned this pull request Feb 21, 2020

Roll engine 84dc383b6428..7a50e4de9f89 (27 commits) flutter/flutter#51170

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2020

60b27fd Reland "Remove usage of Dart_AllocateWithNativeFields" (flutt…

ec5c4db

…er/engine#16713)

engine-flutter-autoroll mentioned this pull request Feb 21, 2020

Roll engine 84dc383b6428..4aaafe0d2862 (28 commits) flutter/flutter#51174

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2020

60b27fd Reland "Remove usage of Dart_AllocateWithNativeFields" (flutt…

7687d88

…er/engine#16713)

engine-flutter-autoroll mentioned this pull request Feb 21, 2020

Roll engine 84dc383b6428..66742b6d3ec2 (30 commits) flutter/flutter#51178

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2020

60b27fd Reland "Remove usage of Dart_AllocateWithNativeFields" (flutt…

a91c45e

…er/engine#16713)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2020

60b27fd Reland "Remove usage of Dart_AllocateWithNativeFields" (flutt…

467ef9a

…er/engine#16713)

This was referenced Feb 21, 2020

Roll engine 84dc383b6428..0bd69f9d5514 (32 commits) flutter/flutter#51181

Closed

Roll engine 84dc383b6428..3e692865fad0 (33 commits) flutter/flutter#51188

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2020

60b27fd Reland "Remove usage of Dart_AllocateWithNativeFields" (flutt…

784aa4a

…er/engine#16713)

engine-flutter-autoroll mentioned this pull request Feb 21, 2020

Roll engine 84dc383b6428..704396b3262a (34 commits) flutter/flutter#51192

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 24, 2020

60b27fd Reland "Remove usage of Dart_AllocateWithNativeFields" (flutt…

94a8b87

…er/engine#16713)

engine-flutter-autoroll mentioned this pull request Feb 24, 2020

Roll engine f2f8c342be32..73fdff1e2b6a (51 commits) flutter/flutter#51323

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 24, 2020

60b27fd Reland "Remove usage of Dart_AllocateWithNativeFields" (flutt…

1f62dad

…er/engine#16713)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 24, 2020

60b27fd Reland "Remove usage of Dart_AllocateWithNativeFields" (flutt…

65a5a75

…er/engine#16713)

This was referenced Feb 24, 2020

Roll engine f2f8c342be32..ecca1750d82b (52 commits) flutter/flutter#51328

Closed

Roll engine f2f8c342be32..4d505170aff1 (53 commits) flutter/flutter#51339

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 24, 2020

60b27fd Reland "Remove usage of Dart_AllocateWithNativeFields" (flutt…

f9aeae5

…er/engine#16713)

engine-flutter-autoroll mentioned this pull request Feb 24, 2020

Roll engine f2f8c342be32..237ddb1bec1a (54 commits) flutter/flutter#51343

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 24, 2020

60b27fd Reland "Remove usage of Dart_AllocateWithNativeFields" (flutt…

51b7aca

…er/engine#16713)

engine-flutter-autoroll mentioned this pull request Feb 24, 2020

Roll engine f2f8c342be32..8da64e067cbe (55 commits) flutter/flutter#51351

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 24, 2020

60b27fd Reland "Remove usage of Dart_AllocateWithNativeFields" (flutt…

80fc985

…er/engine#16713)

engine-flutter-autoroll mentioned this pull request Feb 24, 2020

Roll engine f2f8c342be32..0d87160bdf5a (56 commits) flutter/flutter#51360

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 24, 2020

60b27fd Reland "Remove usage of Dart_AllocateWithNativeFields" (flutt…

14a0ad2

…er/engine#16713)

engine-flutter-autoroll mentioned this pull request Feb 24, 2020

Roll engine f2f8c342be32..1aef7a43692a (57 commits) flutter/flutter#51361

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 24, 2020

60b27fd Reland "Remove usage of Dart_AllocateWithNativeFields" (flutt…

6c172a7

…er/engine#16713)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 24, 2020

60b27fd Reland "Remove usage of Dart_AllocateWithNativeFields" (flutt…

dc0a8dc

…er/engine#16713)

engine-flutter-autoroll mentioned this pull request Feb 24, 2020

Roll engine f2f8c342be32..92abb22c64d1 (61 commits) flutter/flutter#51367

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 25, 2020

60b27fd Reland "Remove usage of Dart_AllocateWithNativeFields" (flutt…

70e487d

…er/engine#16713)

This was referenced Feb 25, 2020

Roll engine f2f8c342be32..971122b314f1 (62 commits) flutter/flutter#51378

Closed

Roll engine f2f8c342be32..5496bb48c58f (64 commits) flutter/flutter#51381

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 25, 2020

60b27fd Reland "Remove usage of Dart_AllocateWithNativeFields" (flutt…

d6b70ff

…er/engine#16713)

engine-flutter-autoroll mentioned this pull request Feb 25, 2020

Roll engine f2f8c342be32..bad1fbe3ca6f (65 commits) flutter/flutter#51385

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 25, 2020

60b27fd Reland "Remove usage of Dart_AllocateWithNativeFields" (flutt…

0031b7f

…er/engine#16713)

engine-flutter-autoroll mentioned this pull request Feb 25, 2020

Roll engine f2f8c342be32..d2e2cc9d4551 (66 commits) flutter/flutter#51386

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 25, 2020

60b27fd Reland "Remove usage of Dart_AllocateWithNativeFields" (flutt…

56d76e2

…er/engine#16713)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 25, 2020

60b27fd Reland "Remove usage of Dart_AllocateWithNativeFields" (flutt…

fdbc029

…er/engine#16713)

engine-flutter-autoroll mentioned this pull request Feb 25, 2020

Roll engine f2f8c342be32..888a62cf2298 (67 commits) flutter/flutter#51391

Merged

NoamDev pushed a commit to NoamDev/engine that referenced this pull request Feb 27, 2020

Reland "Remove usage of Dart_AllocateWithNativeFields" (flutter#16713)

32905c9

NoamDev added a commit to NoamDev/engine that referenced this pull request Feb 27, 2020

Revert "Reland "Remove usage of Dart_AllocateWithNativeFields" (flutt…

0771ced

…er#16713)" This reverts commit 32905c9.

		Dart_Handle CreateDartWrapper(DartState* dart_state);
		void ClaimDartHandle(Dart_Handle wrappable);

Reland "Remove usage of Dart_AllocateWithNativeFields" #16713

Reland "Remove usage of Dart_AllocateWithNativeFields" #16713

Uh oh!

Conversation

dnfield commented Feb 20, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bkonyi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chinmaygarde left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!