1
=== modified file 'include/test/mir/test/validity_matchers.h'
2
--- include/test/mir/test/validity_matchers.h	2016-01-29 08:18:22 +0000
3
+++ include/test/mir/test/validity_matchers.h	2016-12-13 05:39:59 +0000
4
@@ -23,6 +23,8 @@
5
23
23
6
24
#include "mir_toolkit/mir_client_library.h"
24
#include "mir_toolkit/mir_client_library.h"
7
25
25
8
26
class MirRenderSurface;
9
27
10
26
using ::testing::MakePolymorphicMatcher;
28
using ::testing::MakePolymorphicMatcher;
11
27
using ::testing::MatchResultListener;
29
using ::testing::MatchResultListener;
12
28
using ::testing::NotNull;
30
using ::testing::NotNull;
13
@@ -54,6 +56,12 @@
14
54
template<>
56
template<>
15
55
bool IsValidMatcher::MatchAndExplain(MirSurface* surface, MatchResultListener* listener) const;
57
bool IsValidMatcher::MatchAndExplain(MirSurface* surface, MatchResultListener* listener) const;
16
56
58
17
59
template<>
18
60
bool IsValidMatcher::MatchAndExplain(MirBufferStream* stream, MatchResultListener* listener) const;
19
61
20
62
template<>
21
63
bool IsValidMatcher::MatchAndExplain(MirRenderSurface* stream, MatchResultListener* listener) const;
22
64
23
57
// To construct a polymorphic matcher, pass an instance of the class
65
// To construct a polymorphic matcher, pass an instance of the class
24
58
// to MakePolymorphicMatcher().  Note the return type.
66
// to MakePolymorphicMatcher().  Note the return type.
25
59
inline PolymorphicMatcher<IsValidMatcher> IsValid()
67
inline PolymorphicMatcher<IsValidMatcher> IsValid()
26
60
68
27
=== modified file 'src/client/atomic_callback.h'
28
--- src/client/atomic_callback.h	2016-12-01 19:18:27 +0000
29
+++ src/client/atomic_callback.h	2016-12-13 05:39:59 +0000
30
@@ -19,6 +19,7 @@
31
19
#ifndef MIR_CLIENT_ATOMIC_CALLBACK_H_
19
#ifndef MIR_CLIENT_ATOMIC_CALLBACK_H_
32
20
#define MIR_CLIENT_ATOMIC_CALLBACK_H_
20
#define MIR_CLIENT_ATOMIC_CALLBACK_H_
33
21
21
34
22
#include <memory>
35
22
#include <functional>
23
#include <functional>
36
23
#include <mutex>
24
#include <mutex>
37
24
25
38
@@ -31,12 +32,12 @@
39
31
{
32
{
40
32
public:
33
public:
41
33
    AtomicCallback()
34
    AtomicCallback()
43
34
        : callback{[](Args...){}}
35
        : AtomicCallback([](Args...){})
44
35
    {
36
    {
45
36
    }
37
    }
46
37
38
47
38
    AtomicCallback(std::function<void(Args...)> const& fn)
39
    AtomicCallback(std::function<void(Args...)> const& fn)
49
39
        : callback(fn)
40
        : callback{std::make_shared<std::function<void(Args...)>>(fn)}
50
40
    {
41
    {
51
41
    }
42
    }
52
42
43
53
@@ -44,21 +45,25 @@
54
44
45
55
45
    void set_callback(std::function<void(Args...)> const& fn)
46
    void set_callback(std::function<void(Args...)> const& fn)
56
46
    {
47
    {
60
47
        std::lock_guard<std::recursive_mutex> lk(guard);
48
        std::lock_guard<std::mutex> lock{mutex};
61
48
49
        callback = std::make_shared<std::function<void(Args...)>>(fn);
59
49
        callback = fn;
62
50
    }
50
    }
63
51
51
64
52
    void operator()(Args&&... args) const
52
    void operator()(Args&&... args) const
65
53
    {
53
    {
69
54
        std::lock_guard<std::recursive_mutex> lk(guard);
54
        decltype(callback) callback_copy;
70
55
55
71
56
        callback(std::forward<Args>(args)...);
56
        {
72
57
            std::lock_guard<std::mutex> lock{mutex};
73
58
            callback_copy = callback;
74
59
        }
75
60
76
61
        (*callback)(std::forward<Args>(args)...);
77
57
    }
62
    }
78
58
63
79
59
private:
64
private:
82
60
    std::recursive_mutex mutable guard;
65
    std::mutex mutable mutex;
83
61
    std::function<void(Args...)> callback;
66
    std::shared_ptr<std::function<void(Args...)>> callback;
84
62
};
67
};
85
63
}
68
}
86
64
}
69
}
87
65
70
88
=== modified file 'src/client/mir_buffer_stream_api.cpp'
89
--- src/client/mir_buffer_stream_api.cpp	2016-12-12 03:34:50 +0000
90
+++ src/client/mir_buffer_stream_api.cpp	2016-12-13 05:39:59 +0000
91
@@ -92,9 +92,16 @@
92
92
    return connection->release_buffer_stream(buffer_stream, callback, context);
