The problem here is that changing the Visibility is done as "Batch Process".
Because the Model scene model is doing LazyUpdate, when the scene ends the batchprocess state, the model is entirely re-populated, which looses the expand/collapse properties).

qMRMLSceneModel::updateScene should fire a signal before and after repopulating itself.
qMRMLSortAndFilterProxyModel should listen to this signal and propagate it.
qMRMLTreeView should listen to its model, and on sceneAboutToBeUpdated, it should save a list of all the expanded nodes. And on sceneUpdated it should restore the expansion of the nodes that where previously expanded (if they still exist).

As a side note,
vtkMRMLScene::StartState/EndState(vtkMRMLScene::BatchProcessState); shouldn't be done in qMRMLTreeView::toggleVisibility() but in vtkMRMLModelHierarchyLogic::SetChildrenVisibility() instead.

The views should keep track of the open/close state of the tree and then restore it (if it is still valid) after the data changes. Also they should scroll to the same point.

From Julien:

[...]
To expand/collapse nodes, you need to do it through the QTreeView API via QModelIndexes :
QTreeView::isExpanded(), QTreeView::isCollapsed()
QTreeView::expand(), QTreeView::collapse()
signals:
QTreeView::expanded(), QTreeView::collapsed()

There is currently no mechanism in qMRMLTreeView to save/restore the expand/collapse state of the tree.

[...]Steve summarizes the user behavior of what I explained in more details when I wrote:
qMRMLTreeView should listen to its model, and on sceneAboutToBeUpdated, it should save a list of all the expanded nodes. And on sceneUpdated it should restore the expansion of the nodes that where previously expanded (if they still exist)

The main idea is to convert expanded indexes into a list of nodes (for saving the state), and convert back the list of nodes into indexes to expand (for restoring the state).
So qMRMLTreeViewPrivate should probably have a new member variable: a vtkCollection* of nodes to expand.
[...]

I did some elementary work on this to understand how things are wired. Thanks to Julien for his suggestions. I will add the functionality to save and restore the state for the tree view.

Pushed fix to: https://github.com/sankhesh/Slicer/commit/4f1ed29803aa9463b526ee0da583ea55169b2b1e

Kindly review.

Pushed changes: https://github.com/sankhesh/Slicer/commit/2557-Hierarchy-visibility-preservation

Kindly review them and if it is alright, I will merge the two commits before pushing to master.

Fixed in r21523 (http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=21523)

Testing svn 21524.
Right clicking and adding a model hierarchy closes the hierarchy I'm adding it to in the Models module with that brain atlas scene (didn't occur in my simple test with fewer models and creating a whole new hierachy though).
Dropping models onto a new hierarchy doesn't cause it to expand, has to be done manually (the hierachy node GetExpanded() returns 1).
The annotations module also uses the tree view and I'm finding that if I close a ruler hierarchy and then add another fiducial to an expanded hierarchy, the ruler hierarchy opens up automatically. I'm checking for the expand all and expand to level commands that I'd spotted previously and not finding them so far.

Found a culprit:
qMRMLTreeView.cxx: has 3 calls to expandToDepth(2), in:
setSceneModel
setSortFilterProxyModel
setMRMLScene
I'll check the annotations code ot make sure I'm not resetting the mrml sceen too often (in bug 1735, Julien suggests returning if the mrml scene is the same in setMRMLScene).

The Annotations module is calling refreshTree when nodes are added, which is calling setMRMLScene which is expanding the tree to depth 2. If I take out that call (or put the check for changed mrml scene pointer there), the tree doesn't collapse, but I'm back to the problem of when I add a new hierarchy node that it's not expanded by default in the tree.
qMRMLSceneHierarchyModel::updateItemDataFromNode
deals with the state of the checkbox so that it's in synch with the Expanded state of the hierarchy node, but I'm back to the bug of adding a new hierarchy has it collapsed in the tree view by default.

Added a new bug to address this issue: 2840

The three atlas test is failing on scene close after restoring scene views in the first atlas:
http://slicer.cdash.org/testDetails.php?test=3344280&build=59605
Attaching the debugger, the problem seems to be that the Clear call on the mrml scene gets to the scene model catching the scene closed event and calling qMRMLSceneModel::onMRMLSceneClosed, which calls this->updateScene, where the fist thing that's done is
emit sceneAboutToBeUpdated();
which triggers qMRMLTreeView::saveTreeExpandState but the node passed to the qMRMLSortFilterProxyModel::filterAcceptsNode method has a null id, causing a seg fault.

I couldn't quite see where to catch the fact that the scene has been closed and the tree expansion state doesn't need to be saved, could you take another look at it?
Ref: http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=21523

@Nicole, could you please outline the steps you used to reproduce the bug by attaching a debugger to the test?

gdb bin/SlicerApp-real
run "--no-splash" "--testing" "--ignore-slicerrc" "--python-code" "import slicer.testing; slicer.testing.runUnitTest(['/projects/birn/nicole/Slicer42/Slicer4-SuperBuild-Debug/Slicer-build/Applications/SlicerApp/Testing/Python', '/projects/birn/nicole/Slicer42/Slicer/Applications/SlicerApp/Testing/Python'], 'AtlasTests')"

Attaching the back trace.

Pushed topic to https://github.com/sankhesh/Slicer/commits/2557-Failing-Atlas-Tests
Kindly review.

Made the suggested changes. Fixed.

The fix doesn't deal with the underlying event issue I noted in note 7757 [1], and I ran across a similar crash again when restoring a scene view. In that case the node was not null, but it's id was null and searching for it in the hidden nodes list caused a crash.

[1] http://na-mic.org/Mantis/view.php?id=2557#c7757

@Nicole: Thanks for commenting. Does the crash occur following the "Steps To Reproduce" reported above ? If not, could you provide Sankhesh with udpated data and steps allowing to reproduce the crash ?

Narrowed down the crash when it didn't occur through a few steps of the automated test.
Download this .mrb file (accessible via the Brain Atlas test from the Testing, Test Cases, Atlas Tests module):
http://slicer.kitware.com/midas3/download?items=10937
Load the .mrb file
Enter the Models module first, then enter the Scene Views module.
Select the "Visual System" scene and click Restore.

There's a non zero possibility something's wrong with the visual system scene view.

Pushed changes to: https://github.com/sankhesh/Slicer/commits/2557-Failing-Atlas-Tests

The last commit ensures the tree expansion state is saved before batch processing starts and is restored after batch processing ends.
Kindly review and test.

That commit fixed the crash in the Brain Atlas test, and reviewing the code, it looks like it's dealing with the scene restore case more gracfully. Thanks!

Fixed: http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=21881

And the full atlas test is passing on some machines now (as opposed to none):
http://slicer.cdash.org/testSummary.php?project=1&name=py_AtlasTests&date=2013-04-10
The failed ones are mainly due to leaks: http://slicer.cdash.org/testDetails.php?test=3712405&build=84400

Some others are simply reporting an "exit abnormally error":
http://slicer.cdash.org/testDetails.php?test=3547220&build=84401

Jc: Reporting of error on windows seems to have some issue. Other information should be displated in addition to the "exit abnormally error"

factory-south-win7.kitware has a few tests that fail that way:
http://slicer.cdash.org/viewTest.php?onlyfailed&buildid=84455 [^]