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

[Kevin DuBois]

fix LP: #1597717 by checking for 0x0 cursors (such as qtmir's NamedCursor), and hiding the mir cursor if an image with 0x0 is set by the shell.

fixes: LP: #1597717

testing: full stack comes up. Cursor transitions and cursor animation around the browser in pocket desktop mode work as well.

fix LP: #1597717 by checking for 0x0 cursors (such as qtmir's NamedCursor), and hiding the mir cursor if an image with 0x0 is set by the shell.

fixes: LP: #1597717

testing: full stack comes up. Cursor transitions and cursor animation around the browser in pocket desktop mode work as well.

[Mir CI Bot (mir-ci-bot)]

FAILED: Continuous integration, rev:3569
https://mir-jenkins.ubuntu.com/job/mir-ci/1222/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/1405/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1456
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1447
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1447
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1419
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1419/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1419
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1419/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1419
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1419/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1419
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1419/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1419/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1222/rebuild

review: Needs Fixing (continuous-integration)

[Cemil Azizoglu (cemil-azizoglu)]

ok

review: Approve

[Daniel d'Andrada (dandrader)]

Fixes the issue I was experiencing when trying to run qtmir+unity8 with lp:mir/0.24

review: Approve (test)

[Andreas Pokorny (andreas-pokorny)]

lgtm

review: Approve

[Mir CI Bot (mir-ci-bot)]

FAILED: Autolanding.
More details in the following jenkins job:
https://mir-jenkins.ubuntu.com/job/mir-autolanding/369/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/1417/console
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/396/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1468
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1459
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1459
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1431/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1431
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1431/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1431
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1431/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1431
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1431/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1431
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1431/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)

[Alexandros Frantzis (afrantzis)]

CI failure seems unrelated, filed as https://bugs.launchpad.net/mir/+bug/1598802 .

[Mir CI Bot (mir-ci-bot)]

FAILED: Autolanding.
More details in the following jenkins job:
https://mir-jenkins.ubuntu.com/job/mir-autolanding/373/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/1422/console
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/400/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1473
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1464
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1464
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1436
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1436/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1436
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1436/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1436/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1436/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1436
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1436/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1436
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1436/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)

[Chris Halse Rogers (raof)]

I assume that there's some godawful Qt-ish reason why we're doing this rather than just throwing an exception if the shell tries to set a 0x0 cursor and then fixing QtMir?

Assuming there is such a reason, +1.

review: Approve

[Mir CI Bot (mir-ci-bot)]

review: Approve (continuous-integration)

[Mir CI Bot (mir-ci-bot)]

FAILED: Autolanding.
Approved revid is not set in launchpad. This is most likely a launchpad issue and re-approve should fix it. There is also a chance (although a very small one) this is a permission problem of the ps-jenkins bot.
https://mir-jenkins.ubuntu.com/job/mir-autolanding/375/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/1426
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/402/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1477
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1468
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1468
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1440
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1440/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1440
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1440/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1440
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1440/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1440
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1440/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1440
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1440/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)

[Mir CI Bot (mir-ci-bot)]

FAILED: Autolanding.
More details in the following jenkins job:
https://mir-jenkins.ubuntu.com/job/mir-autolanding/375/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/1426
    FAILURE: https://mir-jenkins.ubuntu.com/job/generic-land-mp/402/console
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/403/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1477
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1468
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1468
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1440
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1440/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1440
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1440/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1440
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1440/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1440
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1440/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1440
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1440/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)

[Kevin DuBois (kdub)]

> I assume that there's some godawful Qt-ish reason why we're doing this rather
> than just throwing an exception if the shell tries to set a 0x0 cursor and
> then fixing QtMir?
>
> Assuming there is such a reason, +1.

Right, it appears that they want to disable us showing our cursor, as well as set a Qt-specific cursor name for their renderer to use.

[Kevin DuBois (kdub)]

CI error was "DEBUG: Approved revid is not set (maybe a permission problem?). Failing autolanding."
retriggering.

[Mir CI Bot (mir-ci-bot)]

review: Approve (continuous-integration)

=== modified file 'src/server/graphics/nested/mir_client_host_connection.cpp'
--- src/server/graphics/nested/mir_client_host_connection.cpp	2016-06-09 13:15:34 +0000
+++ src/server/graphics/nested/mir_client_host_connection.cpp	2016-07-01 14:28:05 +0000
@@ -127,6 +127,12 @@
         auto const image_width = image.size().width.as_int();
         auto const image_height = image.size().height.as_int();
 
