[vlc-commits] npapi: Don't wrap callback arguments into Variant' s out of the plugin thread

Hugo Beauzée-Luyssen git at videolan.org
Wed Jan 27 16:20:29 CET 2016


npapi-vlc | branch: master | Hugo Beauzée-Luyssen <hugo at beauzee.fr> | Wed Jan 27 16:18:44 2016 +0100| [0dc92cb3037ff374a02612e87a4dad382e57a8b8] | committer: Hugo Beauzée-Luyssen

npapi: Don't wrap callback arguments into Variant's out of the plugin thread

This completes previous #16292 fix

> https://code.videolan.org/videolan/npapi-vlc/commit/0dc92cb3037ff374a02612e87a4dad382e57a8b8
---

 npapi/utils.hpp          | 120 +++++++++++------------------------------------
 npapi/vlcplugin_base.cpp |  18 ++++---
 2 files changed, 39 insertions(+), 99 deletions(-)

diff --git a/npapi/utils.hpp b/npapi/utils.hpp
index 2b6d20b..3ee47d6 100644
--- a/npapi/utils.hpp
+++ b/npapi/utils.hpp
@@ -24,13 +24,14 @@
 #ifndef UTILS_NPP
 #define UTILS_NPP
 
-#include <npruntime.h>
+#include <array>
+#include <cassert>
+#include <cstring>
 #include <memory>
-#include <type_traits>
+#include <npruntime.h>
 #include <string>
-
-#include <cstring>
-#include <cassert>
+#include <tuple>
+#include <type_traits>
 
 using CStr = std::unique_ptr<char, void(*)(void*)>;
 
@@ -614,107 +615,40 @@ private:
 using Variant = details::Variant<details::policy::Embeded>;
 using OutVariant = details::Variant<details::policy::Wrapped>;
 
-class VariantArray
+namespace details
 {
-    using VPtr = std::unique_ptr<Variant[]>;
-public:
-    VariantArray()
-        : m_variants( nullptr )
-        , m_size( 0 )
-    {
-    }
-    VariantArray(unsigned int nbValue)
-        : m_variants( VPtr( new Variant[nbValue] ) )
-        , m_size( nbValue )
-    {
-    }
-
-    Variant& operator[](size_t idx)
-    {
-        return m_variants[idx];
-    }
-
-    const Variant& operator[](size_t idx) const
-    {
-        return m_variants[idx];
-    }
-
-    // Warning: this assumes the same binary layout between Variant & NPVariant
-    operator NPVariant*()
-    {
-        return (NPVariant*)m_variants.get();
-    }
-
-    operator const NPVariant*() const
-    {
-        return (const NPVariant*)m_variants.get();
-    }
-
-    size_t size() const
+    template <size_t... Ns>
+    struct Seq
     {
-        return m_size;
-    }
-
-    VariantArray(const VariantArray&) = delete;
-    VariantArray& operator=(const VariantArray&) = delete;
-#ifndef _MSC_VER
-    VariantArray(VariantArray&&) = default;
-    VariantArray& operator=(VariantArray&&) = default;
-#else
-    VariantArray(VariantArray&& v)
-        : m_variants(std::move( v.m_variants ) )
-        , m_size( v.m_size )
-    {
-    }
+        using type = Seq<Ns..., sizeof...(Ns)>;
+    };
 
-    VariantArray& operator=(VariantArray&& v)
+    template <size_t N>
+    struct GenSeq
     {
-        if (&v == this)
-            return *this;
-        m_variants = std::move(v.m_variants);
-        m_size = v.m_size;
-        return *this;
-    }
-
-#endif
-private:
-    VPtr m_variants;
-    size_t m_size;
-};
+        // This will recurse down to Seq<> which yields type = Seq<0>
+        // Then, from Seq<0>::type, up to Seq<N-1>::type, which ends
+        // up generating Seq<0, 1, ... N>
+        using type = typename GenSeq<N - 1>::type::type;
+    };
 
