[Gnash-commit] /srv/bzr/gnash/trunk r10469: Rework notification of connection status and slighly move toward the "notional connection" concept. No more failures in remoting.swf.

[Sandro Santilli]

------------------------------------------------------------
revno: 10469
committer: Sandro Santilli <address@hidden>
branch nick: trunk
timestamp: Fri 2008-12-19 00:44:52 +0100
message:
  Rework notification of connection status and slighly move toward the 
"notional connection" concept. No more failures in remoting.swf.
modified:
  libcore/asobj/NetConnection_as.cpp
  testsuite/misc-ming.all/remoting.as

=== modified file 'libcore/asobj/NetConnection_as.cpp'
--- a/libcore/asobj/NetConnection_as.cpp        2008-12-18 22:39:32 +0000
+++ b/libcore/asobj/NetConnection_as.cpp        2008-12-18 23:44:52 +0000
@@ -135,6 +135,11 @@
         _headers["Content-Type"] = "application/x-amf";
     }
 
+    bool hasPendingCalls() const
+    {
+        return _connection || queued_count;
+    }
+
     void enqueue(const SimpleBuffer &amf, const std::string& identifier,
             boost::intrusive_ptr<as_object> callback) {
         push_amf(amf);
@@ -662,19 +667,51 @@
 
 
 void
-NetConnection_as::connect(const std::string& /*uri*/)
+NetConnection_as::connect(const std::string& uri)
 {
-    /// Queue the current call queue
-    if ( _currentCallQueue.get() )
-    {
-        _callQueues.push_back(_currentCallQueue.release());
-    }
-
     _numCalls=0;
 
     // Close any current connections. (why?) Because that's what happens.
     close();
 
+    // TODO: check for other kind of invalidities here...
+    if ( uri.empty() )
+    {
+        _isConnected = false;
+        notifyStatus(CONNECT_FAILED);
+        return;
+    }
+
+    const movie_root& mr = _vm.getRoot();
+    URL url(uri, mr.runInfo().baseURL());
+
+    if ( url.protocol() == "rtmp" )
+    {
+        LOG_ONCE( log_unimpl("NetConnection.connect(%s): RTMP not yet 
supported", url) );
+        notifyStatus(CONNECT_FAILED);
+        return;
+    }
+
+    if ( url.protocol() != "http" )
+    {
+        IF_VERBOSE_ASCODING_ERRORS(
+        log_aserror("NetConnection.connect(%s): invalid connection protocol", 
url);
+        );
+        notifyStatus(CONNECT_FAILED);
+        return;
+    }
+
+    // This is for HTTP remoting
+
+    if (!URLAccessManager::allow(url)) {
+        log_security(_("Gnash is not allowed to NetConnection.connect to %s"), 
url);
+        notifyStatus(CONNECT_FAILED);
+        return;
+    }
+
+    _currentCallQueue.reset(new AMFQueue(*this, url));
+
+
     // FIXME: We should attempt a connection here (this is called when an
     // argument is passed to NetConnection.connect(url).
     // Would probably return true on success and set isConnected.
@@ -688,7 +725,6 @@
     //    and fails immediately.
     // TODO: modify validateURL for doing this.
     _isConnected = false;
-    notifyStatus(CONNECT_FAILED);
 }
 
 
@@ -697,13 +733,22 @@
 void
 NetConnection_as::close()
 {
-    if (!_isConnected) return;
+    bool needSendClosedStatus = _currentCallQueue.get() || _isConnected;
+
+    /// Queue the current call queue if it has pending calls
+    if ( _currentCallQueue.get() && _currentCallQueue->hasPendingCalls() )
+    {
+        _callQueues.push_back(_currentCallQueue.release());
+    }
 
     /// TODO: what should actually happen here? Should an attached
     /// NetStream object be interrupted?
     _isConnected = false;
 
-    notifyStatus(CONNECT_CLOSED);
+    if ( needSendClosedStatus )
+    {
+        notifyStatus(CONNECT_CLOSED);
+    }
 }
 
 
@@ -718,6 +763,12 @@
 NetConnection_as::call(as_object* asCallback, const std::string& methodName,
         const std::vector<as_value>& args, size_t firstArg)
 {
+    if ( ! _currentCallQueue.get() )
+    {
+        log_aserror("NetConnection.call: can't call while not connected");
+        return;
+    }
+
     boost::scoped_ptr<SimpleBuffer> buf (new SimpleBuffer(32));
 
     // method name
@@ -769,18 +820,6 @@
             hexify(buf.data(), buf.size(), false));
 #endif
 
-    // FIXME: Don't do this here. Use a single connection object member
-    // for all calls (depends on the following FIXME), and also check
-    // whether a connection exists and don't call() if it doesn't (can be
-    // done in the AS implementation to save processing arguments when
-    // not connected).
-    URL url(validateURL());
-
-    // This should use the uri set with connect()
-    if (!_currentCallQueue.get()) {
-        _currentCallQueue.reset(new AMFQueue(*this, url));
-    }
-
     if (asCallback) {
 #ifdef GNASH_DEBUG_REMOTING
         log_debug("calling enqueue with callback");
@@ -868,11 +907,6 @@
 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());
@@ -890,6 +924,11 @@
         else break; // queues handling is serialized
     }
 
+    if ( _currentCallQueue.get() ) 
+    {
+        _currentCallQueue->tick();
+    }
+
     // 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() )

=== modified file 'testsuite/misc-ming.all/remoting.as'
--- a/testsuite/misc-ming.all/remoting.as       2008-12-18 16:57:32 +0000
+++ b/testsuite/misc-ming.all/remoting.as       2008-12-18 23:44:52 +0000
@@ -103,11 +103,11 @@
 
     nc.connect(url);
     check_equals(nc.isConnected, false);
-    xcheck_equals(nc.statuses.length, 3);
+    check_equals(nc.statuses.length, 3);
     lastStatusArgs = nc.statuses[nc.statuses.length-1];
     check_equals(lastStatusArgs.length, 1);
-    xcheck_equals(lastStatusArgs[0].level, 'status');
-    xcheck_equals(lastStatusArgs[0].code, 'NetConnection.Connect.Closed');
+    check_equals(lastStatusArgs[0].level, 'status');
+    check_equals(lastStatusArgs[0].code, 'NetConnection.Connect.Closed');
 
     o=new ResultHandler();
     ary1=[1,2,3];
@@ -294,11 +294,11 @@
     nc.connect(url); // reconnect, should reset call id
 
     check_equals(nc.isConnected, false);
-    xcheck_equals(nc.statuses.length, 4);
+    check_equals(nc.statuses.length, 4);
     lastStatusArgs = nc.statuses[nc.statuses.length-1];
     check_equals(lastStatusArgs.length, 1);
-    xcheck_equals(lastStatusArgs[0].level, 'status');
-    xcheck_equals(lastStatusArgs[0].code, 'NetConnection.Connect.Closed');
+    check_equals(lastStatusArgs[0].level, 'status');
+    check_equals(lastStatusArgs[0].code, 'NetConnection.Connect.Closed');
 
     o=new ResultHandler();
     ary13=[]; 
@@ -317,11 +317,11 @@
     nc.connect(url); // reconnect, should reset call id
 
     check_equals(nc.isConnected, false);
-    xcheck_equals(nc.statuses.length, 5);
+    check_equals(nc.statuses.length, 5);
     lastStatusArgs = nc.statuses[nc.statuses.length-1];
     check_equals(lastStatusArgs.length, 1);
-    xcheck_equals(lastStatusArgs[0].level, 'status');
-    xcheck_equals(lastStatusArgs[0].code, 'NetConnection.Connect.Closed');
+    check_equals(lastStatusArgs[0].level, 'status');
+    check_equals(lastStatusArgs[0].code, 'NetConnection.Connect.Closed');
 
     o=new ResultHandler();
     ary13=[]; 
@@ -444,7 +444,7 @@
 function test17()
 {
     check_equals(nc.isConnected, false);
-    xcheck_equals(nc.statuses.length, 5);
+    check_equals(nc.statuses.length, 5);
 
     endOfTest();
 }