+        if ((image_width <= 0) || (image_height <= 0))
+        {
+            hide_cursor();
+            return;
+        }
+
         MirGraphicsRegion g;
 
         if (cursor)

=== modified file 'tests/acceptance-tests/test_nested_mir.cpp'
--- tests/acceptance-tests/test_nested_mir.cpp	2016-06-02 08:19:38 +0000
+++ tests/acceptance-tests/test_nested_mir.cpp	2016-07-01 14:28:05 +0000
@@ -24,11 +24,14 @@
 #include "mir/graphics/display_configuration.h"
 #include "mir/graphics/display_configuration_policy.h"
 #include "mir/graphics/display_configuration_report.h"
+#include "mir/shell/shell.h"
 #include "mir/input/cursor_listener.h"
 #include "mir/cached_ptr.h"
 #include "mir/main_loop.h"
 #include "mir/scene/session_coordinator.h"
 #include "mir/scene/session.h"
+#include "mir/scene/surface.h"
+#include "mir/scene/null_surface_observer.h"
 #include "mir/shell/display_configuration_controller.h"
 #include "mir/shell/host_lifecycle_event_listener.h"
 
@@ -57,6 +60,7 @@
 namespace mt = mir::test;
 namespace mtd = mir::test::doubles;
 namespace mtf = mir_test_framework;
+namespace msc = mir::scene;
 
 using namespace testing;
 using namespace std::chrono_literals;
@@ -206,6 +210,171 @@
     MOCK_METHOD1(configure, void (mg::DisplayConfiguration const&));
 };
 
