Mir

Merge lp:~albaguirre/mir/buffer-stream-error-message-api into lp:mir

  1. buffer-stream-error-message-api
  2. Merge into development-branch
Proposed by Alberto Aguirre
Status: Merged
Approved by: Alberto Aguirre
Approved revision: no longer in the source branch.
Merged at revision: 3154
Proposed branch: lp:~albaguirre/mir/buffer-stream-error-message-api
Merge into: lp:mir
Prerequisite: lp:~albaguirre/mir/fix-1519998
Diff against target: 325 lines (+128/-37)
10 files modified
include/client/mir_toolkit/mir_buffer_stream.h (+11/-0)
src/client/buffer_stream.cpp (+66/-31)
src/client/buffer_stream.h (+3/-0)
src/client/client_buffer_stream.h (+2/-0)
src/client/mir_buffer_stream_api.cpp (+6/-0)
src/client/symbols.map (+1/-0)
tests/acceptance-tests/test_client_library.cpp (+16/-0)
tests/acceptance-tests/throwback/test_client_library_errors.cpp (+18/-0)
tests/include/mir/test/doubles/mock_client_buffer_stream.h (+1/-0)
tests/unit-tests/client/test_client_buffer_stream.cpp (+4/-6)
To merge this branch: bzr merge lp:~albaguirre/mir/buffer-stream-error-message-api
Reviewer Review Type Date Requested Status
Alan Griffiths Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+278762@code.launchpad.net

Commit message

Add mir_buffer_stream_get_error_message

Allow client to retrieve error messages when buffer stream creation fails

Description of the change

Add mir_buffer_stream_get_error_message

Allow client to retrieve error messages when buffer stream creation fails

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:3130
http://jenkins.qa.ubuntu.com/job/mir-ci/5688/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5060
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3966
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5007/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/18/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/18/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5007/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5688/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:3131
http://jenkins.qa.ubuntu.com/job/mir-ci/5690/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5062
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3968
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5009
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/20/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/20/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5009
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5009/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7550
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25565

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5690/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:3131
http://jenkins.qa.ubuntu.com/job/mir-ci/5693/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5066
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3972
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5013
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/23/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/23/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5013
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5013/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7553
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25570

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5693/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:3133
http://jenkins.qa.ubuntu.com/job/mir-ci/5703/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5078
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3984
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5025
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/33
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/33/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/33/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5025
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5025/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7565
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25595

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5703/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:3133
http://jenkins.qa.ubuntu.com/job/mir-ci/5708/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5084
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3990
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5031
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-xenial-touch/39/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/38
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/38/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/38
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/38/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5031
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5031/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7570
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25603
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/37/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5708/rebuild

