Mir

Merge lp:~alan-griffiths/mir/fix-1450797 into lp:mir

  1. fix-1450797
  2. Merge into development-branch
Proposed by Alan Griffiths
Status: Merged
Approved by: Daniel van Vugt
Approved revision: no longer in the source branch.
Merged at revision: 3228
Proposed branch: lp:~alan-griffiths/mir/fix-1450797
Merge into: lp:mir
Diff against target: 159 lines (+14/-14)
12 files modified
include/server/mir/input/surface.h (+1/-1)
include/test/mir/test/doubles/stub_surface.h (+1/-1)
src/server/input/surface_input_dispatcher.cpp (+1/-1)
src/server/scene/basic_surface.cpp (+2/-2)
src/server/scene/basic_surface.h (+1/-1)
tests/acceptance-tests/test_client_input.cpp (+1/-1)
tests/include/mir/test/doubles/mock_input_surface.h (+1/-1)
tests/include/mir/test/doubles/mock_surface.h (+1/-1)
tests/include/mir/test/doubles/stub_scene_surface.h (+1/-1)
tests/mir_test_framework/stub_surface.cpp (+1/-1)
tests/unit-tests/scene/test_basic_surface.cpp (+1/-1)
tests/unit-tests/scene/test_surface.cpp (+2/-2)
To merge this branch: bzr merge lp:~alan-griffiths/mir/fix-1450797
Reviewer Review Type Date Requested Status
Daniel van Vugt Approve
PS Jenkins bot (community) continuous-integration Approve
Kevin DuBois (community) Approve
Review via email: mp+281662@code.launchpad.net

Commit message

Use "MirEvent const*" in the public API

Description of the change

Use "MirEvent const*" in the public API

To post a comment you must log in.
Revision history for this message
Kevin DuBois (kdub) wrote :

alright by me, although not sure why the change is needed.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:3203
http://jenkins.qa.ubuntu.com/job/mir-ci/5941/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5440
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4347
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5396
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-xenial-touch/238
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/265
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/265/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/265
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/265/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5393
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5393/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7887
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26434
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/234
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/234/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-xenial-touch/92
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26436

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

review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Server ABI needs bumping, as I think this will change the mangled name of the consume symbol:

9 - virtual void consume(MirEvent const& event) = 0;
10 + virtual void consume(MirEvent const* event) = 0;

Although you're only changing the interface, existing external callers of the function won't be able to find it any more without rebuilding.

So "Approved" pending a mirserver ABI bump.

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

It isn't strongly motivated. The bug report gives the rationale - which has to be set against it being an API change.

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

This could land right now, so long as there's evidence we have a server ABI bump coming before we branch 0.19.

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

