[Gnash-commit] /srv/bzr/gnash/trunk r12165: Cleanup pass. Make sure attempts at copying an NPVariant also copy the

[Bastiaan Jacques]

------------------------------------------------------------
revno: 12165
committer: Bastiaan Jacques <address@hidden>
branch nick: trunk
timestamp: Wed 2010-04-21 16:33:46 +0200
message:
  Cleanup pass. Make sure attempts at copying an NPVariant also copy the
  contained "value".
  
  This code still lacks an ownership model, which will lead to nasty bugs.
modified:
  plugin/npapi/callbacks.cpp
  plugin/npapi/pluginScriptObject.cpp
  plugin/npapi/pluginScriptObject.h

=== modified file 'plugin/npapi/callbacks.cpp'
--- a/plugin/npapi/callbacks.cpp        2010-04-17 02:44:32 +0000
+++ b/plugin/npapi/callbacks.cpp        2010-04-21 14:33:46 +0000
@@ -143,30 +143,13 @@
         if (value == 0) {
             NULL_TO_NPVARIANT(*result);
         } else {
-            if (NPVARIANT_IS_DOUBLE(*value)) {
-                double num = NPVARIANT_TO_DOUBLE(*value);
-                DOUBLE_TO_NPVARIANT(num, *result);
-            } else if (NPVARIANT_IS_STRING(*value)) {
-                
STRINGN_TO_NPVARIANT(NPVARIANT_TO_STRING(*value).UTF8Characters,
-                                     NPVARIANT_TO_STRING(*value).UTF8Length,
-                                     *result);
-            } else if (NPVARIANT_IS_BOOLEAN(*value)) {
-                BOOLEAN_TO_NPVARIANT(NPVARIANT_TO_BOOLEAN(*value), *result);
-            } else if (NPVARIANT_IS_INT32(*value)) {
-                INT32_TO_NPVARIANT(NPVARIANT_TO_INT32(*value), *result);
-            } else if (NPVARIANT_IS_NULL(*value)) {
-                NULL_TO_NPVARIANT(*result);
-            } else if (NPVARIANT_IS_VOID(*value)) {
-                VOID_TO_NPVARIANT(*result);
-            } else if (NPVARIANT_IS_OBJECT(*value)) {
-                OBJECT_TO_NPVARIANT(NPVARIANT_TO_OBJECT(*value), *result);
-            }
+            CopyVariantValue(*value, *result);
+
             NPN_MemFree(value);
             return true;
         }
     }
     
-    NPN_MemFree(value);
     NPVARIANT_IS_NULL(*result);
     return false;
 }

=== modified file 'plugin/npapi/pluginScriptObject.cpp'
--- a/plugin/npapi/pluginScriptObject.cpp       2010-04-19 23:03:02 +0000
+++ b/plugin/npapi/pluginScriptObject.cpp       2010-04-21 14:33:46 +0000
@@ -464,14 +464,7 @@
     }
 #endif
     
-    std::map<NPIdentifier, NPVariant *>::iterator it;
-    it = _properties.find(name);
-    if (it != _properties.end()) {
-        // log_debug(" FOUND");
-        return true;
-    }
-
-    return false;
+    return _properties.find(name) != _properties.end();
 };
 
 bool
@@ -485,43 +478,16 @@
         log_debug("Getting Property %d\"...", NPN_IntFromIdentifier(name));
     }
 
-    std::map<NPIdentifier, NPVariant *>::iterator it;
+    std::map<NPIdentifier, const NPVariant *>::iterator it;
     it = _properties.find(name);
-    if (it != _properties.end()) {
-//        log_debug(" FOUND = ");
-        // We have to copy the data we hold internally into the result
-        // data object, rather than just resetting the result point to
-        // our internal copy, which doesn't work.
-        NPVariant *value = it->second;
-        if (NPVARIANT_IS_DOUBLE(*value)) {
-            double num = NPVARIANT_TO_DOUBLE(*value);
-            log_debug(" %g", num);
-            DOUBLE_TO_NPVARIANT(num, *result);
-        } else if (NPVARIANT_IS_STRING(*value)) {
-            log_debug(" %s", NPVARIANT_TO_STRING(*value).UTF8Characters);
-            STRINGN_TO_NPVARIANT(NPVARIANT_TO_STRING(*value).UTF8Characters,
-                                 NPVARIANT_TO_STRING(*value).UTF8Length,
-                                 *result);
-        } else if (NPVARIANT_IS_BOOLEAN(*value)) {
-            log_debug(" %d", NPVARIANT_TO_BOOLEAN(*value));
-            BOOLEAN_TO_NPVARIANT(NPVARIANT_TO_BOOLEAN(*value), *result);
-        } else if (NPVARIANT_IS_INT32(*value)) {
-            log_debug(" %d", NPVARIANT_TO_INT32(*value));
-            INT32_TO_NPVARIANT(NPVARIANT_TO_INT32(*value), *result);
-        } else if (NPVARIANT_IS_NULL(*value)) {
-            log_debug(" null value");
-            NULL_TO_NPVARIANT(*result);
-        } else if (NPVARIANT_IS_VOID(*value)) {
-            log_debug(" void value");
-            VOID_TO_NPVARIANT(*result);
-        } else if (NPVARIANT_IS_OBJECT(*value)) {
-            log_debug(" object");
-            OBJECT_TO_NPVARIANT(NPVARIANT_TO_OBJECT(*value), *result);
-        }
-        return true;
+    if (it == _properties.end()) {
+        return false;
     }
 
-    return false;
+    const NPVariant *value = it->second;
+    CopyVariantValue(*value, *result);
+
+    return true;
 };
 
 bool
