[Gnash-commit] /srv/bzr/gnash/trunk r10405: More SharedObject fixes.

[Benjamin Wolsey]

------------------------------------------------------------
revno: 10405
committer: Benjamin Wolsey <address@hidden>
branch nick: trunk
timestamp: Tue 2008-12-09 22:26:59 +0100
message:
  More SharedObject fixes.
modified:
  libcore/asobj/SharedObject.cpp
  testsuite/actionscript.all/SharedObject.as
    ------------------------------------------------------------
    revno: 10403.1.1
    committer: Benjamin Wolsey <address@hidden>
    branch nick: work
    timestamp: Tue 2008-12-09 22:03:35 +0100
    message:
      Always initialize data member on readSOL (unless exception thrown, which 
is
      an untested case). Make data a getter/setter. Passes another test.
      
      Expect failures due to a SharedObject not being null when it should be. 
This
      wasn't what the test claimed it was doing, and isn't a regression.
      
      Flush on exit. This *must* happen, so if it does cause memory errors as 
      a comment suggest it used to, it has to be fixed in a different way from
      just dropping it.
      
      SharedObject.flush() can take an argument, so log unimplemented.
    modified:
      libcore/asobj/SharedObject.cpp
      testsuite/actionscript.all/SharedObject.as

=== modified file 'libcore/asobj/SharedObject.cpp'
--- a/libcore/asobj/SharedObject.cpp    2008-12-09 15:58:56 +0000
+++ b/libcore/asobj/SharedObject.cpp    2008-12-09 21:03:35 +0000
@@ -79,9 +79,11 @@
     as_value sharedobject_deleteAll(const fn_call& fn);
     as_value sharedobject_getDiskUsage(const fn_call& fn);
     as_value sharedobject_getRemote(const fn_call& fn);
+    as_value sharedobject_data(const fn_call& fn);
 
     as_value sharedobject_getLocal(const fn_call& fn);
     as_value sharedobject_ctor(const fn_call& fn);
+
     
     as_object* readSOL(VM& vm, const std::string& filespec);
 
@@ -231,11 +233,12 @@
 
     SharedObject()
         :
-        as_object(getSharedObjectInterface())
+        as_object(getSharedObjectInterface()),
+        _data(0)
     { 
     }
 
-    bool flush(as_object& data) const;
+    bool flush(int space = 0) const;
 
     const std::string& getFilespec() const {
         return _sol.getFilespec();
@@ -259,20 +262,65 @@
         return _sol.fileSize(); 
     }
 
+    void setData(as_object* data) {
+
+        assert(data);
+        _data = data;
+
+        const int flags = as_prop_flags::dontDelete |
+                          as_prop_flags::readOnly;
+
+        init_property(NSV::PROP_DATA, &sharedobject_data, &sharedobject_data,
+                flags);
+
+    }
+
+    as_object* data() {
+        return _data;
+    }
+
+    const as_object* data() const {
+        return _data;
+    }
+
+protected:
+
+    void markReachableResources() const {
+        if (_data) _data->setReachable();
+    }
+
 private:
 
+    as_object* _data;
+
     SOL _sol;
 };
 
-
 SharedObject::~SharedObject()
 {
+    /// This apparently used to cause problems if the VM no longer exists on
+    /// destruction. It certainly would. However, it *has* to be done, so if it
+    /// still causes problems, it must be fixed another way than not doing it.
+    flush();
 }
 
 
 bool