92
    return connection->release_buffer_stream(buffer_stream, callback, context);
93
93
}
93
}
94
94
94
95
95
namespace
96
96
{
97
97
void ignored_callback(MirBufferStream*, void*)
98
98
{
99
99
}
100
100
}
101
101
102
95
void mir_buffer_stream_release_sync(MirBufferStream *buffer_stream)
102
void mir_buffer_stream_release_sync(MirBufferStream *buffer_stream)
103
96
{
103
{
105
97
    mir_buffer_stream_release(buffer_stream, nullptr, nullptr)->wait_for_all();
104
    mir_buffer_stream_release(buffer_stream, &ignored_callback, nullptr);
106
98
}
105
}
107
99
106
108
100
void mir_buffer_stream_get_current_buffer(MirBufferStream* buffer_stream, MirNativeBuffer** buffer_package_out)
107
void mir_buffer_stream_get_current_buffer(MirBufferStream* buffer_stream, MirNativeBuffer** buffer_package_out)
109
101
108
110
=== modified file 'src/client/mir_connection.cpp'
111
--- src/client/mir_connection.cpp	2016-12-12 11:43:17 +0000
112
+++ src/client/mir_connection.cpp	2016-12-13 05:39:59 +0000
113
@@ -463,31 +463,6 @@
114
463
    void* context;
463
    void* context;
115
464
};
464
};
116
465
465
117
466
struct MirConnection::StreamRelease
118
467
{
119
468
    MirBufferStream* stream;
120
469
    MirWaitHandle* handle;
121
470
    mir_buffer_stream_callback callback;
122
471
    void* context;
123
472
    int rpc_id;
124
473
    void* rs;
125
474
};
126
475
127
476
void MirConnection::released(StreamRelease data)
128
477
{
129
478
    if (data.callback)
130
479
        data.callback(data.stream, data.context);
131
480
    if (data.handle)
132
481
        data.handle->result_received();
133
482
134
483
    {
135
484
        std::unique_lock<decltype(mutex)> lk(mutex);
136
485
        if (data.rs)
137
486
            surface_map->erase(data.rs);
138
487
        surface_map->erase(mf::BufferStreamId(data.rpc_id));
139
488
    }
140
489
}
141
490
142
491
void MirConnection::released(SurfaceRelease data)
466
void MirConnection::released(SurfaceRelease data)
143
492
{
467
{
144
493
    data.callback(data.surface, data.context);
468
    data.callback(data.surface, data.context);
145
@@ -1143,23 +1118,17 @@
146
1143
    mir_buffer_stream_callback callback,
1118
    mir_buffer_stream_callback callback,
147
1144
    void *context)
1119
    void *context)
148
1145
{
1120
{
149
1146
    auto new_wait_handle = new MirWaitHandle;
150
1147
151
1148
    StreamRelease stream_release{stream, new_wait_handle, callback, context, stream->rpc_id().as_value(), nullptr };
152
1149
153
1150
    mp::BufferStreamId buffer_stream_id;
1121
    mp::BufferStreamId buffer_stream_id;
154
1151
    buffer_stream_id.set_value(stream->rpc_id().as_value());
1122
    buffer_stream_id.set_value(stream->rpc_id().as_value());
155
1152
1123
160
1153
    {
1124
    surface_map->erase(mf::BufferStreamId{stream->rpc_id().as_value()});
157
1154
        std::lock_guard<decltype(release_wait_handle_guard)> rel_lock(release_wait_handle_guard);
158
1155
        release_wait_handles.push_back(new_wait_handle);
159
1156
    }
161
1157
1125
162
1158
    new_wait_handle->expect_result();
163
1159
    server.release_buffer_stream(
1126
    server.release_buffer_stream(
164
1160
        &buffer_stream_id, void_response.get(),
1127
        &buffer_stream_id, void_response.get(),
167
1161
        google::protobuf::NewCallback(this, &MirConnection::released, stream_release));
1128
        google::protobuf::NewCallback(&google::protobuf::DoNothing));
168
1162
    return new_wait_handle;
1129
169
1130
    callback(stream, context);
170
1131
    return nullptr;
171
1163
}
1132
}
172
1164
1133
173
1165
void MirConnection::release_consumer_stream(MirBufferStream* stream)
1134
void MirConnection::release_consumer_stream(MirBufferStream* stream)
174
@@ -1282,12 +1251,12 @@
175
1282
    auto id = chain->rpc_id();
1251
    auto id = chain->rpc_id();
176
1283
    if (id >= 0)
1252
    if (id >= 0)
