#9867: make DeletePendingObjects() public

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/9867>

#9867: make DeletePendingObjects() public
--------------------------------------------------+-------------------------
 Reporter:  sqlaware                              |       Owner:      
     Type:  defect                                |      Status:  new  
 Priority:  normal                                |   Milestone:      
Component:  base                                  |     Version:  2.8.x
 Keywords:  DeletePendingObjects wxAppBase wxApp  |   Blockedby:      
    Patch:  1                                     |    Blocking:      
--------------------------------------------------+-------------------------
 On rare occasions, a developer needs to ensure that deleted windows are
 really deleted, not just marked for deletion.  If
 wxAppBase::DeletePendingObjects() is public, the developer can call it
 directly.

 For example, a parent wants to send a message to all its child windows but
 the child windows change over time, being added and destroyed.  By calling
 DeletePendingObjects() directly, he can ensure that he sends the message
 to non-zombie windows, without needing to check the status of the window.

 An extra call to DeletePendingObjects() is generally harmless, anyway, so
 making it public poses no danger.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/9867>
_______________________________________________
wx-dev mailing list
wx-dev@...
http://lists.wxwidgets.org/mailman/listinfo/wx-dev

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:1>

#9867: make DeletePendingObjects() public
--------------------------+-------------------------------------------------
  Reporter:  sqlaware     |       Owner:                                      
      Type:  enhancement  |      Status:  new                                
  Priority:  normal       |   Milestone:                                      
 Component:  base         |     Version:  2.8-svn                            
Resolution:               |    Keywords:  DeletePendingObjects wxAppBase wxApp
 Blockedby:               |       Patch:  1                                  
  Blocking:               |  
--------------------------+-------------------------------------------------
Changes (by sqlaware):

  * version:  2.8.x => 2.8-svn
  * type:  defect => enhancement



--
Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:1>
_______________________________________________
wx-dev mailing list
wx-dev@...
http://lists.wxwidgets.org/mailman/listinfo/wx-dev

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:2>

#9867: make DeletePendingObjects() public
--------------------------+-------------------------------------------------
  Reporter:  sqlaware     |       Owner:                                      
      Type:  enhancement  |      Status:  closed                              
  Priority:  normal       |   Milestone:                                      
 Component:  base         |     Version:  2.8-svn                            
Resolution:  wontfix      |    Keywords:  DeletePendingObjects wxAppBase wxApp
 Blockedby:               |       Patch:  1                                  
  Blocking:               |  
--------------------------+-------------------------------------------------
Changes (by vadz):

  * status:  new => closed
  * resolution:  => wontfix


Comment:

 I don't like making this method which is, after all, just an
 implementation detail, public. You can already test if the window is
 marked for deletion by checking for its membership in wxPendingDelete
 (which is another unfortunately exposed implementation detail, of course,
 but this is already done) and you can call ProcessIdle() to clean up the
 pending objects as a side effect. So I don't think we need to make this
 one public too.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:2>
_______________________________________________
wx-dev mailing list
wx-dev@...
http://lists.wxwidgets.org/mailman/listinfo/wx-dev

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:3>

#9867: make DeletePendingObjects() public
--------------------------+-------------------------------------------------
  Reporter:  sqlaware     |       Owner:                                      
      Type:  enhancement  |      Status:  closed                              
  Priority:  normal       |   Milestone:                                      
 Component:  base         |     Version:  2.8-svn                            
Resolution:  wontfix      |    Keywords:  DeletePendingObjects wxAppBase wxApp
 Blockedby:               |       Patch:  1                                  
  Blocking:               |  
--------------------------+-------------------------------------------------

Comment(by sqlaware):

 I accept the decision but I totally disagree.

 When library developers try to hide so-called implementation details, they
 assume that they know better than the developers who use their library.
 They do not know better: they do not know the specific application or how
 it is coded, they do not know their own bugs and they assume that they can
 provide a perfect abstraction (but it invariably leaks with every non-
 trivial application).  By preventing access to the underlying mechanism,
 they make the library awkward; developers must invoke functionality that
 they do not want, collaborating with the library to plug abstraction
 leaks, hoping that the desired side effect will be preserved across
 versions and that the unwanted side effects are harmless across versions.

 Providing explicit and direct access to functionality is the whole point
 of libraries.  Providing abstractions without access to the underlying
 mechanisms to "save the application from its own bad judgement" is not the
 point.

 In this case, I desire to expose DeletePendingObjects() because wxWidgets
 provided the veneer that "an object in the delete pending list is
 essentially the same as deleted" failed.  The abstraction was broken
 because deleted objects were still around.  To ask me to write awkward
 code in order to preserve an abstraction that is broken is wrong.  Let me
 access the underlying mechanism.  Either provide me perfect abstractions
 or provide access to the underlying mechanism.  Since you cannot provide
 perfect abstractions, your only course is to provide access to the
 underlying mechanism.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:3>