-// Private implementation namespace
-namespace details
-{
-    template <size_t Idx, typename T>
-    void wrap( VariantArray& array, T arg )
+    template <>
+    struct GenSeq<0>
     {
-        array[Idx] = Variant<details::policy::Embeded>(arg);
-    }
+        using type = Seq<>;
+    };
 
-    template <size_t Idx, typename T, typename... Args>
-    void wrap( VariantArray& array, T arg, Args&&... args )
+    template <typename Tuple, size_t... Indices>
+    std::array<npapi::Variant, sizeof...(Indices)> wrap( const Tuple& t, Seq<Indices...> )
     {
-        wrap<Idx + 1>( array, std::forward<Args>( args )... );
-        // This needs the Variant wrapper to be the exact size of a NPVariant
-        // For future proofness, we make this explicit:
-        array[Idx] = Variant<details::policy::Embeded>(arg);
+        return { npapi::Variant( std::get<Indices>( t ) )... };
     }
 }
 
-// public entry point. Responsible for allocating the array and initializing
-// the template index parameter (to avoid filling to array in reverse order with
-// sizeof...()
 template <typename... Args>
-VariantArray wrap(Args&&... args)
-{
-    auto array = VariantArray{sizeof...(args)};
-    details::wrap<0>( array, std::forward<Args>( args )... );
-    return array;
-}
-
-inline VariantArray wrap()
+std::array<Variant, sizeof...(Args)> wrap( const std::tuple<Args...>& args )
 {
-    return VariantArray{};
+    return details::wrap( args, typename details::GenSeq<sizeof...(Args)>::type{} );
 }
 
 }
diff --git a/npapi/vlcplugin_base.cpp b/npapi/vlcplugin_base.cpp
index 5160826..a78e640 100644
--- a/npapi/vlcplugin_base.cpp
+++ b/npapi/vlcplugin_base.cpp
@@ -277,28 +277,34 @@ bool VlcPluginBase::handle_event(void *)
     return false;
 }
 
+template <typename... Args>
 struct AsyncEventWrapper
 {
-    AsyncEventWrapper(NPP b, NPObject* l, npapi::VariantArray&& a)
+    template <typename... PFArgs>
+    AsyncEventWrapper(NPP b, NPObject* l, PFArgs&&... args)
         : browser( b )
         , listener( l )
-        , args( std::move( a ) )
+        , args( std::make_tuple( std::forward<Args>( args )... ) )
     {
     }
 
     NPP browser;
     NPObject* listener;
-    npapi::VariantArray args;
+    // If we do not decay Args here, we will end up declaring a tuple holding references
+    // The tuple returned by make_tuple does that, but we end up converting from the correct tuple
+    // type to an invalid tuple type, since we would end up using dangling references.
+    std::tuple<typename std::decay<Args>::type...> args;
 };
 
 template <typename... Args>
 static void invokeEvent( NPP browser, NPObject* listener, Args&&... args )
 {
-    auto wrapper = new AsyncEventWrapper( browser, listener, npapi::wrap( std::forward<Args>( args )... ) );
+    auto wrapper = new AsyncEventWrapper<Args...>( browser, listener, std::forward<Args>( args )... );
     NPN_PluginThreadAsyncCall( browser, [](void* data) {
-        auto w = reinterpret_cast<AsyncEventWrapper*>( data );
+        auto w = reinterpret_cast<AsyncEventWrapper<Args...>*>( data );
+        auto args = npapi::wrap( w->args );
         NPVariant result;
-        if (NPN_InvokeDefault( w->browser, w->listener, w->args, w->args.size(), &result ))
+        if (NPN_InvokeDefault( w->browser, w->listener, reinterpret_cast<NPVariant*>( args.data() ), args.size(), &result ))
         {
             // "result" content is unspecified when invoke fails. Don't clean potential garbage
             NPN_ReleaseVariantValue( &result );



More information about the vlc-commits mailing list