[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [kwindowsystem] /: [xcb] Fix implementation of _NET_WM_FULLSCREEN_MONITORS
From:       Martin_Flöser <null () kde ! org>
Date:       2018-03-21 5:49:58
Message-ID: E1eyWdK-0001Gx-EG () code ! kde ! org
[Download RAW message or body]

Git commit cc5d6fde1abad968a6c54e872a56833db876dca4 by Martin Flöser.
Committed on 18/03/2018 at 10:45.
Pushed by graesslin into branch 'master'.

[xcb] Fix implementation of _NET_WM_FULLSCREEN_MONITORS

Summary:
According to NETWM spec the client "wishing to change this list MUST send
a _NET_WM_FULLSCREEN_MONITORS client message to the root window". This
was not implemented at all, instead the property was updated. The
property must be set by the window manager, or as the spec says:
"The Window Manager MUST keep this list updated to reflect the current
state of the window."

In KWin (as the user of NETWinInfo) this was implemented correctly (see
kwin, file netinfo.cpp method WinInfo::changeFullscreenMonitors).

BUG: 391960

Test Plan:
New test case added which verifies the client/wm interplay,
old and incorrect test case removed

Reviewers: #frameworks, #kwin, #plasma

Tags: #frameworks

Differential Revision: https://phabricator.kde.org/D11414

M  +0    -44   autotests/netwininfotestclient.cpp
M  +85   -0    autotests/netwininfotestwm.cpp
M  +17   -11   src/platforms/xcb/netwm.cpp

https://commits.kde.org/kwindowsystem/cc5d6fde1abad968a6c54e872a56833db876dca4

diff --git a/autotests/netwininfotestclient.cpp b/autotests/netwininfotestclient.cpp
index f34bf49..eb26741 100644
--- a/autotests/netwininfotestclient.cpp
+++ b/autotests/netwininfotestclient.cpp
@@ -70,7 +70,6 @@ private Q_SLOTS:
     void testStrut();
     void testExtendedStrut();
     void testIconGeometry();
-    void testFullscreenMonitors();
     void testWindowType_data();
     void testWindowType();
 
@@ -574,49 +573,6 @@ void NetWinInfoTestClient::testIconGeometry()
     QCOMPARE(geo.size.height, newGeo.size.height);
 }
 
-void NetWinInfoTestClient::testFullscreenMonitors()
-{
-    QVERIFY(connection());
-    ATOM(_NET_WM_FULLSCREEN_MONITORS)
-    INFO
-
-    NETFullscreenMonitors topology = info.fullscreenMonitors();
-    QCOMPARE(topology.bottom, 0);
-    QCOMPARE(topology.left, 0);
-    QCOMPARE(topology.right, 0);
-    QCOMPARE(topology.top, -1);
-
-    NETFullscreenMonitors newTopology;
-    newTopology.bottom = 10;
-    newTopology.left   = 20;
-    newTopology.right  = 30;
-    newTopology.top    = 40;
-    info.setFullscreenMonitors(newTopology);
-    topology = info.fullscreenMonitors();
-    QCOMPARE(topology.bottom, newTopology.bottom);
-    QCOMPARE(topology.left,   newTopology.left);
-    QCOMPARE(topology.right,  newTopology.right);
-    QCOMPARE(topology.top,    newTopology.top);
-
-    // compare with the X property
-    QVERIFY(atom != XCB_ATOM_NONE);
-    GETPROP(XCB_ATOM_CARDINAL, 4, 32)
-    uint32_t *data = reinterpret_cast<uint32_t \
                *>(xcb_get_property_value(reply.data()));
-    QCOMPARE(data[0], uint32_t(newTopology.top));
-    QCOMPARE(data[1], uint32_t(newTopology.bottom));
-    QCOMPARE(data[2], uint32_t(newTopology.left));
-    QCOMPARE(data[3], uint32_t(newTopology.right));
-
-    // and wait for our event
-    QEXPECT_FAIL("", "FullscreenMonitors not handled in events", Continue);
-    waitForPropertyChange(&info, atom, NET::Property(0), \
                NET::WM2FullscreenMonitors);
-    topology = info.fullscreenMonitors();
-    QCOMPARE(topology.bottom, newTopology.bottom);
-    QCOMPARE(topology.left,   newTopology.left);
-    QCOMPARE(topology.right,  newTopology.right);
-    QCOMPARE(topology.top,    newTopology.top);
-}
-
 Q_DECLARE_METATYPE(NET::WindowType)
 void NetWinInfoTestClient::testWindowType_data()
 {
diff --git a/autotests/netwininfotestwm.cpp b/autotests/netwininfotestwm.cpp
index 950c56c..60f7bd3 100644
--- a/autotests/netwininfotestwm.cpp
+++ b/autotests/netwininfotestwm.cpp
@@ -75,6 +75,7 @@ private Q_SLOTS:
     void testFrameExtents();
     void testFrameExtentsKDE();
     void testFrameOverlap();
+    void testFullscreenMonitors();
 
 private:
     bool hasAtomFlag(const xcb_atom_t *atoms, int atomsLenght, const QByteArray \
&actionName); @@ -89,6 +90,7 @@ private:
     QScopedPointer<QProcess> m_xvfb;
     xcb_window_t m_rootWindow;
     xcb_window_t m_testWindow;
+    QByteArray m_displayNumber;
 };
 
 void NetWinInfoTestWM::initTestCase()
@@ -130,6 +132,7 @@ void NetWinInfoTestWM::init()
 
     displayNumber.prepend(QByteArray(":"));
     displayNumber.remove(displayNumber.size() -1, 1);
+    m_displayNumber = displayNumber;
 
     // create X connection
     int screen = 0;
@@ -564,6 +567,88 @@ void NetWinInfoTestWM::testVisibleName()
     QCOMPARE(info.visibleName(), "bar");
 }
 
+class MockWinInfo : public NETWinInfo
+{
+public:
+    MockWinInfo(xcb_connection_t *connection, xcb_window_t window, xcb_window_t \
rootWindow) +        : NETWinInfo(connection, window, rootWindow, \
NET::WMAllProperties, NET::WM2AllProperties, NET::WindowManager) +    {
+    }
+
+protected:
+    void changeFullscreenMonitors(NETFullscreenMonitors topology) override
+    {
+        setFullscreenMonitors(topology);
+    }
+};
+
+void NetWinInfoTestWM::testFullscreenMonitors()
+{
+    // test case for BUG 391960
+    QVERIFY(connection());
+    const uint32_t maskValues[] = {
+                     XCB_EVENT_MASK_SUBSTRUCTURE_REDIRECT |
+                     XCB_EVENT_MASK_KEY_PRESS |
+                     XCB_EVENT_MASK_PROPERTY_CHANGE |
+                     XCB_EVENT_MASK_COLOR_MAP_CHANGE |
+                     XCB_EVENT_MASK_SUBSTRUCTURE_REDIRECT |
+                     XCB_EVENT_MASK_SUBSTRUCTURE_NOTIFY |
+                     XCB_EVENT_MASK_FOCUS_CHANGE | // For NotifyDetailNone
+                     XCB_EVENT_MASK_EXPOSURE
+    };
+    QScopedPointer<xcb_generic_error_t, QScopedPointerPodDeleter> \
redirectCheck(xcb_request_check(connection(), +                                       \
xcb_change_window_attributes_checked(connection(), +                                  \
m_rootWindow, +                                                                       \
XCB_CW_EVENT_MASK, +                                                                  \
maskValues))); +    QVERIFY(redirectCheck.isNull());
+
+    KXUtils::Atom atom(connection(), \
QByteArrayLiteral("_NET_WM_FULLSCREEN_MONITORS")); +
+    // create client connection
+    auto clientConnection = xcb_connect(m_displayNumber.constData(), nullptr);
+    QVERIFY(clientConnection);
+    QVERIFY(!xcb_connection_has_error(clientConnection));
+
+    NETWinInfo clientInfo(clientConnection, m_testWindow, m_rootWindow, \
NET::WMAllProperties, NET::WM2AllProperties); +    NETFullscreenMonitors topology;
+    topology.top = 1;
+    topology.bottom = 2;
+    topology.left = 3;
+    topology.right = 4;
+    clientInfo.setFullscreenMonitors(topology);
+    xcb_flush(clientConnection);
+
+    MockWinInfo info(connection(), m_testWindow, m_rootWindow);
+
+    while (true) {
+        KXUtils::ScopedCPointer<xcb_generic_event_t> \
event(xcb_wait_for_event(connection())); +        if (event.isNull()) {
+            break;
+        }
+        if ((event->response_type & ~0x80) != XCB_CLIENT_MESSAGE) {
+            continue;
+        }
+
+        NET::Properties dirtyProtocols;
+        NET::Properties2 dirtyProtocols2;
+        QCOMPARE(info.fullscreenMonitors().isSet(), false);
+        info.event(event.data(), &dirtyProtocols, &dirtyProtocols2);
+        QCOMPARE(info.fullscreenMonitors().isSet(), true);
+        break;
+    }
+    xcb_flush(connection());
+    // now the property should be updated
+    waitForPropertyChange(&info, atom, NET::Property(0), \
NET::WM2FullscreenMonitors); +
+    QCOMPARE(info.fullscreenMonitors().top, 1);
+    QCOMPARE(info.fullscreenMonitors().bottom, 2);
+    QCOMPARE(info.fullscreenMonitors().left, 3);
+    QCOMPARE(info.fullscreenMonitors().right, 4);
+
+    xcb_disconnect(clientConnection);
+}
+
 QTEST_GUILESS_MAIN(NetWinInfoTestWM)
 
 #include "netwininfotestwm.moc"
