[Gnash-commit] /srv/bzr/gnash/trunk r10441: Hold a *list* of AMFQueue objects, each for each connection.

[Sandro Santilli]

------------------------------------------------------------
revno: 10441
committer: Sandro Santilli <address@hidden>
branch nick: trunk
timestamp: Mon 2008-12-15 16:41:44 +0100
message:
  Hold a *list* of AMFQueue objects, each for each connection.
  Fixes the "connect opens a new connection" testcase.
modified:
  libcore/asobj/NetConnection_as.cpp
  libcore/asobj/NetConnection_as.h
  testsuite/misc-ming.all/remoting.as

=== modified file 'libcore/asobj/NetConnection_as.cpp'
--- a/libcore/asobj/NetConnection_as.cpp        2008-12-15 14:03:46 +0000
+++ b/libcore/asobj/NetConnection_as.cpp        2008-12-15 15:41:44 +0000
@@ -52,7 +52,7 @@
 #include "namedStrings.h"
 
 
-// #define GNASH_DEBUG_REMOTING
+//#define GNASH_DEBUG_REMOTING
 
 // Forward declarations.
 
@@ -135,15 +135,12 @@
         _headers["Content-Type"] = "application/x-amf";
     }
 
-    ~AMFQueue() {
-        stop_ticking();
-    }
-    
     void enqueue(const SimpleBuffer &amf, const std::string& identifier,
             boost::intrusive_ptr<as_object> callback) {
         push_amf(amf);
         push_callback(identifier, callback);
     };
+
     void enqueue(const SimpleBuffer &amf) {
         push_amf(amf);
     };
@@ -153,7 +150,11 @@
     //
     // it handles all networking for NetConnection::call() and dispatches
     // callbacks when needed
-    void tick() {
+    //
+    // @return true if wants to be called again false when done
+    //
+    bool tick()
+    {
 
 #ifdef GNASH_DEBUG_REMOTING
         log_debug("tick running");
@@ -400,9 +401,9 @@
         }
 
         if(!_connection && queued_count > 0) {
-#ifdef GNASH_DEBUG_REMOTING
+//#ifdef GNASH_DEBUG_REMOTING
             log_debug("creating connection");
-#endif
+//#endif
             // set the "number of bodies" header
             (reinterpret_cast<boost::uint16_t*>(postdata.data() + 4))[0] = 
htons(queued_count);
             std::string postdata_str(reinterpret_cast<char*>(postdata.data()), 
postdata.size());
@@ -418,25 +419,12 @@
 #endif
         }
 
-        if(_connection == 0 && queued_count == 0) {
-#ifdef GNASH_DEBUG_REMOTING
-            log_debug("stopping ticking");
-#endif
-            stop_ticking();
-#ifdef GNASH_DEBUG_REMOTING
-            log_debug("ticking stopped");
-#endif
+        if (_connection == 0) {
+            // nothing more to do
+            return false;
         }
-    };
 
-    static as_value amfqueue_tick_wrapper(const fn_call& fn)
-    {
-        boost::intrusive_ptr<NetConnection_as> ptr = 
-            ensureType<NetConnection_as>(fn.this_ptr);
-        // FIXME check if it's possible for the URL of a 
-        // NetConnection to change between call()s
-        ptr->_callQueue->tick();
-        return as_value();
+        return true;
     };
 
     void markReachableResources() const
@@ -449,35 +437,13 @@
     }
 
 private:
