Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1907103002:
X11: Handle change notify for _NET_WORKAREA (Closed)
|
Created:
4 years, 8 months ago by Tom (Use chromium acct) Modified:
4 years, 8 months ago CC:
chromium-reviews, tfarina, Lei Zhang Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionX11: Handle change notify for _NET_WORKAREA
Update window geometry when the WM tells us our work area has changed.
This fixes a race condition where the WM would update the work area
after we already used a stale version of it.
BUG=510079
Committed: https://crrev.com/b71228f3bfea5e5e526abd4bf43110f3eb60417b
Cr-Commit-Position: refs/heads/master@{#388967}
Patch Set 1 #
Total comments: 6
Patch Set 2 : Update window geometry when the WM tells us our work area has changed. #
Messages
Total messages: 22 (10 generated)
The CQ bit was checked by thomasanderson@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1907103002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1907103002/1
thomasanderson@google.com changed reviewers: + erg@chromium.org
Since you only touched this one file, I'm the only owner you need. https://codereview.chromium.org/1907103002/diff/1/ui/views/widget/desktop_aur... File ui/views/widget/desktop_aura/desktop_screen_x11.cc (right): https://codereview.chromium.org/1907103002/diff/1/ui/views/widget/desktop_aur... ui/views/widget/desktop_aura/desktop_screen_x11.cc:249: event->xproperty.atom == atom_cache_.GetAtom("_NET_WORKAREA")) { XEvent is a giant union, not a struct. Check event->type before you access memory at event->xproperty.*.
https://codereview.chromium.org/1907103002/diff/1/ui/views/widget/desktop_aur... File ui/views/widget/desktop_aura/desktop_screen_x11.cc (right): https://codereview.chromium.org/1907103002/diff/1/ui/views/widget/desktop_aur... ui/views/widget/desktop_aura/desktop_screen_x11.cc:36: const char* kAtomsToCache[] = { const char* const, otherwise one can write: kAtomsToCache[0] = nullptr. https://codereview.chromium.org/1907103002/diff/1/ui/views/widget/desktop_aur... ui/views/widget/desktop_aura/desktop_screen_x11.cc:38: NULL nullptr
thestig@chromium.org changed reviewers: + thestig@chromium.org
Oh, and please try to write a more git friendly commit message.
The CQ bit was checked by thomasanderson@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1907103002/10001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1907103002/10001
https://codereview.chromium.org/1907103002/diff/1/ui/views/widget/desktop_aur... File ui/views/widget/desktop_aura/desktop_screen_x11.cc (right): https://codereview.chromium.org/1907103002/diff/1/ui/views/widget/desktop_aur... ui/views/widget/desktop_aura/desktop_screen_x11.cc:36: const char* kAtomsToCache[] = { On 2016/04/21 22:45:31, Lei Zhang wrote: > const char* const, otherwise one can write: kAtomsToCache[0] = nullptr. Done. https://codereview.chromium.org/1907103002/diff/1/ui/views/widget/desktop_aur... ui/views/widget/desktop_aura/desktop_screen_x11.cc:38: NULL On 2016/04/21 22:45:31, Lei Zhang wrote: > nullptr Done. https://codereview.chromium.org/1907103002/diff/1/ui/views/widget/desktop_aur... ui/views/widget/desktop_aura/desktop_screen_x11.cc:249: event->xproperty.atom == atom_cache_.GetAtom("_NET_WORKAREA")) { On 2016/04/21 22:43:05, Elliot Glaysher wrote: > XEvent is a giant union, not a struct. Check event->type before you access > memory at event->xproperty.*. Done.
Description was changed from ========== Update window geometry when the WM tells us our work area has changed. This fixes a race condition where the WM would update the work area after we already used a stale version of it. BUG=510079 ========== to ========== This fixes a race condition where the WM would update the work area after we already used a stale version of it. BUG=510079 ==========
Description was changed from ========== This fixes a race condition where the WM would update the work area after we already used a stale version of it. BUG=510079 ========== to ========== X11: Handle change notify for _NET_WORKAREA Update window geometry when the WM tells us our work area has changed. This fixes a race condition where the WM would update the work area after we already used a stale version of it. BUG=510079 ==========
Description was changed from ========== X11: Handle change notify for _NET_WORKAREA Update window geometry when the WM tells us our work area has changed. This fixes a race condition where the WM would update the work area after we already used a stale version of it. BUG=510079 ========== to ========== X11: Handle change notify for _NET_WORKAREA Update window geometry when the WM tells us our work area has changed. This fixes a race condition where the WM would update the work area after we already used a stale version of it. BUG=510079 ==========
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by thomasanderson@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1907103002/10001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1907103002/10001
Message was sent while issue was closed.
Committed patchset #2 (id:10001)
Message was sent while issue was closed.
Description was changed from ========== X11: Handle change notify for _NET_WORKAREA Update window geometry when the WM tells us our work area has changed. This fixes a race condition where the WM would update the work area after we already used a stale version of it. BUG=510079 ========== to ========== X11: Handle change notify for _NET_WORKAREA Update window geometry when the WM tells us our work area has changed. This fixes a race condition where the WM would update the work area after we already used a stale version of it. BUG=510079 Committed: https://crrev.com/b71228f3bfea5e5e526abd4bf43110f3eb60417b Cr-Commit-Position: refs/heads/master@{#388967} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/b71228f3bfea5e5e526abd4bf43110f3eb60417b Cr-Commit-Position: refs/heads/master@{#388967} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