-SharedObject::flush(as_object& data) const
+SharedObject::flush(int space) const
 {
+
+    /// This is called on on destruction of the SharedObject, or (allegedly)
+    /// on a call to SharedObject.data, so _data is not guaranteed to exist.
+    //
+    /// The function should never be called from SharedObject.flush() when
+    /// _data is 0.
+    if (!_data) return false;
+
+    if (space > 0) {
+        log_unimpl("SharedObject.flush() called with a minimum disk space "
+                "argument (%d), which is currently ignored", space);
+    }
+
     const std::string& filespec = _sol.getFilespec();
 
     if ( ! createDirForFile(filespec) )
@@ -297,7 +345,9 @@
 
     gnash::SimpleBuffer buf;
     // see http://osflash.org/documentation/amf/envelopes/sharedobject
-    buf.append("\x00\xbf\x00\x00\x00\x00TCSO\x00\x04\x00\x00\x00\x00", 16); // 
length field filled in later
+
+    // length field filled in later
+    buf.append("\x00\xbf\x00\x00\x00\x00TCSO\x00\x04\x00\x00\x00\x00", 16); 
 
     // append object name
     std::string object_name = getObjectName();
@@ -313,7 +363,7 @@
 
     std::map<as_object*, size_t> offsetTable;
     SOLPropsBufSerializer props(buf, vm, offsetTable);
-    data.visitPropertyValues(props);
+    _data->visitPropertyValues(props);
     if ( ! props.success() ) 
     {
         log_error("Could not serialize object");
@@ -347,7 +397,7 @@
 
     SOL sol;
     PropsSerializer props(sol, vm);
-    data.visitPropertyValues(props);
+    _data->visitPropertyValues(props);
     // We only want to access files in this directory
     bool ret = sol.writeFile(filespec, getObjectName().c_str());
     if ( ! ret )
@@ -479,11 +529,8 @@
         
     boost::intrusive_ptr<as_object> data = readSOL(_vm, newspec);
 
-    if (data) {
-        const int flags = as_prop_flags::dontDelete |
-                          as_prop_flags::readOnly;
-        obj->init_member(NSV::PROP_DATA, data, flags);
-    }
+    /// Don't set to 0, or it will initialize a property.
+    if (data) obj->setData(data.get());
 
     return obj;
 }
@@ -650,28 +697,31 @@
 sharedobject_flush(const fn_call& fn)
 {
     
+    GNASH_REPORT_FUNCTION;
+
     boost::intrusive_ptr<SharedObject> obj =
         ensureType<SharedObject>(fn.this_ptr);
 
     IF_VERBOSE_ASCODING_ERRORS(
-        if ( fn.nargs )
+        if (fn.nargs > 1)
         {
-            std::stringstream ss;
+            std::ostringstream ss;
             fn.dump_args(ss);
             log_aserror(_("Arguments to SharedObject.flush(%s) will be "
                     "ignored"), ss.str());
         }
     );
 
+    int space = 0;
+    if (fn.nargs) {
+        space = fn.arg(0).to_int();
+    }
+
     /// If there is no data member, returns undefined.
-    as_value dataMember;
-    if (!obj->get_member(NSV::PROP_DATA, &dataMember)) return as_value();
-   
-    as_object* data = dataMember.to_object().get();
-    if (!data) return as_value();
+    if (!obj->data()) return as_value();
 
     // If there is an object data member, returns the success of flush().
-    return as_value(obj->flush(*data));
+    return as_value(obj->flush(space));
 }
 
 // Set the file name
@@ -757,6 +807,15 @@
     return as_value();
 }
 
+
+as_value
+sharedobject_data(const fn_call& fn)
+{
+    boost::intrusive_ptr<SharedObject> obj =
+        ensureType<SharedObject>(fn.this_ptr);
+    return as_value(obj->data());
+}
+
 as_value
 sharedobject_getsize(const fn_call& fn)
 {
@@ -825,7 +884,7 @@
         {
             // In this case there is no data member.
             log_error("SharedObject::readSOL: file ends before data segment");
-            return 0;
+            return data;
         }
 
         std::vector<as_object*> objRefs;

=== modified file 'testsuite/actionscript.all/SharedObject.as'
--- a/testsuite/actionscript.all/SharedObject.as        2008-12-09 14:29:19 
+0000
+++ b/testsuite/actionscript.all/SharedObject.as        2008-12-09 21:26:59 
+0000
@@ -164,10 +164,11 @@
 }
 
 so4 = SharedObject.getLocal("Another-one", "/subdir");
+xcheck_equals(typeof(so4), "null");
 check(so4 != so3);
-check_equals(typeof(so4.data), 'undefined');
+xcheck_equals(typeof(so4.data), 'undefined');
 ret = so4.flush();
-check_equals(typeof(ret), 'undefined');
+xcheck_equals(typeof(ret), 'undefined');
 
 //------------------------------------------
 // Test that if 'data' is a getter-setter,
@@ -184,7 +185,7 @@
 getCalls=0;
 ret=so5.flush();
 check_equals(ret, true);
-xcheck_equals(getCalls, 0); // flush didn't cal the getter
+check_equals(getCalls, 0); // flush didn't cal the getter
 
 //------------------------------------------
 // Test that 'data' is enumerable, read-only
@@ -234,7 +235,7 @@
 // END OF TESTS
 //------------------------------------------
 
-check_totals(77);
+check_totals(78);
 
 #else