Merge #1277

1277: BasicSurface: clear all frame posted callbacks in the destructor r=AlanGriffiths a=wmww See discussion in #1273. This prevents observers being sent pointers to dead surfaces. Co-authored-by: William Wold <[email protected]>
canonical · Feb 14, 2020 · bb447dd · bb447dd
2 parents e3872e1 + 34698bb
commit bb447dd
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 6 deletions.
diff --git a/src/server/scene/basic_surface.cpp b/src/server/scene/basic_surface.cpp
@@ -311,6 +311,8 @@ ms::BasicSurface::BasicSurface(
 
 ms::BasicSurface::~BasicSurface() noexcept
 {
+    for(auto& layer : layers)
+        layer.stream->set_frame_posted_callback([](auto){});
     report->surface_deleted(this, surface_name);
 }
 

diff --git a/tests/integration-tests/test_surface_stack_with_compositor.cpp b/tests/integration-tests/test_surface_stack_with_compositor.cpp
@@ -215,7 +215,8 @@ TEST_F(SurfaceStackCompositor, compositor_runs_until_all_surfaces_buffers_are_co
     ON_CALL(*mock_buffer_stream, buffers_ready_for_compositor(_))
         .WillByDefault(Return(5));
     EXPECT_CALL(*mock_buffer_stream, set_frame_posted_callback(_))
-        .WillOnce(SaveArg<0>(&frame_callback));
+        .WillOnce(SaveArg<0>(&frame_callback))
+        .WillRepeatedly(Return());
     stub_surface->set_streams(std::list<ms::StreamInfo>{ { mock_buffer_stream, {0,0}, geom::Size{100, 100} } });
 
     mc::MultiThreadedCompositor mt_compositor(
@@ -242,7 +243,8 @@ TEST_F(SurfaceStackCompositor, bypassed_compositor_runs_until_all_surfaces_buffe
     ON_CALL(*mock_buffer_stream, lock_compositor_buffer(_))
         .WillByDefault(Return(mt::fake_shared(*stub_buffer)));
     EXPECT_CALL(*mock_buffer_stream, set_frame_posted_callback(_))
-        .WillOnce(SaveArg<0>(&frame_callback));
+        .WillOnce(SaveArg<0>(&frame_callback))
+        .WillRepeatedly(Return());
     stub_surface->set_streams(std::list<ms::StreamInfo>{ { mock_buffer_stream, {0,0}, geom::Size{100, 100} } });
 
     stub_surface->resize(geom::Size{10,10});

diff --git a/tests/unit-tests/scene/test_basic_surface.cpp b/tests/unit-tests/scene/test_basic_surface.cpp
@@ -1319,8 +1319,10 @@ TEST_F(BasicSurfaceTest, registers_frame_callbacks_on_construction)
         { buffer_stream1, {0,0}, {} }
     };
 
-    EXPECT_CALL(*buffer_stream0, set_frame_posted_callback(_));
-    EXPECT_CALL(*buffer_stream1, set_frame_posted_callback(_));
+    EXPECT_CALL(*buffer_stream0, set_frame_posted_callback(_))
+        .Times(AtLeast(1));
+    EXPECT_CALL(*buffer_stream1, set_frame_posted_callback(_))
+        .Times(AtLeast(1));
 
     ms::BasicSurface child{
         nullptr /* session */,
@@ -1344,8 +1346,10 @@ TEST_F(BasicSurfaceTest, registers_frame_callbacks_on_set_streams)
         { buffer_stream1, {0,0}, {} }
     };
 
-    EXPECT_CALL(*buffer_stream0, set_frame_posted_callback(_));
-    EXPECT_CALL(*buffer_stream1, set_frame_posted_callback(_));
+    EXPECT_CALL(*buffer_stream0, set_frame_posted_callback(_))
+        .Times(AtLeast(1));
+    EXPECT_CALL(*buffer_stream1, set_frame_posted_callback(_))
+        .Times(AtLeast(1));
 
     surface.set_streams(streams);
 }