Document //flutter/runtime/dart_snapshot.h #13196

chinmaygarde · 2019-10-17T03:43:47Z

jason-simmons · 2019-10-17T16:47:22Z

runtime/dart_snapshot.h

+///             the host snapshot the state of the Dart heap at specific points
+///             and package the same with the Flutter application. When the Dart
+///             VM on the target is configured to run AOT compiled Dart code,
+///             the tools also compile the developers Flutter application code


typo: "developers" -> "developer's"

jason-simmons · 2019-10-17T16:47:46Z

runtime/dart_snapshot.h

+///             to target specific machine code (instructions). This class deals
+///             with the mapping of both these buffers at runtime on the target.
+///
+///             A flutter application typically needs two instances of this


typo: "flutter" -> "Flutter"

jason-simmons · 2019-10-17T16:48:32Z

runtime/dart_snapshot.h

+///             These snapshots are typically memory-mapped at runtime, or,
+///             referenced directly as symbols present in Mach or ELF binaries.
+///
+///             In case of the core snapshot, the snapshot is collected when the


typo: "In case" -> "In the case"

jason-simmons · 2019-10-17T16:48:54Z

runtime/dart_snapshot.h

+///
+///             A flutter application typically needs two instances of this
+///             class at runtime to run Dart code. One instance is for the VM
+///             and is also called the "core snapshot". The other is the isolate


remove "also"

jason-simmons · 2019-10-17T16:57:24Z

runtime/dart_snapshot.h

 public:
+  //----------------------------------------------------------------------------
+  /// When referencing the heap component of the core snapshot via the dynamic
+  /// linker, the symbols name is expected to be this constant.


typo: "symbols" -> "symbol's"
(same for the other snapshot symbols)

I think this would be clearer with phrasing like: "The symbol name of the heap data in the core snapshot dynamic library"

jason-simmons · 2019-10-17T17:15:50Z

runtime/dart_snapshot.h

+  ///             the core snapshot.
+  ///
+  /// @attention  Depending on the runtime mode of the Flutter application and
+  ///             the target the Flutter is running on, a complex fallback


typo: "target the" -> "target that"

(also applies to VMSnapshotFromSettings and IsolateSnapshotFromSettings)

chinmaygarde

Addressed all comments.

chinmaygarde · 2019-10-17T19:46:14Z

runtime/dart_snapshot.h

+///             the host snapshot the state of the Dart heap at specific points
+///             and package the same with the Flutter application. When the Dart
+///             VM on the target is configured to run AOT compiled Dart code,
+///             the tools also compile the developers Flutter application code


chinmaygarde · 2019-10-17T19:46:47Z

runtime/dart_snapshot.h

+///
+///             A flutter application typically needs two instances of this
+///             class at runtime to run Dart code. One instance is for the VM
+///             and is also called the "core snapshot". The other is the isolate


chinmaygarde · 2019-10-17T19:47:02Z

runtime/dart_snapshot.h

+///             These snapshots are typically memory-mapped at runtime, or,
+///             referenced directly as symbols present in Mach or ELF binaries.
+///
+///             In case of the core snapshot, the snapshot is collected when the


chinmaygarde · 2019-10-17T19:49:20Z

runtime/dart_snapshot.h

 public:
+  //----------------------------------------------------------------------------
+  /// When referencing the heap component of the core snapshot via the dynamic
+  /// linker, the symbols name is expected to be this constant.


chinmaygarde · 2019-10-17T19:49:53Z

runtime/dart_snapshot.h

+  ///             the core snapshot.
+  ///
+  /// @attention  Depending on the runtime mode of the Flutter application and
+  ///             the target the Flutter is running on, a complex fallback


)