177
1284
    {
1253
    {
178
1285
        StreamRelease stream_release{nullptr, nullptr, nullptr, nullptr, chain->rpc_id(), nullptr};
179
1286
        mp::BufferStreamId buffer_stream_id;
1254
        mp::BufferStreamId buffer_stream_id;
180
1287
        buffer_stream_id.set_value(chain->rpc_id());
1255
        buffer_stream_id.set_value(chain->rpc_id());
181
1256
        surface_map->erase(mf::BufferStreamId{chain->rpc_id()});
182
1288
        server.release_buffer_stream(
1257
        server.release_buffer_stream(
183
1289
            &buffer_stream_id, void_response.get(),
1258
            &buffer_stream_id, void_response.get(),
185
1290
            google::protobuf::NewCallback(this, &MirConnection::released, stream_release));
1259
            google::protobuf::NewCallback(&google::protobuf::DoNothing));
186
1291
    }
1260
    }
187
1292
    else
1261
    else
188
1293
    {
1262
    {
189
@@ -1335,19 +1304,15 @@
190
1335
{
1304
{
191
1336
    auto rs = surface_map->render_surface(render_surface);
1305
    auto rs = surface_map->render_surface(render_surface);
192
1337
1306
193
1338
    StreamRelease stream_release{nullptr,
194
1339
                                 nullptr,
195
1340
                                 nullptr,
196
1341
                                 nullptr,
197
1342
                                 rs->stream_id().as_value(),
198
1343
                                 render_surface};
199
1344
200
1345
    mp::BufferStreamId buffer_stream_id;
1307
    mp::BufferStreamId buffer_stream_id;
201
1346
    buffer_stream_id.set_value(rs->stream_id().as_value());
1308
    buffer_stream_id.set_value(rs->stream_id().as_value());
202
1347
1309
203
1310
    surface_map->erase(mf::BufferStreamId(rs->stream_id().as_value()));
204
1311
    surface_map->erase(render_surface);
205
1312
206
1348
    server.release_buffer_stream(
1313
    server.release_buffer_stream(
207
1349
        &buffer_stream_id, void_response.get(),
1314
        &buffer_stream_id, void_response.get(),
209
1350
        google::protobuf::NewCallback(this, &MirConnection::released, stream_release));
1315
        google::protobuf::NewCallback(&google::protobuf::DoNothing));
210
1351
}
1316
}
211
1352
1317
212
1353
void MirConnection::render_surface_created(RenderSurfaceCreationRequest* request_raw)
1318
void MirConnection::render_surface_created(RenderSurfaceCreationRequest* request_raw)
213
1354
1319
214
=== modified file 'src/client/mir_connection.h'
215
--- src/client/mir_connection.h	2016-12-12 11:43:17 +0000
216
+++ src/client/mir_connection.h	2016-12-13 05:39:59 +0000
217
@@ -376,7 +376,6 @@
218
376
    
376
    
219
377
377
220
378
    struct SurfaceRelease;
378
    struct SurfaceRelease;
221
379
    struct StreamRelease;
222
380
379
223
381
    MirConnection* next_valid{nullptr};
380
    MirConnection* next_valid{nullptr};
224
382
381
225
@@ -384,7 +383,6 @@
226
384
    void done_disconnect();
383
    void done_disconnect();
227
385
    void connected(mir_connected_callback callback, void * context);
384
    void connected(mir_connected_callback callback, void * context);
228
386
    void released(SurfaceRelease);
385
    void released(SurfaceRelease);
229
387
    void released(StreamRelease);
230
388
    void done_platform_operation(mir_platform_operation_callback, void* context);
386
    void done_platform_operation(mir_platform_operation_callback, void* context);
231
389
    bool validate_user_display_config(MirDisplayConfiguration const* config);
387
    bool validate_user_display_config(MirDisplayConfiguration const* config);
232
390
388
233
391
389
234
=== modified file 'tests/acceptance-tests/staging/test_render_surface.cpp'
235
--- tests/acceptance-tests/staging/test_render_surface.cpp	2016-12-12 21:58:01 +0000
236
+++ tests/acceptance-tests/staging/test_render_surface.cpp	2016-12-13 05:39:59 +0000
237
@@ -25,6 +25,7 @@
238
25
#include "mir_test_framework/using_stub_client_platform.h"
25
#include "mir_test_framework/using_stub_client_platform.h"
239
26
#include "mir/test/validity_matchers.h"
26
#include "mir/test/validity_matchers.h"
240
27
27
241
28
#include <atomic>
242
28
#include <gtest/gtest.h>
29
#include <gtest/gtest.h>
243
29
#include <gmock/gmock.h>
30
#include <gmock/gmock.h>
244
30
31
245
@@ -316,3 +317,47 @@
246
316
    mir_render_surface_release(rs);
317
    mir_render_surface_release(rs);
247
317
    mir_connection_release(connection);
318
    mir_connection_release(connection);
248
318
}
319
}
249
320
250
321
TEST_F(RenderSurfaceTest, buffer_stream_callback_isnt_called_after_render_surface_release)
251
322
{
252
323
    auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
253
324
254
325
    auto rs = mir_connection_create_render_surface_sync(
255
326
        connection, logical_size.width.as_int(), logical_size.height.as_int());
256
327
257
328
    auto determine_physical_size = [](MirRenderSurface* rs) -> geom::Size
258
329
    {
259
330
        int width = -1;
260
331
        int height = -1;
261
332
        mir_render_surface_get_size(rs, &width, &height);
262
333
        return {width, height};
263
334
    };
264
335
    auto physical_size = determine_physical_size(rs);
265
336
    auto bs = mir_render_surface_get_buffer_stream(
266
337
        rs,
267
338
        physical_size.width.as_int(), physical_size.height.as_int(),
268
339
        mir_pixel_format_abgr_8888,
269
340
        mir_buffer_usage_hardware);
270
341
271
342
    EXPECT_THAT(bs, IsValid());
272
343
273
344
    std::atomic<bool> called{false};
274
345
    mir_buffer_stream_swap_buffers(
275
346
        bs,
276
347
        [](MirBufferStream*, void* called)
277
348
        {
278
349
            *reinterpret_cast<std::atomic<bool>*>(called) = true;
279
350
        },
280
351
        &called);
281
352
282
353
    // Unavoidably racy; we want to test that the swap_buffers callback is never called
283
354
    // after mir_render_surface_release returns.
284
355
    mir_render_surface_release(rs);
285
356
    bool called_post_release = called.load();
286
357
287
358
    mir_connection_release(connection);
288
359
289
360
    // We don't care whether or the swap_buffers callback was called, just that
290
361
    // it didn't get called *after* mir_render_surface_release.
291
362
    EXPECT_THAT(called, Eq(called_post_release));
292
363
}
293
319
364
294
=== modified file 'tests/mir_test/CMakeLists.txt'
295
--- tests/mir_test/CMakeLists.txt	2016-11-09 02:30:14 +0000
296
+++ tests/mir_test/CMakeLists.txt	2016-12-13 05:39:59 +0000
297
@@ -2,6 +2,10 @@
298
2
string (REPLACE " -flto " " " CMAKE_C_FLAGS ${CMAKE_C_FLAGS})
2
string (REPLACE " -flto " " " CMAKE_C_FLAGS ${CMAKE_C_FLAGS})
299
3
string (REPLACE " -flto " " " CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})
3
string (REPLACE " -flto " " " CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})
300
4
4
301
5
include_directories(
302
6
  ${PROJECT_SOURCE_DIR}/src/include/client
303
7
)
304
8
305
5
add_library(mir-public-test OBJECT
9
add_library(mir-public-test OBJECT
306
6
  cross_process_action.cpp
10
  cross_process_action.cpp
307
7
  cross_process_sync.cpp
11
  cross_process_sync.cpp
308
8
12
309
=== modified file 'tests/mir_test/validity_matchers.cpp'
310
--- tests/mir_test/validity_matchers.cpp	2015-06-25 03:00:08 +0000
311
+++ tests/mir_test/validity_matchers.cpp	2016-12-13 05:39:59 +0000
312
@@ -1,5 +1,6 @@
313
1
#include "mir/test/validity_matchers.h"
1
#include "mir/test/validity_matchers.h"
314
2
#include "mir_toolkit/mir_client_library.h"
2
#include "mir_toolkit/mir_client_library.h"
315
3
#include "mir_toolkit/mir_render_surface.h"
316
3
4
317
4
template<>
5
template<>
318
5
bool IsValidMatcher::MatchAndExplain(MirConnection* connection, MatchResultListener* listener) const
6
bool IsValidMatcher::MatchAndExplain(MirConnection* connection, MatchResultListener* listener) const
319
@@ -30,3 +31,33 @@
320
30
    }
31
    }