review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Sensible

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/client/mir_toolkit/mir_buffer_stream.h'
2--- include/client/mir_toolkit/mir_buffer_stream.h 2015-11-27 16:17:28 +0000
3+++ include/client/mir_toolkit/mir_buffer_stream.h 2015-11-27 16:17:28 +0000
4@@ -39,6 +39,17 @@
5 bool mir_buffer_stream_is_valid(MirBufferStream *buffer_stream);
6
7 /**
8+ * Retrieve a text description of the error. The returned string is owned by
9+ * the library and remains valid until the stream or the associated
10+ * connection has been released.
11+ * \param [in] buffer_stream The buffer stream
12+ * \return A text description of any error resulting in an
13+ * invalid stream, or the empty string "" if the
14+ * connection is valid.
15+ */
16+char const *mir_buffer_stream_get_error_message(MirBufferStream *surface);
17+
18+/**
19 * Create a new buffer stream.
20 *
21 * For example, the resulting buffer stream may be used
22
23=== modified file 'src/client/buffer_stream.cpp'
24--- src/client/buffer_stream.cpp 2015-11-27 16:17:28 +0000
25+++ src/client/buffer_stream.cpp 2015-11-27 16:17:28 +0000
26@@ -358,7 +358,7 @@
27 mclr::DisplayServer& server,
28 mcl::BufferStreamMode mode,
29 std::shared_ptr<mcl::ClientPlatform> const& client_platform,
30- mp::BufferStream const& protobuf_bs,
31+ mp::BufferStream const& a_protobuf_bs,
32 std::shared_ptr<mcl::PerfReport> const& perf_report,
33 std::string const& surface_name,
34 geom::Size ideal_size,
35@@ -367,7 +367,7 @@
36 display_server(server),
37 mode(mode),
38 client_platform(client_platform),
39- protobuf_bs{mcl::make_protobuf_object<mir::protobuf::BufferStream>(protobuf_bs)},
40+ protobuf_bs{mcl::make_protobuf_object<mir::protobuf::BufferStream>(a_protobuf_bs)},
41 swap_interval_(1),
42 scale_(1.0f),
43 perf_report(perf_report),
44@@ -376,6 +376,8 @@
45 nbuffers(nbuffers)
46 {
47 created(nullptr, nullptr);
48+ if (!valid())
49+ BOOST_THROW_EXCEPTION(std::runtime_error("Can not create buffer stream: " + std::string(protobuf_bs->error())));
50 perf_report->name_surface(surface_name.c_str());
51 }
52
53@@ -427,33 +429,54 @@
54 create_wait_handle.result_received();
55 }};
56
57- if (!protobuf_bs->has_id() || protobuf_bs->has_error())
58- return;
59-
60- if (protobuf_bs->has_buffer())
61- {
62- cached_buffer_size = geom::Size{protobuf_bs->buffer().width(), protobuf_bs->buffer().height()};
63- buffer_depository = std::make_unique<ExchangeSemantics>(
64- display_server,
65- client_platform->create_buffer_factory(),
66- mir::frontend::client_buffer_cache_size,
67- protobuf_bs->buffer(),
68- cached_buffer_size,
69- static_cast<MirPixelFormat>(protobuf_bs->pixel_format()));
70- }
71- else
72- {
73- buffer_depository = std::make_unique<NewBufferSemantics>(
74- client_platform->create_buffer_factory(),
75- std::make_shared<Requests>(display_server, protobuf_bs->id().value()),
76- ideal_buffer_size, static_cast<MirPixelFormat>(protobuf_bs->pixel_format()), 0, nbuffers);
77- }
78-
79-
80- egl_native_window_ = client_platform->create_egl_native_window(this);
81-
82- if (connection)
83- connection->on_stream_created(protobuf_bs->id().value(), this);
84+ if (!protobuf_bs->has_id())
85+ {
86+ if (!protobuf_bs->has_error())
87+ protobuf_bs->set_error("Error processing buffer stream create response, no ID (disconnected?)");
88+ return;
89+ }
90+
91+ if (protobuf_bs->has_error())
92+ return;
93+
94+ try
95+ {
96+ if (protobuf_bs->has_buffer())
97+ {
98+ cached_buffer_size = geom::Size{protobuf_bs->buffer().width(), protobuf_bs->buffer().height()};
99+ buffer_depository = std::make_unique<ExchangeSemantics>(
100+ display_server,
101+ client_platform->create_buffer_factory(),
102+ mir::frontend::client_buffer_cache_size,
103+ protobuf_bs->buffer(),
104+ cached_buffer_size,
105+ static_cast<MirPixelFormat>(protobuf_bs->pixel_format()));
106+ }
107+ else
108+ {
109+ buffer_depository = std::make_unique<NewBufferSemantics>(
110+ client_platform->create_buffer_factory(),
111+ std::make_shared<Requests>(display_server, protobuf_bs->id().value()),
112+ ideal_buffer_size, static_cast<MirPixelFormat>(protobuf_bs->pixel_format()), 0, nbuffers);
113+ }
114+
115+
116+ egl_native_window_ = client_platform->create_egl_native_window(this);
117+
118+ if (connection)
119+ connection->on_stream_created(protobuf_bs->id().value(), this);
120+ }
121+ catch (std::exception const& error)
122+ {
123+ protobuf_bs->set_error(std::string{"Error processing buffer stream creating response:"} +
124+ boost::diagnostic_information(error));
125+
126+ if (!buffer_depository)
127+ {
128+ for (int i = 0; i < protobuf_bs->buffer().fd_size(); i++)
129+ ::close(protobuf_bs->buffer().fd(i));
130+ }
131+ }
132 }
133
134 mcl::BufferStream::~BufferStream()
135@@ -472,7 +495,7 @@
136 {
137 cached_buffer_size = geom::Size{buffer.width(), buffer.height()};
138 }
139-
140+
141 if (buffer.has_error())
142 {
143 BOOST_THROW_EXCEPTION(std::runtime_error("BufferStream received buffer with error:" + buffer.error()));
144@@ -664,7 +687,7 @@
145 mf::BufferStreamId mcl::BufferStream::rpc_id() const
146 {
147 std::unique_lock<decltype(mutex)> lock(mutex);
148-
149+
150 return mf::BufferStreamId(protobuf_bs->id().value());
151 }
152
153@@ -708,3 +731,15 @@
154 google::protobuf::NewCallback(this, &mcl::BufferStream::on_scale_set, scale));
155 return &scale_wait_handle;
156 }
157+
158+char const * mcl::BufferStream::get_error_message() const
159+{
160+ std::lock_guard<decltype(mutex)> lock(mutex);
161+
162+ if (protobuf_bs->has_error())
163+ {
164+ return protobuf_bs->error().c_str();
165+ }
166+
167+ return error_message.c_str();
168+}
169
170=== modified file 'src/client/buffer_stream.h'
171--- src/client/buffer_stream.h 2015-11-27 16:17:28 +0000
172+++ src/client/buffer_stream.h 2015-11-27 16:17:28 +0000
173@@ -129,6 +129,8 @@
174 void buffer_unavailable() override;
175 void set_size(geometry::Size) override;
176 MirWaitHandle* set_scale(float scale) override;
177+ char const* get_error_message() const override;
178+
179 protected:
180 BufferStream(BufferStream const&) = delete;
181 BufferStream& operator=(BufferStream const&) = delete;
182@@ -174,6 +176,7 @@
183 std::unique_ptr<ServerBufferSemantics> buffer_depository;
184 geometry::Size ideal_buffer_size;
185 size_t const nbuffers;
186+ std::string error_message;
187 };
188
189 }
190
191=== modified file 'src/client/client_buffer_stream.h'
192--- src/client/client_buffer_stream.h 2015-09-24 15:42:21 +0000
193+++ src/client/client_buffer_stream.h 2015-11-27 16:17:28 +0000
194@@ -72,6 +72,8 @@
195 virtual void buffer_unavailable() = 0;
196 virtual void set_size(geometry::Size) = 0;
197 virtual MirWaitHandle* set_scale(float) = 0;
198+ virtual char const* get_error_message() const = 0;
199+
200 protected:
201 ClientBufferStream() = default;
202 ClientBufferStream(const ClientBufferStream&) = delete;
203
204=== modified file 'src/client/mir_buffer_stream_api.cpp'
205--- src/client/mir_buffer_stream_api.cpp 2015-09-24 15:42:21 +0000
206+++ src/client/mir_buffer_stream_api.cpp 2015-11-27 16:17:28 +0000
207@@ -202,3 +202,9 @@
208 if (wh)
209 wh->wait_for_all();
210 }
211+
212+char const* mir_buffer_stream_get_error_message(MirBufferStream* opaque_stream)
213+{
214+ auto buffer_stream = reinterpret_cast<mcl::ClientBufferStream*>(opaque_stream);
215+ return buffer_stream->get_error_message();
216+}
217
218=== modified file 'src/client/symbols.map'
219--- src/client/symbols.map 2015-11-26 10:44:43 +0000
220+++ src/client/symbols.map 2015-11-27 16:17:28 +0000
221@@ -200,6 +200,7 @@
222
223 MIR_CLIENT_unreleased {
224 global:
225+ mir_buffer_stream_get_error_message;
226 mir_keyboard_event_get_cookie;
227 mir_pointer_event_get_cookie;
228 mir_touch_event_get_cookie;
229
230=== modified file 'tests/acceptance-tests/test_client_library.cpp'
231--- tests/acceptance-tests/test_client_library.cpp 2015-09-17 22:16:26 +0000
232+++ tests/acceptance-tests/test_client_library.cpp 2015-11-27 16:17:28 +0000
233@@ -1021,3 +1021,19 @@
234 mir_connection_release(first_client);
235 mir_connection_release(im_client);
236 }
237+
238+TEST_F(ClientLibrary, creates_buffer_streams)
239+{
240+ connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
241+ ASSERT_TRUE(mir_connection_is_valid(connection));
242+
243+ auto stream = mir_connection_create_buffer_stream_sync(connection,
244+ 640, 480, mir_pixel_format_abgr_8888, mir_buffer_usage_software);
245+
246+ ASSERT_THAT(stream, NotNull());
247+ EXPECT_TRUE(mir_buffer_stream_is_valid(stream));
248+ EXPECT_THAT(mir_buffer_stream_get_error_message(stream), StrEq(""));
249+
250+ mir_buffer_stream_release_sync(stream);
251+ mir_connection_release(connection);
252+}
253
254=== modified file 'tests/acceptance-tests/throwback/test_client_library_errors.cpp'
255--- tests/acceptance-tests/throwback/test_client_library_errors.cpp 2015-09-17 15:53:01 +0000
256+++ tests/acceptance-tests/throwback/test_client_library_errors.cpp 2015-11-27 16:17:28 +0000
257@@ -205,6 +205,24 @@
258 mir_connection_release(connection);
259 }
260
261+TEST_F(ClientLibraryErrors, create_buffer_stream_returns_error_object_on_failure)
262+{
263+ mtf::UsingClientPlatform<ConfigurableFailureConfiguration<Method::create_buffer_factory>> stubby;
264+
265+ auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
266+
267+ ASSERT_THAT(connection, IsValid());
268+
269+ auto stream = mir_connection_create_buffer_stream_sync(connection,
270+ 640, 480, mir_pixel_format_abgr_8888, mir_buffer_usage_software);
271+ ASSERT_NE(stream, nullptr);
272+ EXPECT_FALSE(mir_buffer_stream_is_valid(stream));
273+ EXPECT_THAT(mir_buffer_stream_get_error_message(stream), testing::HasSubstr(exception_text));
274+
275+ mir_buffer_stream_release_sync(stream);
276+ mir_connection_release(connection);
277+}
278+
279 namespace
280 {
281 void recording_surface_callback(MirSurface*, void* ctx)
282
283=== modified file 'tests/include/mir/test/doubles/mock_client_buffer_stream.h'
284--- tests/include/mir/test/doubles/mock_client_buffer_stream.h 2015-09-28 19:36:25 +0000
285+++ tests/include/mir/test/doubles/mock_client_buffer_stream.h 2015-11-27 16:17:28 +0000
286@@ -51,6 +51,7 @@
287 MOCK_METHOD0(buffer_unavailable, void());
288 MOCK_METHOD1(set_size, void(geometry::Size));
289 MOCK_METHOD1(set_scale, MirWaitHandle*(float));
290+ MOCK_CONST_METHOD0(get_error_message, char const*(void));
291 };
292
293 }
294
295=== modified file 'tests/unit-tests/client/test_client_buffer_stream.cpp'
296--- tests/unit-tests/client/test_client_buffer_stream.cpp 2015-11-27 16:17:28 +0000
297+++ tests/unit-tests/client/test_client_buffer_stream.cpp 2015-11-27 16:17:28 +0000
298@@ -316,23 +316,21 @@
299
300 auto error_bs = valid_bs;
301 error_bs.set_error("An error");
302- {
303+ EXPECT_THROW({
304 mcl::BufferStream bs(
305 nullptr, mock_protobuf_server, mode,
306 std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
307 error_bs, perf_report, "", size, nbuffers);
308- EXPECT_FALSE(bs.valid());
309- }
310+ }, std::runtime_error);
311
312 auto no_id_bs = valid_bs;
313 no_id_bs.clear_id();
314- {
315+ EXPECT_THROW({
316 mcl::BufferStream bs(
317 nullptr, mock_protobuf_server, mode,
318 std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
319 no_id_bs, perf_report, "", size, nbuffers);
320- EXPECT_FALSE(bs.valid());
321- }
322+ }, std::runtime_error);
323 }
324
325 TEST_P(ClientBufferStream, uses_buffer_message_from_server)

Subscribers

People subscribed via source and target branches