[email protected]:flutter/engine.git/compare/508146f0defb...7a621a7 git log 508146f..7a621a7 --no-merges --oneline 2019-10-18 [email protected] Roll src/third_party/skia 20eafffd2d2f..b80d31f8cbe2 (4 commits) (flutter/engine#13226) 2019-10-18 [email protected] Roll src/third_party/skia da29d70f1a59..20eafffd2d2f (1 commits) (flutter/engine#13223) 2019-10-18 [email protected] Roll src/third_party/skia 63a387395751..da29d70f1a59 (11 commits) (flutter/engine#13221) 2019-10-18 [email protected] Roll fuchsia/sdk/core/linux-amd64 from WpvU_... to _G94w... (flutter/engine#13220) 2019-10-18 [email protected] Specify a human readable reason for an error from the embedder API. (flutter/engine#13218) 2019-10-18 [email protected] Roll fuchsia/sdk/core/mac-amd64 from _9Uy_... to KNygX... (flutter/engine#13219) 2019-10-17 [email protected] Reland ICU update to 64.2 (flutter/engine#13216) 2019-10-17 [email protected] Use `window.devicePixelRatio` in the CanvasKit backend (flutter/engine#13192) 2019-10-17 [email protected] Re-enable WeakPtr ThreadChecker and fix associated failures (flutter/engine#12257) 2019-10-17 [email protected] Re-land "Custom compositor layers must take into account the device pixel ratio." 2019-10-17 [email protected] Add trace events around custom compositor callbacks. (flutter/engine#13212) 2019-10-17 [email protected] Roll src/third_party/skia 93e853bf2b83..63a387395751 (9 commits) (flutter/engine#13208) 2019-10-17 [email protected] Roll src/third_party/dart 9b3c7f64d8..a61c775db8 (5 commits) 2019-10-17 [email protected] Document //flutter/runtime/dart_snapshot.h (flutter/engine#13196) 2019-10-17 [email protected] Revert "Custom compositor layers must take into account the device pixel ratio. (#13193)" (flutter/engine#13211) 2019-10-17 [email protected] wrap the text in text editing. This was causing a missalingment issue in textarea. (flutter/engine#13207) 2019-10-17 [email protected] Custom compositor layers must take into account the device pixel ratio. (flutter/engine#13193) 2019-10-17 [email protected] [web] Environment variable to disable felt snapshot (flutter/engine#13187) 2019-10-17 [email protected] Roll src/third_party/dart 9e636b5ab4..9b3c7f64d8 (5 commits) 2019-10-17 [email protected] Roll src/third_party/skia 0df7697235b4..93e853bf2b83 (1 commits) (flutter/engine#13205) 2019-10-17 [email protected] Roll fuchsia/sdk/core/linux-amd64 from ek5iQ... to WpvU_... (flutter/engine#13203) 2019-10-17 [email protected] Roll fuchsia/sdk/core/mac-amd64 from 6j3Gw... to _9Uy_... (flutter/engine#13202) 2019-10-17 [email protected] Roll src/third_party/skia 6a19e03047cc..0df7697235b4 (1 commits) (flutter/engine#13200) 2019-10-17 [email protected] Roll src/third_party/dart 1e3e9ee04c..9e636b5ab4 (9 commits) 2019-10-17 [email protected] Roll src/third_party/skia f29cb70281d5..6a19e03047cc (5 commits) (flutter/engine#13198) 2019-10-17 [email protected] Roll src/third_party/dart f020ce5d23..1e3e9ee04c (12 commits) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

[email protected]:flutter/engine.git/compare/508146f0defb...7a621a7 git log 508146f..7a621a7 --no-merges --oneline 2019-10-18 [email protected] Roll src/third_party/skia 20eafffd2d2f..b80d31f8cbe2 (4 commits) (flutter/engine#13226) 2019-10-18 [email protected] Roll src/third_party/skia da29d70f1a59..20eafffd2d2f (1 commits) (flutter/engine#13223) 2019-10-18 [email protected] Roll src/third_party/skia 63a387395751..da29d70f1a59 (11 commits) (flutter/engine#13221) 2019-10-18 [email protected] Roll fuchsia/sdk/core/linux-amd64 from WpvU_... to _G94w... (flutter/engine#13220) 2019-10-18 [email protected] Specify a human readable reason for an error from the embedder API. (flutter/engine#13218) 2019-10-18 [email protected] Roll fuchsia/sdk/core/mac-amd64 from _9Uy_... to KNygX... (flutter/engine#13219) 2019-10-17 [email protected] Reland ICU update to 64.2 (flutter/engine#13216) 2019-10-17 [email protected] Use `window.devicePixelRatio` in the CanvasKit backend (flutter/engine#13192) 2019-10-17 [email protected] Re-enable WeakPtr ThreadChecker and fix associated failures (flutter/engine#12257) 2019-10-17 [email protected] Re-land "Custom compositor layers must take into account the device pixel ratio." 2019-10-17 [email protected] Add trace events around custom compositor callbacks. (flutter/engine#13212) 2019-10-17 [email protected] Roll src/third_party/skia 93e853bf2b83..63a387395751 (9 commits) (flutter/engine#13208) 2019-10-17 [email protected] Roll src/third_party/dart 9b3c7f64d8..a61c775db8 (5 commits) 2019-10-17 [email protected] Document //flutter/runtime/dart_snapshot.h (flutter/engine#13196) 2019-10-17 [email protected] Revert "Custom compositor layers must take into account the device pixel ratio. (flutter#13193)" (flutter/engine#13211) 2019-10-17 [email protected] wrap the text in text editing. This was causing a missalingment issue in textarea. (flutter/engine#13207) 2019-10-17 [email protected] Custom compositor layers must take into account the device pixel ratio. (flutter/engine#13193) 2019-10-17 [email protected] [web] Environment variable to disable felt snapshot (flutter/engine#13187) 2019-10-17 [email protected] Roll src/third_party/dart 9e636b5ab4..9b3c7f64d8 (5 commits) 2019-10-17 [email protected] Roll src/third_party/skia 0df7697235b4..93e853bf2b83 (1 commits) (flutter/engine#13205) 2019-10-17 [email protected] Roll fuchsia/sdk/core/linux-amd64 from ek5iQ... to WpvU_... (flutter/engine#13203) 2019-10-17 [email protected] Roll fuchsia/sdk/core/mac-amd64 from 6j3Gw... to _9Uy_... (flutter/engine#13202) 2019-10-17 [email protected] Roll src/third_party/skia 6a19e03047cc..0df7697235b4 (1 commits) (flutter/engine#13200) 2019-10-17 [email protected] Roll src/third_party/dart 1e3e9ee04c..9e636b5ab4 (9 commits) 2019-10-17 [email protected] Roll src/third_party/skia f29cb70281d5..6a19e03047cc (5 commits) (flutter/engine#13198) 2019-10-17 [email protected] Roll src/third_party/dart f020ce5d23..1e3e9ee04c (12 commits) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

Document //flutter/runtime/dart_snapshot.h

2253410

Related to flutter/flutter#42778

chinmaygarde requested review from cbracken and jason-simmons October 17, 2019 03:43

googlebot added the cla: yes label Oct 17, 2019

jason-simmons reviewed Oct 17, 2019

View reviewed changes

Address PR comments.

6be54ca

chinmaygarde commented Oct 17, 2019

View reviewed changes

jason-simmons approved these changes Oct 17, 2019

View reviewed changes

chinmaygarde merged commit c147edd into flutter:master Oct 17, 2019

chinmaygarde deleted the document_dartsnapshot branch October 17, 2019 20:20

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 18, 2019

c147edd Document //flutter/runtime/dart_snapshot.h (flutter/engine#13196

545c8d9

)

engine-flutter-autoroll mentioned this pull request Oct 18, 2019

Roll engine 508146f0defb..7a621a7f7e4f (26 commits) flutter/flutter#43017

Merged

Document //flutter/runtime/dart_snapshot.h #13196

Document //flutter/runtime/dart_snapshot.h #13196

Uh oh!

Conversation

chinmaygarde commented Oct 17, 2019

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants