Added the helper class Fl_Watch to simplify safe handling of widget deletion

in callbacks. This is used e.g. in Fl_Widget::do_callback() to prevent 
accessing widgets after deletion in the callback.

Documentation adjusted, Fl_Menu_Button.cxx changed to use Fl_Watch instead
of Fl::watch_widget_pointer.

Fl::watch_widget_pointer() and Fl::release_widget_pointer() have been
modified to use an array without "holes" (NULL pointers) for storing the
widget pointers for faster access: Fl::release_widget_pointer() now shifts
pointers to close gaps of freed pointers.


git-svn-id: file:///fltk/svn/fltk/branches/branch-1.3@6651 ea41ed52-d2ee-0310-a9c1-e6b18d33e121

6 files changed, 209 insertions, 68 deletions

diff --git a/CHANGES b/CHANGES
index f42e3e9be..89a5863b1 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,9 +1,12 @@
 CHANGES IN FLTK 1.3.0
 
-	- Fl_Help_View handles HTML2 font color specification (STR#890) 
-	- Copyright dates are now updated to 2009 (STR#2036)
+	- New helper class Fl_Watch to simplify safe handling of widget
+	  deletion in callbacks. This is used in Fl_Widget::do_callback()
+	  to prevent accessing widgets after deletion in the callback.
+	- Fl_Help_View handles HTML2 font color specification (STR #890) 
+	- Copyright dates are now updated to 2009 (STR #2036)
 	- Copy/Paste operations should now work as expected, 
-          with utf8,16 support and fltk1.1 compatibility. (STR#2104,2121).
+          with utf8,16 support and fltk1.1 compatibility. (STR #2104,2121).
 	- Widgets now remove themselves from their parent group (if any),
 	  when destroyed (STR #1894)
 	- Added flexible gap size for text buffer (STR #2046)
@@ -72,7 +75,7 @@ CHANGES IN FLTK 1.1.9
 
 CHANGES IN FLTK 1.1.8
 
-	- Documentation fixes (STR #1454, STR #1455, STR #1456,
+	- Documentation fixes  (STR #1454, STR #1455, STR #1456,
 	  STR #1457, STR #1458, STR #1460, STR #1481, STR #1578,
 	  STR #1639, STR #1645, STR #1644, STR #1792, STR #1793,
 	  STR #1742, STR #1777, STR #1794, STR #1827, STR #1843,
diff --git a/FL/Fl.H b/FL/Fl.H
index 1773a3744..2e5815687 100644
--- a/FL/Fl.H
+++ b/FL/Fl.H
@@ -849,17 +849,31 @@ public:
 
   /** \defgroup fl_del_widget Safe widget deletion support functions
 
-	These functions support deletion of widgets inside callbacks.
+    These functions support deletion of widgets inside callbacks.
 
-	There are two groups of related methods:
+    Fl::delete_widget() should be called when deleting widgets
+    or complete widget trees (Fl_Group, Fl_Window, ...) inside
+    callbacks.
 
-	  -# scheduled widget deletion
-	    - Fl::delete_widget() schedules widgets for deletion
-	    - Fl::do_widget_deletion() deletes all scheduled widgets
-	  -# widget watch list ("smart pointers")
-	    - Fl::watch_widget_pointer() adds a widget pointer to the watch list
-	    - Fl::release_widget_pointer() removes a widget pointer from the watch list
-	    - Fl::clear_widget_pointer() clears a widget pointer \e in the watch list
+    The other functions are intended for internal use. The preferred
+    way to use them is by using the helper class Fl_Watch.
+
+    The following is to show how it works ...
+
+    There are three groups of related methods:
+
+      -# scheduled widget deletion
+	- Fl::delete_widget() schedules widgets for deletion
+	- Fl::do_widget_deletion() deletes all scheduled widgets
+      -# widget watch list ("smart pointers")
+	- Fl::watch_widget_pointer() adds a widget pointer to the watch list
+	- Fl::release_widget_pointer() removes a widget pointer from the watch list
+	- Fl::clear_widget_pointer() clears a widget pointer \e in the watch list
+      -# the class Fl_Watch:
+	- the constructor calls Fl::watch_widget_pointer()
+	- the destructor calls Fl::release_widget_pointer()
+	- the access methods can be used to test, if a widget has been deleted
+	  \see Fl_Watch.
 
    @{ */
   // Widget deletion:
@@ -919,6 +933,86 @@ public:
 
 };
 
+/**
+  This class should be used to control safe widget deletion.
+
+  You can use an Fl_Watch object to watch another widget, if you
+  need to know, if this widget has been deleted during a callback.
+
+  This simplifies the use of the "safe widget deletion" methods
+  Fl::watch_widget_pointer() and Fl::release_widget_pointer() and
+  makes their use more reliable, because the destructor autmatically
+  releases the widget pointer from the widget watch list.
+
+  It is intended to be used as an automatic (local/stack) variable,
+  such that the automatic destructor is called when the object's
+  scope is left. This ensures that no stale widget pointers are
+  left in the widget watch list (see example below).
+  
+  You can also create Fl_Watch objects with \e \b new, but then it
+  is your responsibility to delete the object (and thus remove the
+  widget pointer from the watch list) when it is not needed any more.
+
+  Example:
+
+  \code
+    int MyClass::handle (int event) {
+
+      if (...) {
+	Fl_Watch wp(this);	// watch myself
+	do_callback();		// call the callback
+
+	if (wp.deleted()) return 1; // exit, if deleted
+
+	// Now we are sure that the widget has not been deleted.
+	// It is safe to access the widget
+
+	clear_changed();	// access the widget
+      }
+    }
+  \endcode
+
+*/
+class FL_EXPORT Fl_Watch {
+
+  Fl_Widget* wp_;
+
+public:
+
+  Fl_Watch(Fl_Widget *wi);
+  ~Fl_Watch();
+
+  /**
+    returns a pointer to the watched widget.
+
+    This pointer is NULL, if the widget has been deleted.
+  */
+  Fl_Widget *widget() {return wp_;}
+
+  /**
+    returns 1, if the watched widget has been deleted.
+
+    This is a convenience method. You can also use something like
+
+      if (watch.widget() == 0) // ...
+
+    where watch is an Fl_Watch object.
+  */
+  int deleted() {return wp_ == 0;}
+
+  /**
+    returns 1, if the watched widget exists (has not been deleted).
+
+    This is a convenience method. You can also use something like
+
+      if (watch.widget() != 0) // ...
+
+    where watch is an Fl_Watch object.
+  */
+  int exists()  {return wp_ != 0;}
+
+};
+
 #endif // !Fl_H
 
 //
diff --git a/FL/Fl_Widget.H b/FL/Fl_Widget.H
index f8d3fa4ed..7fe67ce11 100644
--- a/FL/Fl_Widget.H
+++ b/FL/Fl_Widget.H
@@ -793,29 +793,20 @@ public:
   static void default_callback(Fl_Widget *cb, void *d);
 
   /** Calls the widget callback.
-      Causes a widget to invoke its callback function, optionally
-      with arbitrary arguments.
+      Causes a widget to invoke its callback function with default arguments.
       \see callback()
    */
-  void do_callback() {callback_(this,user_data_); if (callback_ != default_callback) clear_changed();}
+  void do_callback() {do_callback(this,user_data_);}
 
   /** Calls the widget callback.
-      Causes a widget to invoke its callback function, optionally
-      with arbitrary arguments.
+      Causes a widget to invoke its callback function with arbitrary arguments.
       \param[in] o call the callback with \em o as the widget argument
       \param[in] arg call the callback with \em arg as the user data argument
       \see callback()
    */
-  void do_callback(Fl_Widget* o,void* arg=0) {callback_(o,arg); if (callback_ != default_callback) clear_changed();}
+  void do_callback(Fl_Widget* o,long arg) {do_callback(o,(void*)arg);}
 
-  /** Calls the widget callback.
-      Causes a widget to invoke its callback function, optionally
-      with arbitrary arguments.
-      \param[in] o call the callback with \em o as the widget argument
-      \param[in] arg call the callback with \em arg as the user data argument
-      \see callback()
-   */
-  void do_callback(Fl_Widget* o,long arg) {callback_(o,(void*)arg); if (callback_ != default_callback) clear_changed();}
+  void do_callback(Fl_Widget* o,void* arg=0); // impl. in Fl_Widget.cxx
 
   /** Internal use only. */
   int test_shortcut();
diff --git a/src/Fl.cxx b/src/Fl.cxx
index 2b3ae89d1..0e5a1b251 100644
--- a/src/Fl.cxx
+++ b/src/Fl.cxx
@@ -1518,40 +1518,47 @@ static int max_widget_watch = 0;
 
 /**
   Adds a widget pointer to the widget watch list.
+  
+  \note Internal use only, please use class Fl_Watch instead.
 
-   This should be used, if it is possible that a widget might be deleted during
-   a callback or similar function. The widget pointer must be added to the
-   watch list before calling the callback. After the callback the widget
-   pointer can be queried, if it is NULL. \e If it is NULL, then the widget has been
-   deleted during the callback and must not be accessed anymore. If the widget
-   pointer is \e not NULL, then the widget has not been deleted and can be accessed
-   safely.
+  This can be used, if it is possible that a widget might be deleted during
+  a callback or similar function. The widget pointer must be added to the
+  watch list before calling the callback. After the callback the widget
+  pointer can be queried, if it is NULL. \e If it is NULL, then the widget has been
+  deleted during the callback and must not be accessed anymore. If the widget
+  pointer is \e not NULL, then the widget has not been deleted and can be accessed
+  safely.
 
-   After accessing the widget, the widget pointer should be released from the
-   watch list by calling Fl::release_widget_pointer().
+  After accessing the widget, the widget pointer must be released from the
+  watch list by calling Fl::release_widget_pointer().
 
-   Example for a button that is clicked (from its handle() method):
-   \code
-     Fl_Widget *wp = this;		// save 'this' in a pointer variable
-     Fl::watch_widget_pointer(wp);	// add the pointer to the watch list
-     set_changed();			// set the changed flag
-     do_callback();			// call the callback
-     if (!wp) {				// the widget has been deleted
+  Example for a button that is clicked (from its handle() method):
+  \code
+    Fl_Widget *wp = this;		// save 'this' in a pointer variable
+    Fl::watch_widget_pointer(wp);	// add the pointer to the watch list
+    set_changed();			// set the changed flag
+    do_callback();			// call the callback
+    if (!wp) {				// the widget has been deleted
 
-       // DO NOT ACCESS THE DELETED WIDGET !
+      // DO NOT ACCESS THE DELETED WIDGET !
 
-     } else {				// the widget still exists
-       clear_changed();			// reset the changed flag
-     }
+    } else {				// the widget still exists
+      clear_changed();			// reset the changed flag
+    }
 
-     Fl::release_widget_pointer(wp);	// remove the pointer from the watch list
-    \endcode
+    Fl::release_widget_pointer(wp);	// remove the pointer from the watch list
+   \endcode
 
-    This works, because all widgets call Fl::clear_widget_pointer() in their
-    destructors.
-    
-    \see Fl::release_widget_pointer()
-    \see Fl::clear_widget_pointer()
+   This works, because all widgets call Fl::clear_widget_pointer() in their
+   destructors.
+
+   \see Fl::release_widget_pointer()
+   \see Fl::clear_widget_pointer()
+
+   An easier and more convenient method to control widget deletion during
+   callbacks is to use the class Fl_Watch with a local (automatic) variable.
+
+   \see class Fl_Watch
 */
 void Fl::watch_widget_pointer(Fl_Widget *&w) 
 {
@@ -1560,17 +1567,16 @@ void Fl::watch_widget_pointer(Fl_Widget *&w)
   for (i=0; i<num_widget_watch; ++i) {
     if (widget_watch[i]==wp) return;
   }
-  for (i=0; i<num_widget_watch; ++i) {
-    if (widget_watch[i]==0L) {
-      widget_watch[i] = wp;
-      return;
-    }
-  }
   if (num_widget_watch==max_widget_watch) {
     max_widget_watch += 8;
     widget_watch = (Fl_Widget***)realloc(widget_watch, sizeof(Fl_Widget**)*max_widget_watch);
   }
   widget_watch[num_widget_watch++] = wp;
+#ifdef DEBUG
+  printf ("\nwatch_widget_pointer:   (%d/%d) %8p => %8p\n",
+    num_widget_watch,num_widget_watch,wp,*wp);
+  fflush(stdout);
+#endif // DEBUG
 }
 
 /**
@@ -1578,19 +1584,33 @@ void Fl::watch_widget_pointer(Fl_Widget *&w)
 
   This is used to remove a widget pointer that has been added to the watch list
   with Fl::watch_widget_pointer(), when it is not needed anymore.
+  
+  \note Internal use only, please use class Fl_Watch instead.
 
   \see Fl::watch_widget_pointer()
 */
 void Fl::release_widget_pointer(Fl_Widget *&w)
 {
   Fl_Widget **wp = &w;
-  int i;
+  int i,j=0;
   for (i=0; i<num_widget_watch; ++i) {
-    if (widget_watch[i]==wp) {
-      widget_watch[i] = 0L;
-      return;
+    if (widget_watch[i]!=wp) {
+      if (j<i) widget_watch[j] = widget_watch[i]; // fill gap
+      j++;
     }
+#ifdef DEBUG
+    else { // found widget pointer
+      printf ("release_widget_pointer: (%d/%d) %8p => %8p\n",
+	i+1,num_widget_watch,wp,*wp);
+    }
+#endif //DEBUG
   }
+  num_widget_watch = j;
+#ifdef DEBUG
+  printf ("                        num_widget_watch = %d\n\n",num_widget_watch);
+  fflush(stdout);
+#endif // DEBUG
+  return;
 }
 /**
   Clears a widget pointer \e in the watch list.
@@ -1598,6 +1618,8 @@ void Fl::release_widget_pointer(Fl_Widget *&w)
   This is called when a widget is destroyed (by its destructor). You should never
   call this directly.
   
+  \note Internal use only, please use class Fl_Watch instead.
+  
   This method searches the widget watch list for pointers to the widget and clears
   all pointers that point to it. Widget pointers can be added to the widget
   watch list by calling Fl::watch_widget_pointer().
@@ -1615,6 +1637,24 @@ void Fl::clear_widget_pointer(Fl_Widget const *w)
   }
 }
 
+// Helper class Fl_Watch
+
+/**
+  The constructor adds a widget to the watch list.
+*/
+Fl_Watch::Fl_Watch(Fl_Widget *wi) {
+
+  wp_ = wi;
+  Fl::watch_widget_pointer(wp_); // add pointer to watch list
+}
+
+/**
+  The destructor removes a widget from the watch list.
+*/
+Fl_Watch::~Fl_Watch() {
+
+  Fl::release_widget_pointer(wp_); // remove pointer from watch list
+}
 
 //
 // End of "$Id$".
diff --git a/src/Fl_Menu_Button.cxx b/src/Fl_Menu_Button.cxx
index 988a6e727..9389ca49f 100644
--- a/src/Fl_Menu_Button.cxx
+++ b/src/Fl_Menu_Button.cxx
@@ -58,8 +58,7 @@ const Fl_Menu_Item* Fl_Menu_Button::popup() {
   const Fl_Menu_Item* m;
   pressed_menu_button_ = this;
   redraw();
-  Fl_Widget *mb = this;
-  Fl::watch_widget_pointer(mb);
+  Fl_Watch mb(this);
   if (!box() || type()) {
     m = menu()->popup(Fl::event_x(), Fl::event_y(), label(), mvalue(), this);
   } else {
@@ -67,8 +66,7 @@ const Fl_Menu_Item* Fl_Menu_Button::popup() {
   }
   picked(m);
   pressed_menu_button_ = 0;
-  if (mb) mb->redraw();
-  Fl::release_widget_pointer(mb);
+  if (mb.exists()) redraw();
   return m;
 }
 
diff --git a/src/Fl_Widget.cxx b/src/Fl_Widget.cxx
index 550f1eb1c..740dad6fd 100644
--- a/src/Fl_Widget.cxx
+++ b/src/Fl_Widget.cxx
@@ -293,6 +293,21 @@ Fl_Widget::copy_label(const char *a) {
   redraw_label();
 }
 
+/** Calls the widget callback.
+
+  Causes a widget to invoke its callback function with arbitrary arguments.
+  \param[in] o call the callback with \a o as the widget argument
+  \param[in] arg call the callback with \a arg as the user data argument
+  \see callback()
+*/
+void
+Fl_Widget::do_callback(Fl_Widget* o,void* arg) {
+  Fl_Watch wp(o);
+  callback_(o,arg); 
+  if (wp.deleted()) return;
+  if (callback_ != default_callback) 
+    clear_changed();
+}
 
 //
 // End of "$Id$".