[Gnash-commit] gnash ChangeLog gui/Player.cpp gui/Player.h ser...

[Sandro Santilli]

CVSROOT:        /sources/gnash
Module name:    gnash
Changes by:     Sandro Santilli <strk>  08/05/30 20:23:51

Modified files:
        .              : ChangeLog 
        gui            : Player.cpp Player.h 
        server/parser  : movie_def_impl.cpp movie_def_impl.h 
                         movie_definition.h sprite_definition.h 
        server/swf     : tag_loaders.cpp 

Log message:
                * gui/Player.{cpp,h}: keep the toplevel movie in an 
intrusive_ptr
                  just in case someone else will get it in and out of one while
                  we're still using it (would prematurely destroy it).
                * server/parser/: movie_definition.h, sprite_definition.h,
                  movie_def_impl.{cpp,h}:
                  Add new importResources() method taking care of IMPORT
                  details; have movie_def_impl maintain a list of movies
                  from which it imports from: fixes bug #
                * server/swf/tag_loaders.cpp (import_loader): just parse
                  and load the source movie, let the rest be done
                  by the new importResources() method.
        
        I belive this is worth backporting to branch (see bug #23364).
        Should I proceed ?

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/gnash/ChangeLog?cvsroot=gnash&r1=1.6767&r2=1.6768
http://cvs.savannah.gnu.org/viewcvs/gnash/gui/Player.cpp?cvsroot=gnash&r1=1.116&r2=1.117
http://cvs.savannah.gnu.org/viewcvs/gnash/gui/Player.h?cvsroot=gnash&r1=1.32&r2=1.33
http://cvs.savannah.gnu.org/viewcvs/gnash/server/parser/movie_def_impl.cpp?cvsroot=gnash&r1=1.112&r2=1.113
http://cvs.savannah.gnu.org/viewcvs/gnash/server/parser/movie_def_impl.h?cvsroot=gnash&r1=1.77&r2=1.78
http://cvs.savannah.gnu.org/viewcvs/gnash/server/parser/movie_definition.h?cvsroot=gnash&r1=1.46&r2=1.47
http://cvs.savannah.gnu.org/viewcvs/gnash/server/parser/sprite_definition.h?cvsroot=gnash&r1=1.43&r2=1.44
http://cvs.savannah.gnu.org/viewcvs/gnash/server/swf/tag_loaders.cpp?cvsroot=gnash&r1=1.201&r2=1.202

Patches:
Index: ChangeLog
===================================================================
RCS file: /sources/gnash/gnash/ChangeLog,v
retrieving revision 1.6767
retrieving revision 1.6768
diff -u -b -r1.6767 -r1.6768
--- ChangeLog   30 May 2008 13:18:38 -0000      1.6767
+++ ChangeLog   30 May 2008 20:23:49 -0000      1.6768
@@ -1,5 +1,19 @@
 2008-05-30 Sandro Santilli <address@hidden>
 
+       * gui/Player.{cpp,h}: keep the toplevel movie in an intrusive_ptr
+         just in case someone else will get it in and out of one while
+         we're still using it (would prematurely destroy it).
+       * server/parser/: movie_definition.h, sprite_definition.h,
+         movie_def_impl.{cpp,h}:
+         Add new importResources() method taking care of IMPORT
+         details; have movie_def_impl maintain a list of movies
+         from which it imports from: fixes bug #
+       * server/swf/tag_loaders.cpp (import_loader): just parse
+         and load the source movie, let the rest be done
+         by the new importResources() method.
+
+2008-05-30 Sandro Santilli <address@hidden>
+
        * testsuite/gnashrc.in: limit MovieLibrary to 1 element.
          Exposes the segfault in bug #23364 on make check.
 

Index: gui/Player.cpp
===================================================================
RCS file: /sources/gnash/gnash/gui/Player.cpp,v
retrieving revision 1.116
retrieving revision 1.117
diff -u -b -r1.116 -r1.117
--- gui/Player.cpp      28 May 2008 08:37:13 -0000      1.116
+++ gui/Player.cpp      30 May 2008 20:23:50 -0000      1.117
@@ -213,10 +213,10 @@
 #endif // def GNASH_FPS_DEBUG
 }
 
-movie_definition* 
+boost::intrusive_ptr<movie_definition>
 Player::load_movie()
 {
-    gnash::movie_definition* md = NULL;
+    boost::intrusive_ptr<gnash::movie_definition> md;
 
     RcInitFile& rcfile = RcInitFile::getDefaultInstance();
     URL vurl(_url);
@@ -392,9 +392,16 @@
     _gui->setStage(&root);
 
     // Start loader thread
+    // NOTE: the loader thread might (in IMPORT tag parsing)
+    //       create new movies and register them to the MovieLibrary.
+    //       If MovieLibrary size exceeded, _movieDef might be
+    //       destroyed prematurely. movie_root might actually be
+    //       keeping it alive, as Gui might as well, but why relying
+    //       on luck ? So we made sure to keep _movieDef by 
+    //       intrusive_ptr...
     _movieDef->completeLoad();
 
-    _gui->setMovieDefinition(_movieDef);
+    _gui->setMovieDefinition(_movieDef.get());
 
     if (! _delay) {
       _delay = static_cast<unsigned int>(1000 / movie_fps) ; // milliseconds 
per frame

Index: gui/Player.h
===================================================================
RCS file: /sources/gnash/gnash/gui/Player.h,v
retrieving revision 1.32
retrieving revision 1.33
diff -u -b -r1.32 -r1.33
--- gui/Player.h        5 May 2008 18:58:05 -0000       1.32
+++ gui/Player.h        30 May 2008 20:23:50 -0000      1.33
@@ -26,6 +26,8 @@
 #include "gnash.h" // needed for interface and fscommand callbacks
 #include "sound_handler.h" // for visibility of sound_handler destructor
 #include "gui.h"
+#include "movie_definition.h" // for visibility of movie_definition destructor
+#include "smart_ptr.h" // for intrusive_ptr holding of top-level movie
 
 #include <string>
 #include <map>
@@ -206,7 +208,7 @@
 
        std::string _infile;
 
-       movie_definition* _movieDef;
+       boost::intrusive_ptr<movie_definition> _movieDef;
        
        unsigned long _maxAdvances;
 
@@ -216,7 +218,7 @@
        /// stdin when it equals "-". May throw a GnashException
        /// on failure.
        ///
-       movie_definition* load_movie();
+       boost::intrusive_ptr<movie_definition> load_movie();
 
 #ifdef GNASH_FPS_DEBUG
        float _fpsDebugTime;

Index: server/parser/movie_def_impl.cpp
===================================================================
RCS file: /sources/gnash/gnash/server/parser/movie_def_impl.cpp,v
retrieving revision 1.112
retrieving revision 1.113
diff -u -b -r1.112 -r1.113
--- server/parser/movie_def_impl.cpp    30 May 2008 13:17:04 -0000      1.112
+++ server/parser/movie_def_impl.cpp    30 May 2008 20:23:50 -0000      1.113
@@ -910,4 +910,45 @@
 }
 #endif // GNASH_USE_GC
 
+void
+movie_def_impl::importResources(boost::intrusive_ptr<movie_definition> source, 
Imports& imports)
+{
+       size_t importedSyms=0;
+       for (Imports::iterator i=imports.begin(), e=imports.end(); i!=e; ++i)
+       {
+               int id = i->first;
+               const std::string& symbolName = i->second;
+
+               boost::intrusive_ptr<resource> res = 
source->get_exported_resource(symbolName);
+               if (!res)
+               {
+                       log_error(_("import error: could not find resource '%s' 
in movie '%s'"),
+                               symbolName, source->get_url());
+                       continue;
+               }
+               else if (font* f = res->cast_to_font())
+               {
+                       // Add this shared font to the currently-loading movie.
+                       add_font(id, f);
+                       ++importedSyms;
+               }
+               else if (character_def* ch = res->cast_to_character_def())
+               {
+                       // Add this character to the loading movie.
+                       add_character(id, ch);
+                       ++importedSyms;
+               }
+               else
+               {
+                       log_error(_("importResources error: unsupported import 
of '%s' from movie '%s' has unknown type"),
+                               symbolName, source->get_url());
+               }
+       }
+
+       if ( importedSyms )
+       {
+               _importSources.insert(source);
+       }
+}
+
 } // namespace gnash

Index: server/parser/movie_def_impl.h
===================================================================
RCS file: /sources/gnash/gnash/server/parser/movie_def_impl.h,v
retrieving revision 1.77
retrieving revision 1.78
diff -u -b -r1.77 -r1.78
--- server/parser/movie_def_impl.h      29 May 2008 08:45:56 -0000      1.77
+++ server/parser/movie_def_impl.h      30 May 2008 20:23:51 -0000      1.78
@@ -31,6 +31,7 @@
 #include "StringPredicates.h" // for case-insensitive string comparision 
(ExportMap)
 
 #include <map> // for CharacterDictionary
+#include <set> // for _importSources
 #include <string>
 #include <memory> // for auto_ptr
 #include <boost/thread/thread.hpp>
@@ -292,6 +293,9 @@
        /// A flag set to true when load cancelation is requested
        bool _loadingCanceled;
 
+       /// Movies we import resources from
+       std::set< boost::intrusive_ptr<movie_definition> > _importSources;
+
 public:
 
        movie_def_impl();
@@ -369,6 +373,8 @@
        ///
        virtual boost::intrusive_ptr<resource> get_exported_resource(const 
std::string& symbol);
 
+       virtual void importResources(boost::intrusive_ptr<movie_definition> 
source, Imports& imports);
+
        void add_character(int character_id, character_def* c);
 
        /// \brief

Index: server/parser/movie_definition.h
===================================================================
RCS file: /sources/gnash/gnash/server/parser/movie_definition.h,v
retrieving revision 1.46
retrieving revision 1.47
diff -u -b -r1.46 -r1.47
--- server/parser/movie_definition.h    28 May 2008 08:31:23 -0000      1.46
+++ server/parser/movie_definition.h    30 May 2008 20:23:51 -0000      1.47
@@ -202,6 +202,22 @@
        }
 
 
+       typedef std::pair<int, std::string> ImportSpec;
+       typedef std::vector< ImportSpec > Imports;
+
+       /// Import resources 
+       //
+       /// @param source
+       ///     Movie containing the resources being imported
+       ///
+       /// @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*/)
+       {
+       }
+
+
        /// \brief
        /// Get a character from the dictionary.
        //

Index: server/parser/sprite_definition.h
===================================================================
RCS file: /sources/gnash/gnash/server/parser/sprite_definition.h,v
retrieving revision 1.43
retrieving revision 1.44
diff -u -b -r1.43 -r1.44
--- server/parser/sprite_definition.h   23 May 2008 15:21:28 -0000      1.43
+++ server/parser/sprite_definition.h   30 May 2008 20:23:51 -0000      1.44
@@ -198,6 +198,14 @@
                return m_movie_def->get_exported_resource(sym);
        }
 
