# HG changeset patch
# User Benjamin Golinvaux <bgo@osimis.io>
# Date 1585310067 -3600
# Node ID fd616c4a590432f8024dbd4a18b713dd9cfc8c9f
# Parent  4f8db2d202c8cdfe0b75871b63ef3a66bf4c77bf
Added mechanism to prevent callbacks from being sent on dead WebAssemblyViewport objects

diff -r 4f8db2d202c8 -r fd616c4a5904 Framework/Toolbox/GenericToolbox.h
--- a/Framework/Toolbox/GenericToolbox.h	Wed Mar 25 14:34:27 2020 +0100
+++ b/Framework/Toolbox/GenericToolbox.h	Fri Mar 27 12:54:27 2020 +0100
@@ -20,10 +20,15 @@
 
 #pragma once
 
+#include <Core/Compatibility.h>
+
+#include <boost/shared_ptr.hpp>
+
 #include <string>
 #include <stdint.h>
 #include <math.h>
 
+#include <memory>
 
 namespace OrthancStone
 {
@@ -281,14 +286,52 @@
     /**
     Same as GetRgbValuesFromString
     */
-    bool GetRgbaValuesFromString(uint8_t& red, uint8_t& green, uint8_t& blue, uint8_t& alpha, const char* text);
+    bool GetRgbaValuesFromString(uint8_t& red,
+                                 uint8_t& green,
+                                 uint8_t& blue,
+                                 uint8_t& alpha,
+                                 const char* text);
 
     /**
     Same as GetRgbValuesFromString
     */
-    inline bool GetRgbaValuesFromString(uint8_t& red, uint8_t& green, uint8_t& blue, uint8_t& alpha, const std::string& text)
+    inline bool GetRgbaValuesFromString(uint8_t& red, 
+                                        uint8_t& green, 
+                                        uint8_t& blue, 
+                                        uint8_t& alpha, 
+                                        const std::string& text)
     {
       return GetRgbaValuesFromString(red, green, blue, alpha, text.c_str());
     }
+
+
+    template<typename Wrappee> 
+    struct HoldingRef
+    {
+      HoldingRef(Wrappee* object)
+      {
+        // a crash here means that the object is not stored in a shared_ptr
+        // using shared_ptr is mandatory for this
+        object_ = object->shared_from_this();
+      }
+      boost::shared_ptr<Wrappee> object_;
+
+      static void* Wrap(Wrappee* object)
+      {
+        std::unique_ptr<HoldingRef<Wrappee > > up(new HoldingRef<Wrappee>(object));
+        void* userData = reinterpret_cast<void*>(up.release());
+        return userData;
+      }
+
+      static boost::shared_ptr<Wrappee> Unwrap(void* userData)
+      {
+        // the stored shared_ptr will be deleted because of wrapping
+        // the data in a RAII
+        std::unique_ptr<HoldingRef<Wrappee > >
+          up(reinterpret_cast<HoldingRef<Wrappee>*>(userData));
+        boost::shared_ptr<Wrappee> object = up->object_;
+        return object;
+      }
+    };
   }
 }
diff -r 4f8db2d202c8 -r fd616c4a5904 Framework/Viewport/WebAssemblyViewport.cpp
--- a/Framework/Viewport/WebAssemblyViewport.cpp	Wed Mar 25 14:34:27 2020 +0100
+++ b/Framework/Viewport/WebAssemblyViewport.cpp	Fri Mar 27 12:54:27 2020 +0100
@@ -21,9 +21,12 @@
 
 #include "WebAssemblyViewport.h"
 
+#include "../Toolbox/GenericToolbox.h"
+
 #include <Core/OrthancException.h>
 
 #include <boost/make_shared.hpp>
