Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ios: Fixed the callback for the first frame so that it isn't predicated on having a splash screen. #9952

Merged
merged 3 commits into from Jul 25, 2019

Conversation

@gaaclarke
Copy link
Contributor

commented Jul 19, 2019

Implemented by tweaking splash screen logic.

Relevant issue: flutter/flutter#24519

gaaclarke added 2 commits Jul 19, 2019
having a splash screen.  Implemented by tweaking splash screen logic.
@xster

This comment has been minimized.

Copy link
Contributor

commented Jul 20, 2019

LGTM % tests

Either solution is ok. Though we probably have to answer a few questions about consistent indirections or not in the other PR.

@gaaclarke

This comment has been minimized.

Copy link
Contributor Author

commented Jul 24, 2019

LGTM % tests

I've tried to get automated testing for this but it is not trivial. The library wasn't setup well for tests. I tried using a mock FlutterEngine, but that runs into problems where it eventually has to return C++ objects like the flutter::PlatformView.

If you use a real FlutterEngine I can get the callback installed in the rasterizer, but there is no way to introspect into it. Also nothing gets rendered without an actual Flutter project so I don't get the callback called.

Our best bet might be to create an integration test in framework, maybe in iosAdd2App tests.

@gaaclarke

This comment has been minimized.

Copy link
Contributor Author

commented Jul 25, 2019

I talked offline to @xster. We decided to look into adding this to Dan's integration tests at testing/scenario_app. I realized that isn't going to work well for this case because those tests always launch the FlutterViewController before the tests run, which doesn't give us the opportunity to set the callback. The best place for testing this is in the add2app integration tests.

@xster

This comment has been minimized.

Copy link
Contributor

commented Jul 25, 2019

I think what it would be though is rather than using the same iOS project itself, it would be a new iOS project where the default vc is a normal vc, it has a button or some such that makes a Flutter VC, sets the callback and presents it. Press the button in earl grey, check that the callback is called.

@gaaclarke

This comment has been minimized.

Copy link
Contributor Author

commented Jul 25, 2019

Going to commit this now to make sure I didn't break full screen apps. I'm working on getting an integration working in Engine based off of Dan's Scenario work.

