[Gnash-commit] /srv/bzr/gnash/trunk r10207: Pass ControlTags by auto_ptr to addControlTag to show ownership tranfer.

[Benjamin Wolsey]

------------------------------------------------------------
revno: 10207
committer: Benjamin Wolsey <address@hidden>
branch nick: trunk
timestamp: Thu 2008-10-30 21:06:43 +0100
message:
  Pass ControlTags by auto_ptr to addControlTag to show ownership tranfer.
  Store in a boost::ptr_vector so we don't have to delete them manually.
  Document.
modified:
  libcore/MovieClip.cpp
  libcore/MovieClip.h
  libcore/parser/SWFMovieDefinition.cpp
  libcore/parser/SWFMovieDefinition.h
  libcore/parser/movie_definition.h
  libcore/parser/sprite_definition.cpp
  libcore/parser/sprite_definition.h
  libcore/swf/DoActionTag.h
  libcore/swf/DoInitActionTag.h
  libcore/swf/PlaceObject2Tag.cpp
  libcore/swf/RemoveObjectTag.cpp
  libcore/swf/SetBackgroundColorTag.h
  libcore/swf/StartSoundTag.cpp
  libcore/swf/StreamSoundBlockTag.cpp

=== modified file 'libcore/MovieClip.cpp'
--- a/libcore/MovieClip.cpp     2008-10-30 13:15:12 +0000
+++ b/libcore/MovieClip.cpp     2008-10-30 20:06:43 +0000
@@ -2972,19 +2972,20 @@
     // We set _callingFrameActions to true so that add_action_buffer
     // will execute immediately instead of queuing them.
     // NOTE: in case gotoFrame is executed by code in the called frame
-    //             we'll temporarly clear the _callingFrameActions flag
+    //             we'll temporarily clear the _callingFrameActions flag
     //             to properly queue actions back on the global queue.
     //
     _callingFrameActions=true;
     const PlayList* playlist = m_def->getPlaylist(frame_number);
     if ( playlist )
     {
-    PlayList::const_iterator it = playlist->begin();
+        PlayList::const_iterator it = playlist->begin();
         const PlayList::const_iterator e = playlist->end();
-    for(; it != e; it++)
-    {
-        (*it)->execute_action(this, m_display_list);
-    }
+        for(; it != e; it++)
+        {
+            it->execute_action(this, m_display_list);
+        }
+
     }
     _callingFrameActions=false;
 
@@ -3523,14 +3524,14 @@
         {
             for( ; it != e; it++)
             {
-                (*it)->execute(this, dlist);
+                it->execute(this, dlist);
             }
         }
         else if ( typeflags & TAG_DLIST )
         {
             for( ; it != e; it++)
             {
-                (*it)->execute_state(this, dlist);
+                it->execute_state(this, dlist);
             }
         }
         else
@@ -3538,7 +3539,7 @@
             assert(typeflags & TAG_ACTION);
             for( ; it != e; it++)
             {
-                (*it)->execute_action(this, dlist);
+                it->execute_action(this, dlist);
             }
         }
     }

=== modified file 'libcore/MovieClip.h'
--- a/libcore/MovieClip.h       2008-10-27 10:39:37 +0000
+++ b/libcore/MovieClip.h       2008-10-30 20:06:43 +0000
@@ -71,8 +71,7 @@
 
     typedef std::list<const action_buffer*> ActionList;
 
-    // definition must match movie_definition::PlayList
-    typedef std::vector<ControlTag*> PlayList;
+    typedef movie_definition::PlayList PlayList;
 
     typedef std::vector<swf_event*> SWFEventsVector;
 

=== modified file 'libcore/parser/SWFMovieDefinition.cpp'
--- a/libcore/parser/SWFMovieDefinition.cpp     2008-10-30 13:15:12 +0000
+++ b/libcore/parser/SWFMovieDefinition.cpp     2008-10-30 20:06:43 +0000
@@ -213,18 +213,6 @@
        // Request cancelation of the loading thread
        _loadingCanceled = true;
 
