Mir

Merge lp:~kdub/mir/add-sync-support into lp:~mir-team/mir/trunk

Proposed by Kevin DuBois
Status: Merged
Approved by: Alexandros Frantzis
Approved revision: no longer in the source branch.
Merged at revision: 501
Proposed branch: lp:~kdub/mir/add-sync-support
Merge into: lp:~mir-team/mir/trunk
Diff against target: 478 lines (+275/-15)
12 files modified
src/client/CMakeLists.txt (+1/-0)
src/client/android/android_driver_interpreter.h (+1/-1)
src/client/android/client_surface_interpreter.cpp (+1/-1)
src/client/android/client_surface_interpreter.h (+1/-1)
src/client/android/mir_native_window.cpp (+27/-4)
src/client/android/mir_native_window.h (+2/-1)
src/client/android/syncfence.cpp (+44/-0)
src/client/android/syncfence.h (+76/-0)
tests/unit-tests/client/CMakeLists.txt (+1/-0)
tests/unit-tests/client/test_android_native_window.cpp (+29/-5)
tests/unit-tests/client/test_android_syncfence.cpp (+91/-0)
tests/unit-tests/client/test_client_surface_interpreter.cpp (+1/-2)
To merge this branch: bzr merge lp:~kdub/mir/add-sync-support
Reviewer Review Type Date Requested Status
Alexandros Frantzis (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Alan Griffiths Abstain
Review via email: mp+153017@code.launchpad.net

Commit message

add sync driver support to the android client

fixes: https://bugs.launchpad.net/mir/+bug/1117841

Description of the change

android uses the 'sync' driver available on the android kernel for additional synchronization with the driver. This support was added in the upgrade to the native window type for android. The requirement on us is that we wait on the sync object (via ioctl) and that we close the sync object. After we do this, we get stable client accelerated rendering.

The sync driver is open source in the android kernel. However, the android kernel headers are not available as a package (correct me if I'm wrong :) ), so I scraped the ioctl value and put it in the source, like the android sources do.

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

PASSED: Continuous integration, rev:504
http://jenkins.qa.ubuntu.com/job/mir-ci/60/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/61//console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/mir-ci/60//rebuild/?

review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

243 + SyncFence(int fd);
244 +
245 + //override using real ioctl's with something else. useful for injecting mocks
246 + SyncFence(int fd, std::shared_ptr<IoctlWrapper> const& wrapper);

My preference (as stated in other MPs, too), is to not use default arguments or overloaded constructors as a way to provide default dependencies. The reasons are reduced design clarity (does SyncFence depend on IoctlWrapper or not?), and the introduction of different (even if trivially different) code paths, some of which are not as well tested as the others.

My suggestion is to keep only the "full" constructor and provide an IoctlWrapper implementation at SyncFence construction in mir_native_window.cpp.

225 +class IoctlWrapper
226 +{
233 +class AndroidFence
234 +{

Do we need the copy constructor and assignment operator for these?

252 + std::shared_ptr<IoctlWrapper> ioctl_wrapper;

shared_ptr const

466 +

Nitpick: Do we need this added space here?

review: Needs Fixing
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Also, I wonder if something like this would be a better fit than IoctlWrapper:

FenceFDWrapper
{
    virtual void sync() = 0;
    virtual void close() = 0;
};

FenceFDImpl : FenceFDWrapper
{
   FenceFDImpl(int fd) : fd{fd} {}

   void sync() { if (fd >= 0) ioctl(fd, SYNC_IOC_WAIT, &timeout); }
   void close() { close(fd) }
};

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> 243 + SyncFence(int fd);
> 244 +
> 245 + //override using real ioctl's with something else. useful for
> injecting mocks
> 246 + SyncFence(int fd, std::shared_ptr<IoctlWrapper> const& wrapper);
>
> My preference (as stated in other MPs, too), is to not use default arguments
> or overloaded constructors as a way to provide default dependencies. The
> reasons are reduced design clarity (does SyncFence depend on IoctlWrapper or
> not?), and the introduction of different (even if trivially different) code
> paths, some of which are not as well tested as the others.
>
> My suggestion is to keep only the "full" constructor and provide an
> IoctlWrapper implementation at SyncFence construction in
> mir_native_window.cpp.
>
> 225 +class IoctlWrapper
> 226 +{
> 233 +class AndroidFence
> 234 +{
>
> Do we need the copy constructor and assignment operator for these?
>
> 252 + std::shared_ptr<IoctlWrapper> ioctl_wrapper;
>
> shared_ptr const
>
> 466 +
>
> Nitpick: Do we need this added space here?

+1

Also, not directly aimed at this MP - why don't we have a FileHandle type instead of passing naked ints around?

review: Needs Fixing
Revision history for this message
Kevin DuBois (kdub) wrote :

> 243 + SyncFence(int fd);
> 244 +
> 245 + //override using real ioctl's with something else. useful for
> injecting mocks
> 246 + SyncFence(int fd, std::shared_ptr<IoctlWrapper> const& wrapper);
>
> My preference (as stated in other MPs, too), is to not use default arguments
> or overloaded constructors as a way to provide default dependencies. The
> reasons are reduced design clarity (does SyncFence depend on IoctlWrapper or
> not?), and the introduction of different (even if trivially different) code
> paths, some of which are not as well tested as the others.
>
> My suggestion is to keep only the "full" constructor and provide an
> IoctlWrapper implementation at SyncFence construction in
> mir_native_window.cpp.

I went with this suggestion over the FenceFDWrapper suggestion. I wanted to test right at the point that we call close/ioctl, so we can test how we handle invalid fd's directly.

>
> 225 +class IoctlWrapper
> 226 +{
> 233 +class AndroidFence
> 234 +{
>
> Do we need the copy constructor and assignment operator for these?

no, i have deleted copy/assign

>
> 252 + std::shared_ptr<IoctlWrapper> ioctl_wrapper;
>
> shared_ptr const
>
> 466 +
>
> Nitpick: Do we need this added space here?
addressed, (plus made the 'int fd' const)

Revision history for this message
Kevin DuBois (kdub) wrote :

>
> +1
>
> Also, not directly aimed at this MP - why don't we have a FileHandle type
> instead of passing naked ints around?

My intention with SyncFence was to wrap the FD as soon as we got it. SyncFence is more specialized than a generic type, but I think that's ok for now

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Looks good overall. Some style nitpicks: https://code.launchpad.net/~mir-team/mir/add-sync-support/+merge/153301

(It was quicker to fix the code than propose the fixes :))

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> Looks good overall. Some style nitpicks: https://code.launchpad.net/~mir-
> team/mir/add-sync-support/+merge/153301
>
> (It was quicker to fix the code than propose the fixes :))

With that tidy-up I'd be satisfied.

review: Abstain
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Looks good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/client/CMakeLists.txt'
2--- src/client/CMakeLists.txt 2013-03-13 11:24:59 +0000
3+++ src/client/CMakeLists.txt 2013-03-14 15:40:25 +0000
4@@ -59,6 +59,7 @@
5 android/android_client_platform.cpp
6 android/mir_native_window.cpp
7 android/client_surface_interpreter.cpp
8+ android/syncfence.cpp
9 ${CLIENT_SOURCES}
10 )
11 endif()
12
13=== modified file 'src/client/android/android_driver_interpreter.h'
14--- src/client/android/android_driver_interpreter.h 2013-03-12 15:27:46 +0000
15+++ src/client/android/android_driver_interpreter.h 2013-03-14 15:40:25 +0000
16@@ -31,7 +31,7 @@
17 {
18 public:
19 virtual ANativeWindowBuffer* driver_requests_buffer() = 0;
20- virtual void driver_returns_buffer(ANativeWindowBuffer*, int fence_fd) = 0;
21+ virtual void driver_returns_buffer(ANativeWindowBuffer*) = 0;
22 virtual void dispatch_driver_request_format(int format) = 0;
23 virtual int driver_requests_info(int key) const = 0;
24 protected:
25
26=== modified file 'src/client/android/client_surface_interpreter.cpp'
27--- src/client/android/client_surface_interpreter.cpp 2013-03-12 15:27:46 +0000
28+++ src/client/android/client_surface_interpreter.cpp 2013-03-14 15:40:25 +0000
29@@ -39,7 +39,7 @@
30
31 static void empty(MirSurface * /*surface*/, void * /*client_context*/)
32 {}
33-void mcla::ClientSurfaceInterpreter::driver_returns_buffer(ANativeWindowBuffer*, int /*fence_fd*/)
34+void mcla::ClientSurfaceInterpreter::driver_returns_buffer(ANativeWindowBuffer*)
35 {
36 mir_wait_for(surface.next_buffer(empty, NULL));
37 }
38
39=== modified file 'src/client/android/client_surface_interpreter.h'
40--- src/client/android/client_surface_interpreter.h 2013-03-12 15:27:46 +0000
41+++ src/client/android/client_surface_interpreter.h 2013-03-14 15:40:25 +0000
42@@ -35,7 +35,7 @@
43 explicit ClientSurfaceInterpreter(ClientSurface& surface);
44
45 ANativeWindowBuffer* driver_requests_buffer();
46- void driver_returns_buffer(ANativeWindowBuffer*, int fence_fd);
47+ void driver_returns_buffer(ANativeWindowBuffer*);
48 void dispatch_driver_request_format(int format);
49 int driver_requests_info(int key) const;
50 private:
51
52=== modified file 'src/client/android/mir_native_window.cpp'
53--- src/client/android/mir_native_window.cpp 2013-03-12 16:43:03 +0000
54+++ src/client/android/mir_native_window.cpp 2013-03-14 15:40:25 +0000
55@@ -18,12 +18,30 @@
56
57 #include "mir_native_window.h"
58 #include "android_driver_interpreter.h"
59+#include "syncfence.h"
60+
61+#include <unistd.h>
62+#include <sys/ioctl.h>
63
64 namespace mcl=mir::client;
65 namespace mcla=mir::client::android;
66
67 namespace
68 {
69+
70+class IoctlControl : public mcla::IoctlWrapper
71+{
72+public:
73+ int ioctl(int fd, unsigned long int request, int* timeout) const
74+ {
75+ return ::ioctl(fd, request, timeout);
76+ }
77+ int close(int fd) const
78+ {
79+ return ::close(fd);
80+ }
81+};
82+
83 static int query_static(const ANativeWindow* anw, int key, int* value);
84 static int perform_static(ANativeWindow* anw, int key, ...);
85 static int setSwapInterval_static (struct ANativeWindow* window, int interval);
86@@ -82,14 +100,18 @@
87 struct ANativeWindowBuffer* buffer)
88 {
89 auto self = static_cast<mcla::MirNativeWindow*>(window);
90- return self->queueBuffer(buffer, -1);
91+ auto ioctl_control = std::make_shared<IoctlControl>();
92+ auto fence = std::make_shared<mcla::SyncFence>(-1, ioctl_control);
93+ return self->queueBuffer(buffer, fence);
94 }
95
96 int queueBuffer_static(struct ANativeWindow* window,
97 struct ANativeWindowBuffer* buffer, int fence_fd)
98 {
99 auto self = static_cast<mcla::MirNativeWindow*>(window);
100- return self->queueBuffer(buffer, fence_fd);
101+ auto ioctl_control = std::make_shared<IoctlControl>();
102+ auto fence = std::make_shared<mcla::SyncFence>(fence_fd, ioctl_control);
103+ return self->queueBuffer(buffer, fence);
104
105 }
106
107@@ -146,9 +168,10 @@
108 return 0;
109 }
110
111-int mcla::MirNativeWindow::queueBuffer(struct ANativeWindowBuffer* buffer, int fence_fd)
112+int mcla::MirNativeWindow::queueBuffer(struct ANativeWindowBuffer* buffer, std::shared_ptr<mcla::AndroidFence> const& fence)
113 {
114- driver_interpreter->driver_returns_buffer(buffer, fence_fd);
115+ fence->wait();
116+ driver_interpreter->driver_returns_buffer(buffer);
117 return 0;
118 }
119
120
121=== modified file 'src/client/android/mir_native_window.h'
122--- src/client/android/mir_native_window.h 2013-03-12 16:43:03 +0000
123+++ src/client/android/mir_native_window.h 2013-03-14 15:40:25 +0000
124@@ -30,6 +30,7 @@
125 {
126 namespace android
127 {
128+class AndroidFence;
129 class AndroidDriverInterpreter;
130
131 class MirNativeWindow : public ANativeWindow
132@@ -40,7 +41,7 @@
133 int query(int key, int* value) const;
134 int perform(int key, va_list args );
135 int dequeueBuffer(struct ANativeWindowBuffer** buffer);
136- int queueBuffer(struct ANativeWindowBuffer* buffer, int fence_fd);
137+ int queueBuffer(struct ANativeWindowBuffer* buffer, std::shared_ptr<AndroidFence> const& fence);
138 private:
139
140 std::shared_ptr<AndroidDriverInterpreter> const driver_interpreter;
141
142=== added file 'src/client/android/syncfence.cpp'
143--- src/client/android/syncfence.cpp 1970-01-01 00:00:00 +0000
144+++ src/client/android/syncfence.cpp 2013-03-14 15:40:25 +0000
145@@ -0,0 +1,44 @@
146+/*
147+ * Copyright © 2013 Canonical Ltd.
148+ *
149+ * This program is free software: you can redistribute it and/or modify
150+ * it under the terms of the GNU General Public License version 3 as
151+ * published by the Free Software Foundation.
152+ *
153+ * This program is distributed in the hope that it will be useful,
154+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
155+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
156+ * GNU General Public License for more details.
157+ *
158+ * You should have received a copy of the GNU General Public License
159+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
160+ *
161+ * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
162+ */
163+
164+#include "syncfence.h"
165+
166+namespace mcla = mir::client::android;
167+
168+mcla::SyncFence::SyncFence(int fd, std::shared_ptr<IoctlWrapper> const& wrapper)
169+ : ioctl_wrapper(wrapper),
170+ fence_fd(fd)
171+{
172+}
173+
174+mcla::SyncFence::~SyncFence()
175+{
176+ if (fence_fd > 0)
177+ {
178+ ioctl_wrapper->close(fence_fd);
179+ }
180+}
181+
182+void mcla::SyncFence::wait()
183+{
184+ if (fence_fd > 0)
185+ {
186+ int timeout = -1;
187+ ioctl_wrapper->ioctl(fence_fd, SYNC_IOC_WAIT, &timeout);
188+ }
189+}
190
191=== added file 'src/client/android/syncfence.h'
192--- src/client/android/syncfence.h 1970-01-01 00:00:00 +0000
193+++ src/client/android/syncfence.h 2013-03-14 15:40:25 +0000
194@@ -0,0 +1,76 @@
195+/*
196+ * Copyright © 2013 Canonical Ltd.
197+ *
198+ * This program is free software: you can redistribute it and/or modify
199+ * it under the terms of the GNU General Public License version 3 as
200+ * published by the Free Software Foundation.
201+ *
202+ * This program is distributed in the hope that it will be useful,
203+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
204+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
205+ * GNU General Public License for more details.
206+ *
207+ * You should have received a copy of the GNU General Public License
208+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
209+ *
210+ * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
211+ */
212+#ifndef MIR_CLIENT_ANDROID_SYNCFENCE_H_
213+#define MIR_CLIENT_ANDROID_SYNCFENCE_H_
214+
215+#include <memory>
216+
217+#define SYNC_IOC_WAIT 0x40043E00
218+
219+namespace mir
220+{
221+namespace client
222+{
223+namespace android
224+{
225+
226+class IoctlWrapper
227+{
228+public:
229+ virtual ~IoctlWrapper() {}
230+ virtual int ioctl(int fd, unsigned long int request, int* timeout) const = 0;
231+ virtual int close(int fd) const = 0;
232+
233+protected:
234+ IoctlWrapper() = default;
235+ IoctlWrapper(IoctlWrapper const&) = delete;
236+ IoctlWrapper& operator=(IoctlWrapper const&) = delete;
237+};
238+
239+class AndroidFence
240+{
241+public:
242+ virtual ~AndroidFence() {}
243+ virtual void wait() = 0;
244+
245+protected:
246+ AndroidFence() = default;
247+ AndroidFence(AndroidFence const&) = delete;
248+ AndroidFence& operator=(AndroidFence const&) = delete;
249+};
250+
251+class SyncFence : public AndroidFence
252+{
253+public:
254+ SyncFence(int fd, std::shared_ptr<IoctlWrapper> const& wrapper);
255+ ~SyncFence();
256+
257+ void wait();
258+
259+private:
260+ SyncFence(SyncFence const&) = delete;
261+ SyncFence& operator=(SyncFence const&) = delete;
262+
263+ std::shared_ptr<IoctlWrapper> const ioctl_wrapper;
264+ int const fence_fd;
265+};
266+
267+}
268+}
269+}
270+#endif /* MIR_CLIENT_ANDROID_SYNCFENCE_H_ */
271
272=== modified file 'tests/unit-tests/client/CMakeLists.txt'
273--- tests/unit-tests/client/CMakeLists.txt 2013-03-12 16:43:03 +0000
274+++ tests/unit-tests/client/CMakeLists.txt 2013-03-14 15:40:25 +0000
275@@ -14,6 +14,7 @@
276 ${CMAKE_CURRENT_SOURCE_DIR}/test_android_client_depository.cpp
277 ${CMAKE_CURRENT_SOURCE_DIR}/test_android_client_platform.cpp
278 ${CMAKE_CURRENT_SOURCE_DIR}/test_client_surface_interpreter.cpp
279+ ${CMAKE_CURRENT_SOURCE_DIR}/test_android_syncfence.cpp
280 )
281 endif()
282
283
284=== modified file 'tests/unit-tests/client/test_android_native_window.cpp'
285--- tests/unit-tests/client/test_android_native_window.cpp 2013-03-12 16:43:03 +0000
286+++ tests/unit-tests/client/test_android_native_window.cpp 2013-03-14 15:40:25 +0000
287@@ -19,6 +19,7 @@
288 #include "src/client/mir_client_surface.h"
289 #include "src/client/android/mir_native_window.h"
290 #include "src/client/android/android_driver_interpreter.h"
291+#include "src/client/android/syncfence.h"
292
293 #include <gmock/gmock.h>
294 #include <gtest/gtest.h>
295@@ -27,11 +28,17 @@
296
297 namespace
298 {
299+
300+struct MockSyncFence : public mcla::AndroidFence
301+{
302+ MOCK_METHOD0(wait, void());
303+};
304+
305 class MockAndroidDriverInterpreter : public mcla::AndroidDriverInterpreter
306 {
307 public:
308 MOCK_METHOD0(driver_requests_buffer, ANativeWindowBuffer*());
309- MOCK_METHOD2(driver_returns_buffer, void(ANativeWindowBuffer*, int));
310+ MOCK_METHOD1(driver_returns_buffer, void(ANativeWindowBuffer*));
311 MOCK_METHOD1(dispatch_driver_request_format, void(int));
312 MOCK_CONST_METHOD1(driver_requests_info, int(int));
313 };
314@@ -166,13 +173,30 @@
315 window->queueBuffer(window.get(), tmp, -1);
316 }
317
318-TEST_F(AndroidNativeWindowTest, native_window_queue_passes_buffer_and_fence_back)
319+TEST_F(AndroidNativeWindowTest, native_window_queue_waits_for_sync_before_returning_buffer)
320+{
321+ using namespace testing;
322+ ANativeWindowBuffer* tmp = nullptr;
323+ auto window = std::make_shared<mcla::MirNativeWindow>(mock_driver_interpreter);
324+
325+ auto mock_fence = std::make_shared<testing::NiceMock<MockSyncFence>>();
326+
327+ InSequence s;
328+ EXPECT_CALL(*mock_fence, wait())
329+ .Times(1);
330+ EXPECT_CALL(*mock_driver_interpreter, driver_returns_buffer(_))
331+ .Times(1);
332+
333+ window->queueBuffer(tmp, mock_fence);
334+}
335+
336+TEST_F(AndroidNativeWindowTest, native_window_queue_passes_buffer_back)
337 {
338 ANativeWindowBuffer buffer;
339 int fence_fd = 33;
340 std::shared_ptr<ANativeWindow> window = std::make_shared<mcla::MirNativeWindow>(mock_driver_interpreter);
341
342- EXPECT_CALL(*mock_driver_interpreter, driver_returns_buffer(&buffer, fence_fd))
343+ EXPECT_CALL(*mock_driver_interpreter, driver_returns_buffer(&buffer))
344 .Times(1);
345
346 window->queueBuffer(window.get(), &buffer, fence_fd);
347@@ -187,12 +211,12 @@
348 window->queueBuffer_DEPRECATED(window.get(), tmp);
349 }
350
351-TEST_F(AndroidNativeWindowTest, native_window_queue_deprecated_always_indicates_no_wait)
352+TEST_F(AndroidNativeWindowTest, native_window_queue_deprecated_passes_buffer_back)
353 {
354 ANativeWindowBuffer buffer;
355 std::shared_ptr<ANativeWindow> window = std::make_shared<mcla::MirNativeWindow>(mock_driver_interpreter);
356
357- EXPECT_CALL(*mock_driver_interpreter, driver_returns_buffer(&buffer, -1))
358+ EXPECT_CALL(*mock_driver_interpreter, driver_returns_buffer(&buffer))
359 .Times(1);
360
361 window->queueBuffer_DEPRECATED(window.get(), &buffer);
362
363=== added file 'tests/unit-tests/client/test_android_syncfence.cpp'
364--- tests/unit-tests/client/test_android_syncfence.cpp 1970-01-01 00:00:00 +0000
365+++ tests/unit-tests/client/test_android_syncfence.cpp 2013-03-14 15:40:25 +0000
366@@ -0,0 +1,91 @@
367+/*
368+ * Copyright © 2013 Canonical Ltd.
369+ *
370+ * This program is free software: you can redistribute it and/or modify
371+ * it under the terms of the GNU General Public License version 3 as
372+ * published by the Free Software Foundation.
373+ *
374+ * This program is distributed in the hope that it will be useful,
375+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
376+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
377+ * GNU General Public License for more details.
378+ *
379+ * You should have received a copy of the GNU General Public License
380+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
381+ *
382+ * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
383+ */
384+
385+#include "src/client/android/syncfence.h"
386+
387+#include <gmock/gmock.h>
388+#include <gtest/gtest.h>
389+
390+namespace mcla = mir::client::android;
391+
392+namespace
393+{
394+class MockIoctlWrapper : public mcla::IoctlWrapper
395+{
396+public:
397+ MOCK_CONST_METHOD3(ioctl, int(int, unsigned long int, int*));
398+ MOCK_CONST_METHOD1(close, int(int));
399+};
400+}
401+
402+class SyncFenceTest : public ::testing::Test
403+{
404+public:
405+ SyncFenceTest()
406+ : ioctl_mock(std::make_shared<testing::NiceMock<MockIoctlWrapper>>()),
407+ dummyfd(44), invalid_fd(-1)
408+ {
409+ }
410+
411+ std::shared_ptr<MockIoctlWrapper> const ioctl_mock;
412+ int const dummyfd;
413+ int const invalid_fd;
414+};
415+
416+TEST_F(SyncFenceTest, test_valid_fd_wait)
417+{
418+ using namespace testing;
419+
420+ int timeout_val;
421+ mcla::SyncFence fence(dummyfd, ioctl_mock);
422+ EXPECT_CALL(*ioctl_mock, ioctl(dummyfd, SYNC_IOC_WAIT, _))
423+ .Times(1)
424+ .WillOnce(DoAll(SaveArgPointee<2>(&timeout_val),
425+ Return(0)));
426+
427+ fence.wait();
428+ EXPECT_GT(0, timeout_val);
429+}
430+
431+TEST_F(SyncFenceTest, test_valid_fd_destruction_closes_fd)
432+{
433+ EXPECT_CALL(*ioctl_mock, close(dummyfd))
434+ .Times(1);
435+
436+ mcla::SyncFence fence(dummyfd, ioctl_mock);
437+}
438+
439+TEST_F(SyncFenceTest, test_invalid_fd_does_not_call_ioctl_on_wait)
440+{
441+ using namespace testing;
442+
443+ mcla::SyncFence fence(invalid_fd, ioctl_mock);
444+ EXPECT_CALL(*ioctl_mock, ioctl(_, _, _))
445+ .Times(0);
446+
447+ fence.wait();
448+}
449+
450+TEST_F(SyncFenceTest, test_invalid_fd_destruction_does_not_close_fd)
451+{
452+ using namespace testing;
453+ EXPECT_CALL(*ioctl_mock, close(_))
454+ .Times(0);
455+
456+ mcla::SyncFence fence(invalid_fd, ioctl_mock);
457+}
458
459=== modified file 'tests/unit-tests/client/test_client_surface_interpreter.cpp'
460--- tests/unit-tests/client/test_client_surface_interpreter.cpp 2013-03-12 15:27:46 +0000
461+++ tests/unit-tests/client/test_client_surface_interpreter.cpp 2013-03-14 15:40:25 +0000
462@@ -125,7 +125,6 @@
463 {
464 using namespace testing;
465 ANativeWindowBuffer buffer;
466- int fence_fd = -1;
467
468 MockMirSurface mock_surface{surf_params};
469 mcla::ClientSurfaceInterpreter interpreter(mock_surface);
470@@ -133,7 +132,7 @@
471 EXPECT_CALL(mock_surface, next_buffer(_,_))
472 .Times(1);
473
474- interpreter.driver_returns_buffer(&buffer, fence_fd);
475+ interpreter.driver_returns_buffer(&buffer);
476 }
477
478 /* format is an int that is set by the driver. these are not the HAL_PIXEL_FORMATS in android */

Subscribers

People subscribed via source and target branches