_______________________________________________
wx-dev mailing list
wx-dev@...
http://lists.wxwidgets.org/mailman/listinfo/wx-dev

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:4>

#9867: make DeletePendingObjects() public
--------------------------+-------------------------------------------------
  Reporter:  sqlaware     |       Owner:                                      
      Type:  enhancement  |      Status:  closed                              
  Priority:  normal       |   Milestone:                                      
 Component:  base         |     Version:  2.8-svn                            
Resolution:  wontfix      |    Keywords:  DeletePendingObjects wxAppBase wxApp
 Blockedby:               |       Patch:  1                                  
  Blocking:               |  
--------------------------+-------------------------------------------------

Comment(by vadz):

 Replying to [comment:3 sqlaware]:
 >Either provide me perfect abstractions or provide access to the
 underlying mechanism.  Since you cannot provide perfect abstractions, your
 only course is to provide access to the underlying mechanism.

 We probably cannot but we can at least try. Which is why I'd be interested
 in what problem exactly do you have and try to improve the abstraction so
 that the problem gets solved without directly calling some implementation-
 level functions.

 Libraries isolate the application developers from their implementation
 details not because of some misguided elitist considerations but because
 it is much better if everybody uses public APIs which (hopefully) have
 well-defined meaning. I really have no idea how is DeletePendingObjects()
 going to behave in future wxWidgets versions nor if it's going to be still
 needed at all (I'd really like to get rid of this delayed destruction
 completely some day). Exposing might fix your problem now but it will
 create more problems for both of us in the future.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:4>
_______________________________________________
wx-dev mailing list
wx-dev@...
http://lists.wxwidgets.org/mailman/listinfo/wx-dev

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:5>

#9867: make DeletePendingObjects() public
--------------------------+-------------------------------------------------
  Reporter:  sqlaware     |       Owner:                                      
      Type:  enhancement  |      Status:  closed                              
  Priority:  normal       |   Milestone:                                      
 Component:  base         |     Version:  2.8-svn                            
Resolution:  wontfix      |    Keywords:  DeletePendingObjects wxAppBase wxApp
 Blockedby:               |       Patch:  1                                  
  Blocking:               |  
--------------------------+-------------------------------------------------

Comment(by sqlaware):

 The specific code is:

 {{{
 bool SendEventToChildren( wxWindow * pWnd, wxEvent& event )
 {
     // check arguments
     wxASSERT(pWnd);

     // clean up any windows that are waiting to be destroyed
     //  we don't want to deal with zombies that are waiting
     //  to be destroyed but haven't be destroyed yet
     wxTheApp->DeletePendingObjects();

     // save a list of the child controls
     //  the child window list might change during processing
     //  as handlers can create/destroy child windows
     wxWindowList listCtrls;
     wxWindowList& listCtrlsRef = pWnd->GetChildren();

     for (int iCtrlRef=0; iCtrlRef < (int)listCtrlsRef.GetCount();
 ++iCtrlRef)
     {
         // get the child window pointer
         wxWindow * pWndCtrl = listCtrlsRef[iCtrlRef];
         wxASSERT(pWndCtrl);

         // save the child window pointer
         listCtrls.Append(pWndCtrl);
     }

     // send the event to all children that existed
     //  at the beginning of this method call
     bool bEvtHandled = false;

     for (int iCtrl=0; iCtrl < (int)listCtrls.GetCount(); ++iCtrl)
     {
         // get the child window
         wxWindow * pWndCtrl = listCtrls[iCtrl];
         wxASSERT(pWndCtrl);
         wxASSERT(!pWndCtrl->IsBeingDeleted());

         // get the child window event handler
         wxEvtHandler * pEvtHandler = pWndCtrl->GetEventHandler();
         wxASSERT(pEvtHandler);

         // process the event
         if (pEvtHandler->ProcessEvent(event))
         {
             bEvtHandled = true;
         }
     }

     return bEvtHandled;
 }
 }}}

 The specific reason (I think) is that child windows return
 IsBeingDeleted() as false, even though their parent windows return
 IsBeingDeleted() as true.  So, the only way that a child knows if he is
 being deleted is to traverse up his chain of parents and see if any of his
 parent or grandparents are being deleted.  The simplest solution was to
 actually delete things and not worry about skipping over windows that were
 going to be deleted at the next idle event.

 Realistically, we're not going to replace our call to
 DeletePendingObjects() with a call to ProcessIdle().
 DeletePendingObjects() is straight forward; it's not worth it to retest
 this code just so we can rely on a side effect of ProcessIdle() which, as
 you point out, may secretly break in some future version.  If
 DeletePendingObjects() is removed, at least we will have a compiler error
 instead of a run-time error.

 Of course, I understand the rationale behind public APIs but I disagree.
 The so-called "Law of Leaky Abstractions" indicates that, historically,
 the abstractions nearly always fail at a some point and that the
 underlying mechanisms have to be understood and accessed.  Library
 developers can either make that access easy (to help users easily repair
 the situation and move forward) or make that access hard (to fruitlessly
 attempt to make users stick to the public API which we just violate anyway
 by either writing convoluted code to manipulate the mechanism or hacking
 the source).  I have no objection to a public API but sensible access to
 the underlying mechanisms is just as important.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:5>
