[PATCH] 689338 - Sometimes current frame is wrongly set

[Dodji Seketeli <dodji seketeli org>]

Hello,

I have recently noticed on Fedora 17 that under some circumstances (I
believe after I restart the inferior) stepping in a program with the
context widget visible triggers some weird behaviour.  Basically, at
each step, Nemiver would set the current instruction pointer inside
the 'main' function, no matter where it should be in reality.  That
is, the current instruction pointer is set to the frame that is at the
bottom of the call stack, rather than being the one at the top.

It looks like this is because CallStack::Priv::set_current_frame fails
to update the CallStack::Priv::cur_frame_index member variable,
leaving it set to a dangling random value.  Later, in
CallStack::Priv::on_command_done_signal, we use that random value to
select what we think is the current frame.

The first hunk of the patch below fixes this by correctly setting the
CallStack::Priv::cur_frame_index member variable.  Subsequent hunks
are cleanups I am making to avoid unnecessarily messing with frame
selection code during the clearing of the list store that holds the
frame.

Tested and applied to the master branch.

	* src/persp/dbgperspective/nmv-call-stack.cc
	(CallStack::Priv::set_current_frame): Properly update the
	CallStack::Priv::cur_frame_index member variable.  That way, when
	the CallStack::frame_selected_signal is emitted (in
	on_command_done_signal), the frame index that is advertised is
	going to be correct.  Note that the advertised index is not yet
	used, but still.  This change makes the code correct.
	(CallStack::Priv::clear_frame_list): Block the execution of the
	on_selection_changed callback slot upon clearing of the list
	store.  That way, we ensure that no weird row gets randomly
	selected.
	(CallStack::Priv::on_selection_changed_signal):  Don't try to act
	on an empty list store.
---
 src/persp/dbgperspective/nmv-call-stack.cc | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/src/persp/dbgperspective/nmv-call-stack.cc b/src/persp/dbgperspective/nmv-call-stack.cc
index b2b2ca4..d765fd4 100644
--- a/src/persp/dbgperspective/nmv-call-stack.cc
+++ b/src/persp/dbgperspective/nmv-call-stack.cc
@@ -229,14 +229,15 @@ struct CallStack::Priv {
     set_current_frame (unsigned a_index)
     {
         THROW_IF_FAIL (a_index < frames.size ());
-        cur_frame = frames[a_index];
+        cur_frame_index = a_index;
+        cur_frame = frames[cur_frame_index];
         THROW_IF_FAIL (cur_frame.level () >= 0);
         in_set_cur_frame_trans = true;
 
         LOG_DD ("frame selected: '"<<  (int) cur_frame_index << "'");
         LOG_DD ("frame level: '" << (int) cur_frame.level () << "'");
 
-        debugger->select_frame (a_index);
+        debugger->select_frame (cur_frame_index);
     }
 
     /// If the selected frame is the "expand to see more frames" raw,
@@ -419,7 +420,11 @@ struct CallStack::Priv {
     {
         LOG_FUNCTION_SCOPE_NORMAL_DD;
 
-        NEMIVER_TRY
+        NEMIVER_TRY;
+
+        // Don't try to select a row on an empty call stack.
+        if (store->children ().empty ())
+            return;
 
         Gtk::TreeView *tree_view = dynamic_cast<Gtk::TreeView*> (widget.get ());
         THROW_IF_FAIL (tree_view);
@@ -846,7 +851,12 @@ struct CallStack::Priv {
         }
 
         THROW_IF_FAIL (store);
+        // We really don't need to try to update the selected frame
+        // when we are just clearing the list store, so block the signal
+        // transmission to the callback slot while we clear the store.
+        on_selection_changed_connection.block ();
         store->clear ();
+        on_selection_changed_connection.unblock ();
         frames.clear ();
         params.clear ();
         level_frame_map.clear ();
-- 
		Dodji