-       // Release frame tags
-       for (PlayListMap::iterator i = m_playlist.begin(),
-            e = m_playlist.end(); i != e; ++i)
-       {
-               PlayList& pl = i->second;
-
-               for (PlayList::iterator j = pl.begin(), je = pl.end(); j!=je; 
++j)
-               {
-            delete *j;
-        }
-    }
-
        // It's supposed to be cleaned up in read()
        // TODO: join with loader thread instead ?
        //assert(m_jpeg_in->get() == NULL);

=== modified file 'libcore/parser/SWFMovieDefinition.h'
--- a/libcore/parser/SWFMovieDefinition.h       2008-10-30 13:15:12 +0000
+++ b/libcore/parser/SWFMovieDefinition.h       2008-10-30 20:06:43 +0000
@@ -301,10 +301,14 @@
        // See dox in movie_definition.h
        int get_loading_sound_stream_id() { return m_loading_sound_stream; }
 
-       // See dox in movie_definition.h
-       void    addControlTag(ControlTag* tag)
+    /// Transfer a ControlTag to the SWFMovieDefinition.
+    //
+    /// @param tag  A ControlTag to add to the PlayList m_playlist.
+    ///             The PlayList owns the control tag and is responsible for
+    ///             deleting it.
+       void addControlTag(std::auto_ptr<ControlTag> tag)
        {
-           assert(tag);
+           assert(tag.get());
            boost::mutex::scoped_lock lock(_frames_loaded_mutex);
            m_playlist[_frames_loaded].push_back(tag);
        }

=== modified file 'libcore/parser/movie_definition.h'
--- a/libcore/parser/movie_definition.h 2008-10-30 13:15:12 +0000
+++ b/libcore/parser/movie_definition.h 2008-10-30 20:06:43 +0000
@@ -53,11 +53,13 @@
 
 #include "character_def.h" // for inheritance
 #include "GnashImageJpeg.h"
+#include "swf/ControlTag.h"
 
 #include <string>
 #include <memory> // for auto_ptr
 #include <vector> // for PlayList typedef
 #include <set> 
+#include <boost/ptr_container/ptr_vector.hpp>
 
 // Forward declarations
 namespace gnash {
@@ -65,7 +67,6 @@
        class bitmap_info;
        class movie_instance;
        class MovieClip;
-       class ControlTag;
     class font;
     class ExportableResource;
     class sound_sample;
@@ -97,7 +98,10 @@
 class movie_definition : public character_def
 {
 public:
-       typedef std::vector<ControlTag*> PlayList;
+
+    /// This container owns the ControlTags and deletes them on
+    /// destruction.
+       typedef boost::ptr_vector<ControlTag> PlayList;
 
        virtual int     get_version() const = 0;
        virtual float   get_width_pixels() const = 0;
@@ -174,7 +178,7 @@
 
 
        typedef std::pair<int, std::string> ImportSpec;
-       typedef std::vector< ImportSpec > Imports;
+       typedef std::vector<ImportSpec> Imports;
 
        /// Import resources 
        //
@@ -184,7 +188,9 @@
        /// @param imports
        ///     Resources to import, each with the id to use in our dictionary
        ///
-       virtual void importResources(boost::intrusive_ptr<movie_definition> 
/*source*/, Imports& /*imports*/)
+       virtual void importResources(
+            boost::intrusive_ptr<movie_definition> /*source*/,
+            Imports& /*imports*/)
        {
        }
 
@@ -200,7 +206,7 @@
        /// @return NULL if no character with the given ID is found
        ///         (this is the default)
        ///
-       virtual character_def*  get_character_def(int /*id*/)
+       virtual character_def* get_character_def(int /*id*/)
        {
                return NULL;
        }
@@ -220,7 +226,8 @@
        ///
        /// @return true if a frame with that label was found, false otherwise
        ///
-       virtual bool get_labeled_frame(const std::string& /*label*/, size_t& 
/*frame_number*/)
+       virtual bool get_labeled_frame(const std::string& /*label*/,
+            size_t& /*frame_number*/)
        {
                return false;
        }
@@ -269,19 +276,16 @@
                return 0;
        }
 
-       /// Add an ControlTag to this movie_definition's playlist
+       /// Add a ControlTag to this movie_definition's playlist
        //
-       /// The default implementation is a no-op.
+       /// The movie_definition (or its implementation) owns the ControlTag.
+    /// The default implementation is a no-op.
        ///
        /// @param tag
        ///     The tag to add in the list of executable tags for
        ///     the frame currently being loaded. Ownership is transferred
-       ///     to the SWFMovieDefinition.
-       ///
-       /// TODO: take an auto_ptr.
-       /// NOTE: the default implementation just let the ControlTag leak.
-       ///
-       virtual void    addControlTag(ControlTag* /*c*/)
+       ///     to the movie_definition (SWFMovieDefinition or 
sprite_definition).
+       virtual void addControlTag(std::auto_ptr<ControlTag> /*c*/)
        {
        }
 
@@ -425,8 +429,6 @@
        {
        }
 
-       // ...
-
        /// \brief
        /// Return the URL of the SWF stream this definition has been read
        /// from.

=== modified file 'libcore/parser/sprite_definition.cpp'
--- a/libcore/parser/sprite_definition.cpp      2008-10-28 21:17:19 +0000
+++ b/libcore/parser/sprite_definition.cpp      2008-10-30 20:06:43 +0000
@@ -52,16 +52,6 @@
 
 sprite_definition::~sprite_definition()
 {
-       // Release our playlist data.
-       for (PlayListMap::iterator i=m_playlist.begin(), e=m_playlist.end(); 
i!=e; ++i)
-       {
-               PlayList& pl = i->second;
-
-               for (PlayList::iterator j=pl.begin(), je=pl.end(); j!=je; ++j)
-               {
-            delete *j;
-        }
-    }
 }
 
 /*private*/

=== modified file 'libcore/parser/sprite_definition.h'
--- a/libcore/parser/sprite_definition.h        2008-10-30 13:15:12 +0000
+++ b/libcore/parser/sprite_definition.h        2008-10-30 20:06:43 +0000
@@ -275,11 +275,14 @@
        // Number of frames completely parsed 
        size_t m_loading_frame;
 
-
-       // See dox in movie_definition.h
-       virtual void    addControlTag(ControlTag* c)
+    /// Transfer a ControlTag to the sprite_definition.
+    //
+    /// @param tag  A ControlTag to add to the PlayList m_playlist.
+    ///             The PlayList owns the control tag and is responsible for
+    ///             deleting it.
+       virtual void addControlTag(std::auto_ptr<ControlTag> tag)
        {
-               m_playlist[m_loading_frame].push_back(c);
+               m_playlist[m_loading_frame].push_back(tag);
        }
 
        // See dox in movie_definition.h

=== modified file 'libcore/swf/DoActionTag.h'
--- a/libcore/swf/DoActionTag.h 2008-10-28 21:17:19 +0000
+++ b/libcore/swf/DoActionTag.h 2008-10-30 20:06:43 +0000
@@ -66,7 +66,7 @@
        static void doActionLoader(SWFStream& in, tag_type tag,
             movie_definition& m, const RunInfo& /*r*/)
        {
-               DoActionTag* da = new DoActionTag(m);
+        std::auto_ptr<DoActionTag> da(new DoActionTag(m));
                da->read(in);
 
                IF_VERBOSE_PARSE (
@@ -74,7 +74,7 @@
                log_parse(_("-- actions in frame %d"), m.get_loading_frame());
                );
 
-               m.addControlTag(da); // ownership transferred
+               m.addControlTag(static_cast<std::auto_ptr<ControlTag> >(da));
        }
 
 private:

=== modified file 'libcore/swf/DoInitActionTag.h'
--- a/libcore/swf/DoInitActionTag.h     2008-10-29 09:45:23 +0000
+++ b/libcore/swf/DoInitActionTag.h     2008-10-30 20:06:43 +0000
@@ -69,7 +69,7 @@
     {
         in.ensureBytes(2);
         int cid = in.read_u16();
-        DoInitActionTag* da = new DoInitActionTag(in, m, cid);
+        std::auto_ptr<ControlTag> da(new DoInitActionTag(in, m, cid));
 
         IF_VERBOSE_PARSE (
         log_parse(_("  tag %d: do_init_action_loader"), tag);

=== modified file 'libcore/swf/PlaceObject2Tag.cpp'
--- a/libcore/swf/PlaceObject2Tag.cpp   2008-10-28 21:17:19 +0000
+++ b/libcore/swf/PlaceObject2Tag.cpp   2008-10-30 20:06:43 +0000
@@ -526,11 +526,10 @@
 {
     assert(tag == SWF::PLACEOBJECT || tag == SWF::PLACEOBJECT2 || tag == 
SWF::PLACEOBJECT3);
 
-    // TODO: who owns and is going to remove this tag ?
-    PlaceObject2Tag* ch = new PlaceObject2Tag(m);
+    std::auto_ptr<PlaceObject2Tag> ch(new PlaceObject2Tag(m));
     ch->read(in, tag);
 
-    m.addControlTag(ch);
+    m.addControlTag(static_cast<std::auto_ptr<ControlTag> >(ch));
 }
 
 } // namespace gnash::SWF

=== modified file 'libcore/swf/RemoveObjectTag.cpp'
--- a/libcore/swf/RemoveObjectTag.cpp   2008-10-29 09:45:23 +0000
+++ b/libcore/swf/RemoveObjectTag.cpp   2008-10-30 20:06:43 +0000
@@ -64,13 +64,12 @@
 
     int depth = t->getDepth();
 
-    IF_VERBOSE_PARSE
-    (
-       log_parse(_("  remove_object_2(%d)"), depth);
+    IF_VERBOSE_PARSE(
+        log_parse(_("  remove_object_2(%d)"), depth);
     );
 
     // Ownership transferred to movie_definition
-    m.addControlTag(t.release());
+    m.addControlTag(static_cast<std::auto_ptr<ControlTag> >(t));
 }
 
 } // namespace gnash::SWF

=== modified file 'libcore/swf/SetBackgroundColorTag.h'
--- a/libcore/swf/SetBackgroundColorTag.h       2008-10-28 21:17:19 +0000
+++ b/libcore/swf/SetBackgroundColorTag.h       2008-10-30 20:06:43 +0000
@@ -102,7 +102,7 @@
                assert(tag == SWF::SETBACKGROUNDCOLOR); // 9
 
                // this one may throw, we'll let caller catch it
-               SetBackgroundColorTag* t = new SetBackgroundColorTag(in);
+        std::auto_ptr<ControlTag> t(new SetBackgroundColorTag(in));
                m.addControlTag(t); // takes ownership
        }
 };

=== modified file 'libcore/swf/StartSoundTag.cpp'
--- a/libcore/swf/StartSoundTag.cpp     2008-10-28 21:17:19 +0000
+++ b/libcore/swf/StartSoundTag.cpp     2008-10-30 20:06:43 +0000
@@ -58,7 +58,8 @@
     // NOTE: sound_id is the SWF-defined id,
     //       sam->m_sound_handler_id is the sound_handler-provided id
     //
-    StartSoundTag*  sst = new StartSoundTag(sam->m_sound_handler_id);
+    std::auto_ptr<StartSoundTag> sst(
+            new StartSoundTag(sam->m_sound_handler_id));
     sst->read(in);
 
     IF_VERBOSE_PARSE (
@@ -66,7 +67,7 @@
               sound_id, int(sst->m_stop_playback), sst->m_loop_count);
     );
 
-    m.addControlTag(sst); // takes ownership
+    m.addControlTag(static_cast<std::auto_ptr<ControlTag> >(sst));
 }
 
 /* private */

=== modified file 'libcore/swf/StreamSoundBlockTag.cpp'
--- a/libcore/swf/StreamSoundBlockTag.cpp       2008-10-28 21:43:42 +0000
+++ b/libcore/swf/StreamSoundBlockTag.cpp       2008-10-30 20:06:43 +0000
@@ -118,8 +118,8 @@
 
     // TODO: log_parse ?
 
-    StreamSoundBlockTag* ssst = new StreamSoundBlockTag(handle_id, start);
-    m.addControlTag(ssst); // ownership is transferred to movie_definition
+    std::auto_ptr<ControlTag> ct(new StreamSoundBlockTag(handle_id, start));
+    m.addControlTag(ct); // ownership is transferred to movie_definition
 }
 
 } // namespace gnash::SWF