Summary:
On client side the newer wl_keyboard_release is used which is a
destructor call. On server side the shared destroy callback is used
and it's ensured that KWayland doesn't crash if called into the
KeyboardInterface between unbound and destroyed.
Test Plan:
Test case extended to cover the condition of an unbound
KeyboardInterface.
Reviewers: #plasma_on_wayland
Subscribers: plasma-devel
Tags: #plasma_on_wayland
Differential Revision: https://phabricator.kde.org/D2036
Summary:
On client side use wl_touch_release to get into the proper destroy
handler on server side. There the shared destroy implementation is
used.
The test case is extended to verify the condition and ensure that
our code doesn't crash in case SeatInterface calls into the already
unbound TouchInterface.
Reviewers: #plasma_on_wayland
Subscribers: plasma-devel
Tags: #plasma_on_wayland
Differential Revision: https://phabricator.kde.org/D2035
Summary:
So far SeatInterface handled automatically which DataDeviceInterface
holds the current clipboard selection. While this works fine and is
correct it doesn't support use cases like a clipboard manager where
the clipboard is hold by a different ClientConnection than the one
from the focused keyboard.
This change allows to manually set the selected DataDeviceInterface
to override the automatic selection, though the automatic selection
is still in place. Thus the next update of a selection will override
the manually set selection again.
Reviewers: #plasma_on_wayland
Subscribers: plasma-devel
Tags: #plasma_on_wayland
Differential Revision: https://phabricator.kde.org/D1972
Summary:
A Resource might be unbound, but not yet destroyed. In that case this
return a Resource instead of nullptr.
This change adds an explicit nullptr check.
Reviewers: #plasma_on_wayland
Subscribers: plasma-devel
Tags: #plasma_on_wayland
Differential Revision: https://phabricator.kde.org/D1938
Summary:
Creates a PlasmaShellSurface and allows to set the different roles
through command line argument.
Needed to verify that KWin properly handles the notification type.
Reviewers: #plasma_on_wayland
Subscribers: plasma-devel
Tags: #plasma_on_wayland
Differential Revision: https://phabricator.kde.org/D1932
Summary:
Plasma needs to be able to mark windows as Notifications, so that KWin
can put them into the right layer.
Reviewers: #plasma_on_wayland
Subscribers: plasma-devel
Tags: #plasma_on_wayland
Differential Revision: https://phabricator.kde.org/D1928
Summary:
The QtExtendedSurfaceInterface might be unbound and thus the resource
might be null when calling into close. Thus we need to do a nullptr
check.
Hit a crash there.
Test Plan:
Unfortunately no test case as we don't have a client side
implementation for this.
Reviewers: #plasma_on_wayland
Subscribers: plasma-devel
Tags: #plasma_on_wayland
Differential Revision: https://phabricator.kde.org/D1961
Summary:
This is a gotcha moment:
1. Create Surface with id 1
2. destroy that Surface
3. Create another Surface
Now if in step 3 the id is by pure chance getting reused and also 1, the
wl_resource pointer of the SurfaceInterface of step 1 and step 3 are
the same. This is rather unexpected and causes problems.
When creating a ShellSurface in step 1 and step 3 it would fail. KWayland
finds a ShellSurface which was already created for the Surface. The same
can happen with QtSurfaceExtensionInterface and PlasmaShellInterface which
also go into error state.
On client side this would trigger a protocol error and terminate the
application. An easy way to reproduce is opening the file open dialog
from within Kate multiple times.
This change addresses this problem by setting the surface pointer in
those classes to null when the parent Surface gets destroyed. Thus
creating a new ShellSurface won't find the old referenced Surface any
more.
For ShellSurface and PlasmaShellSurface a test case is added which hit
the condition without this change. For QtSurfaceExtension we don't have
the client side, so we cannot really simulate the condition.
Reviewers: #plasma_on_wayland
Subscribers: plasma-devel
Tags: #plasma_on_wayland
Differential Revision: https://phabricator.kde.org/D1937
Summary:
So far for internal cleanup we mostly listen to QObject::destroyed.
When a Resource gets unbind the wl_resource is set to null and the
Resource gets deleteLater. This creates a short time frame when the
Resource is still there, but the wl_resource is null. For most internal
usages the Resource is completely useless at that point and should no
longer be considered. So far it was still considered and could hit
crashers, if a code path did not nullptr check. Unfortunately
libwayland-server is not nullptr safe: if called with a null value it
tends to crash.
So this check introduces a new signal unbound which can be listend to
in addition to the destroyed signal. It's used in SeatInterface for
DataDeviceInterface, where we experienced a crash related to that.
A test case is added which exposes the crash, but it already needs
the unbound signal to get into the crashy condition. The actual crash
is fixed twice - with the help of the unbound signal, but also by
introducing the nullptr check where it's needed.
Reviewers: #plasma_on_wayland
Subscribers: plasma-devel
Tags: #plasma_on_wayland
Differential Revision: https://phabricator.kde.org/D1868
Summary:
The fact that the buffer is still referenced - that is used - when the
BufferInterface gets destroyed is an error, but it is not fatal.
Unfortunately KWin/Wayland is still hitting this assert from time to
time and the assert is not helping to find the cause as a backtrace
does not show where a reference is still hold.
This change removes the hard assert by a soft warning. The advantage
of the warning is that the compositor is not killed and that we can
observe the reason and find a usage pattern which triggers the condition.
With that we will hopefully be able to find the case where the buffer
is still referenced when being destroyed and fix that.
Reviewers: #plasma_on_wayland
Differential Revision: https://phabricator.kde.org/D1783
Summary:
This improves the cleanup of a shadow from client side. The server now
notices when the client destroyed the shadow.
Reviewers: #plasma_on_wayland
Subscribers: plasma-devel
Differential Revision: https://phabricator.kde.org/D1789
Sometimes the test is failing due to a wait on the signalspy not
working and then the cleanup triggers a heap-use-after-free.
This change tries to address the problem by using a QTRY_COMPARE
instead of a wait on signalspy.
If a PlasmaShellSurface got already created for a Surface, the
PlasmaShell::createSurface should return that one. This change justs
adds a check to an autotest to verify this condition.
Summary:
The event is sent to the client once all initial state is transmitted.
This means the client is able to see the PlasmaWindow completely created
and not in the intermediate state with further updates being pushed after
being created.
The client side API is adjusted to emit the windowCreated signal after
the initial state event is received. In addition if the window is already
unmapped, the signal will never be emitted which means the not valid
windows are not exposed to the client at all.
The tests are adjusted to reflect the new reality, which in most cases
just means removing the comment that this needs to be improved.
There is one kind of unrelated change included: when an empty icon is
set, the client side now creates a QIcon() instead of going through
QIcon::fromTheme. This wrong behavior was exposed now by the auto tests.
Reviewers: #plasma, hein
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D1773
Summary:
If the surface passed in the transient request is to the surface of the
shell surface, KWin aborts. It's an obvious invalid request, so handle
it properly by sending an error.
Reviewers: #plasma
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D1755
Summary:
So far ConnectionThread dispatched events without checking for (protocol)
errors. But Wayland errors are considered fatal. We do need to consider
them.
This change introduces a way to detect errors and expose them in
ConnectionThread. Errors are handled gracefully, they are exposed, but
not considered application fatal. E.g. a new ConnectionThread could be
created afterwards.
This allows to add tests to verify error conditions.
Reviewers: #plasma
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D1753
Summary:
The idea of shipping a test server is to have something like Xvfb for
Wayland. To be able to run a test application with a fake Wayland server.
To make this super easy the test server binary is installed into libexec
directory and provides a cmake function to run a test application with
the test server.
The test server takes full control over the process. It's a guiless
application and starts the passed test application once it is fully
set up. The environment is setup to have the test application connect
to the fake server (WAYLAND_SOCKET env variable and QT_QPA_PLATFORM).
When the started application finishes the test server goes down and
exits with the exit value of the test application. This allows a good
integration with ctest.
The test server is a virtual server which supports the following
interfaces:
* Shm
* Compositor
* Shell
* Seat
* DataDeviceManager
* Idle
* SubCompositor
* Output (1280x1024 at 60 Hz with 96 dpi)
* FakeInput
This is sufficient to bring up a QtWayland based application and
allows some basic interactions from a test application (e.g. fake
input).
So far the server fakes a repaint every 16 msec, but does not yet
pass events to the test applications.
To integrate this into an application for testing use:
find_package(KF5Wayland CONFIG)
add_executable(myTest myTest.cpp)
target_link_libraries(myTest Qt5::Gui Qt5::Test)
kwaylandtest(myTest)
When now running ctest in the build directory the test server gets
started and will start the myTest binary and report the passed/failed
in the expected and normal way.
This way a test case can easily be run against both X11 and Wayland.
Reviewers: #plasma
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D1726
Changelog: Virtual framebuffer server for auto tests
Summary:
So far we directly destroyed the resource on the server side. But this
causes a wayland error when the client tries to cleanup. This results in
the client being destroyed. A problem which brings down plasmashell
regularly when short living windows are shown. This happens e.g. in
Dolphin with the adress auto complete.
This change addresses the problem by creating a temporary
PlasmaWindowInterface for the already unmapped window. It doesn't get
added to the list of known windows and it's only purpose is to properly
handle the unmap and the destroy of the just created resource.
Reviewers: #plasma
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D1734
Summary:
Destroying the SlideInterface on the server side before the client has a
chance to cleanup results in a protocol error:
wl_display@1: error 0: invalid object 7
Which would terminate the client. If we would not destroy the resource,
but only delete the SlideInterface it could result in heap-use-after-free.
So just don't do anything, the client needs to cleanup which will result
in the SlideInterface being deleted.
Reviewers: #plasma
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D1714
Summary:
The destructor was not properly implement. Let's use the generic one
from Resource.
Test case is adjusted to verify that the SlideInterface gets cleaned up.
Reviewers: #plasma
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D1713
Summary:
Unlike the other cases this one is not as dangerous as the shadow
protocol doesn't have a destructor request (yet).
Once that is added the problem would be the same: destroying the
ShadowInterface when the parent SurfaceInterface gets destroyed would
result in a protocol error on client side.
Reviewers: #plasma
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D1711
Summary:
Destroying the ServerSideDecorationInterface on the server side before
the client has a chance to cleanup results in a protocol error:
wl_display@1: error 0: invalid object 7
Which would terminate the client. If we would not destroy the resource,
but only delete the ServerSideDecorationInterface it could result in
heap-use-after-free.
So just don't do anything, the client needs to cleanup which will result
in the ServerSideDecorationInterface being deleted.
Reviewers: #plasma
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D1710
Summary:
Destroying the ContrastInterface on the server side before the client has
a chance to cleanup results in a protocol error:
wl_display@1: error 0: invalid object 7
Which would terminate the client. If we would not destroy the resource,
but only delete the ContrastInterface it could result in
heap-use-after-free.
So just don't do anything, the client needs to cleanup which will result
in the ContrastInterface being deleted.
Reviewers: #plasma
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D1709
Summary:
Destroying the BlurInterface on the server side before the client has a
chance to cleanup results in a protocol error:
wl_display@1: error 0: invalid object 7
Which would terminate the client. If we would not destroy the resource,
but only delete the BlurInterface it could result in heap-use-after-free.
So just don't do anything, the client needs to cleanup which will result
in the BlurInterface being deleted.
Reviewers: #plasma
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D1708
We saw it fail on build.kde.org a few times and it looks like a
entered spy gets triggered timing dependent (no surprise it's on
client side). So better check whether the entered spy got delivered
already and if not wait for the event.
Summary:
This change standardizes the behavior regarding the destructor request.
The destructor should destroy the resource and nothing else. The
Wayland library invokes the static unbind method once the resource is
destroyed. The implementation provided by Resource::Private::unbind
triggers a delete later on the Resource. So there is no need to trigger
a deleteLater from the destructor request callback.
This change adds a generic implementation to Resource::Private which is
now used by all inheriting classes replacing the custom implementations.
Test Plan:
For a few Resources the test is extended to ensure that the Resource
gets deleted on server side.
Reviewers: #plasma
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D1679
