Mir

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

fix-1521795
Merge into development-branch

Proposed by Alberto Aguirre on 2015-12-02

Status:

Merged

Approved by:

Alberto Aguirre on 2015-12-02

Approved revision:

3157

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

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Approve on 2015-12-03
Kevin DuBois (community)			Approve on 2015-12-02
Alan Griffiths		2015-12-02	Approve on 2015-12-02
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.

PS Jenkins bot (ps-jenkins) wrote on 2015-12-02:

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

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-12-02:

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)

Alan Griffiths (alan-griffiths) wrote on 2015-12-02:

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

Kevin DuBois (kdub) wrote on 2015-12-02:

lgtm

review: Approve

PS Jenkins bot (ps-jenkins) wrote on 2015-12-02:

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

review: Approve (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-12-02:

review: Needs Fixing (continuous-integration)

Alberto Aguirre (albaguirre) wrote on 2015-12-02:

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

PS Jenkins bot (ps-jenkins) on 2015-12-03:

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Alberto Aguirre

Emanuele Antonio Faraone

Gerry Boland

Mir development team

 === modified file 'include/server/mir/scene/null_surface_observer.h'
 --- include/server/mir/scene/null_surface_observer.h	2015-06-17 05:20:42 +0000
 +++ include/server/mir/scene/null_surface_observer.h	2015-12-02 02:02:47 +0000
@@ -44,6 +44,7 @@
      void client_surface_close_requested() override;
      void keymap_changed(xkb_rule_names const& names) override;
      void renamed(char const* name) override;
++    void cursor_image_removed() override;
  protected:
      NullSurfaceObserver(NullSurfaceObserver const&) = delete;
 === modified file 'include/server/mir/scene/surface_observer.h'
 --- include/server/mir/scene/surface_observer.h	2015-06-17 05:20:42 +0000
 +++ include/server/mir/scene/surface_observer.h	2015-12-02 02:02:47 +0000
@@ -56,6 +56,7 @@
      virtual void client_surface_close_requested() = 0;
      virtual void keymap_changed(xkb_rule_names const& names) = 0;
      virtual void renamed(char const* name) = 0;
++    virtual void cursor_image_removed() = 0;
  protected:
      SurfaceObserver() = default;
 === added file 'include/test/mir/test/doubles/stub_cursor_image.h'
 --- include/test/mir/test/doubles/stub_cursor_image.h	1970-01-01 00:00:00 +0000
 +++ include/test/mir/test/doubles/stub_cursor_image.h	2015-12-02 02:02:47 +0000
@@ -0,0 +1,42 @@
++/*
++ * Copyright © 2015 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify it
++ * under the terms of the GNU General Public License version 3,
++ * as published by the Free Software Foundation.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Alberto Aguirre <alberto.aguirre@canonical.com>
++ */
++
++#ifndef MIR_TEST_DOUBLES_STUB_CURSOR_IMAGE_H_
++#define MIR_TEST_DOUBLES_STUB_CURSOR_IMAGE_H_
++
++#include "mir/graphics/cursor_image.h"
++
++namespace mir
++{
++namespace test
++{
++namespace doubles
++{
++
++struct StubCursorImage : public mir::graphics::CursorImage
++{
++    void const* as_argb_8888() const override { return nullptr; }
++    mir::geometry::Size size() const override { return geom::Size{16, 16}; }
++    mir::geometry::Displacement hotspot() const override { return geom::Displacement{0, 0}; }
++};
++
++}
++}
++} // namespace mir
++
++#endif /* MIR_TEST_DOUBLES_STUB_CURSOR_H_ */
 === modified file 'src/include/server/mir/scene/surface_observers.h'
 --- src/include/server/mir/scene/surface_observers.h	2015-05-19 21:34:34 +0000
 +++ src/include/server/mir/scene/surface_observers.h	2015-12-02 02:02:47 +0000
@@ -46,6 +46,7 @@
      void client_surface_close_requested() override;
      void keymap_changed(xkb_rule_names const& names) override;
      void renamed(char const*) override;
++    void cursor_image_removed() override;
  };
+ }
 === modified file 'src/server/input/cursor_controller.cpp'
 --- src/server/input/cursor_controller.cpp	2015-11-27 18:38:04 +0000
 +++ src/server/input/cursor_controller.cpp	2015-12-02 02:02:47 +0000
@@ -87,6 +87,10 @@
+     {
          cursor_controller->update_cursor_image();
+     }
++    void cursor_image_removed() override
++    {
++        cursor_controller->update_cursor_image();
++    }
      void orientation_set_to(MirOrientation /* orientation */) override
+     {
          // No need to update cursor for orientation property change alone.
 === modified file 'src/server/scene/basic_surface.cpp'
 --- src/server/scene/basic_surface.cpp	2015-10-20 04:57:13 +0000
 +++ src/server/scene/basic_surface.cpp	2015-12-02 02:02:47 +0000
@@ -124,6 +124,12 @@
          { observer->renamed(name); });
+ }
++void ms::SurfaceObservers::cursor_image_removed()
++{
++    for_each([](std::shared_ptr<SurfaceObserver> const& observer)
++        { observer->cursor_image_removed(); });
++}
++
  struct ms::CursorStreamImageAdapter