https://code.launchpad.net/~vanvugt/mir/server-37/+merge/281816

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cmake/MirCommon.cmake'
2=== modified file 'include/server/mir/input/surface.h'
3--- include/server/mir/input/surface.h 2015-10-20 04:57:13 +0000
4+++ include/server/mir/input/surface.h 2016-01-05 16:45:28 +0000
5@@ -53,7 +53,7 @@
6 virtual std::shared_ptr<input::InputChannel> input_channel() const = 0;
7 virtual std::shared_ptr<graphics::CursorImage> cursor_image() const = 0;
8 virtual InputReceptionMode reception_mode() const = 0;
9- virtual void consume(MirEvent const& event) = 0;
10+ virtual void consume(MirEvent const* event) = 0;
11
12 protected:
13 Surface() = default;
14
15=== modified file 'include/test/mir/test/doubles/stub_surface.h'
16--- include/test/mir/test/doubles/stub_surface.h 2015-10-20 04:57:13 +0000
17+++ include/test/mir/test/doubles/stub_surface.h 2016-01-05 16:45:28 +0000
18@@ -47,7 +47,7 @@
19 geometry::Point top_left() const override;
20 geometry::Rectangle input_bounds() const override;
21 bool input_area_contains(geometry::Point const& point) const override;
22- void consume(MirEvent const& event) override;
23+ void consume(MirEvent const* event) override;
24 void set_alpha(float alpha) override;
25 void set_orientation(MirOrientation orientation) override;
26 void set_transformation(glm::mat4 const&) override;
27
28=== modified file 'src/server/input/surface_input_dispatcher.cpp'
29--- src/server/input/surface_input_dispatcher.cpp 2015-11-30 21:08:18 +0000
30+++ src/server/input/surface_input_dispatcher.cpp 2016-01-05 16:45:28 +0000
31@@ -88,7 +88,7 @@
32 to_deliver.motion.pointer_coordinates[i].y -= sy;
33 }
34 }
35- surface->consume(to_deliver);
36+ surface->consume(&to_deliver);
37 }
38
39 }
40
41=== modified file 'src/server/scene/basic_surface.cpp'
42--- src/server/scene/basic_surface.cpp 2015-12-03 22:21:50 +0000
43+++ src/server/scene/basic_surface.cpp 2016-01-05 16:45:28 +0000
44@@ -841,9 +841,9 @@
45 return max_buf;
46 }
47
48-void ms::BasicSurface::consume(MirEvent const& event)
49+void ms::BasicSurface::consume(MirEvent const* event)
50 {
51- input_validator.validate_and_dispatch(event);
52+ input_validator.validate_and_dispatch(*event);
53 }
54
55 void ms::BasicSurface::set_keymap(xkb_rule_names const& rules)
56
57=== modified file 'src/server/scene/basic_surface.h'
58--- src/server/scene/basic_surface.h 2015-11-30 14:19:30 +0000
59+++ src/server/scene/basic_surface.h 2016-01-05 16:45:28 +0000
60@@ -109,7 +109,7 @@
61 geometry::Point top_left() const override;
62 geometry::Rectangle input_bounds() const override;
63 bool input_area_contains(geometry::Point const& point) const override;
64- void consume(MirEvent const& event) override;
65+ void consume(MirEvent const* event) override;
66 void set_alpha(float alpha) override;
67 void set_orientation(MirOrientation orientation) override;
68 void set_transformation(glm::mat4 const&) override;
69
70=== modified file 'tests/acceptance-tests/test_client_input.cpp'
71--- tests/acceptance-tests/test_client_input.cpp 2015-12-11 03:18:13 +0000
72+++ tests/acceptance-tests/test_client_input.cpp 2016-01-05 16:45:28 +0000
73@@ -590,7 +590,7 @@
74 auto key_event = mir::events::make_event(MirInputDeviceId{0}, 0ns, 0, mir_keyboard_action_down, 0, KEY_M,
75 mir_input_event_modifier_none);
76
77- server.the_shell()->focused_surface()->consume(*key_event);
78+ server.the_shell()->focused_surface()->consume(key_event.get());
79
80 first_client.all_events_received.wait_for_at_most_seconds(2);
81 }
82
83=== modified file 'tests/include/mir/test/doubles/mock_input_surface.h'
84--- tests/include/mir/test/doubles/mock_input_surface.h 2015-02-22 07:46:25 +0000
85+++ tests/include/mir/test/doubles/mock_input_surface.h 2016-01-05 16:45:28 +0000
86@@ -39,7 +39,7 @@
87 MOCK_CONST_METHOD0(input_channel, std::shared_ptr<input::InputChannel>());
88 MOCK_CONST_METHOD0(cursor_image, std::shared_ptr<graphics::CursorImage>());
89 MOCK_CONST_METHOD0(reception_mode, input::InputReceptionMode());
90- MOCK_METHOD1(consume, void(MirEvent const&));
91+ MOCK_METHOD1(consume, void(MirEvent const*));
92 };
93
94 }
95
96=== modified file 'tests/include/mir/test/doubles/mock_surface.h'
97--- tests/include/mir/test/doubles/mock_surface.h 2015-06-18 02:46:16 +0000
98+++ tests/include/mir/test/doubles/mock_surface.h 2016-01-05 16:45:28 +0000
99@@ -71,7 +71,7 @@
100 MOCK_METHOD2(configure, int(MirSurfaceAttrib, int));
101 MOCK_METHOD1(add_observer, void(std::shared_ptr<scene::SurfaceObserver> const&));
102 MOCK_METHOD1(remove_observer, void(std::weak_ptr<scene::SurfaceObserver> const&));
103- MOCK_METHOD1(consume, void(MirEvent const&));
104+ MOCK_METHOD1(consume, void(MirEvent const*));
105
106 MOCK_CONST_METHOD0(primary_buffer_stream, std::shared_ptr<frontend::BufferStream>());
107 MOCK_METHOD1(set_streams, void(std::list<scene::StreamInfo> const&));
108
109=== modified file 'tests/include/mir/test/doubles/stub_scene_surface.h'
110--- tests/include/mir/test/doubles/stub_scene_surface.h 2015-10-20 04:57:13 +0000
111+++ tests/include/mir/test/doubles/stub_scene_surface.h 2016-01-05 16:45:28 +0000
112@@ -84,7 +84,7 @@
113 void remove_observer(std::weak_ptr<scene::SurfaceObserver> const&) override {}
114
115 void set_reception_mode(input::InputReceptionMode mode) override { input_mode = mode; }
116- void consume(MirEvent const&) override {}
117+ void consume(MirEvent const*) override {}
118
119 void set_cursor_image(std::shared_ptr<graphics::CursorImage> const& /* image */) {}
120 std::shared_ptr<graphics::CursorImage> cursor_image() const { return {}; }
121
122=== modified file 'tests/mir_test_framework/stub_surface.cpp'
123--- tests/mir_test_framework/stub_surface.cpp 2015-10-20 04:57:13 +0000
124+++ tests/mir_test_framework/stub_surface.cpp 2016-01-05 16:45:28 +0000
125@@ -100,7 +100,7 @@
126 return false;
127 }
128
129-void mtd::StubSurface::consume(MirEvent const& /*event*/)
130+void mtd::StubSurface::consume(MirEvent const* /*event*/)
131 {
132 }
133
134
135=== modified file 'tests/unit-tests/scene/test_basic_surface.cpp'
136--- tests/unit-tests/scene/test_basic_surface.cpp 2015-12-03 22:21:50 +0000
137+++ tests/unit-tests/scene/test_basic_surface.cpp 2016-01-05 16:45:28 +0000
138@@ -706,7 +706,7 @@
139
140 EXPECT_CALL(mock_sender, send_event(_,_));
141
142- surface.consume(*mev::make_event(mir_prompt_session_state_started));
143+ surface.consume(mev::make_event(mir_prompt_session_state_started).get());
144 }
145
146 TEST_F(BasicSurfaceTest, observer_can_trigger_state_change_within_notification)
147
148=== modified file 'tests/unit-tests/scene/test_surface.cpp'
149--- tests/unit-tests/scene/test_surface.cpp 2015-12-11 03:18:13 +0000
150+++ tests/unit-tests/scene/test_surface.cpp 2016-01-05 16:45:28 +0000
151@@ -417,6 +417,6 @@
152 EXPECT_CALL(mock_sender, send_event(mt::MirKeyboardEventMatches(*key_event), _)).Times(1);
153 EXPECT_CALL(mock_sender, send_event(mt::MirTouchEventMatches(*touch_event), _)).Times(1);
154
155- surface.consume(*key_event);
156- surface.consume(*touch_event);
157+ surface.consume(key_event.get());
158+ surface.consume(touch_event.get());
159 }

Subscribers

People subscribed via source and target branches