+
+struct StubSurfaceObserver : msc::NullSurfaceObserver
+{
+    void cursor_image_set_to(mg::CursorImage const&) override {}
+    void cursor_image_removed() override
+    {
+        std::unique_lock<decltype(mutex)> lk(mutex);
+        removed = true;
+        cv.notify_all();
+    }
+
+    bool wait_for_removal()
+    {
+        using namespace std::chrono_literals;
+        std::unique_lock<decltype(mutex)> lk(mutex);
+        auto rc = cv.wait_for(lk, 4s, [this] { return removed; });
+        return rc;
+    }
+    std::mutex mutex;
+    std::condition_variable cv;
+    bool removed = false;
+};
+
+struct ObservantShell : msh::Shell
+{
+    ObservantShell(
+        std::shared_ptr<msh::Shell> const& wrapped,
+        std::shared_ptr<msc::SurfaceObserver> const& surface_observer) :
+        wrapped(wrapped),
+        surface_observer(surface_observer)
+    {
+    }
+
+    void add_display(geom::Rectangle const& area) override
+    {
+        return wrapped->add_display(area);
+    }
+
+    void remove_display(geom::Rectangle const& area) override
+    {
+        return wrapped->remove_display(area);
+    }
+
+    bool handle(MirEvent const& event) override
+    {
+        return wrapped->handle(event);
+    }
+
+    void focus_next_session() override
+    {
+        wrapped->focus_next_session();
+    }
+
+    auto focused_session() const -> std::shared_ptr<msc::Session>
+    {
+        return wrapped->focused_session();
+    }
+
+    void set_focus_to(
+        std::shared_ptr<msc::Session> const& focus_session,
+        std::shared_ptr<msc::Surface> const& focus_surface) override
+    {
+        return wrapped->set_focus_to(focus_session, focus_surface);
+    }
+
+    std::shared_ptr<msc::Surface> focused_surface() const override
+    {
+        return wrapped->focused_surface();
+    }
+
+    auto surface_at(geom::Point cursor) const -> std::shared_ptr<msc::Surface> override
+    {
+        return wrapped->surface_at(cursor);
+    }
+
+    void raise(msh::SurfaceSet const& surfaces) override
+    {
+        wrapped->raise(surfaces);
+    }
+
+    std::shared_ptr<msc::Session> open_session(
+        pid_t client_pid,
+        std::string const& name,
+        std::shared_ptr<mf::EventSink> const& sink) override
+    {
+        return wrapped->open_session(client_pid, name, sink);
+    }
+
+    void close_session(std::shared_ptr<msc::Session> const& session) override
+    {
+        wrapped->close_session(session);
+    }
+
+    std::shared_ptr<msc::PromptSession> start_prompt_session_for(
+        std::shared_ptr<msc::Session> const& session,
+        msc::PromptSessionCreationParameters const& params) override
+    {
+        return wrapped->start_prompt_session_for(session, params);
+    }
+
+    void add_prompt_provider_for(
+        std::shared_ptr<msc::PromptSession> const& prompt_session,
+        std::shared_ptr<msc::Session> const& session) override
+    {
+        wrapped->add_prompt_provider_for(prompt_session, session);
+    }
+
+    void stop_prompt_session(std::shared_ptr<msc::PromptSession> const& prompt_session) override
+    {
+        wrapped->stop_prompt_session(prompt_session);
+    }
+
+    mf::SurfaceId create_surface(
+        std::shared_ptr<msc::Session> const& session,
+        msc::SurfaceCreationParameters const& params,
+        std::shared_ptr<mf::EventSink> const& sink) override
+    {
+        auto id = wrapped->create_surface(session, params, sink);
+        auto surface = session->surface(id);
+        surface->add_observer(surface_observer);
+        return id;
+    }
+
+    void modify_surface(
+        std::shared_ptr<msc::Session> const& session,
+        std::shared_ptr<msc::Surface> const& surface,
+        msh::SurfaceSpecification  const& modifications) override
+    {
+        wrapped->modify_surface(session, surface, modifications);
+    }
+
+    void destroy_surface(std::shared_ptr<msc::Session> const& session, mf::SurfaceId surface) override
+    {
+        wrapped->destroy_surface(session, surface);
+    }
+
+    int set_surface_attribute(
+        std::shared_ptr<msc::Session> const& session,
+        std::shared_ptr<msc::Surface> const& surface,
+        MirSurfaceAttrib attrib,
+        int value) override
+    {
+        return wrapped->set_surface_attribute(session, surface, attrib, value);
+    }
+
+    int get_surface_attribute(
+        std::shared_ptr<msc::Surface> const& surface,
+        MirSurfaceAttrib attrib) override
+    {
+        return wrapped->get_surface_attribute(surface, attrib);
+    }
+
+    void raise_surface(
+        std::shared_ptr<msc::Session> const& session,
+        std::shared_ptr<msc::Surface> const& surface,
+        uint64_t timestamp) override
+    {
+        return wrapped->raise_surface(session, surface, timestamp);
+    }
+
+private:
+    std::shared_ptr<msh::Shell> const wrapped;
+    std::shared_ptr<msc::SurfaceObserver> const surface_observer;
+};
+
 class NestedMirRunner : public mtf::HeadlessNestedServerRunner
 {
 public:
@@ -264,6 +433,7 @@
 
     std::shared_ptr<MockSessionMediatorReport> mock_session_mediator_report;
     NiceMock<MockDisplay> display{display_geometry};
+    std::shared_ptr<StubSurfaceObserver> stub_observer = std::make_shared<StubSurfaceObserver>();
 
     void SetUp() override
     {
@@ -279,6 +449,11 @@
 
         server.wrap_cursor([this](std::shared_ptr<mg::Cursor> const&) { return the_mock_cursor(); });
 
+        server.wrap_shell([&, this](auto const& wrapped)
+        {
+            return std::make_shared<ObservantShell>(wrapped, stub_observer);
+        });
+
         mtf::HeadlessInProcessServer::SetUp();
     }
 
@@ -832,6 +1007,32 @@
     Mock::VerifyAndClearExpectations(mock_cursor.get());
 }
 
+//LP: #1597717
+TEST_F(NestedServer, showing_a_0x0_cursor_image_sets_disabled_cursor)
+{
+    NestedMirRunner nested_mir{new_connection()};
+
+    ClientWithAPaintedSurfaceAndABufferStream client(nested_mir);
+    auto const mock_cursor = the_mock_cursor();
+
+    server.the_cursor_listener()->cursor_moved_to(489, 9);
+
+    //see NamedCursor in qtmir
+    struct EmptyImage : mg::CursorImage
+    {
+        const void *as_argb_8888() const override { return nullptr; }
+        geom::Size size() const override { return {0,0}; }
+        geom::Displacement hotspot() const override { return {0,0}; }
+    } empty_image;
+    nested_mir.cursor_wrapper->show(empty_image);
+
+    EXPECT_TRUE(stub_observer->wait_for_removal());
+
+    // Need to verify before test server teardown deletes the
+    // surface as the host cursor then reverts to default.
+    Mock::VerifyAndClearExpectations(mock_cursor.get());
+}
+
 TEST_F(NestedServer, applies_display_config_on_startup)
 {
     mt::Signal condition;