_______________________________________________
wx-dev mailing list
wx-dev@...
http://lists.wxwidgets.org/mailman/listinfo/wx-dev

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:6>

#9867: IsBeingDeleted() should return true if parent window is being deleted
--------------------------+-------------------------------------------------
  Reporter:  sqlaware     |       Owner:                                      
      Type:  enhancement  |      Status:  reopened                            
  Priority:  normal       |   Milestone:                                      
 Component:  base         |     Version:  2.8-svn                            
Resolution:               |    Keywords:  DeletePendingObjects wxAppBase wxApp
 Blockedby:               |       Patch:  1                                  
  Blocking:               |  
--------------------------+-------------------------------------------------
Changes (by vadz):

  * status:  closed => reopened
  * resolution:  wontfix =>


Comment:

 > The specific reason (I think) is that child windows return
 IsBeingDeleted() as false, even though their parent windows return
 IsBeingDeleted() as true.

 Well, to me this definitely seems to be the real problem: a child of a
 window being deleted definitely will be deleted too so IMO it does make
 sense to check the parent (recursively). I don't see how can we justify
 IsBeingDeleted() returning false in this case. And if this would also fix
 your problem, all the better. Could you please test this trivial patch:
 {{{
 --- include/wx/window.h (revision 55479)
 +++ include/wx/window.h (working copy)
 @@ -199,7 +199,10 @@
      bool DestroyChildren();

          // is the window being deleted?
 -    bool IsBeingDeleted() const { return m_isBeingDeleted; }
 +    bool IsBeingDeleted() const
 +    {
 +        return m_isBeingDeleted || (m_parent &&
 m_parent->IsBeingDeleted());
 +    }

      // window attributes
      // -----------------
 }}}
 and tell us if you have any problems with it?

 TIA!

 P.S. Does anybody else see a problem with this change?


--
Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:6>
_______________________________________________
wx-dev mailing list
wx-dev@...
http://lists.wxwidgets.org/mailman/listinfo/wx-dev

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:7>

#9867: IsBeingDeleted() should return true if parent window is being deleted
--------------------------+-------------------------------------------------
  Reporter:  sqlaware     |       Owner:                                      
      Type:  enhancement  |      Status:  reopened                            
  Priority:  normal       |   Milestone:                                      
 Component:  base         |     Version:  2.8-svn                            
Resolution:               |    Keywords:  DeletePendingObjects wxAppBase wxApp
 Blockedby:               |       Patch:  1                                  
  Blocking:               |  
--------------------------+-------------------------------------------------

Comment(by roebling):

 If you test the parent (which makes sense) then you probably should test
 all ancestors up to the wxTLW.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:7>
_______________________________________________
wx-dev mailing list
wx-dev@...
http://lists.wxwidgets.org/mailman/listinfo/wx-dev

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:8>

#9867: IsBeingDeleted() should return true if parent window is being deleted
--------------------------+-------------------------------------------------
  Reporter:  sqlaware     |       Owner:                                      
      Type:  enhancement  |      Status:  reopened                            
  Priority:  normal       |   Milestone:                                      
 Component:  base         |     Version:  2.8-svn                            
Resolution:               |    Keywords:  DeletePendingObjects wxAppBase wxApp
 Blockedby:               |       Patch:  1                                  
  Blocking:               |  
--------------------------+-------------------------------------------------

Comment(by csomor):

 which this snippet is doing, as he is calling IsBeingDeleted recursively
 and not just testing the flag of the parent, for Mac this should be ok as
 well, I don't expect problems

 Best,

 stefan


--
Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:8>
_______________________________________________
wx-dev mailing list
wx-dev@...
http://lists.wxwidgets.org/mailman/listinfo/wx-dev

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:9>

