Merge lp:~afrantzis/mir/android-fix-memory-error into lp:~mir-team/mir/trunk

[Alexandros Frantzis]

android: Fix memory error

android: Fix memory error

Note: The conclusions below are based solely on experimentation on the Nexus 4.
Kevin could you please validate that this works properly on the Galaxy Nexus, too?

The native window associated with an EGLSurface is deallocated automatically
by the system when the EGLSurface is destroyed. Deallocating it manually
leads to memory errors and causes all kinds of interesting side effects.

One of these side effects seems to be that we can't create and destroy an
AndroidDisplay multiple times in a process, which forced us to create the display
statically in some integration tests. Since the error is now fixed, we can
remove the workaround and create the AndroidDisplay normally.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:502
http://jenkins.qa.ubuntu.com/job/mir-ci/81/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/82/console
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/82/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

i found something strange was going on yesterday with this bit of code too... will try out this branch

[Kevin DuBois (kdub)]

okay, this is a weird problem that points out the problem of using android types. We pull then nativewindow type over hybris using a c-type, which casts away the ability to use the FramebufferNativeWindow's destructors. If we cast back to this type, we have to use FramebufferNativeWindow's private destructor, which isn't possible to pull over hybris.
*sigh*

you're right that we can't manage this object's deletion with our std::shared_ptr... but we also don't have a route to free up the framebuffers. (other than the process dying, where the kernel will free them up)

long story short.... this problem hits 'the perfect storm' of hybris limitations and silly android classes.
I just ported the annoying class over into our 3rd party, hope to mp, kill the strange dependency, and make multithreaded compositor landable for android.

I'm going to disapprove, in favor of porting the type over

review: Disapprove

[Kevin DuBois (kdub)]

https://code.launchpad.net/~mir-team/mir/android-eliminate-libui-dependency/+merge/153472

[Alexandros Frantzis (afrantzis)]

> you're right that we can't manage this object's deletion with our
> std::shared_ptr... but we also don't have a route to free up the framebuffers.
> (other than the process dying, where the kernel will free them up)

From my experimentation, I was given the impression that the native window type
is freed when the associated EGLSurface is destroyed. Is this not the case?

> long story short.... this problem hits 'the perfect storm' of hybris
> limitations and silly android classes.
> I just ported the annoying class over into our 3rd party, hope to mp, kill the
> strange dependency, and make multithreaded compositor landable for android.
>
> I'm going to disapprove, in favor of porting the type over

Always happy for a better way of doing things :)

=== modified file 'src/server/graphics/android/android_platform.cpp'
--- src/server/graphics/android/android_platform.cpp	2013-03-07 08:04:05 +0000
+++ src/server/graphics/android/android_platform.cpp	2013-03-14 11:29:30 +0000
@@ -40,10 +40,10 @@
     return std::make_shared<mga::AndroidBufferAllocator>();
 }
 
-/* note: gralloc seems to choke when this is opened/closed more than once per process. must investigate drivers further */
 std::shared_ptr<mg::Display> mga::AndroidPlatform::create_display()
 {
-    auto android_window = std::shared_ptr<ANativeWindow>(android_createDisplaySurface());
+    auto android_window = std::shared_ptr<ANativeWindow>(android_createDisplaySurface(),
+                                                         [](ANativeWindow*) {});
     if (!android_window.get())
         BOOST_THROW_EXCEPTION(std::runtime_error("could not open FB window"));
     auto window = std::make_shared<mga::AndroidFramebufferWindow> (android_window);

=== modified file 'tests/integration-tests/graphics/android/test_buffer_integration.cpp'
--- tests/integration-tests/graphics/android/test_buffer_integration.cpp	2013-03-13 07:41:46 +0000
+++ tests/integration-tests/graphics/android/test_buffer_integration.cpp	2013-03-14 11:29:30 +0000
@@ -49,17 +49,12 @@
     static void SetUpTestCase()
     {
         ASSERT_FALSE(mtd::is_surface_flinger_running());
-        ASSERT_NO_THROW(
-        {
-            platform = mg::create_platform(std::make_shared<mg::NullDisplayReport>());
-            display = platform->create_display();
-        });
     }
 
     virtual void SetUp()
     {
-        ASSERT_TRUE(platform != NULL);
-        ASSERT_TRUE(display != NULL);
+        platform = mg::create_platform(std::make_shared<mg::NullDisplayReport>());
+        display = platform->create_display();
 
         auto buffer_initializer = std::make_shared<mg::NullBufferInitializer>();
         allocator = platform->create_buffer_allocator(buffer_initializer);
@@ -70,24 +65,17 @@
         buffer_properties = mc::BufferProperties{size, pf, mc::BufferUsage::software};
     }
 
-    md::glAnimationBasic gl_animation;
+    std::shared_ptr<mg::Platform> platform;
+    std::shared_ptr<mg::Display> display;
     std::shared_ptr<mc::GraphicBufferAllocator> allocator;
     std::shared_ptr<mc::SwapperFactory> strategy;
+    md::glAnimationBasic gl_animation;
     geom::Size size;
     geom::PixelFormat pf;
     mc::BufferProperties buffer_properties;
     mtd::TestGrallocMapper sw_renderer;
-
-    /* note about display: android drivers seem to only be able to open fb once
-       per process (gralloc's framebuffer_close() doesn't seem to work). once we
-       figure out why, we can put display in the test fixture */
-    static std::shared_ptr<mg::Platform> platform;
-    static std::shared_ptr<mg::Display> display;
 };
 
-std::shared_ptr<mg::Display> AndroidBufferIntegration::display;
-std::shared_ptr<mg::Platform> AndroidBufferIntegration::platform;
-
 }
 
 TEST_F(AndroidBufferIntegration, post_does_not_throw)