@gaaclarke gaaclarke merged commit 0ecb793 into flutter:master Jul 25, 2019
19 checks passed
19 checks passed
WIP Ready for review
Details
build_and_test_host Task Summary
Details
build_and_test_host
Details
build_and_test_host_profile Task Summary
Details
build_and_test_host_profile
Details
build_and_test_host_release Task Summary
Details
build_and_test_host_release
Details
build_android Task Summary
Details
build_android
Details
build_fuchsia_profile Task Summary
Details
build_fuchsia_profile
Details
build_windows_debug Task Summary
Details
build_windows_debug
Details
build_windows_debug_unopt Task Summary
Details
build_windows_debug_unopt
Details
cla/google All necessary CLAs are signed
format_and_dart_test Task Summary
Details
format_and_dart_test
Details
luci-engine
Details
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 25, 2019
a-siva added a commit to flutter/flutter that referenced this pull request Jul 26, 2019
* 876eff6c6 Revert "[fuchsia] Use the patched sdk to generate the flutter jit runner far (#10122)" (flutter/engine#10140)

* 36b5b1497 Roll fuchsia/sdk/core/mac-amd64 from 9EcA--gzHNo80R_fAuB_Tq7ECubTP0Uz3UrycPIvRpsC to SHb_OsdvSz75N-G-jbG-tQOM1OyipLcQG0rsVrARsX0C (flutter/engine#10134)

* 29d92c917 Roll src/third_party/skia b3956dc6ba6a..fff996c117c1 (10 commits) (flutter/engine#10133)

* d50f4152c Roll fuchsia/sdk/core/linux-amd64 from NrExSKBKIhY3SnE4sD0-vHjEMCBYYffzarRM2rWONUUC to ylNwUxj7tBGga6sghqrKH_gqu3RzfTLxB4rxZhAk2IAC (flutter/engine#10135)

* 930f58544 Roll fuchsia/clang/linux-amd64 from zHiuOGMDwdWPUkV1B2fHKyRy2fGWGcUgfa2z6dyGAZQC to W8LY5ncb-fknOtxPITszyAZ1g80_OSq2W_QWC3QunXoC (flutter/engine#10137)

* 9c1bba395 Roll fuchsia/clang/mac-amd64 from UYD9C8IEkWVi83ef4zwO1Ump2B8cP9Nd5WECjU8l3AUC to CqfCCMwpILIYr4rqCeh3zF8x3qErehEusnQwl22i7KIC (flutter/engine#10136)

* 0ecb793e1 ios: Fixed the callback for the first frame so that it isn't predicated on having a splash screen. (flutter/engine#9952)

* 35b62dda5 Roll src/third_party/dart 0c97c31b6e..a2aec5eb06 (22 commits) (flutter/engine#10139)

* 250ee3164 Started linking the test targets against Flutter. (flutter/engine#10128)

* 330b0f023 Revert "[macos] Revert check on FlutterCodecs and refactor message function] (#10009)" (flutter/engine#10141)

* 0861e0a76 Disable windows tests (flutter/engine#10143)
christopherfujino added a commit to teach-me-to-code/flutter that referenced this pull request Jul 26, 2019
* 876eff6c6 Revert "[fuchsia] Use the patched sdk to generate the flutter jit runner far (flutter#10122)" (flutter/engine#10140)

* 36b5b1497 Roll fuchsia/sdk/core/mac-amd64 from 9EcA--gzHNo80R_fAuB_Tq7ECubTP0Uz3UrycPIvRpsC to SHb_OsdvSz75N-G-jbG-tQOM1OyipLcQG0rsVrARsX0C (flutter/engine#10134)

* 29d92c917 Roll src/third_party/skia b3956dc6ba6a..fff996c117c1 (10 commits) (flutter/engine#10133)

* d50f4152c Roll fuchsia/sdk/core/linux-amd64 from NrExSKBKIhY3SnE4sD0-vHjEMCBYYffzarRM2rWONUUC to ylNwUxj7tBGga6sghqrKH_gqu3RzfTLxB4rxZhAk2IAC (flutter/engine#10135)

* 930f58544 Roll fuchsia/clang/linux-amd64 from zHiuOGMDwdWPUkV1B2fHKyRy2fGWGcUgfa2z6dyGAZQC to W8LY5ncb-fknOtxPITszyAZ1g80_OSq2W_QWC3QunXoC (flutter/engine#10137)

* 9c1bba395 Roll fuchsia/clang/mac-amd64 from UYD9C8IEkWVi83ef4zwO1Ump2B8cP9Nd5WECjU8l3AUC to CqfCCMwpILIYr4rqCeh3zF8x3qErehEusnQwl22i7KIC (flutter/engine#10136)

* 0ecb793e1 ios: Fixed the callback for the first frame so that it isn't predicated on having a splash screen. (flutter/engine#9952)

* 35b62dda5 Roll src/third_party/dart 0c97c31b6e..a2aec5eb06 (22 commits) (flutter/engine#10139)

* 250ee3164 Started linking the test targets against Flutter. (flutter/engine#10128)

* 330b0f023 Revert "[macos] Revert check on FlutterCodecs and refactor message function] (flutter#10009)" (flutter/engine#10141)

* 0861e0a76 Disable windows tests (flutter/engine#10143)
johnsonmh added a commit to johnsonmh/flutter that referenced this pull request Jul 30, 2019
* 876eff6c6 Revert "[fuchsia] Use the patched sdk to generate the flutter jit runner far (flutter#10122)" (flutter/engine#10140)

* 36b5b1497 Roll fuchsia/sdk/core/mac-amd64 from 9EcA--gzHNo80R_fAuB_Tq7ECubTP0Uz3UrycPIvRpsC to SHb_OsdvSz75N-G-jbG-tQOM1OyipLcQG0rsVrARsX0C (flutter/engine#10134)

* 29d92c917 Roll src/third_party/skia b3956dc6ba6a..fff996c117c1 (10 commits) (flutter/engine#10133)

* d50f4152c Roll fuchsia/sdk/core/linux-amd64 from NrExSKBKIhY3SnE4sD0-vHjEMCBYYffzarRM2rWONUUC to ylNwUxj7tBGga6sghqrKH_gqu3RzfTLxB4rxZhAk2IAC (flutter/engine#10135)

* 930f58544 Roll fuchsia/clang/linux-amd64 from zHiuOGMDwdWPUkV1B2fHKyRy2fGWGcUgfa2z6dyGAZQC to W8LY5ncb-fknOtxPITszyAZ1g80_OSq2W_QWC3QunXoC (flutter/engine#10137)

* 9c1bba395 Roll fuchsia/clang/mac-amd64 from UYD9C8IEkWVi83ef4zwO1Ump2B8cP9Nd5WECjU8l3AUC to CqfCCMwpILIYr4rqCeh3zF8x3qErehEusnQwl22i7KIC (flutter/engine#10136)

* 0ecb793e1 ios: Fixed the callback for the first frame so that it isn't predicated on having a splash screen. (flutter/engine#9952)

* 35b62dda5 Roll src/third_party/dart 0c97c31b6e..a2aec5eb06 (22 commits) (flutter/engine#10139)

* 250ee3164 Started linking the test targets against Flutter. (flutter/engine#10128)

* 330b0f023 Revert "[macos] Revert check on FlutterCodecs and refactor message function] (flutter#10009)" (flutter/engine#10141)

* 0861e0a76 Disable windows tests (flutter/engine#10143)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.