321
31
    return true;
32
    return true;
322
32
}
33
}
323
34
324
35
template<>
325
36
bool IsValidMatcher::MatchAndExplain(MirBufferStream* stream, MatchResultListener* listener) const
326
37
{
327
38
    if (stream == nullptr)
328
39
    {
329
40
        return false;
330
41
    }
331
42
    if (!mir_buffer_stream_is_valid(stream))
332
43
    {
333
44
        *listener << "error is: " << mir_buffer_stream_get_error_message(stream);
334
45
        return false;
335
46
    }
336
47
    return true;
337
48
}
338
49
339
50
template<>
340
51
bool IsValidMatcher::MatchAndExplain(MirRenderSurface* rs, MatchResultListener* listener) const
341
52
{
342
53
    if (rs == nullptr)
343
54
    {
344
55
        return false;
345
56
    }
346
57
    if (!mir_render_surface_is_valid(rs))
347
58
    {
348
59
        *listener << "error is: " << mir_render_surface_get_error_message(rs);
349
60
        return false;
350
61
    }
351
62
    return true;
352
63
}
Status:	Work in progress
Proposed branch:	lp:~raof/mir/client-release-simplifications
Merge into:	lp:mir
Diff against target:	352 lines (+122/-59) 8 files modified include/test/mir/test/validity_matchers.h (+8/-0) src/client/atomic_callback.h (+15/-10) src/client/mir_buffer_stream_api.cpp (+8/-1) src/client/mir_connection.cpp (+11/-46) src/client/mir_connection.h (+0/-2) tests/acceptance-tests/staging/test_render_surface.cpp (+45/-0) tests/mir_test/CMakeLists.txt (+4/-0) tests/mir_test/validity_matchers.cpp (+31/-0)
To merge this branch:	bzr merge lp:~raof/mir/client-release-simplifications
Related bugs:	Link a bug report
Reviewer	Review Type	Date Requested	Status
Cemil Azizoglu (community)			Needs Fixing on 2016-12-13
Andreas Pokorny (community)		2016-12-12	Approve on 2016-12-13
Mir CI Bot	continuous-integration		Needs Fixing on 2016-12-13
Review via email: mp+312995@code.launchpad.net
1	=== modified file 'include/test/mir/test/validity_matchers.h'
2	--- include/test/mir/test/validity_matchers.h 2016-01-29 08:18:22 +0000
3	+++ include/test/mir/test/validity_matchers.h 2016-12-13 05:39:59 +0000
4	@@ -23,6 +23,8 @@
5	23		23
6	24	#include "mir_toolkit/mir_client_library.h"	24	#include "mir_toolkit/mir_client_library.h"
7	25		25
8			26	class MirRenderSurface;
9			27
10	26	using ::testing::MakePolymorphicMatcher;	28	using ::testing::MakePolymorphicMatcher;
11	27	using ::testing::MatchResultListener;	29	using ::testing::MatchResultListener;
12	28	using ::testing::NotNull;	30	using ::testing::NotNull;
13	@@ -54,6 +56,12 @@
14	54	template<>	56	template<>
15	55	bool IsValidMatcher::MatchAndExplain(MirSurface* surface, MatchResultListener* listener) const;	57	bool IsValidMatcher::MatchAndExplain(MirSurface* surface, MatchResultListener* listener) const;
16	56		58
17			59	template<>
18			60	bool IsValidMatcher::MatchAndExplain(MirBufferStream* stream, MatchResultListener* listener) const;
19			61
20			62	template<>
21			63	bool IsValidMatcher::MatchAndExplain(MirRenderSurface* stream, MatchResultListener* listener) const;
22			64
23	57	// To construct a polymorphic matcher, pass an instance of the class	65	// To construct a polymorphic matcher, pass an instance of the class
24	58	// to MakePolymorphicMatcher(). Note the return type.	66	// to MakePolymorphicMatcher(). Note the return type.
25	59	inline PolymorphicMatcher<IsValidMatcher> IsValid()	67	inline PolymorphicMatcher<IsValidMatcher> IsValid()
26	60		68
27	=== modified file 'src/client/atomic_callback.h'
28	--- src/client/atomic_callback.h 2016-12-01 19:18:27 +0000
29	+++ src/client/atomic_callback.h 2016-12-13 05:39:59 +0000
30	@@ -19,6 +19,7 @@
31	19	#ifndef MIR_CLIENT_ATOMIC_CALLBACK_H_	19	#ifndef MIR_CLIENT_ATOMIC_CALLBACK_H_
32	20	#define MIR_CLIENT_ATOMIC_CALLBACK_H_	20	#define MIR_CLIENT_ATOMIC_CALLBACK_H_
33	21		21
34			22	#include <memory>
35	22	#include <functional>	23	#include <functional>
36	23	#include <mutex>	24	#include <mutex>
37	24		25
38	@@ -31,12 +32,12 @@
39	31	{	32	{
40	32	public:	33	public:
41	33	AtomicCallback()	34	AtomicCallback()
43	34	: callback{[](Args...){}}	35	: AtomicCallback([](Args...){})
44	35	{	36	{
45	36	}	37	}
46	37		38
47	38	AtomicCallback(std::function<void(Args...)> const& fn)	39	AtomicCallback(std::function<void(Args...)> const& fn)
49	39	: callback(fn)	40	: callback{std::make_shared<std::function<void(Args...)>>(fn)}
50	40	{	41	{
51	41	}	42	}
52	42		43
53	@@ -44,21 +45,25 @@
54	44		45
55	45	void set_callback(std::function<void(Args...)> const& fn)	46	void set_callback(std::function<void(Args...)> const& fn)
56	46	{	47	{
60	47	std::lock_guard<std::recursive_mutex> lk(guard);	48	std::lock_guard<std::mutex> lock{mutex};
61	48		49	callback = std::make_shared<std::function<void(Args...)>>(fn);
59	49	callback = fn;
62	50	}	50	}
63	51		51
64	52	void operator()(Args&&... args) const	52	void operator()(Args&&... args) const
65	53	{	53	{
69	54	std::lock_guard<std::recursive_mutex> lk(guard);	54	decltype(callback) callback_copy;
70	55		55
71	56	callback(std::forward<Args>(args)...);	56	{
72			57	std::lock_guard<std::mutex> lock{mutex};
73			58	callback_copy = callback;
74			59	}
75			60
76			61	(*callback)(std::forward<Args>(args)...);
77	57	}	62	}
78	58		63
79	59	private:	64	private:
82	60	std::recursive_mutex mutable guard;	65	std::mutex mutable mutex;
83	61	std::function<void(Args...)> callback;	66	std::shared_ptr<std::function<void(Args...)>> callback;
84	62	};	67	};
85	63	}	68	}
86	64	}	69	}
87	65		70
88	=== modified file 'src/client/mir_buffer_stream_api.cpp'
89	--- src/client/mir_buffer_stream_api.cpp 2016-12-12 03:34:50 +0000
90	+++ src/client/mir_buffer_stream_api.cpp 2016-12-13 05:39:59 +0000
91	@@ -92,9 +92,16 @@
92	92	return connection->release_buffer_stream(buffer_stream, callback, context);	92	return connection->release_buffer_stream(buffer_stream, callback, context);
93	93	}	93	}
94	94		94
95			95	namespace
96			96	{
97			97	void ignored_callback(MirBufferStream, void)
98			98	{
99			99	}
100			100	}
101			101
102	95	void mir_buffer_stream_release_sync(MirBufferStream *buffer_stream)	102	void mir_buffer_stream_release_sync(MirBufferStream *buffer_stream)
103	96	{	103	{
105	97	mir_buffer_stream_release(buffer_stream, nullptr, nullptr)->wait_for_all();	104	mir_buffer_stream_release(buffer_stream, &ignored_callback, nullptr);
106	98	}	105	}
107	99		106
108	100	void mir_buffer_stream_get_current_buffer(MirBufferStream* buffer_stream, MirNativeBuffer** buffer_package_out)	107	void mir_buffer_stream_get_current_buffer(MirBufferStream* buffer_stream, MirNativeBuffer** buffer_package_out)
109	101		108
110	=== modified file 'src/client/mir_connection.cpp'
111	--- src/client/mir_connection.cpp 2016-12-12 11:43:17 +0000
112	+++ src/client/mir_connection.cpp 2016-12-13 05:39:59 +0000
113	@@ -463,31 +463,6 @@
114	463	void* context;	463	void* context;
115	464	};	464	};
116	465		465
117	466	struct MirConnection::StreamRelease
118	467	{
119	468	MirBufferStream* stream;
120	469	MirWaitHandle* handle;
121	470	mir_buffer_stream_callback callback;
122	471	void* context;
123	472	int rpc_id;
124	473	void* rs;
125	474	};
126	475
127	476	void MirConnection::released(StreamRelease data)
128	477	{
129	478	if (data.callback)
130	479	data.callback(data.stream, data.context);
131	480	if (data.handle)
132	481	data.handle->result_received();
133	482
134	483	{
135	484	std::unique_lock<decltype(mutex)> lk(mutex);
136	485	if (data.rs)
137	486	surface_map->erase(data.rs);
138	487	surface_map->erase(mf::BufferStreamId(data.rpc_id));
139	488	}
140	489	}
141	490
142	491	void MirConnection::released(SurfaceRelease data)	466	void MirConnection::released(SurfaceRelease data)
143	492	{	467	{
144	493	data.callback(data.surface, data.context);	468	data.callback(data.surface, data.context);
145	@@ -1143,23 +1118,17 @@
146	1143	mir_buffer_stream_callback callback,	1118	mir_buffer_stream_callback callback,
147	1144	void *context)	1119	void *context)
148	1145	{	1120	{
149	1146	auto new_wait_handle = new MirWaitHandle;
150	1147
151	1148	StreamRelease stream_release{stream, new_wait_handle, callback, context, stream->rpc_id().as_value(), nullptr };
152	1149
153	1150	mp::BufferStreamId buffer_stream_id;	1121	mp::BufferStreamId buffer_stream_id;
154	1151	buffer_stream_id.set_value(stream->rpc_id().as_value());	1122	buffer_stream_id.set_value(stream->rpc_id().as_value());
155	1152		1123
160	1153	{	1124	surface_map->erase(mf::BufferStreamId{stream->rpc_id().as_value()});
157	1154	std::lock_guard<decltype(release_wait_handle_guard)> rel_lock(release_wait_handle_guard);
158	1155	release_wait_handles.push_back(new_wait_handle);
159	1156	}
161	1157		1125
162	1158	new_wait_handle->expect_result();
163	1159	server.release_buffer_stream(	1126	server.release_buffer_stream(
164	1160	&buffer_stream_id, void_response.get(),	1127	&buffer_stream_id, void_response.get(),
167	1161	google::protobuf::NewCallback(this, &MirConnection::released, stream_release));	1128	google::protobuf::NewCallback(&google::protobuf::DoNothing));
168	1162	return new_wait_handle;	1129
169			1130	callback(stream, context);
170			1131	return nullptr;
171	1163	}	1132	}
172	1164		1133
173	1165	void MirConnection::release_consumer_stream(MirBufferStream* stream)	1134	void MirConnection::release_consumer_stream(MirBufferStream* stream)
174	@@ -1282,12 +1251,12 @@
175	1282	auto id = chain->rpc_id();	1251	auto id = chain->rpc_id();
176	1283	if (id >= 0)	1252	if (id >= 0)
177	1284	{	1253	{
178	1285	StreamRelease stream_release{nullptr, nullptr, nullptr, nullptr, chain->rpc_id(), nullptr};
179	1286	mp::BufferStreamId buffer_stream_id;	1254	mp::BufferStreamId buffer_stream_id;
180	1287	buffer_stream_id.set_value(chain->rpc_id());	1255	buffer_stream_id.set_value(chain->rpc_id());
181			1256	surface_map->erase(mf::BufferStreamId{chain->rpc_id()});
182	1288	server.release_buffer_stream(	1257	server.release_buffer_stream(
183	1289	&buffer_stream_id, void_response.get(),	1258	&buffer_stream_id, void_response.get(),
185	1290	google::protobuf::NewCallback(this, &MirConnection::released, stream_release));	1259	google::protobuf::NewCallback(&google::protobuf::DoNothing));
186	1291	}	1260	}
187	1292	else	1261	else
188	1293	{	1262	{
189	@@ -1335,19 +1304,15 @@
190	1335	{	1304	{
191	1336	auto rs = surface_map->render_surface(render_surface);	1305	auto rs = surface_map->render_surface(render_surface);
192	1337		1306
193	1338	StreamRelease stream_release{nullptr,
194	1339	nullptr,
195	1340	nullptr,
196	1341	nullptr,
197	1342	rs->stream_id().as_value(),
198	1343	render_surface};
199	1344
200	1345	mp::BufferStreamId buffer_stream_id;	1307	mp::BufferStreamId buffer_stream_id;
201	1346	buffer_stream_id.set_value(rs->stream_id().as_value());	1308	buffer_stream_id.set_value(rs->stream_id().as_value());
202	1347		1309
203			1310	surface_map->erase(mf::BufferStreamId(rs->stream_id().as_value()));
204			1311	surface_map->erase(render_surface);
205			1312
206	1348	server.release_buffer_stream(	1313	server.release_buffer_stream(
207	1349	&buffer_stream_id, void_response.get(),	1314	&buffer_stream_id, void_response.get(),
209	1350	google::protobuf::NewCallback(this, &MirConnection::released, stream_release));	1315	google::protobuf::NewCallback(&google::protobuf::DoNothing));
210	1351	}	1316	}
211	1352		1317
212	1353	void MirConnection::render_surface_created(RenderSurfaceCreationRequest* request_raw)	1318	void MirConnection::render_surface_created(RenderSurfaceCreationRequest* request_raw)
213	1354		1319
214	=== modified file 'src/client/mir_connection.h'
215	--- src/client/mir_connection.h 2016-12-12 11:43:17 +0000
216	+++ src/client/mir_connection.h 2016-12-13 05:39:59 +0000
217	@@ -376,7 +376,6 @@
218	376		376
219	377		377
220	378	struct SurfaceRelease;	378	struct SurfaceRelease;
221	379	struct StreamRelease;
222	380		379
223	381	MirConnection* next_valid{nullptr};	380	MirConnection* next_valid{nullptr};
224	382		381
225	@@ -384,7 +383,6 @@
226	384	void done_disconnect();	383	void done_disconnect();
227	385	void connected(mir_connected_callback callback, void * context);	384	void connected(mir_connected_callback callback, void * context);
228	386	void released(SurfaceRelease);	385	void released(SurfaceRelease);
229	387	void released(StreamRelease);
230	388	void done_platform_operation(mir_platform_operation_callback, void* context);	386	void done_platform_operation(mir_platform_operation_callback, void* context);
231	389	bool validate_user_display_config(MirDisplayConfiguration const* config);	387	bool validate_user_display_config(MirDisplayConfiguration const* config);
232	390		388
233	391		389
234	=== modified file 'tests/acceptance-tests/staging/test_render_surface.cpp'
235	--- tests/acceptance-tests/staging/test_render_surface.cpp 2016-12-12 21:58:01 +0000
236	+++ tests/acceptance-tests/staging/test_render_surface.cpp 2016-12-13 05:39:59 +0000
237	@@ -25,6 +25,7 @@
238	25	#include "mir_test_framework/using_stub_client_platform.h"	25	#include "mir_test_framework/using_stub_client_platform.h"
239	26	#include "mir/test/validity_matchers.h"	26	#include "mir/test/validity_matchers.h"
240	27		27
241			28	#include <atomic>
242	28	#include <gtest/gtest.h>	29	#include <gtest/gtest.h>
243	29	#include <gmock/gmock.h>	30	#include <gmock/gmock.h>
244	30		31
245	@@ -316,3 +317,47 @@
246	316	mir_render_surface_release(rs);	317	mir_render_surface_release(rs);
247	317	mir_connection_release(connection);	318	mir_connection_release(connection);
248	318	}	319	}
249			320
250			321	TEST_F(RenderSurfaceTest, buffer_stream_callback_isnt_called_after_render_surface_release)
251			322	{
252			323	auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
253			324
254			325	auto rs = mir_connection_create_render_surface_sync(
255			326	connection, logical_size.width.as_int(), logical_size.height.as_int());
256			327
257			328	auto determine_physical_size = [](MirRenderSurface* rs) -> geom::Size
258			329	{
259			330	int width = -1;
260			331	int height = -1;
261			332	mir_render_surface_get_size(rs, &width, &height);
262			333	return {width, height};
263			334	};
264			335	auto physical_size = determine_physical_size(rs);
265			336	auto bs = mir_render_surface_get_buffer_stream(
266			337	rs,
267			338	physical_size.width.as_int(), physical_size.height.as_int(),
268			339	mir_pixel_format_abgr_8888,
269			340	mir_buffer_usage_hardware);
270			341
271			342	EXPECT_THAT(bs, IsValid());
272			343
273			344	std::atomic<bool> called{false};
274			345	mir_buffer_stream_swap_buffers(
275			346	bs,
276			347	[](MirBufferStream, void called)
277			348	{
278			349	reinterpret_cast<std::atomic<bool>>(called) = true;
279			350	},
280			351	&called);
281			352
282			353	// Unavoidably racy; we want to test that the swap_buffers callback is never called
283			354	// after mir_render_surface_release returns.
284			355	mir_render_surface_release(rs);
285			356	bool called_post_release = called.load();
286			357
287			358	mir_connection_release(connection);
288			359
289			360	// We don't care whether or the swap_buffers callback was called, just that
290			361	// it didn't get called after mir_render_surface_release.
291			362	EXPECT_THAT(called, Eq(called_post_release));
292			363	}
293	319		364
294	=== modified file 'tests/mir_test/CMakeLists.txt'
295	--- tests/mir_test/CMakeLists.txt 2016-11-09 02:30:14 +0000
296	+++ tests/mir_test/CMakeLists.txt 2016-12-13 05:39:59 +0000
297	@@ -2,6 +2,10 @@
298	2	string (REPLACE " -flto " " " CMAKE_C_FLAGS ${CMAKE_C_FLAGS})	2	string (REPLACE " -flto " " " CMAKE_C_FLAGS ${CMAKE_C_FLAGS})
299	3	string (REPLACE " -flto " " " CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})	3	string (REPLACE " -flto " " " CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})
300	4		4
301			5	include_directories(
302			6	${PROJECT_SOURCE_DIR}/src/include/client
303			7	)
304			8
305	5	add_library(mir-public-test OBJECT	9	add_library(mir-public-test OBJECT
306	6	cross_process_action.cpp	10	cross_process_action.cpp
307	7	cross_process_sync.cpp	11	cross_process_sync.cpp
308	8		12
309	=== modified file 'tests/mir_test/validity_matchers.cpp'
310	--- tests/mir_test/validity_matchers.cpp 2015-06-25 03:00:08 +0000
311	+++ tests/mir_test/validity_matchers.cpp 2016-12-13 05:39:59 +0000
312	@@ -1,5 +1,6 @@
313	1	#include "mir/test/validity_matchers.h"	1	#include "mir/test/validity_matchers.h"
314	2	#include "mir_toolkit/mir_client_library.h"	2	#include "mir_toolkit/mir_client_library.h"
315			3	#include "mir_toolkit/mir_render_surface.h"
316	3		4
317	4	template<>	5	template<>
318	5	bool IsValidMatcher::MatchAndExplain(MirConnection* connection, MatchResultListener* listener) const	6	bool IsValidMatcher::MatchAndExplain(MirConnection* connection, MatchResultListener* listener) const
319	@@ -30,3 +31,33 @@
320	30	}	31	}
321	31	return true;	32	return true;
322	32	}	33	}
323			34
324			35	template<>
325			36	bool IsValidMatcher::MatchAndExplain(MirBufferStream* stream, MatchResultListener* listener) const
326			37	{
327			38	if (stream == nullptr)
328			39	{
329			40	return false;
330			41	}
331			42	if (!mir_buffer_stream_is_valid(stream))
332			43	{
333			44	*listener << "error is: " << mir_buffer_stream_get_error_message(stream);
334			45	return false;
335			46	}
336			47	return true;
337			48	}
338			49
339			50	template<>
340			51	bool IsValidMatcher::MatchAndExplain(MirRenderSurface* rs, MatchResultListener* listener) const
341			52	{
342			53	if (rs == nullptr)
343			54	{
344			55	return false;
345			56	}
346			57	if (!mir_render_surface_is_valid(rs))
347			58	{
348			59	*listener << "error is: " << mir_render_surface_get_error_message(rs);
349			60	return false;
350			61	}
351			62	return true;
352			63	}