[wayland] Finish initialising ShellClient only when commited to the surface

Summary:
Everything on the wl_surface is double buffered.

When we create an XdgShell toplevel or popup we shouldn't treat it as
attached until it's committed to the surface.

A client should commit the surface after it's sent it's initial state of
the Xdg topLevel; minimumSize, title, app_id, etc.

By blocking sending configure events we will have flushed the correct
initial state before sending a single atomic correct event to the
client. It also adds a hook to re-evaluate rules now that all properties
are set.

Arguably this applies to WlShellSurface too, but I've left it unchanged
as it's deprecated and hard to verify real client behaviour.

Test Plan: Ran all unit tests

Reviewers: #kwin, zzag

Reviewed By: #kwin, zzag

Subscribers: zzag, kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D18583
icc-effect-5.17.5
David Edmundson 2019-02-26 13:41:07 +00:00
parent dc7ea09e8d
commit 2bad2b48fe
7 changed files with 264 additions and 31 deletions

View File

@ -116,12 +116,14 @@ void MaximizeAnimationTest::testMaximizeRestore()
QVERIFY(!surface.isNull());
QFETCH(Test::ShellSurfaceType, type);
QScopedPointer<XdgShellSurface> shellSurface(qobject_cast<XdgShellSurface *>(
Test::createShellSurface(type, surface.data())));
QScopedPointer<XdgShellSurface> shellSurface(createXdgShellSurface(type, surface.data(), nullptr, Test::CreationSetup::CreateOnly));
// Wait for the initial configure event.
XdgShellSurface::States states;
QSignalSpy configureRequestedSpy(shellSurface.data(), &XdgShellSurface::configureRequested);
surface->commit(Surface::CommitFlag::None);
QVERIFY(configureRequestedSpy.isValid());
QVERIFY(configureRequestedSpy.wait());
QCOMPARE(configureRequestedSpy.count(), 1);

View File

@ -142,12 +142,48 @@ enum class ShellSurfaceType {
XdgShellV6,
XdgShellStable
};
enum class CreationSetup {
CreateOnly,
CreateAndConfigure, /// commit and wait for the configure event, making this surface ready to commit buffers
};
/**
* Creates either a ShellSurface * or XdgShellSurface * as defined by @arg type
* For XDG top levels this method will block for a configure event, make this surface ready to commit buffers
*/
QObject *createShellSurface(ShellSurfaceType type, KWayland::Client::Surface *surface, QObject *parent = nullptr);
KWayland::Client::ShellSurface *createShellSurface(KWayland::Client::Surface *surface, QObject *parent = nullptr);
KWayland::Client::XdgShellSurface *createXdgShellV5Surface(KWayland::Client::Surface *surface, QObject *parent = nullptr);
KWayland::Client::XdgShellSurface *createXdgShellV6Surface(KWayland::Client::Surface *surface, QObject *parent = nullptr);
KWayland::Client::XdgShellSurface *createXdgShellStableSurface(KWayland::Client::Surface *surface, QObject *parent = nullptr);
KWayland::Client::XdgShellPopup *createXdgShellStablePopup(KWayland::Client::Surface *surface, KWayland::Client::XdgShellSurface *parentSurface, const KWayland::Client::XdgPositioner &positioner, QObject *parent = nullptr);
KWayland::Client::XdgShellSurface *createXdgShellSurface(ShellSurfaceType type,
KWayland::Client::Surface *surface,
QObject *parent = nullptr,
CreationSetup creationSetup = CreationSetup::CreateAndConfigure);
KWayland::Client::ShellSurface *createShellSurface(KWayland::Client::Surface *surface,
QObject *parent = nullptr);
KWayland::Client::XdgShellSurface *createXdgShellV5Surface(KWayland::Client::Surface *surface,
QObject *parent = nullptr,
CreationSetup = CreationSetup::CreateAndConfigure);
KWayland::Client::XdgShellSurface *createXdgShellV6Surface(KWayland::Client::Surface *surface,
QObject *parent = nullptr,
CreationSetup = CreationSetup::CreateAndConfigure);
KWayland::Client::XdgShellSurface *createXdgShellStableSurface(KWayland::Client::Surface *surface,
QObject *parent = nullptr,
CreationSetup = CreationSetup::CreateAndConfigure);
KWayland::Client::XdgShellPopup *createXdgShellStablePopup(KWayland::Client::Surface *surface,
KWayland::Client::XdgShellSurface *parentSurface,
const KWayland::Client::XdgPositioner &positioner,
QObject *parent = nullptr,
CreationSetup = CreationSetup::CreateAndConfigure);
/**
* Commits the XdgShellSurface to the given surface, and waits for the configure event from the compositor
*/
void initXdgShellSurface(KWayland::Client::Surface *surface, KWayland::Client::XdgShellSurface *shellSurface);
void initXdgShellPopup(KWayland::Client::Surface *surface, KWayland::Client::XdgShellPopup *popup);
/**
* Creates a shared memory buffer of @p size in @p color and attaches it to the @p surface.

View File

@ -464,11 +464,19 @@ void QuickTilingTest::testQuickTilingPointerMoveXdgShell()
QScopedPointer<Surface> surface(Test::createSurface());
QVERIFY(!surface.isNull());
QScopedPointer<XdgShellSurface> shellSurface(Test::createXdgShellV6Surface(surface.data()));
QScopedPointer<XdgShellSurface> shellSurface(Test::createXdgShellV6Surface(
surface.data(), surface.data(), Test::CreationSetup::CreateOnly));
QVERIFY(!shellSurface.isNull());
// wait for the initial configure event
QSignalSpy configureRequestedSpy(shellSurface.data(), &XdgShellSurface::configureRequested);
QVERIFY(configureRequestedSpy.isValid());
surface->commit(Surface::CommitFlag::None);
QVERIFY(configureRequestedSpy.wait());
QCOMPARE(configureRequestedSpy.count(), 1);
// let's render
shellSurface->ackConfigure(configureRequestedSpy.last().at(2).value<quint32>());
auto c = Test::renderAndWaitForShown(surface.data(), QSize(100, 50), Qt::blue);
QVERIFY(c);
@ -476,6 +484,8 @@ void QuickTilingTest::testQuickTilingPointerMoveXdgShell()
QCOMPARE(c->geometry(), QRect(0, 0, 100, 50));
QCOMPARE(c->quickTileMode(), QuickTileMode(QuickTileFlag::None));
QCOMPARE(c->maximizeMode(), MaximizeRestore);
// we have to receive a configure event when the client becomes active
QVERIFY(configureRequestedSpy.wait());
QTRY_COMPARE(configureRequestedSpy.count(), 2);
@ -526,11 +536,19 @@ void QuickTilingTest::testQuickTilingTouchMoveXdgShell()
QVERIFY(!surface.isNull());
QScopedPointer<ServerSideDecoration> deco(Test::waylandServerSideDecoration()->create(surface.data()));
QScopedPointer<XdgShellSurface> shellSurface(Test::createXdgShellV6Surface(surface.data()));
QScopedPointer<XdgShellSurface> shellSurface(Test::createXdgShellV6Surface(
surface.data(), surface.data(), Test::CreationSetup::CreateOnly));
QVERIFY(!shellSurface.isNull());
// wait for the initial configure event
QSignalSpy configureRequestedSpy(shellSurface.data(), &XdgShellSurface::configureRequested);
QVERIFY(configureRequestedSpy.isValid());
surface->commit(Surface::CommitFlag::None);
QVERIFY(configureRequestedSpy.wait());
QCOMPARE(configureRequestedSpy.count(), 1);
// let's render
shellSurface->ackConfigure(configureRequestedSpy.last().at(2).value<quint32>());
auto c = Test::renderAndWaitForShown(surface.data(), QSize(1000, 50), Qt::blue);
QVERIFY(c);
@ -542,6 +560,8 @@ void QuickTilingTest::testQuickTilingTouchMoveXdgShell()
50 + decoration->borderTop() + decoration->borderBottom()));
QCOMPARE(c->quickTileMode(), QuickTileMode(QuickTileFlag::None));
QCOMPARE(c->maximizeMode(), MaximizeRestore);
// we have to receive a configure event when the client becomes active
QVERIFY(configureRequestedSpy.wait());
QTRY_COMPARE(configureRequestedSpy.count(), 2);

View File

@ -103,6 +103,10 @@ private Q_SLOTS:
void testMinimizeWindowWithTransients();
void testXdgDecoration_data();
void testXdgDecoration();
void testXdgNeverCommitted();
void testXdgInitialState();
void testXdgInitiallyMaximised();
void testXdgInitiallyMinimized();
};
void TestShellClient::initTestCase()
@ -540,25 +544,36 @@ void TestShellClient::testFullscreenRestore()
// this test verifies that windows created fullscreen can be later properly restored
QScopedPointer<Surface> surface(Test::createSurface());
QFETCH(Test::ShellSurfaceType, type);
QScopedPointer<QObject> shellSurface(Test::createShellSurface(type, surface.data()));
XdgShellSurface *xdgShellSurface = Test::createXdgShellSurface(type, surface.data(), surface.data(), Test::CreationSetup::CreateOnly);
QSignalSpy configureRequestedSpy(xdgShellSurface, SIGNAL(configureRequested(QSize, KWayland::Client::XdgShellSurface::States, quint32)));
XdgShellSurface *xdgShellSurface = nullptr;
// fullscreen the window
xdgShellSurface = qobject_cast<XdgShellSurface*>(shellSurface.data());
xdgShellSurface->setFullscreen(true);
surface->commit(Surface::CommitFlag::None);
auto c = Test::renderAndWaitForShown(surface.data(), QSize(screens()->size(0)), Qt::blue);
configureRequestedSpy.wait();
QCOMPARE(configureRequestedSpy.count(), 1);
const auto size = configureRequestedSpy.first()[0].value<QSize>();
const auto state = configureRequestedSpy.first()[1].value<KWayland::Client::XdgShellSurface::States>();
QCOMPARE(size, screens()->size(0));
QVERIFY(state & KWayland::Client::XdgShellSurface::State::Fullscreen);
xdgShellSurface->ackConfigure(configureRequestedSpy.first()[2].toUInt());
auto c = Test::renderAndWaitForShown(surface.data(), size, Qt::blue);
QVERIFY(c);
QVERIFY(c->isFullScreen());
configureRequestedSpy.wait(100);
QSignalSpy fullscreenChangedSpy(c, &ShellClient::fullScreenChanged);
QVERIFY(fullscreenChangedSpy.isValid());
QSignalSpy geometryChangedSpy(c, &ShellClient::geometryChanged);
QVERIFY(geometryChangedSpy.isValid());
QSignalSpy configureRequestedSpy(shellSurface.data(), SIGNAL(configureRequested(QSize, KWayland::Client::XdgShellSurface::States, quint32)));
QVERIFY(configureRequestedSpy.isValid());
// swap back to normal
configureRequestedSpy.clear();
xdgShellSurface->setFullscreen(false);
QVERIFY(fullscreenChangedSpy.wait());
@ -567,7 +582,7 @@ void TestShellClient::testFullscreenRestore()
QVERIFY(!c->isFullScreen());
for (const auto &it: configureRequestedSpy) {
xdgShellSurface->ackConfigure(it[2].toInt());
xdgShellSurface->ackConfigure(it[2].toUInt());
}
Test::render(surface.data(), QSize(100, 50), Qt::red);
@ -630,17 +645,25 @@ void TestShellClient::testUserSetFullscreenXdgShell()
{
QScopedPointer<Surface> surface(Test::createSurface());
QFETCH(Test::ShellSurfaceType, type);
QScopedPointer<XdgShellSurface> shellSurface(dynamic_cast<XdgShellSurface*>(Test::createShellSurface(type, surface.data())));
QScopedPointer<XdgShellSurface> shellSurface(Test::createXdgShellSurface(
type, surface.data(), surface.data(), Test::CreationSetup::CreateOnly));
QVERIFY(!shellSurface.isNull());
// wait for the initial configure event
QSignalSpy configureRequestedSpy(shellSurface.data(), &XdgShellSurface::configureRequested);
QVERIFY(configureRequestedSpy.isValid());
surface->commit(Surface::CommitFlag::None);
QVERIFY(configureRequestedSpy.wait());
QCOMPARE(configureRequestedSpy.count(), 1);
shellSurface->ackConfigure(configureRequestedSpy.last().at(2).value<quint32>());
auto c = Test::renderAndWaitForShown(surface.data(), QSize(100, 50), Qt::blue);
QVERIFY(c);
QVERIFY(c->isActive());
QVERIFY(!c->isFullScreen());
// two, one for initial sync, second as it becomes active
QTRY_COMPARE(configureRequestedSpy.count(), 2);
// The client gets activated, which gets another configure event. Though that's not relevant to the test
configureRequestedSpy.wait(10);
QSignalSpy fullscreenChangedSpy(c, &AbstractClient::fullScreenChanged);
QVERIFY(fullscreenChangedSpy.isValid());
@ -1354,5 +1377,87 @@ void TestShellClient::testXdgDecoration()
QCOMPARE(c->isDecorated(), expectedMode == XdgDecoration::Mode::ServerSide);
}
void TestShellClient::testXdgNeverCommitted()
{
//check we don't crash if we create a shell object but delete the ShellClient before committing it
QScopedPointer<Surface> surface(Test::createSurface());
QScopedPointer<XdgShellSurface> shellSurface(Test::createXdgShellStableSurface(surface.data(), nullptr, Test::CreationSetup::CreateOnly));
}
void TestShellClient::testXdgInitialState()
{
QScopedPointer<Surface> surface(Test::createSurface());
QScopedPointer<XdgShellSurface> shellSurface(Test::createXdgShellStableSurface(surface.data(), nullptr, Test::CreationSetup::CreateOnly));
QSignalSpy configureRequestedSpy(shellSurface.data(), &XdgShellSurface::configureRequested);
surface->commit(Surface::CommitFlag::None);
configureRequestedSpy.wait();
QCOMPARE(configureRequestedSpy.count(), 1);
const auto size = configureRequestedSpy.first()[0].value<QSize>();
QCOMPARE(size, QSize(0, 0)); //client should chose it's preferred size
shellSurface->ackConfigure(configureRequestedSpy.first()[2].toUInt());
auto c = Test::renderAndWaitForShown(surface.data(), QSize(200,100), Qt::blue);
QCOMPARE(c->size(), QSize(200, 100));
}
void TestShellClient::testXdgInitiallyMaximised()
{
QScopedPointer<Surface> surface(Test::createSurface());
QScopedPointer<XdgShellSurface> shellSurface(Test::createXdgShellStableSurface(surface.data(), nullptr, Test::CreationSetup::CreateOnly));
QSignalSpy configureRequestedSpy(shellSurface.data(), &XdgShellSurface::configureRequested);
shellSurface->setMaximized(true);
surface->commit(Surface::CommitFlag::None);
configureRequestedSpy.wait();
QCOMPARE(configureRequestedSpy.count(), 1);
const auto size = configureRequestedSpy.first()[0].value<QSize>();
const auto state = configureRequestedSpy.first()[1].value<KWayland::Client::XdgShellSurface::States>();
QCOMPARE(size, QSize(1280, 1024));
QVERIFY(state & KWayland::Client::XdgShellSurface::State::Maximized);
shellSurface->ackConfigure(configureRequestedSpy.first()[2].toUInt());
auto c = Test::renderAndWaitForShown(surface.data(), size, Qt::blue);
QCOMPARE(c->maximizeMode(), MaximizeFull);
QCOMPARE(c->size(), QSize(1280, 1024));
}
void TestShellClient::testXdgInitiallyMinimized()
{
QScopedPointer<Surface> surface(Test::createSurface());
QScopedPointer<XdgShellSurface> shellSurface(Test::createXdgShellStableSurface(surface.data(), nullptr, Test::CreationSetup::CreateOnly));
QSignalSpy configureRequestedSpy(shellSurface.data(), &XdgShellSurface::configureRequested);
shellSurface->requestMinimize();
surface->commit(Surface::CommitFlag::None);
configureRequestedSpy.wait();
QCOMPARE(configureRequestedSpy.count(), 1);
const auto size = configureRequestedSpy.first()[0].value<QSize>();
const auto state = configureRequestedSpy.first()[1].value<KWayland::Client::XdgShellSurface::States>();
QCOMPARE(size, QSize(0, 0));
QCOMPARE(state, 0);
shellSurface->ackConfigure(configureRequestedSpy.first()[2].toUInt());
QEXPECT_FAIL("", "Client created in a minimised state is not exposed to kwin bug 404838", Abort);
auto c = Test::renderAndWaitForShown(surface.data(), size, Qt::blue, QImage::Format_ARGB32, 10);
QVERIFY(c);
QVERIFY(c->isMinimized());
}
WAYLANDTEST_MAIN(TestShellClient)
#include "shell_client_test.moc"

View File

@ -484,7 +484,7 @@ ShellSurface *createShellSurface(Surface *surface, QObject *parent)
return s;
}
XdgShellSurface *createXdgShellV5Surface(Surface *surface, QObject *parent)
XdgShellSurface *createXdgShellV5Surface(Surface *surface, QObject *parent, CreationSetup creationSetup)
{
if (!s_waylandConnection.xdgShellV5) {
return nullptr;
@ -494,10 +494,13 @@ XdgShellSurface *createXdgShellV5Surface(Surface *surface, QObject *parent)
delete s;
return nullptr;
}
if (creationSetup == CreationSetup::CreateAndConfigure) {
initXdgShellSurface(surface, s);
}
return s;
}
XdgShellSurface *createXdgShellV6Surface(Surface *surface, QObject *parent)
XdgShellSurface *createXdgShellV6Surface(Surface *surface, QObject *parent, CreationSetup creationSetup)
{
if (!s_waylandConnection.xdgShellV6) {
return nullptr;
@ -507,10 +510,13 @@ XdgShellSurface *createXdgShellV6Surface(Surface *surface, QObject *parent)
delete s;
return nullptr;
}
if (creationSetup == CreationSetup::CreateAndConfigure) {
initXdgShellSurface(surface, s);
}
return s;
}
XdgShellSurface *createXdgShellStableSurface(Surface *surface, QObject *parent)
XdgShellSurface *createXdgShellStableSurface(Surface *surface, QObject *parent, CreationSetup creationSetup)
{
if (!s_waylandConnection.xdgShellStable) {
return nullptr;
@ -520,10 +526,13 @@ XdgShellSurface *createXdgShellStableSurface(Surface *surface, QObject *parent)
delete s;
return nullptr;
}
if (creationSetup == CreationSetup::CreateAndConfigure) {
initXdgShellSurface(surface, s);
}
return s;
}
XdgShellPopup *createXdgShellStablePopup(Surface *surface, XdgShellSurface *parentSurface, const XdgPositioner &positioner, QObject *parent)
XdgShellPopup *createXdgShellStablePopup(Surface *surface, XdgShellSurface *parentSurface, const XdgPositioner &positioner, QObject *parent, CreationSetup creationSetup)
{
if (!s_waylandConnection.xdgShellStable) {
return nullptr;
@ -533,26 +542,64 @@ XdgShellPopup *createXdgShellStablePopup(Surface *surface, XdgShellSurface *pare
delete s;
return nullptr;
}
if (creationSetup == CreationSetup::CreateAndConfigure) {
initXdgShellPopup(surface, s);
}
return s;
}
void initXdgShellSurface(KWayland::Client::Surface *surface, KWayland::Client::XdgShellSurface *shellSurface)
{
//wait for configure
QSignalSpy configureRequestedSpy(shellSurface, &KWayland::Client::XdgShellSurface::configureRequested);
QVERIFY(configureRequestedSpy.isValid());
surface->commit(Surface::CommitFlag::None);
QVERIFY(configureRequestedSpy.wait());
shellSurface->ackConfigure(configureRequestedSpy.last()[2].toInt());
}
void initXdgShellPopup(KWayland::Client::Surface *surface, KWayland::Client::XdgShellPopup *shellPopup)
{
//wait for configure
QSignalSpy configureRequestedSpy(shellPopup, &KWayland::Client::XdgShellPopup::configureRequested);
QVERIFY(configureRequestedSpy.isValid());
surface->commit(Surface::CommitFlag::None);
QVERIFY(configureRequestedSpy.wait());
shellPopup->ackConfigure(configureRequestedSpy.last()[1].toInt());
}
QObject *createShellSurface(ShellSurfaceType type, KWayland::Client::Surface *surface, QObject *parent)
{
switch (type) {
case ShellSurfaceType::WlShell:
return createShellSurface(surface, parent);
case ShellSurfaceType::XdgShellV5:
return createXdgShellV5Surface(surface, parent);
return createXdgShellV5Surface(surface, parent, CreationSetup::CreateAndConfigure);
case ShellSurfaceType::XdgShellV6:
return createXdgShellV6Surface(surface, parent);
return createXdgShellV6Surface(surface, parent, CreationSetup::CreateAndConfigure);
case ShellSurfaceType::XdgShellStable:
return createXdgShellStableSurface(surface, parent);
return createXdgShellStableSurface(surface, parent, CreationSetup::CreateAndConfigure);
default:
Q_UNREACHABLE();
return nullptr;
}
}
KWayland::Client::XdgShellSurface *createXdgShellSurface(ShellSurfaceType type, KWayland::Client::Surface *surface, QObject *parent, CreationSetup creationSetup)
{
switch (type) {
case ShellSurfaceType::XdgShellV5:
return createXdgShellV5Surface(surface, parent, creationSetup);
case ShellSurfaceType::XdgShellV6:
return createXdgShellV6Surface(surface, parent, creationSetup);
case ShellSurfaceType::XdgShellStable:
return createXdgShellStableSurface(surface, parent, creationSetup);
default:
return nullptr;
}
}
bool waitForWindowDestroyed(AbstractClient *client)
{
QSignalSpy destroyedSpy(client, &QObject::destroyed);

View File

@ -88,7 +88,9 @@ ShellClient::ShellClient(XdgShellSurfaceInterface *surface)
, m_internal(surface->client() == waylandServer()->internalConnection())
{
setSurface(surface->surface());
m_requestGeometryBlockCounter++;
init();
connect(surface->surface(), &SurfaceInterface::committed, this, &ShellClient::finishInit);
}
ShellClient::ShellClient(XdgShellPopupInterface *surface)
@ -99,7 +101,9 @@ ShellClient::ShellClient(XdgShellPopupInterface *surface)
, m_internal(surface->client() == waylandServer()->internalConnection())
{
setSurface(surface->surface());
m_requestGeometryBlockCounter++;
init();
connect(surface->surface(), &SurfaceInterface::committed, this, &ShellClient::finishInit);
}
ShellClient::~ShellClient() = default;
@ -314,7 +318,6 @@ void ShellClient::init()
}
m_xdgShellSurface->configure(xdgSurfaceStates(), m_requestedClientSize);
};
configure();
connect(this, &AbstractClient::activeChanged, this, configure);
connect(this, &AbstractClient::clientStartUserMovedResized, this, configure);
connect(this, &AbstractClient::clientFinishUserMovedResized, this, configure);
@ -330,9 +333,6 @@ void ShellClient::init()
m_lastAckedConfigureRequest = serial;
});
QRect position = QRect(m_xdgShellPopup->transientOffset(), m_xdgShellPopup->initialSize());
m_xdgShellPopup->configure(position);
connect(m_xdgShellPopup, &XdgShellPopupInterface::destroyed, this, &ShellClient::destroyClient);
}
@ -370,6 +370,21 @@ void ShellClient::init()
}
}
void ShellClient::finishInit() {
SurfaceInterface *s = surface();
disconnect(s, &SurfaceInterface::committed, this, &ShellClient::finishInit);
if (m_xdgShellPopup) {
QRect area = workspace()->clientArea(PlacementArea, Screens::self()->current(), desktop());
placeIn(area);
}
m_requestGeometryBlockCounter--;
if (m_requestGeometryBlockCounter == 0) {
requestGeometry(m_blockedRequestGeometry);
}
}
void ShellClient::destroyClient()
{
m_closing = true;

View File

@ -195,7 +195,15 @@ private Q_SLOTS:
void clientFullScreenChanged(bool fullScreen);
private:
/**
* Called when the shell is created.
**/
void init();
/**
* Called for the XDG case when the shell surface is committed to the surface.
* At this point all initial properties should have been set by the client.
**/
void finishInit();
template <class T>
void initSurface(T *shellSurface);
void createDecoration(const QRect &oldgeom);
@ -258,7 +266,7 @@ private:
bool m_hasPopupGrab = false;
qreal m_opacity = 1.0;
class RequestGeometryBlocker {
class RequestGeometryBlocker { //TODO rename ConfigureBlocker when this class is Xdg only
public:
RequestGeometryBlocker(ShellClient *client)
: m_client(client)