[Gnash-commit] /srv/bzr/gnash/trunk r11553: More property lookup improvements.

[Benjamin Wolsey]

------------------------------------------------------------
revno: 11553 [merge]
committer: Benjamin Wolsey <address@hidden>
branch nick: trunk
timestamp: Thu 2009-10-08 15:45:57 +0200
message:
  More property lookup improvements.
modified:
  libcore/MovieClip.h
  libcore/as_object.cpp
  libcore/as_object.h
  testsuite/swfdec/PASSING

=== modified file 'libcore/MovieClip.h'
--- a/libcore/MovieClip.h       2009-10-08 10:34:32 +0000
+++ b/libcore/MovieClip.h       2009-10-08 13:09:26 +0000
@@ -522,6 +522,14 @@
     void loadVariables(const std::string& urlstr,
             VariablesMethod sendVarsMethod);
 
+    /// Get special MovieClip properties
+    //
+    /// These are properties not attached as genuine members to the MovieClip
+    /// object. Currently they include DisplayList members and TextField
+    /// variables.
+    //
+    /// TODO: work out if there is a better way of doing this, such as
+    /// attaching special properties as real members.
     bool getMovieClipProperty(string_table::key name, as_value& val);
 
     // See dox in as_object.h

=== modified file 'libcore/as_object.cpp'
--- a/libcore/as_object.cpp     2009-10-08 09:50:44 +0000
+++ b/libcore/as_object.cpp     2009-10-08 12:33:01 +0000
@@ -65,6 +65,14 @@
 
     };
 
+    class Exists
+    {
+    public:
+        Exists(as_object*) {}
+        bool operator()(const Property* const) const {
+            return true;
+        }
+    };
 }
 
 template<typename T>
@@ -107,16 +115,6 @@
         return _object && !_object->_displayObject;
     }
 
-    /// Return the object reached in searching the chain.
-    //
-    /// This will abort if there is no current object, so make sure
-    /// operator() returns true and that the PrototypeRecursor was
-    /// initialized with a valid as_object.
-    as_object* currentObject() const {
-        assert(_object);
-        return _object;
-    }
-
     /// Return the wanted property if it exists and satisfies the predicate.
     //
     /// This will abort if there is no current object.
@@ -401,7 +399,7 @@
                        cacheVal = trig.call(cacheVal, as_value(), *this);
 
                        // The trigger call could have deleted the property,
-                       // so we check for its existance again, and do NOT put
+                       // so we check for its existence again, and do NOT put
                        // it back in if it was deleted
                        prop = _members.getProperty(k);
                        if ( ! prop )
@@ -420,10 +418,12 @@
 
 /// Order of property lookup:
 //
-/// 1. Own properties.
+/// 1. Visible own properties.
 /// 2. If DisplayObject, magic properties
-/// 3. Own properties of all __proto__ objects (a DisplayObject ends the 
chain).
-/// 4. __resolve properties of this object and all __proto__ objects.
+/// 3. Visible own properties of all __proto__ objects (a DisplayObject
+///    ends the chain).
+/// 4. __resolve property of this object and all __proto__ objects (a Display
+///    Object ends the chain). This should ignore visibility but doesn't.
 bool
 as_object::get_member(string_table::key name, as_value* val,
        string_table::key nsname)
@@ -590,39 +590,26 @@
 Property*
 as_object::findUpdatableProperty(const ObjectURI& uri)
 {
-    const string_table::key key = getName(uri), nsname = getNamespace(uri);
 
        const int swfVersion = getSWFVersion(*this);
 
-       Property* prop = _members.getProperty(key, nsname);
-       // 
+    PrototypeRecursor<Exists> pr(this, uri);
+
+       Property* prop = pr.getProperty();
+
        // We won't scan the inheritance chain if we find a member,
        // even if invisible.
-       // 
-       if ( prop )     return prop;  // TODO: what about visible ?
-
-       std::set<as_object*> visited;
-       visited.insert(this);
-
-       int i = 0;
-
-       boost::intrusive_ptr<as_object> obj = get_prototype();
-
-    // TODO: does this recursion protection exist in the PP?
-       while (obj && visited.insert(obj.get()).second)
-       {
-               ++i;
-               if ((i > 255 && swfVersion == 5) || i > 257)
-                       throw ActionLimitException("Property lookup depth 
exceeded.");
-
-               Property* p = obj->_members.getProperty(key, nsname);
-               if (p && (p->isGetterSetter() | p->isStatic()) && 
p->visible(swfVersion))
-               {
-                       return p; // What should we do if this is not a 
getter/setter ?
-               }
-               obj = obj->get_prototype();
+       if (prop) return prop; 
+
+    while (pr()) {
+        if ((prop = pr.getProperty())) {
+            if ((prop->isStatic() || prop->isGetterSetter()) &&
+                    prop->visible(swfVersion)) {
+                return prop;
+            }
+        }
        }
-       return NULL;
+       return 0;
 }
 
 void
@@ -702,7 +689,7 @@
             boost::bind(SecondElement<TriggerContainer::value_type>(), _1)));
                     
     // The trigger call could have deleted the property,
