[fix] the grouping of add/remove controls wasn't correctly waiting fo…

…r all of the controls to be passed.
Montellese · Nov 13, 2012 · f7809b0 · f7809b0
1 parent 4f196ae
commit f7809b0
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 15 deletions.
diff --git a/xbmc/interfaces/legacy/Window.cpp b/xbmc/interfaces/legacy/Window.cpp
@@ -37,6 +37,19 @@ namespace XBMCAddon
 {
   namespace xbmcgui
   {
+    /**
+     * Used in add/remove control. It only locks if it's given a 
+     * non-NULL CCriticalSection. It's given a NULL CCriticalSection
+     * when a function higher in the call stack already has a 
+     */
+    class MaybeLock
+    {
+      CCriticalSection* lock;
+    public:
+      inline MaybeLock(CCriticalSection* p_lock) : lock(p_lock) { if (lock) lock->lock(); }
+      inline ~MaybeLock() { if (lock) lock->unlock(); }
+    };
+
     /**
      * Explicit template instantiation
      */
@@ -536,15 +549,24 @@ namespace XBMCAddon
     }
 
     void Window::removeControl(Control* pControl) throw (WindowException)
+    {
+      TRACE;
+      DelayedCallGuard dg(languageHook);
+      doRemoveControl(pControl,&g_graphicsContext,true);
+    }
+
+    void Window::doRemoveControl(Control* pControl, CCriticalSection* gcontext, bool wait) throw (WindowException)
     {
       TRACE;
       // type checking, object should be of type Control
       if(pControl == NULL)
         throw WindowException("Object should be of type Control");
 
-      CSingleLock lock(g_graphicsContext);
+      {
-      if(!ref(window)->GetControl(pControl->iControlId))
+        MaybeLock mlock(gcontext);
-        throw WindowException("Control does not exist in window");
+        if(!ref(window)->GetControl(pControl->iControlId))
+          throw WindowException("Control does not exist in window");
+      }
 
       // delete control from vecControls in window object
       std::vector<AddonClass::Ref<Control> >::iterator it = vecControls.begin();
@@ -559,7 +581,7 @@ namespace XBMCAddon
 
       CGUIMessage msg(GUI_MSG_REMOVE_CONTROL, 0, 0);
       msg.SetPointer(pControl->pGUIControl);
-      CApplicationMessenger::Get().SendGUIMessage(msg, iWindowId, true);
+      CApplicationMessenger::Get().SendGUIMessage(msg, iWindowId, wait);
 
       // initialize control to zero
       pControl->pGUIControl = NULL;
@@ -569,8 +591,11 @@ namespace XBMCAddon
 
     void Window::removeControls(std::vector<Control*> pControls) throw (WindowException)
     {
-      for (std::vector<Control*>::iterator iter = pControls.begin(); iter != pControls.end(); iter++)
+      TRACE;
-        removeControl(*iter);
+      DelayedCallGuard dg(languageHook);
+      int count = 1; int size = pControls.size();
+      for (std::vector<Control*>::iterator iter = pControls.begin(); iter != pControls.end(); count++, iter++)
+        doRemoveControl(*iter,NULL, count == size);
     }
 
     long Window::getHeight()
@@ -688,6 +713,13 @@ namespace XBMCAddon
     }
 
     void Window::addControl(Control* pControl) throw (WindowException)
+    {
+      TRACE;
+      DelayedCallGuard dg(languageHook);
+      doAddControl(pControl,&g_graphicsContext,true);
+    }
+
+    void Window::doAddControl(Control* pControl, CCriticalSection* gcontext, bool wait) throw (WindowException)
     {
       TRACE;
       if(pControl == NULL)
@@ -697,12 +729,14 @@ namespace XBMCAddon
         throw WindowException("Control is already used");
 
       // lock xbmc GUI before accessing data from it
-      CSingleLock lock(g_graphicsContext);
       pControl->iParentId = iWindowId;
-      // assign control id, if id is already in use, try next id
+
-      // TODO: This is not thread safe
+      {
-      do pControl->iControlId = ++iCurrentControlId;
+        MaybeLock mlock(gcontext);
-      while (ref(window)->GetControl(pControl->iControlId));
+        // assign control id, if id is already in use, try next id
+        do pControl->iControlId = ++iCurrentControlId;
+        while (ref(window)->GetControl(pControl->iControlId));
+      }
 
       pControl->Create();
 
@@ -722,13 +756,17 @@ namespace XBMCAddon
       // This calls the CGUIWindow parent class to do the final add
       CGUIMessage msg(GUI_MSG_ADD_CONTROL, 0, 0);
       msg.SetPointer(pControl->pGUIControl);
-      CApplicationMessenger::Get().SendGUIMessage(msg, iWindowId, true);
+      CApplicationMessenger::Get().SendGUIMessage(msg, iWindowId, wait);
     }
 
     void Window::addControls(std::vector<Control*> pControls) throw (WindowException)
     {
-      for (std::vector<Control*>::iterator iter = pControls.begin(); iter != pControls.end(); iter++)
+      TRACE;
-        addControl(*iter);
+      DelayedCallGuard dg(languageHook);
+      CSingleLock lock(g_graphicsContext);
+      int count = 1; int size = pControls.size();
+      for (std::vector<Control*>::iterator iter = pControls.begin(); iter != pControls.end(); count++, iter++)
+        doAddControl(*iter,NULL, count == size);
     }
 
     Control* Window::getControl(int iControlId) throw (WindowException)
@@ -737,7 +775,6 @@ namespace XBMCAddon
       return GetControlById(iControlId);
     }
 
-
     void Action::setFromCAction(const CAction& action)
     {
       TRACE;

diff --git a/xbmc/interfaces/legacy/Window.h b/xbmc/interfaces/legacy/Window.h
@@ -78,6 +78,10 @@ namespace XBMCAddon
     {
       friend class WindowDialogMixin;
       bool isDisposed;
+
+      void doAddControl(Control* pControl, CCriticalSection* gcontext, bool wait) throw (WindowException);
+      void doRemoveControl(Control* pControl, CCriticalSection* gcontext, bool wait) throw (WindowException);
+
     protected:
 #ifndef SWIG
       InterceptorBase* window;