#9867: IsBeingDeleted() should return true if parent window is being deleted
--------------------------+-------------------------------------------------
  Reporter:  sqlaware     |       Owner:                                      
      Type:  enhancement  |      Status:  confirmed                          
  Priority:  normal       |   Milestone:                                      
 Component:  base         |     Version:  2.8-svn                            
Resolution:               |    Keywords:  DeletePendingObjects wxAppBase wxApp
 Blockedby:               |       Patch:  1                                  
  Blocking:               |  
--------------------------+-------------------------------------------------
Changes (by vadz):

  * status:  reopened => confirmed


Comment:

 Yes, thanks Stefan, but Robert still has a point: we probably shouldn't go
 beyond the TLW so the function would become
 {{{
     bool IsBeingDeleted() const
     {
         return m_isBeingDeleted ||
                 (!IsTopLevel() && m_parent && m_parent->IsBeingDeleted());
     }
 }}}

 Or should we? I'm not sure what should the exact semantics be here...


--
Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:9>
_______________________________________________
wx-dev mailing list
wx-dev@...
http://lists.wxwidgets.org/mailman/listinfo/wx-dev

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:10>

#9867: IsBeingDeleted() should return true if parent window is being deleted
--------------------------+-------------------------------------------------
  Reporter:  sqlaware     |       Owner:                                      
      Type:  enhancement  |      Status:  confirmed                          
  Priority:  normal       |   Milestone:                                      
 Component:  base         |     Version:  2.8-svn                            
Resolution:               |    Keywords:  DeletePendingObjects wxAppBase wxApp
 Blockedby:               |       Patch:  1                                  
  Blocking:               |  
--------------------------+-------------------------------------------------

Comment(by vadz):

 Oops... even trivial patches need testing, this version doesn't compile,
 here is one which does:
 {{{
 Index: include/wx/window.h
 ===================================================================
 --- include/wx/window.h (revision 55493)
 +++ include/wx/window.h (working copy)
 @@ -199,7 +199,7 @@
      bool DestroyChildren();

          // is the window being deleted?
 -    bool IsBeingDeleted() const { return m_isBeingDeleted; }
 +    bool IsBeingDeleted() const;

      // window attributes
      // -----------------
 Index: src/common/wincmn.cpp
 ===================================================================
 --- src/common/wincmn.cpp       (revision 55493)
 +++ src/common/wincmn.cpp       (working copy)
 @@ -379,6 +379,12 @@
  #endif
  }

 +bool wxWindowBase::IsBeingDeleted() const
 +{
 +    return m_isBeingDeleted ||
 +            (!IsTopLevel() && m_parent && m_parent->IsBeingDeleted());
 +}
 +
  void wxWindowBase::SendDestroyEvent()
  {
      wxWindowDestroyEvent event;
 }}}


--
Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:10>
_______________________________________________
wx-dev mailing list
wx-dev@...
http://lists.wxwidgets.org/mailman/listinfo/wx-dev

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:11>

#9867: IsBeingDeleted() should return true if parent window is being deleted
--------------------------+-------------------------------------------------
  Reporter:  sqlaware     |       Owner:                                      
      Type:  enhancement  |      Status:  confirmed                          
  Priority:  normal       |   Milestone:                                      
 Component:  base         |     Version:  2.8-svn                            
Resolution:               |    Keywords:  DeletePendingObjects wxAppBase wxApp
 Blockedby:               |       Patch:  1                                  
  Blocking:               |  
--------------------------+-------------------------------------------------

Comment(by VZ):

 (In [55570]) return true from IsBeingDeleted() if any of the parent
 windows is marked for destruction too (see #9867)


--
Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:11>
_______________________________________________
wx-dev mailing list
wx-dev@...
http://lists.wxwidgets.org/mailman/listinfo/wx-dev

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:12>

#9867: IsBeingDeleted() should return true if parent window is being deleted
--------------------------+-------------------------------------------------
  Reporter:  sqlaware     |       Owner:                                      
      Type:  enhancement  |      Status:  closed                              
  Priority:  normal       |   Milestone:                                      
 Component:  base         |     Version:  2.8-svn                            
Resolution:  fixed        |    Keywords:  DeletePendingObjects wxAppBase wxApp
 Blockedby:               |       Patch:  1                                  
  Blocking:               |  
--------------------------+-------------------------------------------------
Changes (by vadz):

  * status:  confirmed => closed
  * resolution:  => fixed


Comment:

 Patch committed but testing would still be most welcome, TIA!


--
Ticket URL: <http://trac.wxwidgets.org/ticket/9867#comment:12>
_______________________________________________
wx-dev mailing list
wx-dev@...
http://lists.wxwidgets.org/mailman/listinfo/wx-dev