DevHeads.net

Review Request 115515: [oxygen] Check whether we are on platform X11 before calling into xcb

Review request for kde-workspace and Hugo Pereira Da Costa.

Repository: kde-workspace

Description
[oxygen] Check whether we are on platform X11 before calling into xcb

Just because we compiled with X11 present doesn't mean we run on X11.
This fixes quite a lot of crashers when trying to run framework apps
on Wayland.

@Hugo: do you know of further files which use xcb unconditionally and which I just haven't hit yet?

Diffs
kstyles/oxygen/CMakeLists.txt ca9cc1a1710187f5013482ef502c456238fd4373
kstyles/oxygen/oxygenshadowhelper.cpp f77093daa4f907afd55333032c6f6b618ad2f47f
kstyles/oxygen/oxygenstylehelper.cpp 7e2de41822bf0205e2cec7ea82c8e3f6751a2a6b
kstyles/oxygen/oxygenwindowmanager.h 7ef54d3172b0b0a92b6c33152858cb923b9d3f1e
kstyles/oxygen/oxygenwindowmanager.cpp 308ce4d049b6ce4c9c2cdd67448d351747f34b19
libs/oxygen/oxygenhelper.h 8d734e6e3f3a23c0257cc400417c340ba6d48bea
libs/oxygen/oxygenhelper.cpp 1fe78c24c053e74f2179de00ee9a5701997a0acb

Diff: <a href="https://git.reviewboard.kde.org/r/115515/diff/" title="https://git.reviewboard.kde.org/r/115515/diff/">https://git.reviewboard.kde.org/r/115515/diff/</a>

Testing
running Kate on Wayland till it crashes (or doesn't)

Thanks,

Martin Gräßlin

Comments

Re: Review Request 115515: [oxygen] Check whether we are on plat

By Martin =?ISO-88... at 02/06/2014 - 07:22

(Updated Feb. 6, 2014, 1:22 p.m.)

Review request for kde-workspace and Hugo Pereira Da Costa.

Changes
added test to blurhelper. It hadn't crashed as createAtom was adjusted to return 0.

Repository: kde-workspace

Description
[oxygen] Check whether we are on platform X11 before calling into xcb

Just because we compiled with X11 present doesn't mean we run on X11.
This fixes quite a lot of crashers when trying to run framework apps
on Wayland.

@Hugo: do you know of further files which use xcb unconditionally and which I just haven't hit yet?

Diffs (updated)
kstyles/oxygen/CMakeLists.txt ca9cc1a1710187f5013482ef502c456238fd4373
kstyles/oxygen/oxygenblurhelper.cpp d774b2c45f8ec452311d34f35adf4d9bcc39693d
kstyles/oxygen/oxygenshadowhelper.cpp f77093daa4f907afd55333032c6f6b618ad2f47f
kstyles/oxygen/oxygenstylehelper.cpp 7e2de41822bf0205e2cec7ea82c8e3f6751a2a6b
kstyles/oxygen/oxygenwindowmanager.h 7ef54d3172b0b0a92b6c33152858cb923b9d3f1e
kstyles/oxygen/oxygenwindowmanager.cpp 308ce4d049b6ce4c9c2cdd67448d351747f34b19
libs/oxygen/oxygenhelper.h 8d734e6e3f3a23c0257cc400417c340ba6d48bea
libs/oxygen/oxygenhelper.cpp 1fe78c24c053e74f2179de00ee9a5701997a0acb

Diff: <a href="https://git.reviewboard.kde.org/r/115515/diff/" title="https://git.reviewboard.kde.org/r/115515/diff/">https://git.reviewboard.kde.org/r/115515/diff/</a>

Testing
running Kate on Wayland till it crashes (or doesn't)

Thanks,

Martin Gräßlin

Re: Review Request 115515: [oxygen] Check whether we are on plat

By Martin =?ISO-88... at 02/07/2014 - 07:13

(Updated Feb. 7, 2014, 12:13 p.m.)

Status
This change has been marked as submitted.

Review request for kde-workspace and Hugo Pereira Da Costa.

Repository: kde-workspace

Description
[oxygen] Check whether we are on platform X11 before calling into xcb

Just because we compiled with X11 present doesn't mean we run on X11.
This fixes quite a lot of crashers when trying to run framework apps
on Wayland.

@Hugo: do you know of further files which use xcb unconditionally and which I just haven't hit yet?

Diffs
kstyles/oxygen/CMakeLists.txt ca9cc1a1710187f5013482ef502c456238fd4373
kstyles/oxygen/oxygenblurhelper.cpp d774b2c45f8ec452311d34f35adf4d9bcc39693d
kstyles/oxygen/oxygenshadowhelper.cpp f77093daa4f907afd55333032c6f6b618ad2f47f
kstyles/oxygen/oxygenstylehelper.cpp 7e2de41822bf0205e2cec7ea82c8e3f6751a2a6b
kstyles/oxygen/oxygenwindowmanager.h 7ef54d3172b0b0a92b6c33152858cb923b9d3f1e
kstyles/oxygen/oxygenwindowmanager.cpp 308ce4d049b6ce4c9c2cdd67448d351747f34b19
libs/oxygen/oxygenhelper.h 8d734e6e3f3a23c0257cc400417c340ba6d48bea
libs/oxygen/oxygenhelper.cpp 1fe78c24c053e74f2179de00ee9a5701997a0acb

Diff: <a href="https://git.reviewboard.kde.org/r/115515/diff/" title="https://git.reviewboard.kde.org/r/115515/diff/">https://git.reviewboard.kde.org/r/115515/diff/</a>

Testing
running Kate on Wayland till it crashes (or doesn't)

Thanks,

Martin Gräßlin

Re: Review Request 115515: [oxygen] Check whether we are on plat

By Commit Hook at 02/07/2014 - 07:13

This review has been submitted with commit 1a4df7587968a1de26c5811f3106e17b07090516 by Martin Gräßlin to branch master.

- Commit Hook

On Feb. 6, 2014, 12:22 p.m., Martin Gräßlin wrote:

Re: Review Request 115515: [oxygen] Check whether we are on plat

By Hugo Pereira Da... at 02/07/2014 - 07:09

Ship it!

ship it, again :)

- Hugo Pereira Da Costa

On Feb. 6, 2014, 12:22 p.m., Martin Gräßlin wrote:

Re: Review Request 115515: [oxygen] Check whether we are on plat

By Hugo Pereira Da... at 02/06/2014 - 06:26

Ship it!

@Martin
in kstyles/oxygen
you are missing oxygenblurhelper (and likely kate will crash when showing a tooltip)

in kwin/clients/oxygen (but might be another review)
oxygenclient
oxygensizegrip
config/oxygendetectwidget

Other than that, ship it !

- Hugo Pereira Da Costa

On Feb. 6, 2014, 10:55 a.m., Martin Gräßlin wrote:

Re: Review Request 115515: [oxygen] Check whether we are on plat

By Martin =?ISO-88... at 02/06/2014 - 07:15

kwin/clients doesn't matter (yet) ;-)

- Martin

On Feb. 6, 2014, 11:55 a.m., Martin Gräßlin wrote: