gnash-commit
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Gnash-commit] /srv/bzr/gnash/trunk r9674: Make Player's Gui non-static,


From: Sandro Santilli
Subject: [Gnash-commit] /srv/bzr/gnash/trunk r9674: Make Player's Gui non-static, rework core to host communication design
Date: Thu, 04 Sep 2008 05:08:33 +0200
User-agent: Bazaar (1.5)

------------------------------------------------------------
revno: 9674
committer: Sandro Santilli <address@hidden>
branch nick: trunk
timestamp: Thu 2008-09-04 05:08:33 +0200
message:
  Make Player's Gui non-static, rework core to host communication design
modified:
  gui/Player.cpp
  gui/Player.h
  gui/gui.cpp
  libbase/curl_adapter.cpp
  libcore/asobj/Mouse.cpp
  libcore/asobj/System.cpp
  libcore/movie_root.cpp
  libcore/movie_root.h
  libcore/vm/ASHandlers.cpp
  utilities/processor.cpp
    ------------------------------------------------------------
    revno: 9666.1.1
    committer: Sandro Santilli <address@hidden>
    branch nick: mybranch
    timestamp: Thu 2008-09-04 05:04:47 +0200
    message:
      Rework FS command and INTERFACE callbacks to use functors rather then
      C-style callbacks. This allows callbacks to have a state, which we'd
      in particular use to avoid statics. The alternative would be taking
      an additional void pointer (user data), but I belive the C++ way is
      more flexible for this (despite being more verbose).
      Most changes are for hiding the callback type (two new methods of
      movie_root take care of dispatching the notifications and calls to
      hosting application).
      
      Thanks to this we drop the 'static' Gui member of Player, thus
      fixing bug #24194.
    modified:
      gui/Player.cpp
      gui/Player.h
      gui/gui.cpp
      libbase/curl_adapter.cpp
      libcore/asobj/Mouse.cpp
      libcore/asobj/System.cpp
      libcore/movie_root.cpp
      libcore/movie_root.h
      libcore/vm/ASHandlers.cpp
      utilities/processor.cpp
=== modified file 'gui/Player.cpp'
--- a/gui/Player.cpp    2008-07-11 12:32:22 +0000
+++ b/gui/Player.cpp    2008-09-04 03:04:47 +0000
@@ -62,8 +62,6 @@
 gnash::LogFile& dbglogfile = gnash::LogFile::getDefaultInstance();
 }
 
-std::auto_ptr<Gui> Player::_gui(NULL);
-
 /*static private*/
 void
 Player::setFlashVars(const std::string& varstr)
@@ -400,13 +398,15 @@
 
     SystemClock clock; // use system clock here...
     movie_root& root = VM::init(*_movieDef, clock).getRoot();
+
+    _callbacksHandler.reset(new CallbacksHandler(_gui.get())); 
     
     // Register Player to receive events from the core (Mouse, Stage,
     // System etc)
-    root.registerEventCallback(&interfaceEventCallback);
+    root.registerEventCallback(_callbacksHandler.get());
     
     // Register Player to receive FsCommand events from the core.
-    root.registerFSCommandCallback(&fs_callback);
+    root.registerFSCommandCallback(_callbacksHandler.get());
 
     // Set host requests fd (if any)
     if ( _hostfd != -1 ) root.setHostFD(_hostfd);
@@ -449,16 +449,78 @@
     return EXIT_SUCCESS;
 }
 
-// static private
-// For handling notification callbacks from ActionScript. The callback is
-// always sent to a hosting application (i.e. if a file descriptor is
-// supplied). It is never acted on by Gnash when running as a plugin.
-//
-// TODO: drop first argument ?
-//
+std::string
+Player::CallbacksHandler::call(const std::string& event, const std::string& 
arg)
+{
+    if (event == "Mouse.hide")
+    {
+       return _gui->showMouse(false) ? "true" : "false";
+    }
+
+    if (event == "Mouse.show")
+    {
+       return _gui->showMouse(true) ? "true" : "false";
+    }
+    
+    if (event == "Stage.displayState")
+    {
+       if (arg == "fullScreen") _gui->setFullscreen();
+       else if (arg == "normal") _gui->unsetFullscreen();
+       return "";
+    }
+
+    if (event == "Stage.scaleMode" || event == "Stage.align" )
+    {
+       _gui->updateStageMatrix();
+       return "";
+    }
+    
+    if (event == "System.capabilities.screenResolutionX")
+    {
+       std::ostringstream ss;
+       ss << _gui->getScreenResX();
+       return ss.str();
+    }
+
+    if (event == "System.capabilities.screenResolutionY")
+    {
+       std::ostringstream ss;
+       ss << _gui->getScreenResY();
+       return ss.str();
+    }
+
+    if (event == "System.capabilities.pixelAspectRatio")
+    {
+       std::ostringstream ss;
+       // Whether the pp actively limits the precision or simply
+       // gets a slightly different result isn't clear.
+       ss << std::setprecision(7) << _gui->getPixelAspectRatio();
+       return ss.str();
+    }
+
+    if (event == "System.capabilities.screenDPI")
+    {
+       std::ostringstream ss;
+       ss << _gui->getScreenDPI();
+       return ss.str();
+    }
+
+    if (event == "System.capabilities.screenColor")
+    {
+       return _gui->getScreenColor();
+    }
+
+    if (event == "System.capabilities.playerType")
+    {
+       return _gui->isPlugin() ? "PlugIn" : "StandAlone";
+    }
+
+    log_error(_("Unhandled callback %s with arguments %s"), event, arg);
+    return "";
+}
+
 void
-Player::fs_callback(gnash::sprite_instance* /*movie*/, const std::string& 
command,
-                                const std::string& args)
+Player::CallbacksHandler::notify(const std::string& command, const 
std::string& args)
 {
     //log_debug(_("fs_callback(%p): %s %s"), (void*)movie, command, args);
 
@@ -578,75 +640,6 @@
 
 }
 
-std::string
-Player::interfaceEventCallback(const std::string& event, const std::string& 
arg)
-{
-    if (event == "Mouse.hide")
-    {
-        return _gui->showMouse(false) ? "true" : "false";
-    }
-
-    if (event == "Mouse.show")
-    {
-        return _gui->showMouse(true) ? "true" : "false";
-    }
-    
-    if (event == "Stage.displayState")
-    {
-        if (arg == "fullScreen") _gui->setFullscreen();
-        else if (arg == "normal") _gui->unsetFullscreen();
-        return "";
-    }
-
-    if (event == "Stage.scaleMode" || event == "Stage.align" )
-    {
-        _gui->updateStageMatrix();
-        return "";
-    }
-    
-    if (event == "System.capabilities.screenResolutionX")
-    {
-        std::ostringstream ss;
-        ss << _gui->getScreenResX();
-        return ss.str();
-    }
-
-    if (event == "System.capabilities.screenResolutionY")
-    {
-        std::ostringstream ss;
-        ss << _gui->getScreenResY();
-        return ss.str();
-    }
-
-    if (event == "System.capabilities.pixelAspectRatio")
-    {
-        std::ostringstream ss;
-        // Whether the pp actively limits the precision or simply
-        // gets a slightly different result isn't clear.
-        ss << std::setprecision(7) << _gui->getPixelAspectRatio();
-        return ss.str();
-    }
-
-    if (event == "System.capabilities.screenDPI")
-    {
-        std::ostringstream ss;
-        ss << _gui->getScreenDPI();
-        return ss.str();
-    }
-
-    if (event == "System.capabilities.screenColor")
-    {
-        return _gui->getScreenColor();
-    }
-
-    if (event == "System.capabilities.playerType")
-    {
-        return _gui->isPlugin() ? "PlugIn" : "StandAlone";
-    }
-
-    log_error(_("Unhandled callback %s with arguments %s"), event, arg);
-    return "";
-}
 
 // private
 std::auto_ptr<Gui>
@@ -687,3 +680,7 @@
     return std::auto_ptr<Gui>(new NullGui(_doLoop));
 }
 
+Player::~Player()
+{
+       log_debug("~Player - _movieDef refcount: %d (1 will be dropped now)", 
_movieDef->get_ref_count());
+}

=== modified file 'gui/Player.h'
--- a/gui/Player.h      2008-06-03 13:48:51 +0000
+++ b/gui/Player.h      2008-09-04 03:04:47 +0000
@@ -29,6 +29,7 @@
 #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 "movie_root.h" // for Abstract callbacks
 
 #include <string>
 #include <map>
@@ -54,7 +55,7 @@
 
        Player();
 
-       ~Player() {}
+       ~Player();
 
        /// Play the movie at the given url/path.
        //
@@ -142,6 +143,30 @@
        
 private:
 
+       class CallbacksHandler
+               : public movie_root::AbstractIfaceCallback,
+                 public movie_root::AbstractFsCallback
+       {
+       public:
+               CallbacksHandler(Gui* gui)
+                       :
+                       _gui(gui)
+               {}
+
+               std::string call(const std::string& event, const std::string& 
arg);
+
+               // For handling notification callbacks from ActionScript. The 
callback is
+               // always sent to a hosting application (i.e. if a file 
descriptor is
+               // supplied). It is never acted on by Gnash when running as a 
plugin.
+               void notify(const std::string& event, const std::string& arg);
+
+       private:
+
+               Gui* _gui;
+       };
+
+       std::auto_ptr<CallbacksHandler> _callbacksHandler;
+
        void init();
 
        /// This aux streamer returns a silent audio stream
@@ -171,12 +196,6 @@
 
        void setFlashVars(const std::string& varstr);
 
-       static void fs_callback(sprite_instance* movie,
-                       const std::string& command, const std::string& args);
-
-       static std::string interfaceEventCallback(const std::string& event,
-                                                       const std::string& arg);
-
        // Movie parameters (for -P)
        std::map<std::string, std::string> params;
 
@@ -203,7 +222,7 @@
 
        std::string _baseurl;
 
-       static std::auto_ptr<Gui> _gui;
+       std::auto_ptr<Gui> _gui;
 
        std::auto_ptr<media::sound_handler> _soundHandler;
 

=== modified file 'gui/gui.cpp'
--- a/gui/gui.cpp       2008-08-07 13:21:24 +0000
+++ b/gui/gui.cpp       2008-09-04 03:04:47 +0000
@@ -157,6 +157,7 @@
 
 Gui::~Gui()
 {
+    log_debug("~Gui - _movieDef refcount: %d", _movieDef->get_ref_count());
 
     delete _renderer;
 #ifdef GNASH_FPS_DEBUG

=== modified file 'libbase/curl_adapter.cpp'
--- a/libbase/curl_adapter.cpp  2008-08-18 23:53:04 +0000
+++ b/libbase/curl_adapter.cpp  2008-09-04 03:04:47 +0000
@@ -189,15 +189,16 @@
 
 CurlSession::~CurlSession()
 {
+       log_debug("~CurlSession");
        exportCookies();
 
-       CURLSHcode code = curl_share_cleanup(_shandle);
-       if ( code != CURLSHE_OK )
+       CURLSHcode code;
+       while ( (code=curl_share_cleanup(_shandle)) != CURLSHE_OK )
        {
-               log_error("Failed cleaning up share handle: %s", 
curl_share_strerror(code));
+               log_error("Failed cleaning up share handle: %s. Will try again 
in a second.", curl_share_strerror(code));
+               sleep(1);
        }
        _shandle = 0;
-
        curl_global_cleanup();
 }
 

=== modified file 'libcore/asobj/Mouse.cpp'
--- a/libcore/asobj/Mouse.cpp   2008-07-21 09:56:09 +0000
+++ b/libcore/asobj/Mouse.cpp   2008-09-04 03:04:47 +0000
@@ -67,14 +67,7 @@
 
     movie_root& m = obj->getVM().getRoot();
 
-       if (m.interfaceHandle)
-       {
-               success = ((*m.interfaceHandle)("Mouse.hide", "") == "true") ? 
1 : 0;
-       }
-       else
-       {
-               log_error(_("No callback to handle Mouse.hide"));
-       }
+    success = (m.callInterface("Mouse.hide", "") == "true") ? 1 : 0;
 
     // returns 1 if mouse was visible before call.
     return as_value(success);
@@ -90,14 +83,7 @@
 
     movie_root& m = obj->getVM().getRoot();
 
-       if (m.interfaceHandle)
-       {
-               success = ((*m.interfaceHandle)("Mouse.show", "") == "true") ? 
1 : 0;
-       }
-       else
-       {
-               log_error(_("No callback to handle Mouse.show"));
-       }
+    success = (m.callInterface("Mouse.show", "") == "true") ? 1 : 0;
 
     // returns 1 if Mouse was visible before call.
     return as_value(success);

=== modified file 'libcore/asobj/System.cpp'
--- a/libcore/asobj/System.cpp  2008-08-25 10:16:16 +0000
+++ b/libcore/asobj/System.cpp  2008-09-04 03:04:47 +0000
@@ -134,22 +134,20 @@
 
     const movie_root& m = vm.getRoot();
 
-    if (m.interfaceHandle) {
-        ss.str((*m.interfaceHandle)("System.capabilities.screenResolutionX", 
""));
-        ss >> screenResolutionX;
+    ss.str(m.callInterface("System.capabilities.screenResolutionX", ""));
+    ss >> screenResolutionX;
         
-        ss.clear();
-        ss.str((*m.interfaceHandle)("System.capabilities.screenResolutionY", 
""));
-        ss >> screenResolutionY;
+    ss.clear();
+    ss.str(m.callInterface("System.capabilities.screenResolutionY", ""));
+    ss >> screenResolutionY;
 
-        ss.clear();
-        ss.str((*m.interfaceHandle)("System.capabilities.screenDPI", ""));
-        ss >> screenDPI;
+    ss.clear();
+    ss.str(m.callInterface("System.capabilities.screenDPI", ""));
+    ss >> screenDPI;
         
-        pixelAspectRatio = 
(*m.interfaceHandle)("System.capabilities.pixelAspectRatio", "");
-        playerType = (*m.interfaceHandle)("System.capabilities.playerType", 
"");
-        screenColor = (*m.interfaceHandle)("System.capabilities.screenColor", 
"");
-    }
+    pixelAspectRatio = m.callInterface("System.capabilities.pixelAspectRatio", 
"");
+    playerType = m.callInterface("System.capabilities.playerType", "");
+    screenColor = m.callInterface("System.capabilities.screenColor", "");
 
     //
        // Media

=== modified file 'libcore/movie_root.cpp'
--- a/libcore/movie_root.cpp    2008-09-01 18:52:30 +0000
+++ b/libcore/movie_root.cpp    2008-09-04 03:04:47 +0000
@@ -90,8 +90,8 @@
 
 movie_root::movie_root()
        :
-       interfaceHandle(0),
-       fsCommandHandle(0),
+       _interfaceHandler(0),
+       _fsCommandHandler(0),
        m_viewport_x0(0),
        m_viewport_y0(0),
        m_viewport_width(1),
@@ -1436,7 +1436,7 @@
 movie_root::setStageAlignment(short s)
 {
     _alignMode = s;
-    if (interfaceHandle) (*interfaceHandle)("Stage.align", "");
+    callInterface("Stage.align", "");
 }
 
 /// Returns a pair of enum values giving the actual alignment
@@ -1497,7 +1497,7 @@
     }
 
     _scaleMode = sm;
-    if (interfaceHandle) (*interfaceHandle)("Stage.align", "");    
+    callInterface("Stage.align", "");    
 
     if ( notifyResize )
     {
@@ -1514,15 +1514,15 @@
     boost::intrusive_ptr<Stage> stage = getStageObject();
     if ( stage ) stage->notifyFullScreen( (_displayState == fullScreen) );
 
-       if (!movie_root::interfaceHandle) return; // No registered callback
+       if (!_interfaceHandler) return; // No registered callback
        
        if (_displayState == fullScreen)
        {
-           (*movie_root::interfaceHandle)("Stage.displayState", "fullScreen");
+           callInterface("Stage.displayState", "fullScreen");
        }
        else if (_displayState == normal)
        {
-           (*movie_root::interfaceHandle)("Stage.displayState", "normal");
+           callInterface("Stage.displayState", "normal");
        }   
 }
 
@@ -2261,5 +2261,21 @@
 }
 #endif
 
+void
+movie_root::handleFsCommand(const std::string& cmd, const std::string& arg) 
const
+{
+       if ( _fsCommandHandler ) _fsCommandHandler->notify(cmd, arg);
+}
+
+std::string
+movie_root::callInterface(const std::string& cmd, const std::string& arg) const
+{
+       if ( _interfaceHandler ) return _interfaceHandler->call(cmd, arg);
+
+       log_error("Hosting application registered no callback for 
events/queries");
+
+       return "<no iface to hosting app>";
+}
+
 } // namespace gnash
 

=== modified file 'libcore/movie_root.h'
--- a/libcore/movie_root.h      2008-08-26 13:30:20 +0000
+++ b/libcore/movie_root.h      2008-09-04 03:04:47 +0000
@@ -700,28 +700,12 @@
         return _hostfd;
     }
 
-    /// Signature of interface event callback.
-    typedef std::string (*interfaceEventCallback)(const std::string& event,
-                                              const std::string& arg);
-
-       /// A callback to the GUI (or whatever is listening) for sending
-       /// events and receiving replies. Used for ActionScript interface
-       /// with the gui (Mouse visibility, Stage alignment etc and System
-       /// information, for instance).
-       interfaceEventCallback interfaceHandle;
-
-       DSOEXPORT void registerEventCallback(interfaceEventCallback handler)
-       {
-               interfaceHandle = handler;
-       }
-
-    /// Signature of fscommand callback function
-    typedef void (*fsCommandCallback)(sprite_instance* movie,
-                  const std::string& command, const std::string& arg);
-
-    /// Callback to send FsCommands somewhere.
-    fsCommandCallback fsCommandHandle;
-
+    /// Abstract base class for FS handlers
+    class AbstractFsCallback {
+    public:
+        virtual void notify(const std::string& cmd, const std::string& arg)=0;
+        virtual ~AbstractFsCallback() {}
+    };
 
     /// ActionScript embedded in a movie can use the built-in
     /// fscommand() function to send data back to the host
@@ -732,10 +716,39 @@
     /// The handler gets the sprite_instance* that the script is
     /// embedded in, and the two string arguments passed by the
     /// script to fscommand().
-    DSOEXPORT void registerFSCommandCallback(fsCommandCallback handler)
-    {
-        fsCommandHandle = handler;
-    }
+    ///
+    DSOEXPORT void registerFSCommandCallback(AbstractFsCallback* handler)
+    {
+        _fsCommandHandler = handler;
+    }
+
+    /// Call this to notify FS commands
+    DSOEXPORT void handleFsCommand(const std::string& cmd, const std::string& 
arg) const;
+
+    /// Abstract base class for hosting app handler
+    class AbstractIfaceCallback {
+    public:
+        virtual std::string call(const std::string& cmd, const std::string& 
arg)=0;
+        virtual ~AbstractIfaceCallback() {}
+    };
+
+    /// A callback to the GUI (or whatever is listening) for sending
+    /// events and receiving replies. Used for ActionScript interface
+    /// with the gui (Mouse visibility, Stage alignment etc and System
+    /// information, for instance).
+    ///
+    /// See callInterface method
+    ///
+    DSOEXPORT void registerEventCallback(AbstractIfaceCallback* handler)
+    {
+        _interfaceHandler = handler;
+    }
+
+    /// Call into the hosting application
+    ///
+    /// Will use callback set with registerEventCallback
+    ///
+    DSOEXPORT std::string callInterface(const std::string& cmd, const 
std::string& arg) const;
 
     /// Called from the ScriptLimits tag parser to set the
     /// global script limits. It is expected behaviour that
@@ -775,6 +788,12 @@
 
 private:
 
+    /// Registered Interface command handler, if any
+    AbstractIfaceCallback* _interfaceHandler;
+
+    /// Registered FsCommand handler, if any
+    AbstractFsCallback* _fsCommandHandler;
+
     /// A load movie request
     class LoadMovieRequest {
     public:

=== modified file 'libcore/vm/ASHandlers.cpp'
--- a/libcore/vm/ASHandlers.cpp 2008-09-03 13:43:49 +0000
+++ b/libcore/vm/ASHandlers.cpp 2008-09-04 03:08:33 +0000
@@ -2194,14 +2194,7 @@
     StringNoCaseEqual noCaseCompare;
     if (noCaseCompare(urlTarget.substr(0, 10), "FSCommand:"))
     {
-        if (m.fsCommandHandle)
-        {
-            // Call into the app.
-            // NOTE: the first argument is a movie_instance, but isn't used
-            // anyway, so we avoid attempting to fetch one
-            (*m.fsCommandHandle)(0, urlTarget.substr(10), 
target_string.c_str());
-        }
-
+        m.handleFsCommand(urlTarget.substr(10), target_string);
         return;
     }
 

=== modified file 'utilities/processor.cpp'
--- a/utilities/processor.cpp   2008-08-07 10:02:35 +0000
+++ b/utilities/processor.cpp   2008-09-04 03:04:47 +0000
@@ -135,61 +135,69 @@
 //
 static int quitrequested = false;
 
-void execFsCommand(sprite_instance* movie, const std::string& command, const 
std::string& args)
-{
-    log_debug(_("fs_callback(%p): %s %s"), (void*)movie, command, args);
-
-    StringNoCaseEqual ncasecomp;
-   
-    if ( ncasecomp(command, "quit") ) quitrequested = true;
-}
-
-std::string eventCallback(const std::string& event,
-                                                       const std::string& arg)
-{
-    log_debug(_("eventCallback: %s %s"), event, arg);
-
-    static bool mouseShown = true;
-
-    // These should return "true" if the mouse was visible before
-    // the call.
-    if ( event == "Mouse.hide" ) {
-        bool state = mouseShown;
-        mouseShown = false;
-        return state ? "true" : "false";
-    }
-
-    if ( event == "Mouse.show" ) {
-        bool state = mouseShown;
-        mouseShown = true;
-        return state ? "true" : "false" ;
-    }
-    
-    // Some fake values for consistent test results.
-    
-    if ( event == "System.capabilities.screenResolutionX" ) {
-        return "800";
-    }
-
-    if ( event == "System.capabilities.screenResolutionY" ) {
-        return "640";
-    } 
-
-    if ( event == "System.capabilities.screenDPI" ) {
-        return "72";
-    }        
-
-    if ( event == "System.capabilities.screenColor" ) {
-        return "Color";
-    } 
-
-    if ( event == "System.capabilities.playerType" ) {
-        return "StandAlone";
-    } 
-
-    return "";
-
-}
+class FsCommandExecutor: public movie_root::AbstractFsCallback {
+public:
+       void notify(const std::string& command, const std::string& args)
+       {
+           log_debug(_("fs_callback(%p): %s %s"), command, args);
+
+           StringNoCaseEqual ncasecomp;
+          
+           if ( ncasecomp(command, "quit") ) quitrequested = true;
+       }
+};
+
+class EventCallback: public movie_root::AbstractIfaceCallback {
+public:
+       std::string call(const std::string& event, const std::string& arg)
+       {
+           log_debug(_("eventCallback: %s %s"), event, arg);
+
+           static bool mouseShown = true;
+
+           // These should return "true" if the mouse was visible before
+           // the call.
+           if ( event == "Mouse.hide" ) {
+               bool state = mouseShown;
+               mouseShown = false;
+               return state ? "true" : "false";
+           }
+
+           if ( event == "Mouse.show" ) {
+               bool state = mouseShown;
+               mouseShown = true;
+               return state ? "true" : "false" ;
+           }
+           
+           // Some fake values for consistent test results.
+           
+           if ( event == "System.capabilities.screenResolutionX" ) {
+               return "800";
+           }
+
+           if ( event == "System.capabilities.screenResolutionY" ) {
+               return "640";
+           } 
+
+           if ( event == "System.capabilities.screenDPI" ) {
+               return "72";
+           }        
+
+           if ( event == "System.capabilities.screenColor" ) {
+               return "Color";
+           } 
+
+           if ( event == "System.capabilities.playerType" ) {
+               return "StandAlone";
+           } 
+
+           return "";
+
+       }
+};
+
+EventCallback eventCallback;
+FsCommandExecutor execFsCommand;
 
 int
 main(int argc, char *argv[])


reply via email to

[Prev in Thread] Current Thread [Next in Thread]