add windows embedding test #12423

chunhtai · 2019-09-24T18:08:57Z

flutter/flutter#41004

BUILD.gn

shell/platform/windows/BUILD.gn

shell/platform/windows/win32_window_unittests.cc

shell/platform/windows/testing/win32_flutter_window_test.h

chinmaygarde · 2019-09-25T21:30:45Z

shell/platform/windows/testing/win32_flutter_window_test.cc

+
+namespace flutter {
+namespace testing {
+  void Win32FlutterWindowTest::OnFontChange() {


OnFontChanged or OnFontsDidChange sound more descriptive.

I was following the pattern in

engine/shell/platform/windows/win32_window.cc

Line 123 in d8d0d3f

switch (message) {

I think most of the pattern is On'message name'. The message for font changes is WM_FONTCHANGE.
If you think this is not a good pattern, i can update all of them

shell/platform/windows/win32_window_unittests.cc

shell/platform/windows/testing/win32_flutter_window_test.h

chinmaygarde · 2019-09-25T22:53:00Z

Also, please amend the commit message to correct the typo.

stuartmorgan-g · 2019-09-26T23:10:55Z

shell/platform/windows/BUILD.gn

+    "win32_flutter_window.h",
+    "win32_window.cc",
+    "win32_window.h",
+    "window_state.h",


Why is this duplicating everything instead of depending on flutter_windows_source?

shell/platform/windows/testing/win32_flutter_window_test.h

shell/platform/windows/win32_window_unittests.cc

googlebot · 2019-09-27T18:15:58Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

googlebot · 2019-09-27T18:15:58Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

googlebot · 2019-09-27T18:20:28Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

googlebot · 2019-09-27T18:20:29Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

chunhtai · 2019-09-27T19:14:28Z

.cirrus.yml

      compile_host_script: |
        cd %ENGINE_PATH%/src
        python flutter/tools/gn --runtime-mode debug --unoptimized
        ninja -C out/host_debug_unopt
+      test_host_script: |
+        cd $ENGINE_PATH/src
+        python flutter/testing/run_tests.py --type=engine --engine-filter=flutter_windows_unittests,client_wrapper_windows_unittests


some of the unit tests fail on windows platform, I think only run relevant tests should be enough

chunhtai · 2019-10-02T23:56:59Z

.cirrus.yml

@@ -90,7 +90,7 @@ task:
 task:
  gce_instance:
    image_project: flutter-cirrus
-    image_name: flutter-engine-windows-server-2016-core
+    image_name: flutter-engine-windows-server-2016


The windows unit test requires opengl to run, I updated it to use a gui version of image

chunhtai · 2019-10-03T16:55:02Z

This is ready for re review

[email protected]:flutter/engine.git/compare/7d67e275ff82...8aa4732 git log 7d67e27..8aa4732 --no-merges --oneline 2019-10-03 [email protected] Fix Metal builds. (flutter/engine#12777) 2019-10-03 [email protected] Revert "Manage resource and onscreen contexts using separate IOSGLContext objects (#12277)" (flutter/engine#12773) 2019-10-03 [email protected] Roll src/third_party/dart afac6a3714..07a63a17a4 (6 commits) 2019-10-03 [email protected] roll buildroot to 01e9235 (flutter/engine#12771) 2019-10-03 [email protected] Create a package-able incremental compiler (flutter/engine#12681) 2019-10-03 [email protected] add windows embedding test (flutter/engine#12423) 2019-10-03 [email protected] Roll fuchsia/sdk/core/mac-amd64 from g-PD1... to wYLiQ... (flutter/engine#12770) 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/7d67e275ff82...8aa4732 git log 7d67e27..8aa4732 --no-merges --oneline 2019-10-03 [email protected] Fix Metal builds. (flutter/engine#12777) 2019-10-03 [email protected] Revert "Manage resource and onscreen contexts using separate IOSGLContext objects (flutter#12277)" (flutter/engine#12773) 2019-10-03 [email protected] Roll src/third_party/dart afac6a3714..07a63a17a4 (6 commits) 2019-10-03 [email protected] roll buildroot to 01e9235 (flutter/engine#12771) 2019-10-03 [email protected] Create a package-able incremental compiler (flutter/engine#12681) 2019-10-03 [email protected] add windows embedding test (flutter/engine#12423) 2019-10-03 [email protected] Roll fuchsia/sdk/core/mac-amd64 from g-PD1... to wYLiQ... (flutter/engine#12770) 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

googlebot added cla: yes labels Sep 24, 2019

chunhtai requested a review from stuartmorgan-g September 24, 2019 18:09

chunhtai commented Sep 24, 2019

View reviewed changes

BUILD.gn Show resolved Hide resolved

chinmaygarde reviewed Sep 25, 2019

View reviewed changes

chunhtai force-pushed the issues/41004 branch from ba71a1c to a6ee021 Compare September 26, 2019 22:23

chunhtai changed the title ~~add wundows embedding test~~ add windows embedding test Sep 26, 2019

stuartmorgan-g suggested changes Sep 26, 2019

View reviewed changes

googlebot added cla: no and removed cla: yes labels Sep 27, 2019

chunhtai force-pushed the issues/41004 branch from 90bd415 to e76c282 Compare September 27, 2019 18:20

googlebot added cla: yes and removed cla: no labels Sep 27, 2019

chunhtai force-pushed the issues/41004 branch from e76c282 to 336ed86 Compare September 27, 2019 19:06

chunhtai commented Sep 27, 2019

View reviewed changes

chunhtai added the Work in progress (WIP) Not ready (yet) for review! label Sep 27, 2019

chunhtai force-pushed the issues/41004 branch 2 times, most recently from f1d95dd to 9f17c44 Compare September 27, 2019 21:48

chunhtai commented Oct 2, 2019

View reviewed changes

add windows embedding test

23ceb41

chunhtai force-pushed the issues/41004 branch from 349e309 to 23ceb41 Compare October 3, 2019 16:23

chunhtai removed the Work in progress (WIP) Not ready (yet) for review! label Oct 3, 2019

chinmaygarde approved these changes Oct 3, 2019

View reviewed changes

chunhtai merged commit daf1eb9 into flutter:master Oct 3, 2019

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

daf1eb9 add windows embedding test (flutter/engine#12423)

78a6a45

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

Roll engine 7d67e275ff82..8aa47326ae73 (7 commits) flutter/flutter#41944

Merged

liyuqian mentioned this pull request Oct 4, 2019

Run engine unit tests on Windows presubmit checks flutter/flutter#42024

Closed

add windows embedding test #12423

add windows embedding test #12423

Uh oh!

Conversation

chunhtai commented Sep 24, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chinmaygarde Sep 25, 2019

Choose a reason for hiding this comment

Uh oh!

chunhtai Sep 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chinmaygarde commented Sep 25, 2019

Uh oh!

stuartmorgan-g Sep 26, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

googlebot commented Sep 27, 2019

Uh oh!

googlebot commented Sep 27, 2019

Uh oh!

googlebot commented Sep 27, 2019

Uh oh!

googlebot commented Sep 27, 2019

Uh oh!

chunhtai Sep 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chunhtai Oct 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chunhtai commented Oct 3, 2019

Uh oh!

Uh oh!

chunhtai Sep 26, 2019 •

edited

Loading

chunhtai Sep 27, 2019 •

edited

Loading

chunhtai Oct 2, 2019 •

edited

Loading