From b76d939032fc8952aaed8fc07428e05310fbe8ca Mon Sep 17 00:00:00 2001
From: Jason Simmons <jason-simmons@users.noreply.github.com>
Date: Mon, 16 Jul 2018 12:04:05 -0700
Subject: [PATCH] Handle Android vsync callbacks that occur after the
 VsyncWaiter has been deleted (flutter/engine#5749)

Fixes https://github.com/flutter/flutter/issues/19159
---
 engine/src/flutter/shell/common/animator.h    |  2 +-
 .../src/flutter/shell/common/platform_view.h  |  1 -
 .../src/flutter/shell/common/vsync_waiter.h   |  8 ++---
 .../platform/android/vsync_waiter_android.cc  | 36 ++++++-------------
 .../platform/android/vsync_waiter_android.h   |  1 +
 5 files changed, 16 insertions(+), 32 deletions(-)

diff --git a/engine/src/flutter/shell/common/animator.h b/engine/src/flutter/shell/common/animator.h
index 53b2ac6884..8ddd852b67 100644
--- a/engine/src/flutter/shell/common/animator.h
+++ b/engine/src/flutter/shell/common/animator.h
@@ -64,7 +64,7 @@ class Animator final {
 
   Delegate& delegate_;
   blink::TaskRunners task_runners_;
-  std::unique_ptr<VsyncWaiter> waiter_;
+  std::shared_ptr<VsyncWaiter> waiter_;
 
   fxl::TimePoint last_begin_frame_time_;
   int64_t dart_frame_deadline_;
diff --git a/engine/src/flutter/shell/common/platform_view.h b/engine/src/flutter/shell/common/platform_view.h
index d803baa8e1..9f5fca77d2 100644
--- a/engine/src/flutter/shell/common/platform_view.h
+++ b/engine/src/flutter/shell/common/platform_view.h
@@ -125,7 +125,6 @@ class PlatformView {
  protected:
   PlatformView::Delegate& delegate_;
   const blink::TaskRunners task_runners_;
-  std::unique_ptr<VsyncWaiter> vsync_waiter_;
 
   SkISize size_;
   fml::WeakPtrFactory<PlatformView> weak_factory_;
diff --git a/engine/src/flutter/shell/common/vsync_waiter.h b/engine/src/flutter/shell/common/vsync_waiter.h
index 82231fdf4f..b6d9ee2255 100644
--- a/engine/src/flutter/shell/common/vsync_waiter.h
+++ b/engine/src/flutter/shell/common/vsync_waiter.h
@@ -14,7 +14,7 @@
 
 namespace shell {
 
-class VsyncWaiter {
+class VsyncWaiter : public std::enable_shared_from_this<VsyncWaiter> {
  public:
   using Callback = std::function<void(fxl::TimePoint frame_start_time,
                                       fxl::TimePoint frame_target_time)>;
@@ -23,6 +23,9 @@ class VsyncWaiter {
 
   void AsyncWaitForVsync(Callback callback);
 
+  void FireCallback(fxl::TimePoint frame_start_time,
+                    fxl::TimePoint frame_target_time);
+
  protected:
   const blink::TaskRunners task_runners_;
   std::mutex callback_mutex_;
@@ -32,9 +35,6 @@ class VsyncWaiter {
 
   virtual void AwaitVSync() = 0;
 
-  void FireCallback(fxl::TimePoint frame_start_time,
-                    fxl::TimePoint frame_target_time);
-
   FXL_DISALLOW_COPY_AND_ASSIGN(VsyncWaiter);
 };
 
diff --git a/engine/src/flutter/shell/platform/android/vsync_waiter_android.cc b/engine/src/flutter/shell/platform/android/vsync_waiter_android.cc
index 052de023b9..51c2f461bc 100644
--- a/engine/src/flutter/shell/platform/android/vsync_waiter_android.cc
+++ b/engine/src/flutter/shell/platform/android/vsync_waiter_android.cc
@@ -16,8 +16,6 @@
 
 namespace shell {
 
-static jlong CreatePendingCallback(VsyncWaiter::Callback callback);
-
 static void ConsumePendingCallback(jlong java_baton,
                                    fxl::TimePoint frame_start_time,
                                    fxl::TimePoint frame_target_time);
@@ -32,12 +30,9 @@ VsyncWaiterAndroid::~VsyncWaiterAndroid() = default;
 
 // |shell::VsyncWaiter|
 void VsyncWaiterAndroid::AwaitVSync() {
-  auto java_baton =
-      CreatePendingCallback(std::bind(&VsyncWaiterAndroid::FireCallback,  //
-                                      this,                               //
-                                      std::placeholders::_1,              //
-                                      std::placeholders::_2               //
-                                      ));
+  std::weak_ptr<VsyncWaiter>* weak_this =
+      new std::weak_ptr<VsyncWaiter>(shared_from_this());
+  jlong java_baton = reinterpret_cast<jlong>(weak_this);
 
   task_runners_.GetPlatformTaskRunner()->PostTask([java_baton]() {
     JNIEnv* env = fml::jni::AttachCurrentThread();
@@ -86,27 +81,16 @@ bool VsyncWaiterAndroid::Register(JNIEnv* env) {
   return env->RegisterNatives(clazz, methods, arraysize(methods)) == 0;
 }
 
-struct PendingCallbackData {
-  VsyncWaiter::Callback callback;
-
-  PendingCallbackData(VsyncWaiter::Callback p_callback)
-      : callback(std::move(p_callback)) {
-    FXL_DCHECK(callback);
-  }
-};
-
-static jlong CreatePendingCallback(VsyncWaiter::Callback callback) {
-  // This delete for this new is balanced in the consume call.
-  auto data = new PendingCallbackData(std::move(callback));
-  return reinterpret_cast<jlong>(data);
-}
-
 static void ConsumePendingCallback(jlong java_baton,
                                    fxl::TimePoint frame_start_time,
                                    fxl::TimePoint frame_target_time) {
-  auto data = reinterpret_cast<PendingCallbackData*>(java_baton);
-  data->callback(frame_start_time, frame_target_time);
-  delete data;
+  auto weak_this = reinterpret_cast<std::weak_ptr<VsyncWaiter>*>(java_baton);
+  auto shared_this = weak_this->lock();
+  delete weak_this;
+
+  if (shared_this) {
+    shared_this->FireCallback(frame_start_time, frame_target_time);
+  }
 }
 
 }  // namespace shell
diff --git a/engine/src/flutter/shell/platform/android/vsync_waiter_android.h b/engine/src/flutter/shell/platform/android/vsync_waiter_android.h
index fd72a0a21f..857bce3e2c 100644
--- a/engine/src/flutter/shell/platform/android/vsync_waiter_android.h
+++ b/engine/src/flutter/shell/platform/android/vsync_waiter_android.h
@@ -6,6 +6,7 @@
 #define SHELL_PLATFORM_ANDROID_VSYNC_WAITER_ANDROID_H_
 
 #include <jni.h>
+#include <memory>
 #include "flutter/fml/memory/weak_ptr.h"
 #include "flutter/shell/common/vsync_waiter.h"
 #include "lib/fxl/macros.h"