Call native animation callbacks only once

change/react-native-windows-2019-10-31-23-56-14-user-asklar-callbacks.json

@@ -0,0 +1,8 @@
+{
+  "type": "prerelease",
+  "comment": "Call native animation callbacks only once",
+  "packageName": "react-native-windows",
+  "email": "asklar@microsoft.com",
+  "commit": "141e2654c5b5ed958591c4933a82d85213b56e40",
+  "date": "2019-11-01T06:56:14.460Z"
+}

vnext/ReactUWP/Modules/Animated/AnimationDriver.cpp

@@ -23,6 +23,19 @@ AnimationDriver::AnimationDriver(
   }();
 }
 
+void AnimationDriver::DoCallback(bool value) {
+  if (m_endCallback) {
+    m_endCallback(std::vector<folly::dynamic>{folly::dynamic::object("finished", value)});
+  }
+#ifdef DEBUG
+  m_debug_callbackAttempts++;
+  if (m_debug_callbackAttempts > 1) {
+    throw std::string("The \"finished\" callback for a native animation was called more than once.");
+  }
+#endif //  DEBUG
+  m_endCallback = {};
+}
+
 AnimationDriver::~AnimationDriver() {
   if (m_scopedBatch)
     m_scopedBatch.Completed(m_scopedBatchCompletedToken);
@@ -41,20 +54,16 @@ void AnimationDriver::StartAnimation() {
   }
   scopedBatch.End();
 
-  m_scopedBatchCompletedToken = scopedBatch.Completed(
-      [EndCallback = m_endCallback, weakManager = m_manager, valueTag = m_animatedValueTag, id = m_id](
-          auto sender, auto) {
-        if (EndCallback) {
-          EndCallback(std::vector<folly::dynamic>{folly::dynamic::object("finished", true)});
-        }
-        if (auto manager = weakManager.lock()) {
-          if (auto const animatedValue = manager->GetValueAnimatedNode(valueTag)) {
-            animatedValue->RemoveActiveAnimation(id);
-            animatedValue->FlattenOffset();
-          }
-          manager->RevmoveActiveAnimation(id);
-        }
-      });
+  m_scopedBatchCompletedToken = scopedBatch.Completed([&](auto sender, auto) {
+    DoCallback(true);
+    if (auto manager = m_manager.lock()) {
+      if (auto const animatedValue = manager->GetValueAnimatedNode(m_animatedValueTag)) {
+        animatedValue->RemoveActiveAnimation(m_id);
+        animatedValue->FlattenOffset();
+      }
+      manager->RemoveActiveAnimation(m_id);
+    }
+  });
 
   m_animation = animation;
   m_scopedBatch = scopedBatch;
@@ -67,8 +76,7 @@ void AnimationDriver::StopAnimation(bool ignoreCompletedHandlers) {
       animatedValue->RemoveActiveAnimation(m_id);
 
       if (m_scopedBatch) {
-        if (m_endCallback)
-          m_endCallback(std::vector<folly::dynamic>{folly::dynamic::object("finished", false)});
+        DoCallback(false);
         m_scopedBatch.Completed(m_scopedBatchCompletedToken);
         m_scopedBatch = nullptr;
       }

vnext/ReactUWP/Modules/Animated/AnimationDriver.h

@@ -57,7 +57,6 @@ class AnimationDriver {
 
   int64_t m_id{0};
   int64_t m_animatedValueTag{};
-  Callback m_endCallback{};
   int64_t m_iterations{0};
   folly::dynamic m_config{};
   std::weak_ptr<NativeAnimatedNodeManager> m_manager{};
@@ -67,6 +66,13 @@ class AnimationDriver {
   // auto revoker for scopedBatch.Completed is broken, tracked by internal bug
   // #22399779
   winrt::event_token m_scopedBatchCompletedToken{};
+
+ private:
+  Callback m_endCallback{};
+  void DoCallback(bool value);
+#ifdef DEBUG
+  int m_debug_callbackAttempts{0};
+#endif // DEBUG
 };
 } // namespace uwp
 } // namespace react

vnext/ReactUWP/Modules/Animated/NativeAnimatedNodeManager.cpp

@@ -396,7 +396,7 @@ TrackingAnimatedNode *NativeAnimatedNodeManager::GetTrackingAnimatedNode(int64_t
   return nullptr;
 }
 
-void NativeAnimatedNodeManager::RevmoveActiveAnimation(int64_t tag) {
+void NativeAnimatedNodeManager::RemoveActiveAnimation(int64_t tag) {
   m_activeAnimations.erase(tag);
 }
 } // namespace uwp

vnext/ReactUWP/Modules/Animated/NativeAnimatedNodeManager.h

@@ -88,7 +88,7 @@ class NativeAnimatedNodeManager {
   StyleAnimatedNode *GetStyleAnimatedNode(int64_t tag);
   TransformAnimatedNode *GetTransformAnimatedNode(int64_t tag);
   TrackingAnimatedNode *GetTrackingAnimatedNode(int64_t tag);
-  void RevmoveActiveAnimation(int64_t tag);
+  void RemoveActiveAnimation(int64_t tag);
 
  private:
   std::unordered_map<int64_t, std::unique_ptr<ValueAnimatedNode>> m_valueNodes{};

yarn.lock

@@ -9863,10 +9863,10 @@ react-is@^16.8.1, react-is@^16.8.4, react-is@^16.8.6:
   resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.9.0.tgz#21ca9561399aad0ff1a7701c01683e8ca981edcb"
   integrity sha512-tJBzzzIgnnRfEm046qRcURvwQnZVXmuCbscxUO5RWrGTXpon2d4c8mI0D8WE6ydVIm29JiLB6+RslkIvym9Rjw==
 
-"react-native@https://github.com/microsoft/react-native/archive/v0.60.0-microsoft.13.tar.gz":
-  version "0.60.0-microsoft.13"
-  uid ed08bc48f6601a1ca854016767567fd30f9a6b2e
-  resolved "https://github.com/microsoft/react-native/archive/v0.60.0-microsoft.13.tar.gz#ed08bc48f6601a1ca854016767567fd30f9a6b2e"
+"react-native@https://github.com/microsoft/react-native/archive/v0.60.0-microsoft.14.tar.gz":
+  version "0.60.0-microsoft.14"
+  uid fa964a148d696ef3913f414224695b9e34abf363
+  resolved "https://github.com/microsoft/react-native/archive/v0.60.0-microsoft.14.tar.gz#fa964a148d696ef3913f414224695b9e34abf363"
   dependencies:
     "@babel/core" "^7.4.0"
     "@babel/generator" "^7.4.0"