-    // so we check for its existance again, and do NOT put
+    // so we check for its existence again, and do NOT put
     // it back in if it was deleted
     prop = findUpdatableProperty(uri);
     if (!prop) {
@@ -716,15 +703,44 @@
     
 }
 
-// Handles read_only and static properties properly.
+/// Order of property lookup:
+//
+/// 1. Own properties even if invisible or not getter-setters. 
+/// 2. If DisplayObject, magic properties
+/// 3. Visible own getter-setter properties of all __proto__ objects
+///    (a DisplayObject ends the chain).
 bool
 as_object::set_member(string_table::key key, const as_value& val,
        string_table::key nsname, bool ifFound)
 {
 
-    ObjectURI uri(key, nsname);
-       Property* prop = findUpdatableProperty(uri);
-       
+    const ObjectURI uri(key, nsname);
+    
+    PrototypeRecursor<Exists> pr(this, uri);
+
+       Property* prop = pr.getProperty();
+
+       // We won't scan the inheritance chain if we find a member,
+       // even if invisible.
+       if (!prop) { 
+
+        if (_displayObject) {
+            if (setDisplayObjectProperty(*this, key, val)) return true;
+            // TODO: should we execute triggers?
+        }
+
+        const int version = getSWFVersion(*this);
+        while (pr()) {
+            if ((prop = pr.getProperty())) {
+                if ((prop->isStatic() || prop->isGetterSetter()) &&
+                        prop->visible(version)) {
+                    break;
+                }
+                else prop = 0;
+            }
+        }
+    }
+
     if (prop) {
 
                if (prop->isReadOnly()) {
@@ -746,11 +762,6 @@
                return true;
        }
 
-    if (_displayObject) {
-        if (setDisplayObjectProperty(*this, key, val)) return true;
-        // Execute triggers?
-    }
-
        // Else, add new property...
        if (ifFound) return false;
 

=== modified file 'libcore/as_object.h'
--- a/libcore/as_object.h       2009-10-08 07:53:12 +0000
+++ b/libcore/as_object.h       2009-10-08 12:24:21 +0000
@@ -54,7 +54,6 @@
     class asName;
     class RunResources;
     class Global_as;
-    class PrototypeRecursor;
 }
 
 namespace gnash {

=== modified file 'testsuite/swfdec/PASSING'
--- a/testsuite/swfdec/PASSING  2009-10-08 09:50:27 +0000
+++ b/testsuite/swfdec/PASSING  2009-10-08 13:13:25 +0000
@@ -964,6 +964,9 @@
 prototype-propflags-5.swf:43f4ffa51653f54d0182610a01db8f1f
 prototype-propflags-8.swf:3c6f02bf6a3348879f3f50c28ee599cb
 prototype-recursion-addProperty-5.swf:cb268cb74f2ebda340868ef624c02672
+prototype-recursion-addProperty-6.swf:f5630bb38117ae3811df5fe8f6a9a052
+prototype-recursion-addProperty-7.swf:e16fd0192b80416787f27c2b032cd15f
+prototype-recursion-addProperty-8.swf:0bf8bcd4c0b3245a29f1cf2dd5acd612
 prototype-recursion-get-5.swf:fbb029ff389d27c79338d8bc7248137b
 prototype-recursion-get-6.swf:47fa73c88db1f3acedd832e2ddec6315
 prototype-recursion-get-7.swf:766d65bdfdb4a79073728607ce8dc0d3