Mir

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

Proposed by Kevin DuBois
Status: Merged
Approved by: Alan Griffiths
Approved revision: 1274
Merged at revision: 1277
Proposed branch: lp:~kdub/mir/fix-1258056
Merge into: lp:mir
Diff against target: 54 lines (+20/-7)
2 files modified
src/platform/graphics/android/android_platform.cpp (+3/-7)
tests/unit-tests/graphics/android/test_android_platform.cpp (+17/-0)
To merge this branch: bzr merge lp:~kdub/mir/fix-1258056
Reviewer Review Type Date Requested Status
Daniel van Vugt Approve
Alan Griffiths Approve
PS Jenkins bot (community) continuous-integration Approve
Alexandros Frantzis (community) Approve
Review via email: mp+197906@code.launchpad.net

Commit message

The code path to initialize nested display was going and opening up the display hardware functions (but not using them). This was causing problems because it meant older devices that can't open up the display hardware in more than one process were choking, and newer devices required 'video' group permissions to work. This changes things so that nested display doesn't access any display hardware

(LP: #1258056)

Description of the change

The code path to initialize nested display was going and opening up the display hardware functions (but not using them). This was causing problems because it meant older devices that can't open up the display hardware in more than one process were choking, and newer devices required 'video' group permissions to work. This changes things so that nested display doesn't access any display hardware

(LP: #1258056)

To post a comment you must log in.
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

9 +mga::AndroidPlatform::AndroidPlatform(

We could just call the original AndroidPlatform(a,b) constructor directly, so adding a constructor that we want to remove seems unnecessary.

review: Needs Fixing
Revision history for this message
Michael Terry (mterry) wrote :

Just FYI, I've tested this on mako (worked great) and maguro (worked great). In each, it fixed a problem I was running into with nested mode.

lp:~kdub/mir/fix-1258056 updated
1274. By Kevin DuBois

remove second constructor from AndroidPlatform

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

Looks good.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

OK for now. Clearly something about the dependencies needs sorting out.

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/platform/graphics/android/android_platform.cpp'
--- src/platform/graphics/android/android_platform.cpp 2013-12-02 16:48:04 +0000
+++ src/platform/graphics/android/android_platform.cpp 2013-12-05 16:36:47 +0000
@@ -119,11 +119,7 @@
119119
120extern "C" std::shared_ptr<mg::NativePlatform> create_native_platform(std::shared_ptr<mg::DisplayReport> const& display_report)120extern "C" std::shared_ptr<mg::NativePlatform> create_native_platform(std::shared_ptr<mg::DisplayReport> const& display_report)
121{121{
122 auto should_use_fb_fallback = false;122 //TODO: remove nullptr parameter once platform classes are sorted.
123 auto buffer_initializer = std::make_shared<mg::NullBufferInitializer>();123 // mg::NativePlatform cannot create a display anyways, so it doesnt need a display builder
124 auto fb_allocator = std::make_shared<mga::AndroidGraphicBufferAllocator>(buffer_initializer);124 return std::make_shared<mga::AndroidPlatform>(nullptr, display_report);
125 auto display_resource_factory = std::make_shared<mga::ResourceFactory>();
126 auto display_builder = std::make_shared<mga::OutputBuilder>(
127 fb_allocator, display_resource_factory, display_report, should_use_fb_fallback);
128 return std::make_shared<mga::AndroidPlatform>(display_builder, display_report);
129}125}
130126
=== modified file 'tests/unit-tests/graphics/android/test_android_platform.cpp'
--- tests/unit-tests/graphics/android/test_android_platform.cpp 2013-12-02 16:48:04 +0000
+++ tests/unit-tests/graphics/android/test_android_platform.cpp 2013-12-05 16:36:47 +0000
@@ -17,10 +17,12 @@
17 */17 */
1818
19#include "mir/graphics/null_display_report.h"19#include "mir/graphics/null_display_report.h"
20#include "mir/graphics/native_platform.h"
20#include "mir/graphics/buffer_ipc_packer.h"21#include "mir/graphics/buffer_ipc_packer.h"
21#include "mir/options/program_option.h"22#include "mir/options/program_option.h"
22#include "src/platform/graphics/android/android_platform.h"23#include "src/platform/graphics/android/android_platform.h"
23#include "mir_test_doubles/mock_buffer.h"24#include "mir_test_doubles/mock_buffer.h"
25#include "mir_test_doubles/mock_android_hw.h"
24#include "mir_test_doubles/mock_buffer_packer.h"26#include "mir_test_doubles/mock_buffer_packer.h"
25#include "mir_test_doubles/mock_display_report.h"27#include "mir_test_doubles/mock_display_report.h"
26#include "mir_test_doubles/stub_display_builder.h"28#include "mir_test_doubles/stub_display_builder.h"
@@ -121,3 +123,18 @@
121123
122 EXPECT_EQ(EGL_DEFAULT_DISPLAY, platform.egl_native_display());124 EXPECT_EQ(EGL_DEFAULT_DISPLAY, platform.egl_native_display());
123}125}
126
127TEST(NestedPlatformCreation, doesnt_access_display_hardware)
128{
129 using namespace testing;
130
131 mtd::HardwareAccessMock hwaccess;
132 mtd::MockDisplayReport stub_report;
133
134 EXPECT_CALL(hwaccess, hw_get_module(StrEq(HWC_HARDWARE_MODULE_ID), _))
135 .Times(0);
136 EXPECT_CALL(hwaccess, hw_get_module(StrEq(GRALLOC_HARDWARE_MODULE_ID), _))
137 .Times(AtMost(1));
138
139 auto platform = mg::create_native_platform(mt::fake_shared(stub_report));
140}

Subscribers

People subscribed via source and target branches