diff --git a/src/platforms/xcb/netwm.cpp b/src/platforms/xcb/netwm.cpp
index 3760cac..4d6fdac 100644
--- a/src/platforms/xcb/netwm.cpp
+++ b/src/platforms/xcb/netwm.cpp
@@ -2841,20 +2841,24 @@ void NETWinInfo::setStrut(NETStrut strut)
 
 void NETWinInfo::setFullscreenMonitors(NETFullscreenMonitors topology)
 {
-    if (p->role != Client) {
-        return;
-    }
+    if (p->role == Client) {
+        const uint32_t data[5] = {
+            topology.top, topology.bottom, topology.left, topology.right, 1
+        };
 
-    p->fullscreen_monitors = topology;
+        send_client_message(p->conn, netwm_sendevent_mask, p->root, p->window, \
p->atom(_NET_WM_FULLSCREEN_MONITORS), data); +    } else {
+        p->fullscreen_monitors = topology;
 
-    uint32_t data[4];
-    data[0] = topology.top;
-    data[1] = topology.bottom;
-    data[2] = topology.left;
-    data[3] = topology.right;
+        uint32_t data[4];
+        data[0] = topology.top;
+        data[1] = topology.bottom;
+        data[2] = topology.left;
+        data[3] = topology.right;
 
-    xcb_change_property(p->conn, XCB_PROP_MODE_REPLACE, p->window, \
                p->atom(_NET_WM_FULLSCREEN_MONITORS),
-                        XCB_ATOM_CARDINAL, 32, 4, (const void *) data);
+        xcb_change_property(p->conn, XCB_PROP_MODE_REPLACE, p->window, \
p->atom(_NET_WM_FULLSCREEN_MONITORS), +                            XCB_ATOM_CARDINAL, \
32, 4, (const void *) data); +    }
 }
 
 void NETWinInfo::setState(NET::States state, NET::States mask)
@@ -3775,6 +3779,8 @@ void NETWinInfo::event(xcb_generic_event_t *event, \
NET::Properties *properties,  dirty2 |= WM2OpaqueRegion;
         } else if (pe->atom == p->atom(_KDE_NET_WM_DESKTOP_FILE)) {
             dirty2 = WM2DesktopFileName;
+        } else if (pe->atom == p->atom(_NET_WM_FULLSCREEN_MONITORS)) {
+            dirty2 = WM2FullscreenMonitors;
         }
 
         do_update = true;


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic