Add new test for role actions

* subcompositor.c (ViewSetSubcompositor):
* subsurface.c (Teardown, XLSubsurfaceParentDestroyed):
* surface.c (HandleSurfaceDestroy): Restore previous code with
more comments.

* tests/subsurface_test.c (test_single_step): Test that surface
actions work with unconfirmed subsurfaces.

subcompositor.c

@@ -1286,8 +1286,8 @@ ViewSetSubcompositor (View *view, Subcompositor *subcompositor)
 
   list = view->link;
 
-  /* Attach the subcompositor recursively for all of view's
+  /* Attach the subcompositor recursively for all of view's inferiors.
-     inferiors.  */
+     view should not be attached to a subcompositor.  */
 
   do
     {
@@ -1296,7 +1296,7 @@ ViewSetSubcompositor (View *view, Subcompositor *subcompositor)
 
       list = list->next;
     }
-  while (list != view->inferior);
+  while (list != view->link);
 }
 
 

subsurface.c

@@ -912,18 +912,15 @@ Teardown (Surface *surface, Role *role)
   NoteSubsurfaceTeardown (subsurface);
 
   role->surface = NULL;
+  subcompositor = NULL;
 
   if (subsurface->parent)
     {
-      subcompositor = ViewGetSubcompositor (surface->view);
+      if (!subsurface->pending)
-
-      /* Assert that the subcompositor is NULL if the subsurface is
-	 pending.  */
-      XLAssert (!subsurface->pending || !subcompositor);
-
-      if (subcompositor)
 	{
-	  /* Detach the views if the subcompositor is set.  */
+	  subcompositor = ViewGetSubcompositor (surface->view);
+
+	  /* Detach the views if the subsurface is not pending.  */
 
 	  ViewUnparent (surface->view);
 	  ViewSetSubcompositor (surface->view, NULL);
@@ -1144,22 +1141,19 @@ XLSubsurfaceParentDestroyed (Role *role)
   Subsurface *subsurface;
 
   subsurface = SubsurfaceFromRole (role);
-  subsurface->parent = NULL;
 
   /* The callback is freed with the parent.  */
   subsurface->commit_callback = NULL;
 
   if (subsurface->role.surface)
     {
-      /* Set the subcompositor to NULL, as it may no longer be
+      /* Unparent the view.  The parent is responsible for clearing
-	 present.  */
+	 the subcompositor.  */
-      ViewSetSubcompositor (subsurface->role.surface->view,
-			    NULL);
-      ViewSetSubcompositor (subsurface->role.surface->under,
-			    NULL);
       ViewUnparent (subsurface->role.surface->view);
       ViewUnparent (subsurface->role.surface->under);
     }
+
+  subsurface->parent = NULL;
 }
 
 void

surface.c

@@ -1390,15 +1390,16 @@ HandleSurfaceDestroy (struct wl_resource *resource)
 
   surface = wl_resource_get_user_data (resource);
 
-  /* Free all subsurfaces.  This must come before the subcompositor is
+  if (surface->role)
-     destroyed.  */
+    XLSurfaceReleaseRole (surface, surface->role);
+
+  /* Detach all subsurfaces from the parent.  This *must* be done
+     after the role is torn down, because that is where the toplevel
+     subcompositor is detached from the roles.  */
   XLListFree (surface->subsurfaces,
 	      NotifySubsurfaceDestroyed);
   surface->subsurfaces = NULL;
 
-  if (surface->role)
-    XLSurfaceReleaseRole (surface, surface->role);
-
   /* Keep surface->resource around until the role is released; some
      code (such as dnd.c) assumes that surface->resource will always
      be available in unmap callbacks.  */

tests/subsurface_test.c

@@ -917,6 +917,27 @@ test_single_step (enum test_kind kind)
       wait_frame_callback (subsurfaces[0]->surface);
       sleep_or_verify ();
 
+      /* Test that subsurface actions are applied in the right order
+	 for newly unparented (pending) subsurfaces.  Destroy and
+	 recreate subsurfaces[9] and subsurfaces[10].  */
+      delete_subsurface_role (subsurfaces[9]);
+      delete_subsurface_role (subsurfaces[10]);
+      recreate_subsurface (subsurfaces[9], subsurfaces[6]->surface);
+      recreate_subsurface (subsurfaces[10], subsurfaces[6]->surface);
+      wl_subsurface_set_position (subsurfaces[9]->subsurface, 600, 600);
+      wl_subsurface_set_position (subsurfaces[10]->subsurface, 600, 600);
+
+      /* Place subsurfaces[9] above subsurfaces[10].  At this point,
+	 the subsurfaces have not yet been confirmed.  */
+      wl_subsurface_place_above (subsurfaces[9]->subsurface,
+				 subsurfaces[10]->surface);
+
+      /* Commit the parents.  W should now be tinted red.  */
+      wl_surface_commit (subsurfaces[6]->surface);
+      wl_surface_commit (subsurfaces[4]->surface);
+      wait_frame_callback (subsurfaces[0]->surface);
+      sleep_or_verify ();
+
       break;
     }