-    void start_ticking() 
-    {
-
-        if (ticker) return;
-
-        boost::intrusive_ptr<builtin_function> ticker_as = 
-                new builtin_function(&AMFQueue::amfqueue_tick_wrapper);
-
-        std::auto_ptr<Timer> timer(new Timer);
-        unsigned long delayMS = 50; // FIXME crank up to 50 or so
-        timer->setInterval(*ticker_as, delayMS, &_nc);
-        ticker = _nc.getVM().getRoot().add_interval_timer(timer, true);
-    }
 
     void push_amf(const SimpleBuffer &amf) 
     {
-        GNASH_REPORT_FUNCTION;
+        //GNASH_REPORT_FUNCTION;
 
         postdata.append(amf.data(), amf.size());
         queued_count++;
-
-        start_ticking();
-    }
-
-    void stop_ticking() 
-    {
-        if (!ticker) return;
-        _nc.getVM().getRoot().clear_interval_timer(ticker);
-        ticker=0;
     }
 
     void push_callback(const std::string& id,
@@ -509,8 +475,10 @@
 NetConnection_as::NetConnection_as()
     :
     as_object(getNetConnectionInterface()),
-    _callQueue(0),
-    _isConnected(false)
+    _callQueues(),
+    _currentCallQueue(0),
+    _isConnected(false),
+    _advanceTimer(0)
 {
     attachProperties(*this);
 }
@@ -539,13 +507,25 @@
 // here to have AMFQueue definition available
 NetConnection_as::~NetConnection_as()
 {
+    for (std::list<AMFQueue*>::iterator
+            i=_callQueues.begin(), e=_callQueues.end();
+            i!=e; ++i)
+    {
+        delete *i;
+    }
 }
 
 
 void
 NetConnection_as::markReachableResources() const
 {
-    if ( _callQueue.get() ) _callQueue->markReachableResources();
+    if ( _currentCallQueue.get() ) _currentCallQueue->markReachableResources();
+    for (std::list<AMFQueue*>::const_iterator
+            i=_callQueues.begin(), e=_callQueues.end();
+            i!=e; ++i)
+    {
+        (*i)->markReachableResources();
+    }
     markAsObjectReachable();
 }
 
@@ -653,7 +633,13 @@
 void
 NetConnection_as::connect(const std::string& /*uri*/)
 {
-    // Close any current connections.
+    /// Queue the current call queue
+    if ( _currentCallQueue.get() )
+    {
+        _callQueues.push_back(_currentCallQueue.release());
+    }
+
+    // Close any current connections. (why?)
     close();
 
     // FIXME: We should attempt a connection here (this is called when an
@@ -712,28 +698,32 @@
     // not connected).
     URL url(validateURL());
 
-    // The URL depends on the URL passed to NetConnection.connect();
-    if (!_callQueue.get()) {
-        _callQueue.reset(new AMFQueue(*this, url));
+    // FIXME check if it's possible for the URL of a NetConnection
+    // to change between call()s
+    if (!_currentCallQueue.get()) {
+        _currentCallQueue.reset(new AMFQueue(*this, url));
     }
 
     if (asCallback) {
 #ifdef GNASH_DEBUG_REMOTING
         log_debug("calling enqueue with callback");
 #endif
-        _callQueue->enqueue(buf, callNumber, asCallback);
+        _currentCallQueue->enqueue(buf, callNumber, asCallback);
     }
     
     else {
 #ifdef GNASH_DEBUG_REMOTING
         log_debug("calling enqueue without callback");
 #endif
-        _callQueue->enqueue(buf);
+        _currentCallQueue->enqueue(buf);
     }
+
 #ifdef GNASH_DEBUG_REMOTING
     log_debug("called enqueue");
 #endif
 
+    startAdvanceTimer();
+
 }
 
 std::auto_ptr<IOChannel>
@@ -752,6 +742,92 @@
 
 }
 
+as_value
+NetConnection_as::advanceWrapper(const fn_call& fn)
+{
+    boost::intrusive_ptr<NetConnection_as> ptr = 
+        ensureType<NetConnection_as>(fn.this_ptr);
+    // FIXME check if it's possible for the URL of a 
+    // NetConnection to change between call()s
+    ptr->advance();
+    return as_value();
+};
+
+void
+NetConnection_as::startAdvanceTimer() 
+{
+
+    if (_advanceTimer)
+    {
+        //log_debug("startAdvanceTimer: already running %d", _advanceTimer);
+        return;
+    }
+
+    boost::intrusive_ptr<builtin_function> ticker_as = 
+            new builtin_function(&NetConnection_as::advanceWrapper);
+
+    std::auto_ptr<Timer> timer(new Timer);
+    unsigned long delayMS = 50; // FIXME crank up to 50 or so
+    timer->setInterval(*ticker_as, delayMS, this);
+    _advanceTimer = getVM().getRoot().add_interval_timer(timer, true);
+
+    log_debug("startAdvanceTimer: registered advance timer %d", _advanceTimer);
+}
+
+void
+NetConnection_as::stopAdvanceTimer() 
+{
+    if (!_advanceTimer)
+    {
+        log_debug("stopAdvanceTimer: not running");
+        return;
+    }
+
+    getVM().getRoot().clear_interval_timer(_advanceTimer);
+    log_debug("stopAdvanceTimer: deregistered timer %d", _advanceTimer);
+    _advanceTimer=0;
+}
+
+void
+NetConnection_as::advance()
+{
+    // Advance
+    if ( _currentCallQueue.get() ) 
+    {
+        _callQueues.push_back(_currentCallQueue.release());
+        assert(!_currentCallQueue.get());
+    }
+
+#ifdef GNASH_DEBUG_REMOTING
+    log_debug("NetConnection_as::advance: %d calls to advance", 
_callQueues.size());
+#endif
+
+    while ( ! _callQueues.empty() )
+    {
+        AMFQueue* que = _callQueues.front();
+        if ( ! que->tick() )
+        {
+            log_debug("AMFQueue done, dropping");
+            _callQueues.pop_front();
+            delete que;
+        }
+        else break; // queues handling is serialized
+    }
+
+    // ticking of the queue might have triggered creation
+    // of a new queue, so we won't stop the tick in that case
+    if ( _callQueues.empty() && ! _currentCallQueue.get() )
+    {
+#ifdef GNASH_DEBUG_REMOTING
+        log_debug("stopping ticking");
+#endif
+        stopAdvanceTimer();
+#ifdef GNASH_DEBUG_REMOTING
+        log_debug("ticking stopped");
+#endif
+    }
+}
+
 /// Anonymous namespace for NetConnection AMF-reading helper functions
 /// (shouldn't be here).
 

=== modified file 'libcore/asobj/NetConnection_as.h'
--- a/libcore/asobj/NetConnection_as.h  2008-12-15 14:03:46 +0000
+++ b/libcore/asobj/NetConnection_as.h  2008-12-15 15:41:44 +0000
@@ -20,10 +20,12 @@
 #define GNASH_NETCONNECTION_H
 
 #include "IOChannel.h"
-#include <string>
 #include "as_object.h" // for inheritance
 #include "fn_call.h"
 
+#include <string>
+#include <list>
+
 // Forward declarations
 namespace gnash {
        class AMFQueue;
@@ -53,6 +55,11 @@
        NetConnection_as();
        ~NetConnection_as();
 
+    /// Process connection stuff
+    virtual void advance();
+
+    static as_value advanceWrapper(const fn_call& fn);
+
     /// Make the stored URI into a valid and checked URL.
        std::string validateURL() const;
 
@@ -100,13 +107,20 @@
        /// Extend the URL to be used for playing
        void addToURL(const std::string& url);
 
-       std::auto_ptr<AMFQueue> _callQueue;
+       std::list<AMFQueue*> _callQueues;
+
+    std::auto_ptr<AMFQueue> _currentCallQueue; 
 
        /// the url prefix optionally passed to connect()
        std::string _uri;
 
     bool _isConnected;
 
+    void startAdvanceTimer();
+
+    void stopAdvanceTimer();
+
+    int _advanceTimer;
 };
 
 void netconnection_class_init(as_object& global);

=== modified file 'testsuite/misc-ming.all/remoting.as'
--- a/testsuite/misc-ming.all/remoting.as       2008-12-15 12:16:12 +0000
+++ b/testsuite/misc-ming.all/remoting.as       2008-12-15 15:41:44 +0000
@@ -261,8 +261,7 @@
     nc.call("ary_newconnect2", o, ary13); //
     o.onResult = function(res) {
         //note(printInfo(res));
-        // Gnash uses the same connection, even on re-connect !
-        xcheck(res.remote_port != connectionPort);
+        check(res.remote_port != connectionPort);
         connectionPort = res.remote_port;
         xcheck_equals(res.request_id, '/1'); // connection is reset
         check_equals(res.message, 'ary_newconnect2');