Expose the Flutter engine, Dart and Skia versions to Dart. #7634

iskakaushik · 2019-01-30T02:01:06Z

Add get*Version methods to the Window class.
We pass the needed information from shell.

jason-simmons · 2019-01-30T02:21:20Z

lib/ui/window.dart

@@ -841,6 +841,11 @@ class Window {
  void _respondToPlatformMessage(int responseId, ByteData data)
      native 'Window_respondToPlatformMessage';

+  /// Return the version information for the engine, skia and dart.
+  String getFlutterEngineVersion() native 'Window_GetFlutterEngineVersion';


I think this could be simplified by merging these into one private native method that returns a list of version strings. Then wrap that with a public Dart method that unpacks the list into a Dart object with fields for each version.

That sounds better, will do it.

jason-simmons · 2019-01-30T02:34:21Z

runtime/runtime_controller.cc

@@ -326,7 +331,18 @@ RuntimeController::Locale::Locale(std::string language_code_,

 RuntimeController::Locale::~Locale() = default;

-RuntimeController::WindowData::WindowData() = default;
+RuntimeController::Versions::Versions(const char * dart_version_,


It should be possible to avoid defining this structure by adding a GetVersions method to the RuntimeDelegate interface implemented by the Engine.

RuntimeController can invoke the engine's GetVersions through the client_. The engine can then construct a WindowClient::Versions and return it through the chain of callers.

amirh · 2019-01-30T04:22:25Z

Are the versions a property of a window? Would it make sense to expose them as a top level dart:ui property instead?
If Flutter ends up supporting multiple windows(flutter/flutter#25526) I think exposing the engine version as a property of each window object might look a little weird.
I'm also wondering whether we will ever run a backgrounded app without a window object.

chinmaygarde · 2019-01-30T04:27:00Z

lib/ui/window.dart

@@ -841,6 +841,11 @@ class Window {
  void _respondToPlatformMessage(int responseId, ByteData data)
      native 'Window_respondToPlatformMessage';

+  /// Return the version information for the engine, skia and dart.
+  String getFlutterEngineVersion() native 'Window_GetFlutterEngineVersion';
+  String getSkiaVersion() native 'Window_GetSkiaVersion';


Capitalization of the native entrypoints does not match the convention of existing calls.

chinmaygarde · 2019-01-30T04:28:27Z

lib/ui/window/window.h

@@ -38,6 +38,20 @@ enum class AccessibilityFeatureFlag : int32_t {

 class WindowClient {
 public:
+  struct Versions {


This seems redundant as you can directly call Get*Version(). from any point within the engine. Versions will remain the same for all Flutter applications created using the same engine.

From what i was able to gather, you can only call Get*Version from within the shell and runtime code, adding a dependency on shell code from within ui, causes one of the build checks to be triggered. I added this struct as a convenience wrapper around it.

chinmaygarde · 2019-01-30T04:29:42Z

testing/dart/window_versions_test.dart

+// found in the LICENSE file.
+
+// HACK: pretend to be dart.ui in order to access its internals
+library dart.ui;


This test isn't tied to any harness. Did you forget to check in changes to a GN file?

oops, forgot to check it in!

Looks like flutter/testing/dart/*.dart are all being run by run_tests.sh.

chinmaygarde · 2019-01-30T04:30:14Z

testing/dart/window_versions_test.dart

+      expect(_isNotEmpty(flutterEngineVersion), equals(true));
+    });
+
+}


Nit: Newline at the end of the file please.

iskakaushik · 2019-01-30T04:40:50Z

Are the versions a property of a window? Would it make sense to expose them as a top level dart:ui property instead?
If Flutter ends up supporting multiple windows(flutter/flutter#25526) I think exposing the engine version as a property of each window object might look a little weird.
I'm also wondering whether we will ever run a backgrounded app without a window object.

@amirh, I was also thinking along similar lines. Versions don't really make sense to be on the Window object. Will move them to an outer class of their own.

For the backgrounded app point, do you think I should have to do anything special to accommodate that case or was it more of a concern if I put it on the window?

amirh · 2019-01-30T05:21:14Z

For the backgrounded app point, do you think I should have to do anything special to accommodate that case or was it more of a concern if I put it on the window?

Just a concern if it's a window property.

iskakaushik · 2019-01-30T08:17:15Z

I decided to move shell/version/* to common/version/*, since these versions are not inherently tied to the shell. This made accessing this simpler throughout. I also ended up moving these to versions.dart as these properties are not tied to the window.

jason-simmons · 2019-01-30T22:12:05Z

lib/ui/versions.cc

+void GetVersions(Dart_NativeArguments args) {
+  const std::vector<std::string> versions_list = {
+      GetDartVersion(), GetSkiaVersion(), GetFlutterEngineVersion()};
+  const auto& dart_val =


Declare this as a Dart_Handle, not a reference

jason-simmons · 2019-01-30T22:14:09Z

lib/ui/versions.dart

+
+part of dart.ui;
+
+/// Wraps version information for dart, skia and flutter.


Capitalize "Dart", "Skia", and "Flutter" in this and other comments

…lutter/engine#7634)

flutter/engine@e7eb1c8...74839e2 git log e7eb1c8..74839e2 --no-merges --oneline 74839e2 Roll src/third_party/skia 8c14038e56c3..d7a5a1d27e1d (5 commits) (flutter/engine#7683) 615ca59 Roll src/third_party/skia 69130631e23d..8c14038e56c3 (1 commits) (flutter/engine#7682) 482e43f Roll src/third_party/skia 21ca3702f8eb..69130631e23d (2 commits) (flutter/engine#7681) cc349a3 Roll src/third_party/skia b45f47dc3ef5..21ca3702f8eb (1 commits) (flutter/engine#7680) 26b8339 Roll src/third_party/skia 9c7a006a8e2d..b45f47dc3ef5 (1 commits) (flutter/engine#7679) 940b420 Roll src/third_party/skia 32d8cce070dd..9c7a006a8e2d (1 commits) (flutter/engine#7677) 0a9dadb Roll src/third_party/skia 71e434dc9dea..32d8cce070dd (1 commits) (flutter/engine#7675) 6266da8 Roll src/third_party/skia d396dd0347d3..71e434dc9dea (1 commits) (flutter/engine#7674) 94ec2c5 Roll src/third_party/skia 0d84e805c30c..d396dd0347d3 (1 commits) (flutter/engine#7673) 48055b1 Roll src/third_party/skia 48913465db5f..0d84e805c30c (1 commits) (flutter/engine#7672) 7f9d59c Roll src/third_party/skia d6a45a8aee6e..48913465db5f (1 commits) (flutter/engine#7671) 3f4ac2b Roll src/third_party/skia 8ec9a60bdb6f..d6a45a8aee6e (2 commits) (flutter/engine#7670) b00c7c8 Roll src/third_party/skia 72c687807f5e..8ec9a60bdb6f (2 commits) (flutter/engine#7669) 1b6f139 Roll src/third_party/skia 8f5aeebdd8eb..72c687807f5e (3 commits) (flutter/engine#7668) 637fadb Roll src/third_party/skia 3ed198faf93a..8f5aeebdd8eb (8 commits) (flutter/engine#7667) 1b65785 Add kernel-worker and dart2js to BUILD.gn (flutter/engine#7660) 1cf6061 Roll src/third_party/skia 5892553ad020..3ed198faf93a (3 commits) (flutter/engine#7666) 377e665 Roll src/third_party/skia 569dda7216cd..5892553ad020 (5 commits) (flutter/engine#7665) 44245c8 Roll src/third_party/skia 72a0e3347cdf..569dda7216cd (1 commits) (flutter/engine#7664) 80db23f Roll src/third_party/skia ae64f52786b3..72a0e3347cdf (2 commits) (flutter/engine#7663) 4889c74 Roll src/third_party/skia 95b014790f93..ae64f52786b3 (3 commits) (flutter/engine#7662) fac24d9 Roll src/third_party/skia 88bfed46ab6e..95b014790f93 (3 commits) (flutter/engine#7661) 5dcf3cf Roll src/third_party/skia 5b257abb87d2..88bfed46ab6e (8 commits) (flutter/engine#7657) 9345274 Initial import of FDE macOS framework (flutter/engine#7642) 54f5467 Roll src/third_party/skia 1bd245b91801..5b257abb87d2 (5 commits) (flutter/engine#7656) e71daf6 Roll src/third_party/skia ea2dc6a6bc11..1bd245b91801 (1 commits) (flutter/engine#7655) 4f5e170 Roll src/third_party/skia e9742a5a0db1..ea2dc6a6bc11 (1 commits) (flutter/engine#7654) ef1a7f8 Roll src/third_party/skia 8a484d0203d9..e9742a5a0db1 (2 commits) (flutter/engine#7653) 28339c8 Roll src/third_party/skia b7af275ebbf5..8a484d0203d9 (3 commits) (flutter/engine#7652) 7fa77ef Fix two typos in embedder docs (flutter/engine#7649) e9419bf Roll src/third_party/skia 9c8ad0316147..b7af275ebbf5 (3 commits) (flutter/engine#7650) b94e759 Expose the Flutter engine, Dart and Skia versions to Dart. (flutter/engine#7634) 64e1707 Document make_resource_current on FlutterOpenGLRendererConfig and warn if the callback is not set. (flutter/engine#7648) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff (dnfield@google.com), and stop the roller if necessary.

Hixie · 2019-02-08T00:56:49Z

I don't think we should do this. It's not at all clear to me what the use case is, and historically adding version numbers to APIs has always led to more strife.

flutter/engine@e7eb1c8...74839e2 git log e7eb1c8..74839e2 --no-merges --oneline 74839e2 Roll src/third_party/skia 8c14038e56c3..d7a5a1d27e1d (5 commits) (flutter/engine#7683) 615ca59 Roll src/third_party/skia 69130631e23d..8c14038e56c3 (1 commits) (flutter/engine#7682) 482e43f Roll src/third_party/skia 21ca3702f8eb..69130631e23d (2 commits) (flutter/engine#7681) cc349a3 Roll src/third_party/skia b45f47dc3ef5..21ca3702f8eb (1 commits) (flutter/engine#7680) 26b8339 Roll src/third_party/skia 9c7a006a8e2d..b45f47dc3ef5 (1 commits) (flutter/engine#7679) 940b420 Roll src/third_party/skia 32d8cce070dd..9c7a006a8e2d (1 commits) (flutter/engine#7677) 0a9dadb Roll src/third_party/skia 71e434dc9dea..32d8cce070dd (1 commits) (flutter/engine#7675) 6266da8 Roll src/third_party/skia d396dd0347d3..71e434dc9dea (1 commits) (flutter/engine#7674) 94ec2c5 Roll src/third_party/skia 0d84e805c30c..d396dd0347d3 (1 commits) (flutter/engine#7673) 48055b1 Roll src/third_party/skia 48913465db5f..0d84e805c30c (1 commits) (flutter/engine#7672) 7f9d59c Roll src/third_party/skia d6a45a8aee6e..48913465db5f (1 commits) (flutter/engine#7671) 3f4ac2b Roll src/third_party/skia 8ec9a60bdb6f..d6a45a8aee6e (2 commits) (flutter/engine#7670) b00c7c8 Roll src/third_party/skia 72c687807f5e..8ec9a60bdb6f (2 commits) (flutter/engine#7669) 1b6f139 Roll src/third_party/skia 8f5aeebdd8eb..72c687807f5e (3 commits) (flutter/engine#7668) 637fadb Roll src/third_party/skia 3ed198faf93a..8f5aeebdd8eb (8 commits) (flutter/engine#7667) 1b65785 Add kernel-worker and dart2js to BUILD.gn (flutter/engine#7660) 1cf6061 Roll src/third_party/skia 5892553ad020..3ed198faf93a (3 commits) (flutter/engine#7666) 377e665 Roll src/third_party/skia 569dda7216cd..5892553ad020 (5 commits) (flutter/engine#7665) 44245c8 Roll src/third_party/skia 72a0e3347cdf..569dda7216cd (1 commits) (flutter/engine#7664) 80db23f Roll src/third_party/skia ae64f52786b3..72a0e3347cdf (2 commits) (flutter/engine#7663) 4889c74 Roll src/third_party/skia 95b014790f93..ae64f52786b3 (3 commits) (flutter/engine#7662) fac24d9 Roll src/third_party/skia 88bfed46ab6e..95b014790f93 (3 commits) (flutter/engine#7661) 5dcf3cf Roll src/third_party/skia 5b257abb87d2..88bfed46ab6e (8 commits) (flutter/engine#7657) 9345274 Initial import of FDE macOS framework (flutter/engine#7642) 54f5467 Roll src/third_party/skia 1bd245b91801..5b257abb87d2 (5 commits) (flutter/engine#7656) e71daf6 Roll src/third_party/skia ea2dc6a6bc11..1bd245b91801 (1 commits) (flutter/engine#7655) 4f5e170 Roll src/third_party/skia e9742a5a0db1..ea2dc6a6bc11 (1 commits) (flutter/engine#7654) ef1a7f8 Roll src/third_party/skia 8a484d0203d9..e9742a5a0db1 (2 commits) (flutter/engine#7653) 28339c8 Roll src/third_party/skia b7af275ebbf5..8a484d0203d9 (3 commits) (flutter/engine#7652) 7fa77ef Fix two typos in embedder docs (flutter/engine#7649) e9419bf Roll src/third_party/skia 9c8ad0316147..b7af275ebbf5 (3 commits) (flutter/engine#7650) b94e759 Expose the Flutter engine, Dart and Skia versions to Dart. (flutter/engine#7634) 64e1707 Document make_resource_current on FlutterOpenGLRendererConfig and warn if the callback is not set. (flutter/engine#7648) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff (dnfield@google.com), and stop the roller if necessary.

…lutter#7634)" This reverts commit b94e759.

* Revert "Fix versions implementation (#7726)" This reverts commit 3c38dd3. * Revert "Expose the Flutter engine, Dart and Skia versions to Dart. (#7634)" This reverts commit b94e759. * remove namespace shell stuff * fix format * fix licenses

Kaushik Iska added 4 commits January 29, 2019 15:55

Window has the versions functions

ce2d39e

Merge branch 'master' of github.com:flutter/engine into 26380

a6df536

more fixes

3132f55

fix tests

91bddcb

googlebot added the cla: yes label Jan 30, 2019

iskakaushik requested review from chinmaygarde and jason-simmons January 30, 2019 02:02

jason-simmons reviewed Jan 30, 2019

View reviewed changes

Merge remote-tracking branch 'upstream/master' into 26380

3a681f4

chinmaygarde suggested changes Jan 30, 2019

View reviewed changes

Kaushik Iska added 4 commits January 29, 2019 22:27

Move things to ui_dart_state

81ad5e1

Move shell/versions to common/versions

5a6c2cb

fix few compile errors

e7d608c

simplify code

f9c6cc0

fix formatting

a6346c8

iskakaushik force-pushed the 26380 branch from 975c174 to a6346c8 Compare January 30, 2019 08:36

Kaushik Iska added 2 commits January 30, 2019 00:45

fix licenses

735dd2e

Merge branch 'master' of github.com:flutter/engine into 26380

b594095

iskakaushik force-pushed the 26380 branch from 968a2fc to b594095 Compare January 30, 2019 20:10

jason-simmons suggested changes Jan 30, 2019

View reviewed changes

iskakaushik force-pushed the 26380 branch from 1a36661 to a4151b4 Compare January 30, 2019 22:26

capitalize Dart, Skia and Flutter. Use value not reference

b6da02f

iskakaushik force-pushed the 26380 branch from a4151b4 to b6da02f Compare January 30, 2019 22:27

jason-simmons approved these changes Jan 30, 2019

View reviewed changes

chinmaygarde approved these changes Jan 31, 2019

View reviewed changes

iskakaushik merged commit b94e759 into flutter:master Jan 31, 2019

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

b94e759 Expose the Flutter engine, Dart and Skia versions to Dart. (f…

bc8af56

…lutter/engine#7634)

engine-flutter-autoroll mentioned this pull request Feb 1, 2019

Roll engine 15f2b92cce91..1b657855e38f (24 commits) flutter/flutter#27415

Closed

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

b94e759 Expose the Flutter engine, Dart and Skia versions to Dart. (f…

0c80fa1

…lutter/engine#7634)

This was referenced Feb 2, 2019

Roll engine 15f2b92cce91..637fadba7127 (25 commits) flutter/flutter#27416

Closed

Roll engine 15f2b92cce91..1b6f1397c8cb (26 commits) flutter/flutter#27421

Closed

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

b94e759 Expose the Flutter engine, Dart and Skia versions to Dart. (f…

7c9fb2a

…lutter/engine#7634)

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

b94e759 Expose the Flutter engine, Dart and Skia versions to Dart. (f…

65c2b99

…lutter/engine#7634)

engine-flutter-autoroll mentioned this pull request Feb 2, 2019

Roll engine 15f2b92cce91..b00c7c87f549 (27 commits) flutter/flutter#27423

Closed

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

b94e759 Expose the Flutter engine, Dart and Skia versions to Dart. (f…

c4e133a

…lutter/engine#7634)

engine-flutter-autoroll mentioned this pull request Feb 2, 2019

Roll engine 15f2b92cce91..48055b102d25 (30 commits) flutter/flutter#27440

Closed

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

b94e759 Expose the Flutter engine, Dart and Skia versions to Dart. (f…

c73775e

…lutter/engine#7634)

engine-flutter-autoroll mentioned this pull request Feb 3, 2019

Roll engine 15f2b92cce91..0a9dadb0441a (33 commits) flutter/flutter#27470

Closed

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

b94e759 Expose the Flutter engine, Dart and Skia versions to Dart. (f…

7b48d9a

…lutter/engine#7634)

engine-flutter-autoroll mentioned this pull request Feb 3, 2019

Roll engine 15f2b92cce91..940b42030bdd (34 commits) flutter/flutter#27475

Closed

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

b94e759 Expose the Flutter engine, Dart and Skia versions to Dart. (f…

ef5a7a8

…lutter/engine#7634)

engine-flutter-autoroll mentioned this pull request Feb 4, 2019

Roll engine 15f2b92cce91..26b83393b9ec (35 commits) flutter/flutter#27482

Closed

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

b94e759 Expose the Flutter engine, Dart and Skia versions to Dart. (f…

90193a4

…lutter/engine#7634)

engine-flutter-autoroll mentioned this pull request Feb 4, 2019

Roll engine 15f2b92cce91..615ca598da18 (38 commits) flutter/flutter#27498

Closed

GaryQian mentioned this pull request Feb 4, 2019

Roll engine e7eb1c8bf..615ca598da18 (32 commits) flutter/flutter#27503

Closed

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

b94e759 Expose the Flutter engine, Dart and Skia versions to Dart. (f…

528cbdd

…lutter/engine#7634)

engine-flutter-autoroll mentioned this pull request Feb 4, 2019

Roll engine e7eb1c8bf655..74839e207b48 (33 commits) flutter/flutter#27507

Merged

dnfield mentioned this pull request Feb 13, 2019

Flutter engine artifacts version mismatch flutter/flutter#27574

Closed

iskakaushik pushed a commit to iskakaushik/engine that referenced this pull request Feb 14, 2019

Revert "Expose the Flutter engine, Dart and Skia versions to Dart. (f…

075e580

…lutter#7634)" This reverts commit b94e759.

iskakaushik pushed a commit to iskakaushik/engine that referenced this pull request Apr 11, 2019

Revert "Expose the Flutter engine, Dart and Skia versions to Dart. (f…

2b0b7d5

…lutter#7634)" This reverts commit b94e759.


		part of dart.ui;

		/// Wraps version information for dart, skia and flutter.

Expose the Flutter engine, Dart and Skia versions to Dart. #7634

Expose the Flutter engine, Dart and Skia versions to Dart. #7634

Uh oh!

Conversation

iskakaushik commented Jan 30, 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!

amirh commented Jan 30, 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!

iskakaushik commented Jan 30, 2019

Uh oh!

amirh commented Jan 30, 2019

Uh oh!

iskakaushik commented Jan 30, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hixie commented Feb 8, 2019

Uh oh!

Uh oh!