Mir

Merge lp:~albaguirre/mir/fix-1521795 into lp:mir

  1. fix-1521795
  2. Merge into development-branch
Proposed by Alberto Aguirre
Status: Merged
Approved by: Alberto Aguirre
Approved revision: no longer in the source branch.
Merged at revision: 3165
Proposed branch: lp:~albaguirre/mir/fix-1521795
Merge into: lp:mir
Diff against target: 230 lines (+96/-2)
11 files modified
include/server/mir/scene/null_surface_observer.h (+1/-0)
include/server/mir/scene/surface_observer.h (+1/-0)
include/test/mir/test/doubles/stub_cursor_image.h (+42/-0)
src/include/server/mir/scene/surface_observers.h (+1/-0)
src/server/input/cursor_controller.cpp (+4/-0)
src/server/scene/basic_surface.cpp (+11/-2)
src/server/scene/legacy_surface_change_notification.cpp (+4/-0)
src/server/scene/legacy_surface_change_notification.h (+1/-0)
src/server/scene/null_surface_observer.cpp (+1/-0)
src/server/symbols.map (+1/-0)
tests/unit-tests/scene/test_basic_surface.cpp (+29/-0)
To merge this branch: bzr merge lp:~albaguirre/mir/fix-1521795
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Kevin DuBois (community) Approve
Alan Griffiths Approve
Review via email: mp+279218@code.launchpad.net

Commit message

Add notification for cursor image removal.

Avoid creating references to null pointers when notifying observers when a cursor image has been removed from a surface.
Instead add an explicit call in SurfaceObserver to notify about removal.

Description of the change

Add notification for cursor image removal.

Avoid creating references to null pointers when notifying observers when a cursor image has been removed from a surface.
Instead add an explicit call in SurfaceObserver to notify about removal.

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

FAILED: Continuous integration, rev:3157
http://jenkins.qa.ubuntu.com/job/mir-ci/5740/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5141
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4047/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5089
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/69
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/69/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/69
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/69/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5089
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5089/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7619
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25694

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:3157
http://jenkins.qa.ubuntu.com/job/mir-ci/5741/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5145
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4051
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5093
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/70
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/70/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/70/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5093
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5093/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7623
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25699

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

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

xenial-i386 failure doesn't seem related:

Do you want to ignore this warning and proceed anyway?
To continue, enter "Yes"; to abort, enter "No": Abort.
E: pbuilder-satisfydepends failed.
I: Copying back the cached apt archive contents
I: unmounting /var/lib/jenkins/slaves/prodstack-worker-med-2/workspace/mir-xenial-i386-ci/work/results filesystem
Connection Timeout: disconnecting client after 300.0 seconds
I: unmounting dev/pts filesystem
I: unmounting run/shm filesystem
I: unmounting proc filesystem
I: cleaning the build env
I: removing directory /var/cache/pbuilder/build//13162 and its subdirectories
bzr: ERROR: The build failed.

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

lgtm

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

PASSED: Continuous integration, rev:3157
http://jenkins.qa.ubuntu.com/job/mir-ci/5747/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5154
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4060
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5102
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-xenial-touch/77/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/76
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/76/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/76
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/76/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5102
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5102/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7632
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25724
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/75/console

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

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

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/2213/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5163
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4069
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5111/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-autolanding/47
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-autolanding/47/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-autolanding/47
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-autolanding/47/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5111
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5111/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7641/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25740

review: Needs Fixing (continuous-integration)
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

