diff --git a/include/wx/event.h b/include/wx/event.h index ec3c65d1a9..24e5545175 100644 --- a/include/wx/event.h +++ b/include/wx/event.h @@ -3587,8 +3587,6 @@ public: userData); } - wxList* GetDynamicEventTable() const { return m_dynamicEvents ; } - // User data can be associated with each wxEvtHandler void SetClientObject( wxClientData *data ) { DoSetClientObject(data); } wxClientData *GetClientObject() const { return DoGetClientObject(); } @@ -3610,6 +3608,14 @@ public: wxEvtHandler *handler, wxEvent& event); + // Allow iterating over all connected dynamic event handlers: you must pass + // the same "cookie" to GetFirst() and GetNext() and call them until null + // is returned. + // + // These functions are for internal use only. + wxDynamicEventTableEntry* GetFirstDynamicEntry(size_t& cookie) const; + wxDynamicEventTableEntry* GetNextDynamicEntry(size_t& cookie) const; + virtual bool SearchEventTable(wxEventTable& table, wxEvent& event); bool SearchDynamicEventTable( wxEvent& event ); @@ -3680,7 +3686,10 @@ protected: wxEvtHandler* m_nextHandler; wxEvtHandler* m_previousHandler; - wxList* m_dynamicEvents; + + typedef wxVector DynamicEvents; + DynamicEvents* m_dynamicEvents; + wxList* m_pendingEvents; #if wxUSE_THREADS diff --git a/src/common/event.cpp b/src/common/event.cpp index 9d9eb18a95..eae17196ce 100644 --- a/src/common/event.cpp +++ b/src/common/event.cpp @@ -1122,13 +1122,11 @@ wxEvtHandler::~wxEvtHandler() if (m_dynamicEvents) { - for ( wxList::iterator it = m_dynamicEvents->begin(), - end = m_dynamicEvents->end(); - it != end; - ++it ) + size_t cookie; + for ( wxDynamicEventTableEntry* entry = GetFirstDynamicEntry(cookie); + entry; + entry = GetNextDynamicEntry(cookie) ) { - wxDynamicEventTableEntry *entry = (wxDynamicEventTableEntry*)*it; - // Remove ourselves from sink destructor notifications // (this has usually been done, in wxTrackable destructor) wxEvtHandler *eventSink = entry->m_fn->GetEvtHandler(); @@ -1696,10 +1694,12 @@ void wxEvtHandler::DoBind(int id, new wxDynamicEventTableEntry(eventType, id, lastId, func, userData); if (!m_dynamicEvents) - m_dynamicEvents = new wxList; + m_dynamicEvents = new DynamicEvents; - // Insert at the front of the list so most recent additions are found first - m_dynamicEvents->Insert( (wxObject*) entry ); + // We prefer to push back the entry here and then iterate over the vector + // in reverse direction in GetNextDynamicEntry() as it's more efficient + // than inserting the element at the front. + m_dynamicEvents->push_back(entry); // Make sure we get to know when a sink is destroyed wxEvtHandler *eventSink = func->GetEvtHandler(); @@ -1723,11 +1723,11 @@ wxEvtHandler::DoUnbind(int id, if (!m_dynamicEvents) return false; - wxList::compatibility_iterator node = m_dynamicEvents->GetFirst(); - while (node) + size_t cookie; + for ( wxDynamicEventTableEntry* entry = GetFirstDynamicEntry(cookie); + entry; + entry = GetNextDynamicEntry(cookie) ) { - wxDynamicEventTableEntry *entry = (wxDynamicEventTableEntry*)node->GetData(); - if ((entry->m_id == id) && ((entry->m_lastId == lastId) || (lastId == wxID_ANY)) && ((entry->m_eventType == eventType) || (eventType == wxEVT_NULL)) && @@ -1744,28 +1744,78 @@ wxEvtHandler::DoUnbind(int id, } delete entry->m_callbackUserData; - m_dynamicEvents->Erase( node ); + + // We can't delete the entry from the vector if we're currently + // iterating over it. As we don't know whether we're or not, just + // null it for now and we will really erase it when we do finish + // iterating over it the next time. + // + // Notice that we rely on "cookie" being just the index into the + // vector, which is not guaranteed by our API, but here we can use + // this implementation detail. + (*m_dynamicEvents)[cookie] = NULL; + delete entry; return true; } - node = node->GetNext(); } return false; } +wxDynamicEventTableEntry* +wxEvtHandler::GetFirstDynamicEntry(size_t& cookie) const +{ + if ( !m_dynamicEvents ) + return NULL; + + // The handlers are in LIFO order, so we must start at the end. + cookie = m_dynamicEvents->size(); + return GetNextDynamicEntry(cookie); +} + +wxDynamicEventTableEntry* +wxEvtHandler::GetNextDynamicEntry(size_t& cookie) const +{ + // On entry here cookie is one greater than the index of the entry to + // return, so if it is 0, it means that there are no more entries. + while ( cookie ) + { + // Otherwise return the element at the previous index, skipping any + // null elements which indicate removed entries. + wxDynamicEventTableEntry* const entry = m_dynamicEvents->at(--cookie); + if ( entry ) + return entry; + } + + return NULL; +} + bool wxEvtHandler::SearchDynamicEventTable( wxEvent& event ) { wxCHECK_MSG( m_dynamicEvents, false, wxT("caller should check that we have dynamic events") ); - wxList::compatibility_iterator node = m_dynamicEvents->GetFirst(); - while (node) - { - wxDynamicEventTableEntry *entry = (wxDynamicEventTableEntry*)node->GetData(); + DynamicEvents& dynamicEvents = *m_dynamicEvents; - // get next node before (maybe) calling the event handler as it could - // call Disconnect() invalidating the current node - node = node->GetNext(); + bool processed = false; + bool needToPruneDeleted = false; + + // We can't use Get{First,Next}DynamicEntry() here as they hide the deleted + // but not yet pruned entries from the caller, but here we do want to know + // about them, so iterate directly. Remember to do it in the reverse order + // to honour the order of handlers connection. + for ( size_t n = dynamicEvents.size(); n; n-- ) + { + wxDynamicEventTableEntry* const entry = dynamicEvents[n - 1]; + + if ( !entry ) + { + // This entry must have been unbound at some time in the past, so + // skip it now and really remove it from the vector below, once we + // finish iterating. + needToPruneDeleted = true; + continue; + } if ( event.GetEventType() == entry->m_eventType ) { @@ -1773,11 +1823,27 @@ bool wxEvtHandler::SearchDynamicEventTable( wxEvent& event ) if ( !handler ) handler = this; if ( ProcessEventIfMatchesId(*entry, handler, event) ) - return true; + { + processed = true; + break; + } } } - return false; + if ( needToPruneDeleted ) + { + size_t nNew = 0; + for ( size_t n = 0; n != dynamicEvents.size(); n++ ) + { + if ( dynamicEvents[n] ) + dynamicEvents[nNew++] = dynamicEvents[n]; + } + + wxASSERT( nNew != dynamicEvents.size() ); + dynamicEvents.resize(nNew); + } + + return processed; } void wxEvtHandler::DoSetClientObject( wxClientData *data ) @@ -1844,19 +1910,20 @@ void wxEvtHandler::OnSinkDestroyed( wxEvtHandler *sink ) wxASSERT(m_dynamicEvents); // remove all connections with this sink - wxList::compatibility_iterator node = m_dynamicEvents->GetFirst(), node_nxt; - while (node) + size_t cookie; + for ( wxDynamicEventTableEntry* entry = GetFirstDynamicEntry(cookie); + entry; + entry = GetNextDynamicEntry(cookie) ) { - wxDynamicEventTableEntry *entry = (wxDynamicEventTableEntry*)node->GetData(); - node_nxt = node->GetNext(); - if ( entry->m_fn->GetEvtHandler() == sink ) { delete entry->m_callbackUserData; - m_dynamicEvents->Erase( node ); delete entry; + + // Just as in DoUnbind(), we use our knowledge of + // GetNextDynamicEntry() implementation here. + (*m_dynamicEvents)[cookie] = NULL; } - node = node_nxt; } } diff --git a/src/common/xtistrm.cpp b/src/common/xtistrm.cpp index 343962596d..b4ec20d411 100644 --- a/src/common/xtistrm.cpp +++ b/src/common/xtistrm.cpp @@ -119,34 +119,30 @@ void wxObjectWriter::FindConnectEntry(const wxEvtHandler * evSource, const wxObject* &sink, const wxHandlerInfo *&handler) { - wxList *dynamicEvents = evSource->GetDynamicEventTable(); - - if ( dynamicEvents ) + size_t cookie; + for ( wxDynamicEventTableEntry* entry = evSource->GetFirstDynamicEntry(cookie); + entry; + entry = evSource->GetNextDynamicEntry(cookie) ) { - for ( wxList::const_iterator node = dynamicEvents->begin(); node != dynamicEvents->end(); ++node ) + // find the match + if ( entry->m_fn && + (dti->GetEventType() == entry->m_eventType) && + (entry->m_id == -1 ) && + (entry->m_fn->GetEvtHandler() != NULL ) ) { - wxDynamicEventTableEntry *entry = (wxDynamicEventTableEntry*)(*node); - - // find the match - if ( entry->m_fn && - (dti->GetEventType() == entry->m_eventType) && - (entry->m_id == -1 ) && - (entry->m_fn->GetEvtHandler() != NULL ) ) + sink = entry->m_fn->GetEvtHandler(); + const wxClassInfo* sinkClassInfo = sink->GetClassInfo(); + const wxHandlerInfo* sinkHandler = sinkClassInfo->GetFirstHandler(); + while ( sinkHandler ) { - sink = entry->m_fn->GetEvtHandler(); - const wxClassInfo* sinkClassInfo = sink->GetClassInfo(); - const wxHandlerInfo* sinkHandler = sinkClassInfo->GetFirstHandler(); - while ( sinkHandler ) + if ( sinkHandler->GetEventFunction() == entry->m_fn->GetEvtMethod() ) { - if ( sinkHandler->GetEventFunction() == entry->m_fn->GetEvtMethod() ) - { - handler = sinkHandler; - break; - } - sinkHandler = sinkHandler->GetNext(); + handler = sinkHandler; + break; } - break; + sinkHandler = sinkHandler->GetNext(); } + break; } } } diff --git a/tests/events/evthandler.cpp b/tests/events/evthandler.cpp index 1488c44839..a0dd3e604e 100644 --- a/tests/events/evthandler.cpp +++ b/tests/events/evthandler.cpp @@ -164,6 +164,7 @@ private: CPPUNIT_TEST( BindFunctionUsingBaseEvent ); CPPUNIT_TEST( BindNonHandler ); CPPUNIT_TEST( InvalidBind ); + CPPUNIT_TEST( UnbindFromHandler ); CPPUNIT_TEST_SUITE_END(); void BuiltinConnect(); @@ -178,6 +179,7 @@ private: void BindFunctionUsingBaseEvent(); void BindNonHandler(); void InvalidBind(); + void UnbindFromHandler(); // these member variables exceptionally don't use "m_" prefix because @@ -459,3 +461,48 @@ void EvtHandlerTestCase::InvalidBind() myHandler.Bind(MyEventType, &MyHandler::OnMyEvent, &mySink); #endif } + +void EvtHandlerTestCase::UnbindFromHandler() +{ + struct Handler1 + { + void OnDontCall(MyEvent&) + { + // Although this handler is bound, the second one below is bound + // later and so will be called first and will disconnect this one + // before it has a chance to be called. + CPPUNIT_FAIL("shouldn't be called"); + } + } h1; + + handler.Bind(MyEventType, &Handler1::OnDontCall, &h1); + + class Handler2 + { + public: + Handler2(MyHandler& handler, Handler1& h1) + { + m_handler = &handler; + m_h1 = &h1; + } + + void OnUnbind(MyEvent& e) + { + m_handler->Unbind(MyEventType, &Handler1::OnDontCall, m_h1); + + // Check that the now disconnected first handler is not executed. + e.Skip(); + } + + private: + // Use pointers instead of references just to avoid warnings about the + // class being non-copyable even though these pointers don't change and + // are non-NULL. + MyHandler* m_handler; + Handler1* m_h1; + } h2(handler, h1); + + handler.Bind(MyEventType, &Handler2::OnUnbind, &h2); + + handler.ProcessEvent(e); +}