@@ -529,7 +495,7 @@
 {
     // log_debug(__PRETTY_FUNCTION__);
 
-    _properties[name] = const_cast<NPVariant *>(value);
+    _properties[name] = value;
 
     return false;
 }
@@ -539,7 +505,7 @@
 {
     // log_debug(__PRETTY_FUNCTION__);
 
-    std::map<NPIdentifier, NPVariant *>::iterator it;
+    std::map<NPIdentifier, const NPVariant *>::iterator it;
     it = _properties.find(name);
     if (it != _properties.end()) {
         _properties.erase(it);
@@ -580,14 +546,7 @@
     }
 #endif
 
-    std::map<NPIdentifier, NPInvokeFunctionPtr>::iterator it;
-    it = _methods.find(name);
-    if (it != _methods.end()) {
-        // log_debug(" FOUND");
-        return true;
-    }    
-    
-    return false;
+    return _methods.find(name) != _methods.end();
 }
 
 bool
@@ -1087,6 +1046,34 @@
     return plugin->handleInvoke(iochan, cond);
 }
 
+void
+CopyVariantValue(const NPVariant& from, NPVariant& to)
+{
+    // First, we'll make a shallow copy, which is fine for most variant types.
+    to = from;
+
+    // For deep copies for strings we obviously have to duplicate the string,
+    // and object pointer copies need to have their reference count increased.
+    switch(from.type) {
+        case NPVariantType_String:
+        {
+            const NPString& fromstr = NPVARIANT_TO_STRING(from);
+            const uint32_t& len = fromstr.UTF8Length;
+
+            NPUTF8* tostr = static_cast<NPUTF8*>(NPN_MemAlloc(len));
+            std::copy(fromstr.UTF8Characters, fromstr.UTF8Characters+len, 
tostr);
+
+            STRINGN_TO_NPVARIANT(tostr, len, to);
+            break;
+        }
+        case NPVariantType_Object:
+            NPN_RetainObject(NPVARIANT_TO_OBJECT(to));
+            break;
+        default:
+        {}
+    }
+}
+
 // local Variables:
 // mode: C++
 // indent-tabs-mode: nil

=== modified file 'plugin/npapi/pluginScriptObject.h'
--- a/plugin/npapi/pluginScriptObject.h 2010-04-18 00:35:41 +0000
+++ b/plugin/npapi/pluginScriptObject.h 2010-04-21 14:33:46 +0000
@@ -16,7 +16,6 @@
 // Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
 //
 
-#include <glib.h>
 
 // Test:
 //     Use browser to open plugin/scriptable-test.html.
@@ -39,12 +38,19 @@
 #include <map>
 #include <string>
 
+#include <glib.h>
 #include "npapi.h"
 #include "npruntime.h"
 
 #define READFD 0
 #define WRITEFD 1
 
+/// Makes a deep copy of a NPVariant.
+/// @param from The source NPVariant to copy values from.
+/// @param to The destination NPVariant.
+void
+CopyVariantValue(const NPVariant& from, NPVariant& to);
+
 class GnashPluginScriptObject : public NPObject
 {
 public:
@@ -173,8 +179,9 @@
     void setInstance(NPP inst) { _nppinstance = inst; };
     
     // _nppinstance->pdata should be the nsPluginInstance once NPP_New() is 
finished.
-    NPP         _nppinstance;
-    std::map<NPIdentifier, NPVariant *> _properties;
+    NPP _nppinstance;
+    
+    std::map<NPIdentifier, const NPVariant*> _properties;
     std::map<NPIdentifier,  NPInvokeFunctionPtr> _methods;
     // 0 for reading, 1 for writing
     int         _sockfds[2];