^---Network hiccup - let's try again.

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/server/mir/scene/null_surface_observer.h'
2--- include/server/mir/scene/null_surface_observer.h 2015-06-17 05:20:42 +0000
3+++ include/server/mir/scene/null_surface_observer.h 2015-12-02 02:02:47 +0000
4@@ -44,6 +44,7 @@
5 void client_surface_close_requested() override;
6 void keymap_changed(xkb_rule_names const& names) override;
7 void renamed(char const* name) override;
8+ void cursor_image_removed() override;
9
10 protected:
11 NullSurfaceObserver(NullSurfaceObserver const&) = delete;
12
13=== modified file 'include/server/mir/scene/surface_observer.h'
14--- include/server/mir/scene/surface_observer.h 2015-06-17 05:20:42 +0000
15+++ include/server/mir/scene/surface_observer.h 2015-12-02 02:02:47 +0000
16@@ -56,6 +56,7 @@
17 virtual void client_surface_close_requested() = 0;
18 virtual void keymap_changed(xkb_rule_names const& names) = 0;
19 virtual void renamed(char const* name) = 0;
20+ virtual void cursor_image_removed() = 0;
21
22 protected:
23 SurfaceObserver() = default;
24
25=== added file 'include/test/mir/test/doubles/stub_cursor_image.h'
26--- include/test/mir/test/doubles/stub_cursor_image.h 1970-01-01 00:00:00 +0000
27+++ include/test/mir/test/doubles/stub_cursor_image.h 2015-12-02 02:02:47 +0000
28@@ -0,0 +1,42 @@
29+/*
30+ * Copyright © 2015 Canonical Ltd.
31+ *
32+ * This program is free software: you can redistribute it and/or modify it
33+ * under the terms of the GNU General Public License version 3,
34+ * as published by the Free Software Foundation.
35+ *
36+ * This program is distributed in the hope that it will be useful,
37+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
38+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
39+ * GNU General Public License for more details.
40+ *
41+ * You should have received a copy of the GNU General Public License
42+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
43+ *
44+ * Authored by: Alberto Aguirre <alberto.aguirre@canonical.com>
45+ */
46+
47+#ifndef MIR_TEST_DOUBLES_STUB_CURSOR_IMAGE_H_
48+#define MIR_TEST_DOUBLES_STUB_CURSOR_IMAGE_H_
49+
50+#include "mir/graphics/cursor_image.h"
51+
52+namespace mir
53+{
54+namespace test
55+{
56+namespace doubles
57+{
58+
59+struct StubCursorImage : public mir::graphics::CursorImage
60+{
61+ void const* as_argb_8888() const override { return nullptr; }
62+ mir::geometry::Size size() const override { return geom::Size{16, 16}; }
63+ mir::geometry::Displacement hotspot() const override { return geom::Displacement{0, 0}; }
64+};
65+
66+}
67+}
68+} // namespace mir
69+
70+#endif /* MIR_TEST_DOUBLES_STUB_CURSOR_H_ */
71
72=== modified file 'src/include/server/mir/scene/surface_observers.h'
73--- src/include/server/mir/scene/surface_observers.h 2015-05-19 21:34:34 +0000
74+++ src/include/server/mir/scene/surface_observers.h 2015-12-02 02:02:47 +0000
75@@ -46,6 +46,7 @@
76 void client_surface_close_requested() override;
77 void keymap_changed(xkb_rule_names const& names) override;
78 void renamed(char const*) override;
79+ void cursor_image_removed() override;
80 };
81
82 }
83
84=== modified file 'src/server/input/cursor_controller.cpp'
85--- src/server/input/cursor_controller.cpp 2015-11-27 18:38:04 +0000
86+++ src/server/input/cursor_controller.cpp 2015-12-02 02:02:47 +0000
87@@ -87,6 +87,10 @@
88 {
89 cursor_controller->update_cursor_image();
90 }
91+ void cursor_image_removed() override
92+ {
93+ cursor_controller->update_cursor_image();
94+ }
95 void orientation_set_to(MirOrientation /* orientation */) override
96 {
97 // No need to update cursor for orientation property change alone.
98
99=== modified file 'src/server/scene/basic_surface.cpp'
100--- src/server/scene/basic_surface.cpp 2015-10-20 04:57:13 +0000
101+++ src/server/scene/basic_surface.cpp 2015-12-02 02:02:47 +0000
102@@ -124,6 +124,12 @@
103 { observer->renamed(name); });
104 }
105
106+void ms::SurfaceObservers::cursor_image_removed()
107+{
108+ for_each([](std::shared_ptr<SurfaceObserver> const& observer)
109+ { observer->cursor_image_removed(); });
110+}
111+
112 struct ms::CursorStreamImageAdapter
113 {
114 CursorStreamImageAdapter(ms::BasicSurface &surface)
115@@ -613,8 +619,11 @@
116 cursor_image_ = image;
117 }
118
119- observers.cursor_image_set_to(*image);
120-}
121+ if (image)
122+ observers.cursor_image_set_to(*image);
123+ else
124+ observers.cursor_image_removed();
125+}
126
127 std::shared_ptr<mg::CursorImage> ms::BasicSurface::cursor_image() const
128 {
129
130=== modified file 'src/server/scene/legacy_surface_change_notification.cpp'
131--- src/server/scene/legacy_surface_change_notification.cpp 2015-09-18 15:37:25 +0000
132+++ src/server/scene/legacy_surface_change_notification.cpp 2015-12-02 02:02:47 +0000
133@@ -76,6 +76,10 @@
134 {
135 }
136
137+void ms::LegacySurfaceChangeNotification::cursor_image_removed()
138+{
139+}
140+
141 void ms::LegacySurfaceChangeNotification::reception_mode_set_to(mi::InputReceptionMode /*mode*/)
142 {
143 notify_scene_change();
144
145=== modified file 'src/server/scene/legacy_surface_change_notification.h'
146--- src/server/scene/legacy_surface_change_notification.h 2015-06-17 05:20:42 +0000
147+++ src/server/scene/legacy_surface_change_notification.h 2015-12-02 02:02:47 +0000
148@@ -50,6 +50,7 @@
149 void client_surface_close_requested() override;
150 void keymap_changed(xkb_rule_names const& names) override;
151 void renamed(char const*) override;
152+ void cursor_image_removed() override;
153
154 private:
155 std::function<void()> const notify_scene_change;
156
157=== modified file 'src/server/scene/null_surface_observer.cpp'
158--- src/server/scene/null_surface_observer.cpp 2015-06-17 05:20:42 +0000
159+++ src/server/scene/null_surface_observer.cpp 2015-12-02 02:02:47 +0000
160@@ -34,3 +34,4 @@
161 void ms::NullSurfaceObserver::client_surface_close_requested() {}
162 void ms::NullSurfaceObserver::keymap_changed(xkb_rule_names const& /* names */) {}
163 void ms::NullSurfaceObserver::renamed(char const*) {}
164+void ms::NullSurfaceObserver::cursor_image_removed() {}
165
166=== modified file 'src/server/symbols.map'
167--- src/server/symbols.map 2015-11-27 14:57:46 +0000
168+++ src/server/symbols.map 2015-12-02 02:02:47 +0000
169@@ -102,6 +102,7 @@
170 mir::scene::NullSurfaceObserver::alpha_set_to*;
171 mir::scene::NullSurfaceObserver::attrib_changed*;
172 mir::scene::NullSurfaceObserver::client_surface_close_requested*;
173+ mir::scene::NullSurfaceObserver::cursor_image_removed*;
174 mir::scene::NullSurfaceObserver::cursor_image_set_to*;
175 mir::scene::NullSurfaceObserver::frame_posted*;
176 mir::scene::NullSurfaceObserver::hidden_set_to*;
177
178=== modified file 'tests/unit-tests/scene/test_basic_surface.cpp'
179--- tests/unit-tests/scene/test_basic_surface.cpp 2015-10-07 12:06:14 +0000
180+++ tests/unit-tests/scene/test_basic_surface.cpp 2015-12-02 02:02:47 +0000
181@@ -27,6 +27,7 @@
182 #include "mir/scene/null_surface_observer.h"
183 #include "mir/events/event_builders.h"
184
185+#include "mir/test/doubles/stub_cursor_image.h"
186 #include "mir/test/doubles/mock_buffer_stream.h"
187 #include "mir/test/doubles/mock_input_sender.h"
188 #include "mir/test/doubles/stub_input_sender.h"
189@@ -67,6 +68,8 @@
190 MOCK_METHOD1(hidden_set_to, void(bool));
191 MOCK_METHOD1(renamed, void(char const*));
192 MOCK_METHOD0(client_surface_close_requested, void());
193+ MOCK_METHOD1(cursor_image_set_to, void(mir::graphics::CursorImage const& image));
194+ MOCK_METHOD0(cursor_image_removed, void());
195 };
196
197 void post_a_frame(ms::BasicSurface& surface)
198@@ -661,6 +664,32 @@
199 INSTANTIATE_TEST_CASE_P(SurfaceFocusAttributeTest, BasicSurfaceAttributeTest,
200 ::testing::Values(surface_focus_test_parameters));
201
202+TEST_F(BasicSurfaceTest, notifies_about_cursor_image_change)
203+{
204+ using namespace testing;
205+
206+ NiceMock<MockSurfaceObserver> mock_surface_observer;
207+
208+ auto cursor_image = std::make_shared<mtd::StubCursorImage>();
209+ EXPECT_CALL(mock_surface_observer, cursor_image_set_to(_));
210+
211+ surface.add_observer(mt::fake_shared(mock_surface_observer));
212+ surface.set_cursor_image(cursor_image);
213+}
214+
215+TEST_F(BasicSurfaceTest, notifies_about_cursor_image_removal)
216+{
217+ using namespace testing;
218+
219+ NiceMock<MockSurfaceObserver> mock_surface_observer;
220+
221+ EXPECT_CALL(mock_surface_observer, cursor_image_set_to(_)).Times(0);
222+ EXPECT_CALL(mock_surface_observer, cursor_image_removed());
223+
224+ surface.add_observer(mt::fake_shared(mock_surface_observer));
225+ surface.set_cursor_image({});
226+}
227+
228 TEST_F(BasicSurfaceTest, calls_send_event_on_consume)
229 {
230 using namespace ::testing;

Subscribers

People subscribed via source and target branches