Re: [Gnash-commit] /srv/bzr/gnash/trunk r12163: implemented most of the methods used by the ExternalInterface AS class.

[Benjamin Wolsey]

>  
> +as_value externalinterface_addCallback(const fn_call& fn);
> +as_value externalinterface_call(const fn_call& fn);
> +as_value externalInterfaceConstructor(const fn_call& fn);
> +as_value externalinterface_available(const fn_call& fn);
> +as_value externalinterface_marshallExceptions(const fn_call& fn);
> +as_value externalinterface_objectID(const fn_call& fn);

Please leave these functions in an anonymous namespace; they shouldn't
have external linkage. All file-scope functions should be in an
anonymous namespace.

> +    // FIXME: for now, always make these available as ming doesn't support v9

Yes it does: the version is nothing more than a byte in the header.
However, properties are practically never attached by version, they are
attached with a visibility flag. 

> +//    if (getSWFVersion(o) > 8) {
> +    o.init_readonly_property("marshallExceptions", 
> &externalinterface_marshallExceptions);
> +    o.init_property("objectID", externalinterface_objectID, 
> externalinterface_objectID);
> +//    }
> +    

> +    if (getSWFVersion(o) > 6) {
> +        o.init_member("_argumentsToXML",
> +                      gl.createFunction(externalinterface_uArgumentsToXML), 
> flags);

The same applies here: the onlySWF7Up flag will set the visibility
(though onlySWF8Up seems more likely).

> +as_value
> +externalinterface_available(const fn_call& /* fn */)
> +{
> +//    GNASH_REPORT_FUNCTION;
> +    
> +    // Yes, Gnash supports the ExternalInterface
> +    return as_value(true);
> +}

The return of this function depends on the allowscriptaccess attribute
of the embed tag, doesn't it?

> +as_value
> +externalinterface_uArgumentsToAS(const fn_call& fn)
> +{
> +    // GNASH_REPORT_FUNCTION;
> +    
> +    std::string str(fn.arg(0).to_string());
> +    ExternalInterface_as &ptr = (ExternalInterface_as &)(fn);

Er?

This is a good way of causing a crash. It "works" currently because the
ExternalInterface_as member functions you use don't access any data
members (which itself is a pretty good clue that they shouldn't be
member functions at all), but it's horrible UB nonetheless.

None of these AS function implementations should rely on the 'this'
object being an ExternalInterface, if only for the simple reason that
there is no native ExternalInterface type in AS. In fact, there is no
need for a 'this' object at all, as all functions are static.

> +/// Convert an AS object to an XML string.
> +std::string
> +ExternalInterface_as::arrayToXML(as_object *obj)
> +{
> +    // GNASH_REPORT_FUNCTION;
> +    std::stringstream ss;
> +    if (obj == 0) {
> +        //log_error("Need a valid AS Object!");
> +        return ss.str();
> +    }
> +
> +    VM& vm = getVM(*obj);    
> +    
> +    ss << "<array>";
> +    PropsSerializer props(*this, vm);
> +    obj->visitProperties<IsEnumerable>(props);

An array most likely needs the function template foreachArray() in
Array_as.h, which will only serialize array indices rather than all
properties.

> +as_value
> +ExternalInterface_as::argumentsToXML(std::vector<as_value> &args)
> +{
> +    // GNASH_REPORT_FUNCTION;
> +    std::vector<as_value>::iterator it;
> +    std::stringstream ss;
> +
> +    ss << "<arguments>";
> +    for (it=args.begin(); it != args.end(); it++) {
> +        as_value val = *it;
> +        ss << toXML(val);
> +    }
> +    ss << "</arguments>";
> +    
> +    return as_value(ss.str());
>  }

There's no need for this to be a member function. It can be implemented
in the AS function so that it works (as expected) without a 'this'
object.
 
> +
> +class ExternalInterface_as : public ActiveRelay

The ExternalInterface class can't be instantiated and has only static
methods. Such "classes" should be implemented as objects; it's incorrect
to have a separate native type for them.

> +    // Returns the id attribute of the object tag in Internet Explorer,
> +    // or the name attribute of the embed tag in Netscape. 
> +    std::string &objectID() { return _objectid; };

Is there a reason to return by reference to non-const?

 
> -check(r instanceOf EI);
> -// But it doesn't do much.
> -check_equals(r._toXML(o), undefined);

You've dropped the most important test. This is the one that shows the
functions aren't inherited.

> -// All methods are static.

As the also-dropped comment says.

bwy

--
The current release of Gnash is 0.8.7
http://www.gnu.org/software/gnash/

Benjamin Wolsey, Software Developer - http://benjaminwolsey.de
C++ and Open-Source Flash blog - http://www.benjaminwolsey.de/bwysblog

xmpp:address@hidden
http://identi.ca/bwy

signature.asc
Description: Dies ist ein digital signierter Nachrichtenteil