Merge lp:~kdub/mir/fix-1406725 into lp:mir

[Kevin DuBois]

Allow clients to call mir_buffer_stream_get_graphics_region() multiple times between mir_buffer_stream_swap_buffers() without side effects (eg, cache flushing) on android.

This fixes the main corruption that xmir was seeing in lp: #1406725. xmir was using this function to query buffer size, and this would have trashed the android's cpu cache on every size query.

fixes lp: #1406725

Allow clients to call mir_buffer_stream_get_graphics_region() multiple times between mir_buffer_stream_swap_buffers() without side effects (eg, cache flushing) on android.

This fixes the main corruption that xmir was seeing in lp: #1406725. xmir was using this function to query buffer size, and this would have trashed the android's cpu cache on every size query.

fixes lp: #1406725

note: This does not fix the corruption seen with egl_demo_client_flicker (and this corruption is rare, but possible with xmir). Further fixes are coming for lp: #1517205)

[Alberto Aguirre (albaguirre)]

LGTM.

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3106
http://jenkins.qa.ubuntu.com/job/mir-ci/5575/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4904
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3811
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4847
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/804/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1729
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1729/artifact/work/output/*zip*/output.zip
    None: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/804/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4848
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4848/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7426
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25222
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/805/console

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

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/2131/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4913
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3819/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4859
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/815/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/684
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/684/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/813
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/813/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4859
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4859/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7434
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25248
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/816/console

review: Needs Fixing (continuous-integration)

[Kevin DuBois (kdub)]

the errors all look like jenkins just didn't run the jobs.

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'include/client/mir_toolkit/mir_buffer_stream.h'
--- include/client/mir_toolkit/mir_buffer_stream.h	2015-09-15 18:56:59 +0000
+++ include/client/mir_toolkit/mir_buffer_stream.h	2015-11-18 13:10:34 +0000
@@ -152,8 +152,10 @@
 void mir_buffer_stream_swap_buffers_sync(MirBufferStream *buffer_stream);
 
 /**
- * Retrieve a buffer stream's graphics region, e.g. map the graphics buffer to main
- * memory.
+ * Retrieve a buffer stream's graphics region
+ *   \warning Depending on platform, this can map the graphics buffer each
+ *            time its called. The region remains mapped until
+ *            mir_buffer_stream_swap_buffers().
  *   \pre                          The buffer stream is valid
  *   \param [in] buffer stream     The buffer stream
  *   \param [out] graphics_region  Structure to be populated

=== modified file 'src/client/buffer_stream.cpp'
--- src/client/buffer_stream.cpp	2015-10-26 19:27:50 +0000
+++ src/client/buffer_stream.cpp	2015-11-18 13:10:34 +0000
@@ -537,7 +537,8 @@
     auto buffer = buffer_depository->current_buffer();
     std::unique_lock<decltype(mutex)> lock(mutex);
 
-    secured_region = buffer->secure_for_cpu_write();
+    if (!secured_region)
+        secured_region = buffer->secure_for_cpu_write();
     return secured_region;
 }
 

=== modified file 'tests/unit-tests/client/test_client_buffer_stream.cpp'
--- tests/unit-tests/client/test_client_buffer_stream.cpp	2015-10-21 13:05:45 +0000
+++ tests/unit-tests/client/test_client_buffer_stream.cpp	2015-11-18 13:10:34 +0000
@@ -481,6 +481,34 @@
     EXPECT_EQ(&expected_memory_region, bs.secure_for_cpu_write().get());
 }
 
+//lp: #1463873
+TEST_P(ClientBufferStream, maps_graphics_region_only_once_per_swapbuffers)
+{
+    MockClientBuffer mock_client_buffer(size);
+    ON_CALL(mock_factory, create_buffer(BufferPackageMatches(buffer_package),_,_))
+        .WillByDefault(Return(mt::fake_shared(mock_client_buffer)));
+    mcl::BufferStream bs(
+        nullptr, mock_protobuf_server, mode,
+        std::make_shared<StubClientPlatform>(mt::fake_shared(mock_factory)),
+        response, perf_report, "", size, nbuffers);
+    service_requests_for(bs, 2);
+
+    mcl::MemoryRegion first_expected_memory_region;
+    mcl::MemoryRegion second_expected_memory_region;
+    EXPECT_CALL(mock_client_buffer, secure_for_cpu_write())
+        .Times(2)
+        .WillOnce(Return(mt::fake_shared(first_expected_memory_region)))
+        .WillOnce(Return(mt::fake_shared(second_expected_memory_region)));
+    EXPECT_EQ(&first_expected_memory_region, bs.secure_for_cpu_write().get());
+    bs.secure_for_cpu_write();
+    bs.secure_for_cpu_write();
+
+    bs.request_and_wait_for_next_buffer();
+    EXPECT_EQ(&second_expected_memory_region, bs.secure_for_cpu_write().get());
+    bs.secure_for_cpu_write();
+    bs.secure_for_cpu_write();
+}
+
 TEST_P(ClientBufferStream, passes_name_to_perf_report)
 {
     NiceMock<MockPerfReport> mock_perf_report;