[Gnash-commit] /srv/bzr/gnash/trunk r11694: Factories of ref_counted objects must return by intrusive_ptr, or assignments of 'this' to a temporary intrusive_ptr would destroy the created object. This was happening on 'make check' during attach*Imported.swf tests with a LibraryLimit of 1...

[Sandro Santilli]

------------------------------------------------------------
revno: 11694
committer: Sandro Santilli <address@hidden>
branch nick: trunk
timestamp: Mon 2009-12-14 22:12:35 +0100
message:
  Factories of ref_counted objects must return by intrusive_ptr, or assignments 
of 'this' to a temporary intrusive_ptr would destroy the created object. This 
was happening on 'make check' during attach*Imported.swf tests with a 
LibraryLimit of 1...
modified:
  libcore/MovieFactory.cpp
  libcore/MovieFactory.h
  testsuite/MovieTester.cpp
  testsuite/MovieTester.h

=== modified file 'libcore/MovieFactory.cpp'
--- a/libcore/MovieFactory.cpp  2009-11-13 23:21:54 +0000
+++ b/libcore/MovieFactory.cpp  2009-12-14 21:12:35 +0000
@@ -125,31 +125,33 @@
 // Create a SWFMovieDefinition from an SWF stream
 // NOTE: this method assumes this *is* an SWF stream
 //
-SWFMovieDefinition*
+boost::intrusive_ptr<SWFMovieDefinition>
 createSWFMovie(std::auto_ptr<IOChannel> in, const std::string& url,
         const RunResources& runResources, bool startLoaderThread)
 {
 
-    std::auto_ptr<SWFMovieDefinition> m (new SWFMovieDefinition(runResources));
+    boost::intrusive_ptr<SWFMovieDefinition> m = new 
SWFMovieDefinition(runResources);
 
     const std::string& absURL = URL(url).str();
 
     if (!m->readHeader(in, absURL)) return 0;
     if (startLoaderThread && !m->completeLoad()) return 0;
 
-    return m.release();
+    return m;
 }
 
 // Create a movie_definition from an image format stream
 // NOTE: this method assumes this *is* the format described in the
 // FileType type
 // TODO: The pp won't display PNGs for SWF7 or below.
-movie_definition*
+boost::intrusive_ptr<BitmapMovieDefinition>
 createBitmapMovie(std::auto_ptr<IOChannel> in, const std::string& url,
         const RunResources& r, FileType type)
 {
     assert (in.get());
 
+    boost::intrusive_ptr<BitmapMovieDefinition> ret;
+
     // readImageData takes a shared pointer because JPEG streams sometimes need
     // to transfer ownership.
     boost::shared_ptr<IOChannel> imageData(in.release());
@@ -161,31 +163,32 @@
 
         if (!im.get()) {
             log_error(_("Can't read image file from %s"), url);
-            return NULL;
+            return ret;
         }
 
         Renderer* renderer = r.renderer();
 
-        BitmapMovieDefinition* mdef =
-            new BitmapMovieDefinition(im, renderer, url);
+        ret =  new BitmapMovieDefinition(im, renderer, url);
 
-        return mdef;
+        return ret;
 
     }
     catch (ParserException& e)
     {
         log_error(_("Parsing error: %s"), e.what());
-        return NULL;
+        return ret;
     }
 
 }
 
-movie_definition*
+boost::intrusive_ptr<movie_definition>
 createNonLibraryMovie(const URL& url, const RunResources& runResources,
         const char* reset_url, bool startLoaderThread,
         const std::string* postdata)
 {
 
+  boost::intrusive_ptr<movie_definition> ret;
+
   std::auto_ptr<IOChannel> in;
 
   const StreamProvider& streamProvider = runResources.streamProvider();
@@ -200,36 +203,35 @@
   if ( ! in.get() )
   {
       log_error(_("failed to open '%s'; can't create movie"), url);
-      return NULL;
+      return ret;
   }
   
   if (in->bad())
   {
       log_error(_("streamProvider opener can't open '%s'"), url);
-      return NULL;
+      return ret;
   }
 
   std::string movie_url = reset_url ? reset_url : url.str();
-  movie_definition* ret = MovieFactory::makeMovie(in, movie_url, runResources,
+  ret = MovieFactory::makeMovie(in, movie_url, runResources,
           startLoaderThread);
 
   return ret;
 
-
 }
 
 
-
-
 } // anonymous namespace
 
 
 MovieLibrary MovieFactory::movieLibrary;
 
-movie_definition*
+boost::intrusive_ptr<movie_definition>
 MovieFactory::makeMovie(std::auto_ptr<IOChannel> in, const std::string& url,
         const RunResources& runResources, bool startLoaderThread)
 {
+  boost::intrusive_ptr<movie_definition> ret;
+
   assert(in.get());
 
   // see if it's a jpeg or an swf
@@ -248,34 +250,37 @@
                            "image, for which we don't yet have the "
                            "concept of a 'loading thread'"));
             }
-            return createBitmapMovie(in, url, runResources, type);
+            ret = createBitmapMovie(in, url, runResources, type);
+            break;
         }
 
 
         case GNASH_FILETYPE_SWF:
-            return createSWFMovie(in, url, runResources, startLoaderThread);
+            ret = createSWFMovie(in, url, runResources, startLoaderThread);
+            break;
 
         case GNASH_FILETYPE_FLV:
             log_unimpl(_("FLV can't be loaded directly as a movie"));
-            return NULL;
+            return ret;
 
         default:
             log_error(_("unknown file type (%s)"), type);
             break;
     }
 
-    return NULL;
+    return ret;
 }
 
 // Try to load a movie from the given url, if we haven't
 // loaded it already.  Add it to our library on success, and
 // return a pointer to it.
 //
-movie_definition*
+boost::intrusive_ptr<movie_definition>
 MovieFactory::makeMovie(const URL& url, const RunResources& runResources,
         const char* real_url, bool startLoaderThread,
         const std::string* postdata)
 {
+    boost::intrusive_ptr<movie_definition>  mov;
 
     // Use real_url as label for cache if available 
     std::string cache_label = real_url ? URL(real_url).str() : url.str();
@@ -283,11 +288,10 @@
     // Is the movie already in the library? (don't check if we have post data!)
     if (!postdata)
     {
-        boost::intrusive_ptr<movie_definition>  m;
-        if ( movieLibrary.get(cache_label, &m) )
+        if ( movieLibrary.get(cache_label, &mov) )
         {
             log_debug(_("Movie %s already in library"), cache_label);
-            return m.get();
+            return mov;
         }
     }
 
@@ -295,20 +299,18 @@
     // the loader thread now to avoid IMPORT tag loaders from 
     // calling createMovie() again and NOT finding
     // the just-created movie.
-    movie_definition* mov = createNonLibraryMovie(url, runResources,
-            real_url, false,
-            postdata);
+    mov = createNonLibraryMovie(url, runResources, real_url, false, postdata);
 
     if (!mov)
     {
         log_error(_("Couldn't load library movie '%s'"), url.str());
-        return NULL;
+        return mov;
     }
 
     // Movie is good, add to the library 
     if (!postdata) // don't add if we POSTed
     {
-        movieLibrary.add(cache_label, mov);
+        movieLibrary.add(cache_label, mov.get());
         log_debug(_("Movie %s (SWF%d) added to library"),
                 cache_label, mov->get_version());
     }

=== modified file 'libcore/MovieFactory.h'
--- a/libcore/MovieFactory.h    2009-11-13 23:21:54 +0000
+++ b/libcore/MovieFactory.h    2009-12-14 21:12:35 +0000
@@ -80,7 +80,8 @@
     /// @param postdata
     /// If not NULL, use POST method (only valid for HTTP).
     /// NOTE: when POSTing, the movies library won't be used.
-    static DSOEXPORT movie_definition* makeMovie(const URL& url,
+    static DSOEXPORT boost::intrusive_ptr<movie_definition> makeMovie(
+        const URL& url,
         const RunResources& runResources, const char* real_url = NULL,
         bool startLoaderThread = true, const std::string* postdata = NULL);
     
@@ -108,7 +109,8 @@
     /// This is typically used to postpone parsing until a VirtualMachine
     /// is initialized. Initializing the VirtualMachine requires a target
     /// SWF version, which can be found in the SWF header.
-    static DSOEXPORT movie_definition* makeMovie(std::auto_ptr<IOChannel> in,
+    static DSOEXPORT boost::intrusive_ptr<movie_definition> makeMovie(
+            std::auto_ptr<IOChannel> in,
             const std::string& url, const RunResources& runResources,
             bool startLoaderThread);
 

=== modified file 'testsuite/MovieTester.cpp'
--- a/testsuite/MovieTester.cpp 2009-10-21 07:10:41 +0000
+++ b/testsuite/MovieTester.cpp 2009-12-14 21:12:35 +0000
@@ -153,7 +153,7 @@
 
        // Finally, place the root movie on the stage ...
     MovieClip::MovieVariables v;
-    _movie_root->init(_movie_def, v);
+    _movie_root->init(_movie_def.get(), v);
 
        // ... and render it
        render();
@@ -625,7 +625,7 @@
 {
        _movie_root->clear(); // restart();
     MovieClip::MovieVariables v;
-       _movie_root->init(_movie_def, v);
+       _movie_root->init(_movie_def.get(), v);
 
        // Set _movie before calling ::render
        render();

=== modified file 'testsuite/MovieTester.h'
--- a/testsuite/MovieTester.h   2009-10-21 07:34:31 +0000
+++ b/testsuite/MovieTester.h   2009-12-14 21:12:35 +0000
@@ -320,7 +320,7 @@
 
        gnash::movie_root* _movie_root;
 
-       gnash::movie_definition* _movie_def;
+       boost::intrusive_ptr<gnash::movie_definition> _movie_def;
 
     boost::shared_ptr<sound::sound_handler> _sound_handler;