+ {
      CursorStreamImageAdapter(ms::BasicSurface &surface)
@@ -613,8 +619,11 @@
          cursor_image_ = image;
+     }
--    observers.cursor_image_set_to(*image);
--}
++    if (image)
++        observers.cursor_image_set_to(*image);
++    else
++        observers.cursor_image_removed();
++}
  std::shared_ptr<mg::CursorImage> ms::BasicSurface::cursor_image() const
+ {
 === modified file 'src/server/scene/legacy_surface_change_notification.cpp'
 --- src/server/scene/legacy_surface_change_notification.cpp	2015-09-18 15:37:25 +0000
 +++ src/server/scene/legacy_surface_change_notification.cpp	2015-12-02 02:02:47 +0000
@@ -76,6 +76,10 @@
+ {
+ }
++void ms::LegacySurfaceChangeNotification::cursor_image_removed()
++{
++}
++
  void ms::LegacySurfaceChangeNotification::reception_mode_set_to(mi::InputReceptionMode /*mode*/)
+ {
      notify_scene_change();
 === modified file 'src/server/scene/legacy_surface_change_notification.h'
 --- src/server/scene/legacy_surface_change_notification.h	2015-06-17 05:20:42 +0000
 +++ src/server/scene/legacy_surface_change_notification.h	2015-12-02 02:02:47 +0000
@@ -50,6 +50,7 @@
      void client_surface_close_requested() override;
      void keymap_changed(xkb_rule_names const& names) override;
      void renamed(char const*) override;
++    void cursor_image_removed() override;
  private:
      std::function<void()> const notify_scene_change;
 === modified file 'src/server/scene/null_surface_observer.cpp'
 --- src/server/scene/null_surface_observer.cpp	2015-06-17 05:20:42 +0000
 +++ src/server/scene/null_surface_observer.cpp	2015-12-02 02:02:47 +0000
@@ -34,3 +34,4 @@
  void ms::NullSurfaceObserver::client_surface_close_requested() {}
  void ms::NullSurfaceObserver::keymap_changed(xkb_rule_names const& /* names */) {}
  void ms::NullSurfaceObserver::renamed(char const*) {}
++void ms::NullSurfaceObserver::cursor_image_removed() {}
 === modified file 'src/server/symbols.map'
 --- src/server/symbols.map	2015-11-27 14:57:46 +0000
 +++ src/server/symbols.map	2015-12-02 02:02:47 +0000
@@ -102,6 +102,7 @@
      mir::scene::NullSurfaceObserver::alpha_set_to*;
      mir::scene::NullSurfaceObserver::attrib_changed*;
      mir::scene::NullSurfaceObserver::client_surface_close_requested*;
++    mir::scene::NullSurfaceObserver::cursor_image_removed*;
      mir::scene::NullSurfaceObserver::cursor_image_set_to*;
      mir::scene::NullSurfaceObserver::frame_posted*;
      mir::scene::NullSurfaceObserver::hidden_set_to*;
 === modified file 'tests/unit-tests/scene/test_basic_surface.cpp'
 --- tests/unit-tests/scene/test_basic_surface.cpp	2015-10-07 12:06:14 +0000
 +++ tests/unit-tests/scene/test_basic_surface.cpp	2015-12-02 02:02:47 +0000
@@ -27,6 +27,7 @@
  #include "mir/scene/null_surface_observer.h"
  #include "mir/events/event_builders.h"
++#include "mir/test/doubles/stub_cursor_image.h"
  #include "mir/test/doubles/mock_buffer_stream.h"
  #include "mir/test/doubles/mock_input_sender.h"
  #include "mir/test/doubles/stub_input_sender.h"
@@ -67,6 +68,8 @@
      MOCK_METHOD1(hidden_set_to, void(bool));
      MOCK_METHOD1(renamed, void(char const*));
      MOCK_METHOD0(client_surface_close_requested, void());
++    MOCK_METHOD1(cursor_image_set_to, void(mir::graphics::CursorImage const& image));
++    MOCK_METHOD0(cursor_image_removed, void());
  };
  void post_a_frame(ms::BasicSurface& surface)
@@ -661,6 +664,32 @@
  INSTANTIATE_TEST_CASE_P(SurfaceFocusAttributeTest, BasicSurfaceAttributeTest,
     ::testing::Values(surface_focus_test_parameters));
++TEST_F(BasicSurfaceTest, notifies_about_cursor_image_change)
++{
++    using namespace testing;
++
++    NiceMock<MockSurfaceObserver> mock_surface_observer;
++
++    auto cursor_image = std::make_shared<mtd::StubCursorImage>();
++    EXPECT_CALL(mock_surface_observer, cursor_image_set_to(_));
++
++    surface.add_observer(mt::fake_shared(mock_surface_observer));
++    surface.set_cursor_image(cursor_image);
++}
++
++TEST_F(BasicSurfaceTest, notifies_about_cursor_image_removal)
++{
++    using namespace testing;
++
++    NiceMock<MockSurfaceObserver> mock_surface_observer;
++
++    EXPECT_CALL(mock_surface_observer, cursor_image_set_to(_)).Times(0);
++    EXPECT_CALL(mock_surface_observer, cursor_image_removed());
++
++    surface.add_observer(mt::fake_shared(mock_surface_observer));
++    surface.set_cursor_image({});
++}
++
  TEST_F(BasicSurfaceTest, calls_send_event_on_consume)
+ {
      using namespace ::testing;