-
Notifications
You must be signed in to change notification settings - Fork 6k
Flush all embedded Android views on hot restart. #5929
Conversation
Adds an OnEngineRestarted method to PlatformView, this is currently only implemented for Android where we need to use it for embedded views.
fcf1276
to
42804e2
Compare
shell/common/shell.cc
Outdated
@@ -740,6 +740,21 @@ void Shell::OnEngineHandlePlatformMessage( | |||
}); | |||
} | |||
|
|||
// |shell::Engine::Delegate| | |||
void Shell::OnEngineRestart() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will deadlock on configurations where the task runners are the same (test/embedder configurations). Please use fml::TaskRunner::RunNowOrPostTask
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add the assertions for setup and the thread checker as done in all the other methods in this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow good catch on the threading issue! thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the same assertions, though I believe we don't really care to assert that we're on the ui thread
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we are just being super paranoid about threading here. Also, I like the assertion as a nice way of quickly figuring out who initiates the restart call. For example, it is possible that the service protocol handler fires off this delegate method. But now that you added the assertion, I can easily tell that the engine has had a chance to service the call.
shell/common/engine.cc
Outdated
@@ -103,6 +103,7 @@ bool Engine::Restart(RunConfiguration configuration) { | |||
} | |||
runtime_controller_ = runtime_controller_->Clone(); | |||
UpdateAssetManager(nullptr); | |||
delegate_.OnEngineRestart(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to check if the run call actually succeeded before firing the delegate method. As written, failed attempts to relaunch the engine will still fire the delegate callback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That might be a problem - I need to make sure I flushed the embedded views before the dart code starts...
What happens when a hot restart fails? do we retry? what if all retries failed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a hot restart fails, we just sit there waiting for another restart (all restarts are initiated by the service protocol).
If you really want a callback just before the restart, I would name this delegate method OnEngineWillRestart
to better indicate that the restart has not yet happened. Also, move this delegate call up before the runtime_controller_->Clone()
call. On line 106, the old runtime controller has already been destroyed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Renamed to OnPreEngineRestart
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after the delegate call rename and move of the invocation to before the clone on the runtime controller. Thanks.
flutter/engine@4893b07...ffbafc8 git log 4893b07..ffbafc8 --date=short --no-merges --format='%%ad %%ae %%s' 2018-08-03 amirh@users.noreply.github.com Flush all embedded Android views on hot restart. (flutter/engine#5929) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 578ef2847b72..1400d38e0863 (7 commits) (flutter/engine#5933) 2018-08-02 amirh@users.noreply.github.com Don't drop MotionEvents with unknown tool type. (flutter/engine#5931) 2018-08-02 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 64cc576b1fa7..578ef2847b72 (20 commits) (flutter/engine#5930) 2018-08-02 regis@google.com Roll Dart to b04def964c428ada007cca7ef6b4936001db965d (flutter/engine#5928) 2018-08-02 jamesderlin@users.noreply.github.com Add an explicit `-[FlutterViewController init]` implementation (flutter/engine#5924) The AutoRoll server is located here: https://flutter-engine-flutter-roll.skia.org 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, who should be CC'd on the roll, and stop the roller if necessary.
flutter/engine@4893b07...715b64e git log 4893b07..715b64e --date=short --no-merges --format='%%ad %%ae %%s' 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 1400d38e0863..cdfa16d086b8 (10 commits) (flutter/engine#5934) 2018-08-03 amirh@users.noreply.github.com Flush all embedded Android views on hot restart. (flutter/engine#5929) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 578ef2847b72..1400d38e0863 (7 commits) (flutter/engine#5933) 2018-08-02 amirh@users.noreply.github.com Don't drop MotionEvents with unknown tool type. (flutter/engine#5931) 2018-08-02 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 64cc576b1fa7..578ef2847b72 (20 commits) (flutter/engine#5930) 2018-08-02 regis@google.com Roll Dart to b04def964c428ada007cca7ef6b4936001db965d (flutter/engine#5928) 2018-08-02 jamesderlin@users.noreply.github.com Add an explicit `-[FlutterViewController init]` implementation (flutter/engine#5924) The AutoRoll server is located here: https://flutter-engine-flutter-roll.skia.org 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, who should be CC'd on the roll, and stop the roller if necessary.
flutter/engine@4893b07...597a508 git log 4893b07..597a508 --date=short --no-merges --format='%%ad %%ae %%s' 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia cdfa16d086b8..4c3b63e289c5 (9 commits) (flutter/engine#5936) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 1400d38e0863..cdfa16d086b8 (10 commits) (flutter/engine#5934) 2018-08-03 amirh@users.noreply.github.com Flush all embedded Android views on hot restart. (flutter/engine#5929) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 578ef2847b72..1400d38e0863 (7 commits) (flutter/engine#5933) 2018-08-02 amirh@users.noreply.github.com Don't drop MotionEvents with unknown tool type. (flutter/engine#5931) 2018-08-02 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 64cc576b1fa7..578ef2847b72 (20 commits) (flutter/engine#5930) 2018-08-02 regis@google.com Roll Dart to b04def964c428ada007cca7ef6b4936001db965d (flutter/engine#5928) 2018-08-02 jamesderlin@users.noreply.github.com Add an explicit `-[FlutterViewController init]` implementation (flutter/engine#5924) The AutoRoll server is located here: https://flutter-engine-flutter-roll.skia.org 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, who should be CC'd on the roll, and stop the roller if necessary.
flutter/engine@4893b07...ecbb2b2 git log 4893b07..ecbb2b2 --date=short --no-merges --format='%%ad %%ae %%s' 2018-08-03 liyuqian@google.com Call drawPaint instead of drawPath if there's clip (flutter/engine#5937) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia cdfa16d086b8..4c3b63e289c5 (9 commits) (flutter/engine#5936) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 1400d38e0863..cdfa16d086b8 (10 commits) (flutter/engine#5934) 2018-08-03 amirh@users.noreply.github.com Flush all embedded Android views on hot restart. (flutter/engine#5929) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 578ef2847b72..1400d38e0863 (7 commits) (flutter/engine#5933) 2018-08-02 amirh@users.noreply.github.com Don't drop MotionEvents with unknown tool type. (flutter/engine#5931) 2018-08-02 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 64cc576b1fa7..578ef2847b72 (20 commits) (flutter/engine#5930) 2018-08-02 regis@google.com Roll Dart to b04def964c428ada007cca7ef6b4936001db965d (flutter/engine#5928) 2018-08-02 jamesderlin@users.noreply.github.com Add an explicit `-[FlutterViewController init]` implementation (flutter/engine#5924) The AutoRoll server is located here: https://flutter-engine-flutter-roll.skia.org 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, who should be CC'd on the roll, and stop the roller if necessary.
flutter/engine@4893b07...aaf4a9a git log 4893b07..aaf4a9a --date=short --no-merges --format='%%ad %%ae %%s' 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 4c3b63e289c5..71fe8baccc01 (3 commits) (flutter/engine#5940) 2018-08-03 liyuqian@google.com Call drawPaint instead of drawPath if there's clip (flutter/engine#5937) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia cdfa16d086b8..4c3b63e289c5 (9 commits) (flutter/engine#5936) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 1400d38e0863..cdfa16d086b8 (10 commits) (flutter/engine#5934) 2018-08-03 amirh@users.noreply.github.com Flush all embedded Android views on hot restart. (flutter/engine#5929) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 578ef2847b72..1400d38e0863 (7 commits) (flutter/engine#5933) 2018-08-02 amirh@users.noreply.github.com Don't drop MotionEvents with unknown tool type. (flutter/engine#5931) 2018-08-02 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 64cc576b1fa7..578ef2847b72 (20 commits) (flutter/engine#5930) 2018-08-02 regis@google.com Roll Dart to b04def964c428ada007cca7ef6b4936001db965d (flutter/engine#5928) 2018-08-02 jamesderlin@users.noreply.github.com Add an explicit `-[FlutterViewController init]` implementation (flutter/engine#5924) The AutoRoll server is located here: https://flutter-engine-flutter-roll.skia.org 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, who should be CC'd on the roll, and stop the roller if necessary.
flutter/engine@4893b07...97aea09 git log 4893b07..97aea09 --date=short --no-merges --format='%%ad %%ae %%s' 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 71fe8baccc01..59aabbcf3b0d (1 commits) (flutter/engine#5942) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 4c3b63e289c5..71fe8baccc01 (3 commits) (flutter/engine#5940) 2018-08-03 liyuqian@google.com Call drawPaint instead of drawPath if there's clip (flutter/engine#5937) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia cdfa16d086b8..4c3b63e289c5 (9 commits) (flutter/engine#5936) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 1400d38e0863..cdfa16d086b8 (10 commits) (flutter/engine#5934) 2018-08-03 amirh@users.noreply.github.com Flush all embedded Android views on hot restart. (flutter/engine#5929) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 578ef2847b72..1400d38e0863 (7 commits) (flutter/engine#5933) 2018-08-02 amirh@users.noreply.github.com Don't drop MotionEvents with unknown tool type. (flutter/engine#5931) 2018-08-02 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 64cc576b1fa7..578ef2847b72 (20 commits) (flutter/engine#5930) 2018-08-02 regis@google.com Roll Dart to b04def964c428ada007cca7ef6b4936001db965d (flutter/engine#5928) 2018-08-02 jamesderlin@users.noreply.github.com Add an explicit `-[FlutterViewController init]` implementation (flutter/engine#5924) The AutoRoll server is located here: https://flutter-engine-flutter-roll.skia.org 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, who should be CC'd on the roll, and stop the roller if necessary.
flutter/engine@4893b07...63ede2e git log 4893b07..63ede2e --date=short --no-merges --format='%%ad %%ae %%s' 2018-08-05 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 59aabbcf3b0d..2e77f54f46e8 (1 commits) (flutter/engine#5943) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 71fe8baccc01..59aabbcf3b0d (1 commits) (flutter/engine#5942) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 4c3b63e289c5..71fe8baccc01 (3 commits) (flutter/engine#5940) 2018-08-03 liyuqian@google.com Call drawPaint instead of drawPath if there's clip (flutter/engine#5937) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia cdfa16d086b8..4c3b63e289c5 (9 commits) (flutter/engine#5936) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 1400d38e0863..cdfa16d086b8 (10 commits) (flutter/engine#5934) 2018-08-03 amirh@users.noreply.github.com Flush all embedded Android views on hot restart. (flutter/engine#5929) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 578ef2847b72..1400d38e0863 (7 commits) (flutter/engine#5933) 2018-08-02 amirh@users.noreply.github.com Don't drop MotionEvents with unknown tool type. (flutter/engine#5931) 2018-08-02 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 64cc576b1fa7..578ef2847b72 (20 commits) (flutter/engine#5930) 2018-08-02 regis@google.com Roll Dart to b04def964c428ada007cca7ef6b4936001db965d (flutter/engine#5928) 2018-08-02 jamesderlin@users.noreply.github.com Add an explicit `-[FlutterViewController init]` implementation (flutter/engine#5924) The AutoRoll server is located here: https://flutter-engine-flutter-roll.skia.org 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, who should be CC'd on the roll, and stop the roller if necessary.
flutter/engine@4893b07...c6baaaf git log 4893b07..c6baaaf --date=short --no-merges --format='%%ad %%ae %%s' 2018-08-05 petrhosek@users.noreply.github.com Replace acquire+release thread annotation with excludes (flutter/engine#5944) 2018-08-05 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 59aabbcf3b0d..2e77f54f46e8 (1 commits) (flutter/engine#5943) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 71fe8baccc01..59aabbcf3b0d (1 commits) (flutter/engine#5942) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 4c3b63e289c5..71fe8baccc01 (3 commits) (flutter/engine#5940) 2018-08-03 liyuqian@google.com Call drawPaint instead of drawPath if there's clip (flutter/engine#5937) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia cdfa16d086b8..4c3b63e289c5 (9 commits) (flutter/engine#5936) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 1400d38e0863..cdfa16d086b8 (10 commits) (flutter/engine#5934) 2018-08-03 amirh@users.noreply.github.com Flush all embedded Android views on hot restart. (flutter/engine#5929) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 578ef2847b72..1400d38e0863 (7 commits) (flutter/engine#5933) 2018-08-02 amirh@users.noreply.github.com Don't drop MotionEvents with unknown tool type. (flutter/engine#5931) 2018-08-02 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 64cc576b1fa7..578ef2847b72 (20 commits) (flutter/engine#5930) 2018-08-02 regis@google.com Roll Dart to b04def964c428ada007cca7ef6b4936001db965d (flutter/engine#5928) 2018-08-02 jamesderlin@users.noreply.github.com Add an explicit `-[FlutterViewController init]` implementation (flutter/engine#5924) The AutoRoll server is located here: https://flutter-engine-flutter-roll.skia.org 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, who should be CC'd on the roll, and stop the roller if necessary.
flutter/engine@4893b07...f4464a8 git log 4893b07..f4464a8 --date=short --no-merges --format='%%ad %%ae %%s' 2018-08-06 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 2e77f54f46e8..12fb9cfeee07 (1 commits) (flutter/engine#5945) 2018-08-05 petrhosek@users.noreply.github.com Replace acquire+release thread annotation with excludes (flutter/engine#5944) 2018-08-05 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 59aabbcf3b0d..2e77f54f46e8 (1 commits) (flutter/engine#5943) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 71fe8baccc01..59aabbcf3b0d (1 commits) (flutter/engine#5942) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 4c3b63e289c5..71fe8baccc01 (3 commits) (flutter/engine#5940) 2018-08-03 liyuqian@google.com Call drawPaint instead of drawPath if there's clip (flutter/engine#5937) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia cdfa16d086b8..4c3b63e289c5 (9 commits) (flutter/engine#5936) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 1400d38e0863..cdfa16d086b8 (10 commits) (flutter/engine#5934) 2018-08-03 amirh@users.noreply.github.com Flush all embedded Android views on hot restart. (flutter/engine#5929) 2018-08-03 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 578ef2847b72..1400d38e0863 (7 commits) (flutter/engine#5933) 2018-08-02 amirh@users.noreply.github.com Don't drop MotionEvents with unknown tool type. (flutter/engine#5931) 2018-08-02 37626415+skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 64cc576b1fa7..578ef2847b72 (20 commits) (flutter/engine#5930) 2018-08-02 regis@google.com Roll Dart to b04def964c428ada007cca7ef6b4936001db965d (flutter/engine#5928) 2018-08-02 jamesderlin@users.noreply.github.com Add an explicit `-[FlutterViewController init]` implementation (flutter/engine#5924) The AutoRoll server is located here: https://flutter-engine-flutter-roll.skia.org 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, who should be CC'd on the roll, and stop the roller if necessary.
ecbb2b2 Call drawPaint instead of drawPath if there's clip (flutter/engine#5937) 597a508 Roll src/third_party/skia cdfa16d086b8..4c3b63e289c5 (9 commits) (flutter/engine#5936) 715b64e Roll src/third_party/skia 1400d38e0863..cdfa16d086b8 (10 commits) (flutter/engine#5934) ffbafc8 Flush all embedded Android views on hot restart. (flutter/engine#5929) 9fe6a57 Roll src/third_party/skia 578ef2847b72..1400d38e0863 (7 commits) (flutter/engine#5933) 3b66f20 Don't drop MotionEvents with unknown tool type. (flutter/engine#5931) 391ac2f Roll src/third_party/skia 64cc576b1fa7..578ef2847b72 (20 commits) (flutter/engine#5930) Goldens: 64b7a3a Add updated golden files cb1fa8a Update golden files of PhysicalModel and PhysicalShape 3baed8d Add a goldens package for the embedded Android views integration test.
ecbb2b2 Call drawPaint instead of drawPath if there's clip (flutter/engine#5937) 597a508 Roll src/third_party/skia cdfa16d086b8..4c3b63e289c5 (9 commits) (flutter/engine#5936) 715b64e Roll src/third_party/skia 1400d38e0863..cdfa16d086b8 (10 commits) (flutter/engine#5934) ffbafc8 Flush all embedded Android views on hot restart. (flutter/engine#5929) 9fe6a57 Roll src/third_party/skia 578ef2847b72..1400d38e0863 (7 commits) (flutter/engine#5933) 3b66f20 Don't drop MotionEvents with unknown tool type. (flutter/engine#5931) 391ac2f Roll src/third_party/skia 64cc576b1fa7..578ef2847b72 (20 commits) (flutter/engine#5930) Goldens: 64b7a3a Add updated golden files cb1fa8a Update golden files of PhysicalModel and PhysicalShape 3baed8d Add a goldens package for the embedded Android views integration test.
Adds an OnEngineRestarted method to PlatformView, this is currently only
implemented for Android where we need to use it for embedded views.
Fixes flutter/flutter#19576