+#include <boost/enable_shared_from_this.hpp>
 
 namespace OrthancStone
 {
@@ -103,12 +106,12 @@
 
   EM_BOOL WebAssemblyViewport::OnRequestAnimationFrame(double time, void *userData)
   {
-    WebAssemblyViewport& that = *reinterpret_cast<WebAssemblyViewport*>(userData);
+    boost::shared_ptr<WebAssemblyViewport> that = GenericToolbox::HoldingRef<WebAssemblyViewport>::Unwrap(userData);
 
-    if (that.compositor_.get() != NULL &&
-        that.controller_ /* should always be true */)
+    if (that->compositor_.get() != NULL &&
+        that->controller_ /* should always be true */)
     {
-      that.Paint(*that.compositor_, *that.controller_);
+      that->Paint(*that->compositor_, *that->controller_);
     }
       
     return true;
@@ -116,12 +119,12 @@
 
   EM_BOOL WebAssemblyViewport::OnResize(int eventType, const EmscriptenUiEvent *uiEvent, void *userData)
   {
-    WebAssemblyViewport& that = *reinterpret_cast<WebAssemblyViewport*>(userData);
+    boost::shared_ptr<WebAssemblyViewport> that = GenericToolbox::HoldingRef<WebAssemblyViewport>::Unwrap(userData);
 
-    if (that.compositor_.get() != NULL)
+    if (that->compositor_.get() != NULL)
     {
-      that.UpdateSize(*that.compositor_);
-      that.Invalidate();
+      that->UpdateSize(*that->compositor_);
+      that->Invalidate();
     }
       
     return true;
@@ -130,20 +133,20 @@
 
   EM_BOOL WebAssemblyViewport::OnMouseDown(int eventType, const EmscriptenMouseEvent *mouseEvent, void *userData)
   {
-    WebAssemblyViewport& that = *reinterpret_cast<WebAssemblyViewport*>(userData);
+    boost::shared_ptr<WebAssemblyViewport> that = GenericToolbox::HoldingRef<WebAssemblyViewport>::Unwrap(userData);
 
-    LOG(INFO) << "mouse down: " << that.GetFullCanvasId();      
+    LOG(INFO) << "mouse down: " << that->GetFullCanvasId();      
 
-    if (that.compositor_.get() != NULL &&
-        that.interactor_.get() != NULL)
+    if (that->compositor_.get() != NULL &&
+        that->interactor_.get() != NULL)
     {
       PointerEvent pointer;
-      ConvertMouseEvent(pointer, *mouseEvent, *that.compositor_);
+      ConvertMouseEvent(pointer, *mouseEvent, *that->compositor_);
 
-      that.controller_->HandleMousePress(*that.interactor_, pointer,
-                                         that.compositor_->GetCanvasWidth(),
-                                         that.compositor_->GetCanvasHeight());        
-      that.Invalidate();
+      that->controller_->HandleMousePress(*that->interactor_, pointer,
+                                         that->compositor_->GetCanvasWidth(),
+                                         that->compositor_->GetCanvasHeight());        
+      that->Invalidate();
     }
 
     return true;
@@ -152,44 +155,41 @@
     
   EM_BOOL WebAssemblyViewport::OnMouseMove(int eventType, const EmscriptenMouseEvent *mouseEvent, void *userData)
   {
-    WebAssemblyViewport& that = *reinterpret_cast<WebAssemblyViewport*>(userData);
+    boost::shared_ptr<WebAssemblyViewport> that = GenericToolbox::HoldingRef<WebAssemblyViewport>::Unwrap(userData);
 
-    if (that.compositor_.get() != NULL &&
-        that.controller_->HasActiveTracker())
+    if (that->compositor_.get() != NULL &&
+        that->controller_->HasActiveTracker())
     {
       PointerEvent pointer;
-      ConvertMouseEvent(pointer, *mouseEvent, *that.compositor_);
-      if (that.controller_->HandleMouseMove(pointer))
+      ConvertMouseEvent(pointer, *mouseEvent, *that->compositor_);
+      if (that->controller_->HandleMouseMove(pointer))
       {
-        that.Invalidate();
+        that->Invalidate();
       }
     }
 
     return true;
   }
+    
+  EM_BOOL WebAssemblyViewport::OnMouseUp(int eventType, const EmscriptenMouseEvent *mouseEvent, void *userData)
+  {
+    boost::shared_ptr<WebAssemblyViewport> that = GenericToolbox::HoldingRef<WebAssemblyViewport>::Unwrap(userData);
+
+    if (that->compositor_.get() != NULL)
+    {
+      PointerEvent pointer;
+      ConvertMouseEvent(pointer, *mouseEvent, *that->compositor_);
+      that->controller_->HandleMouseRelease(pointer);
+      that->Invalidate();
+    }
+
+    return true;
+  }
 
-    
-  EM_BOOL WebAssemblyViewport::OnMouseUp(int eventType, const EmscriptenMouseEvent *mouseEvent, void *userData)
-  {
-    WebAssemblyViewport& that = *reinterpret_cast<WebAssemblyViewport*>(userData);
-
-    if (that.compositor_.get() != NULL)
-    {
-      PointerEvent pointer;
-      ConvertMouseEvent(pointer, *mouseEvent, *that.compositor_);
-      that.controller_->HandleMouseRelease(pointer);
-      that.Invalidate();
-    }
-
-    return true;
-  }
-
-    
   void WebAssemblyViewport::Invalidate()
   {
-    emscripten_request_animation_frame(OnRequestAnimationFrame, this);
+    emscripten_request_animation_frame(OnRequestAnimationFrame, GenericToolbox::HoldingRef<WebAssemblyViewport>::Wrap(this));
   }
-    
 
   void WebAssemblyViewport::AcquireCompositor(ICompositor* compositor /* takes ownership */)
   {
diff -r 4f8db2d202c8 -r fd616c4a5904 Framework/Viewport/WebAssemblyViewport.h
--- a/Framework/Viewport/WebAssemblyViewport.h	Wed Mar 25 14:34:27 2020 +0100
+++ b/Framework/Viewport/WebAssemblyViewport.h	Fri Mar 27 12:54:27 2020 +0100
@@ -36,7 +36,9 @@
 
 namespace OrthancStone
 {
-  class WebAssemblyViewport : public IViewport
+  class WebAssemblyViewport : public IViewport,
+                              public boost::enable_shared_from_this<WebAssemblyViewport>
+
   {
   private:
     class WasmLock;
@@ -84,6 +86,9 @@
 
     virtual ILock* Lock() ORTHANC_OVERRIDE;
 
+    /**
+    This method takes ownership
+    */
     void AcquireInteractor(IViewportInteractor* interactor);
 
     const std::string& GetShortCanvasId() const
diff -r 4f8db2d202c8 -r fd616c4a5904 UnitTestsSources/GenericToolboxTests.cpp
--- a/UnitTestsSources/GenericToolboxTests.cpp	Wed Mar 25 14:34:27 2020 +0100
+++ b/UnitTestsSources/GenericToolboxTests.cpp	Fri Mar 27 12:54:27 2020 +0100
@@ -22,6 +22,7 @@
 
 #include <boost/date_time/posix_time/posix_time.hpp>
 #include <boost/lexical_cast.hpp>
+#include <boost/enable_shared_from_this.hpp>
 
 #include "gtest/gtest.h"
 
@@ -4243,3 +4244,51 @@
   EXPECT_EQ(0, green);
   EXPECT_EQ(0, blue);
 }
+
+static int Test_GenericToolbox_HoldingRef_ctor_trace = 100;
+static int Test_GenericToolbox_HoldingRef_dtor_trace = 1000;
+
+class Test_GenericToolbox_HoldingRef : public boost::enable_shared_from_this<Test_GenericToolbox_HoldingRef>
+{
+public:
+  Test_GenericToolbox_HoldingRef() : answer_(42) { Test_GenericToolbox_HoldingRef_ctor_trace++; }
+  unsigned int answer_;
+
+  ~Test_GenericToolbox_HoldingRef()
+  {
+    Test_GenericToolbox_HoldingRef_dtor_trace--;
+  }
+};
+
+TEST(GenericToolbox, HoldingRef)
+{
+  using namespace OrthancStone::GenericToolbox;
+
+  void* userData = NULL;
+
+  {
+    boost::shared_ptr<Test_GenericToolbox_HoldingRef> test(new Test_GenericToolbox_HoldingRef() );
+    Test_GenericToolbox_HoldingRef* This = test.get();
+    userData = HoldingRef<Test_GenericToolbox_HoldingRef>::Wrap(This);
+    EXPECT_EQ(42u,This->answer_);
+    This->answer_ = 0xdeadbeef;
+    EXPECT_EQ(0xdeadbeef,This->answer_);
+  }
+  
+  EXPECT_EQ(101,Test_GenericToolbox_HoldingRef_ctor_trace);
+  EXPECT_EQ(1000,Test_GenericToolbox_HoldingRef_dtor_trace);
+  
+  {
+    boost::shared_ptr<Test_GenericToolbox_HoldingRef> test2 = HoldingRef<Test_GenericToolbox_HoldingRef>::Unwrap(userData);
+    EXPECT_EQ(0xdeadbeef,test2->answer_);
+  }
+  
+  EXPECT_EQ(1001,Test_GenericToolbox_HoldingRef_dtor_trace); // make sure wrapper was deleted once
+}
+
+
+
+
+
+
+