+       /// Overridden just for complaining  about malformed SWF
+       virtual void importResources(boost::intrusive_ptr<movie_definition> 
/*source*/, Imports& /*imports*/)
+       {
+               IF_VERBOSE_MALFORMED_SWF (
+               log_swferror(_("IMPORT tag appears in DEFINESPRITE tag"));
+               );
+       }
+
        /// \brief
        /// Get a character_def from this Sprite's root movie
        /// CharacterDictionary.

Index: server/swf/tag_loaders.cpp
===================================================================
RCS file: /sources/gnash/gnash/server/swf/tag_loaders.cpp,v
retrieving revision 1.201
retrieving revision 1.202
diff -u -b -r1.201 -r1.202
--- server/swf/tag_loaders.cpp  29 May 2008 10:34:56 -0000      1.201
+++ server/swf/tag_loaders.cpp  30 May 2008 20:23:51 -0000      1.202
@@ -61,6 +61,7 @@
 #include <map>
 #include <limits>
 #include <cassert>
+#include <utility> // for std::make_pair
 
 namespace gnash {
 
@@ -1083,16 +1084,15 @@
 
 
     // Try to load the source movie into the movie library.
-    movie_definition*  source_movie = NULL;
+       boost::intrusive_ptr<movie_definition> source_movie;
 
        try {
            source_movie = create_library_movie(abs_url);
        } catch (gnash::GnashException& e) {
            log_error(_("Exception: %s"), e.what());
-           source_movie = NULL;
        }
 
-       if (source_movie == NULL)
+       if (!source_movie)
        {
            // Give up on imports.
            log_error(_("can't import movie from url %s"), abs_url.str());
@@ -1109,6 +1109,8 @@
            return;
        }
 
+       movie_definition::Imports imports;
+
     // Get the imports.
     for (int i = 0; i < count; i++)
     {
@@ -1116,35 +1118,13 @@
            boost::uint16_t     id = in->read_u16();
            std::string symbolName;
            in->read_string(symbolName);
-           IF_VERBOSE_PARSE
-           (
+               IF_VERBOSE_PARSE (
                log_parse(_("  import: id = %d, name = %s"), id, symbolName);
            );
-
-        boost::intrusive_ptr<resource> res = 
source_movie->get_exported_resource(symbolName);
-        if (res == NULL)
-        {
-        log_error(_("import error: could not find resource '%s' in movie 
'%s'"),
-                 symbolName, source_url);
-        }
-        else if (font* f = res->cast_to_font())
-        {
-            // Add this shared font to the currently-loading movie.
-            m->add_font(id, f);
-        }
-        else if (character_def* ch = res->cast_to_character_def())
-        {
-            // Add this character to the loading movie.
-            m->add_character(id, ch);
-        }
-        else
-        {
-        log_error(_("import error: resource '%s' from movie '%s' has unknown 
type"),
-                 symbolName, source_url);
-        }
-
+               imports.push_back( std::make_pair(id, symbolName) );
     }
 
+       m->importResources(source_movie, imports);
 }
 
 // Read a DefineText tag.