Merge #1273

1273: BasicSurface: allow frame to be posted after surface destroyed r=AlanGriffiths a=wmww

Reading through the code, it looks like setting all buffer stream callbacks to `[](){}` is allowed (that's what we already do in `BasicSurface::set_streams()`, and why shouldn't it be?). If an noop callback is allowed, a conditionally noop callback should also be allowed. I see no reason to throw this error, as it is perfectly reasonable for a surface to be destroyed before its buffer streams.

Fixes #1272

Co-authored-by: William Wold <[email protected]>

src/server/scene/basic_surface.cpp

@@ -287,8 +287,6 @@ ms::BasicSurface::BasicSurface(
         {
             if (auto const o = observers.lock())
                 o->frame_posted(this, 1, size);
-            else
-                BOOST_THROW_EXCEPTION(std::runtime_error("Frame posted to dead surface"));
         };
 
     for (auto& layer : layers)
@@ -925,8 +923,6 @@ void ms::BasicSurface::set_streams(std::list<scene::StreamInfo> const& s)
                 {
                     if (auto const o = observers.lock())
                         o->frame_posted(this, 1, size);
-                    else
-                        BOOST_THROW_EXCEPTION(std::runtime_error("Frame posted to dead surface"));
                 });
         surface_top_left = surface_rect.top_left;
     }

tests/unit-tests/scene/test_basic_surface.cpp

@@ -1582,3 +1582,55 @@ TEST_F(BasicSurfaceTest, notifies_when_first_visible)
     EXPECT_THAT(observer->exposes(), Eq(1));
     EXPECT_THAT(observer->hides(), Eq(0));
 }
+
+TEST_F(BasicSurfaceTest, buffer_can_be_submitted_to_original_stream_after_surface_destroyed)
+{
+    using namespace testing;
+
+    auto local_stream = std::make_shared<NiceMock<mtd::MockBufferStream>>();
+    std::list<ms::StreamInfo> local_stream_list = { { local_stream, {}, {} } };
+    std::function<void(geom::Size const&)> callback = [](auto){};
+
+    EXPECT_CALL(*local_stream, set_frame_posted_callback(_))
+        .Times(AtLeast(1))
+        .WillRepeatedly(SaveArg<0>(&callback));
+
+    auto surface = std::make_unique<ms::BasicSurface>(
+        nullptr, // session
+        name,
+        rect,
+        mir_pointer_unconfined,
+        local_stream_list,
+        std::shared_ptr<mg::CursorImage>(),
+        report);
+
+    surface.reset();
+    callback({10, 10});
+}
+
+TEST_F(BasicSurfaceTest, buffer_can_be_submitted_to_set_stream_after_surface_destroyed)
+{
+    using namespace testing;
+
+    auto local_stream = std::make_shared<NiceMock<mtd::MockBufferStream>>();
+    std::list<ms::StreamInfo> local_stream_list = { { local_stream, {}, {} } };
+    std::function<void(geom::Size const&)> callback = [](auto){};
+
+    EXPECT_CALL(*local_stream, set_frame_posted_callback(_))
+        .Times(AtLeast(1))
+        .WillRepeatedly(SaveArg<0>(&callback));
+
+    auto surface = std::make_unique<ms::BasicSurface>(
+        nullptr, // session
+        name,
+        rect,
+        mir_pointer_unconfined,
+        streams, // use the default list from the class initially
+        std::shared_ptr<mg::CursorImage>(),
+        report);
+
+    surface->set_streams(local_stream_list);
+
+    surface.reset();
+    callback({10, 10});
+}