Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(246)

Issue 264713007: Add unittests for X11TopmostWindowFinder (Closed)

Created:
6 years, 7 months ago by pkotwicz
Modified:
6 years, 6 months ago
Reviewers:
sadrul
CC:
chromium-reviews, tdanderson+views_chromium.org, yusukes+watch_chromium.org, derat+watch_chromium.org, tfarina, ben+views_chromium.org
Visibility:
Public.

Description

Add unittests for X11TopmostWindowFinder A couple of additional changes: - Moves X11PropertyChangeWaiter to its own class so that it can be shared among several tests - Moves the dependency on Xlib.h out of path_x11.h and uses forward declaration instead. This is necessary because the definitions of "None" and "Bool" in Xlib.h conflict with those in gtest. - Moves X11WindowEventFilter::SetUseHostWindowBorders() into x11_util.cc so that it can be used in tests on X windows which are not associated with a views::Widget BUG=None TEST=X11TopmostWindowFinderTest.* R=sadrul TBR=sky (For trivial change to the includes in ui/gfx/path_x11.*) Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=277116

Patch Set 1 : #

Total comments: 1

Patch Set 2 : #

Total comments: 1

Patch Set 3 : #

Total comments: 1

Patch Set 4 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+590 lines, -192 lines) Patch
M ui/base/x/x11_util.h View 1 2 3 1 chunk +4 lines, -0 lines 0 comments Download
M ui/base/x/x11_util.cc View 1 2 3 1 chunk +8 lines, -0 lines 0 comments Download
M ui/gfx/path_x11.h View 1 2 1 chunk +2 lines, -3 lines 0 comments Download
M ui/gfx/path_x11.cc View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
A ui/views/test/x11_property_change_waiter.h View 1 2 3 1 chunk +64 lines, -0 lines 0 comments Download
A ui/views/test/x11_property_change_waiter.cc View 1 2 3 1 chunk +84 lines, -0 lines 0 comments Download
M ui/views/views.gyp View 1 2 3 3 chunks +4 lines, -0 lines 0 comments Download
M ui/views/widget/desktop_aura/desktop_screen_x11_unittest.cc View 1 2 3 3 chunks +0 lines, -111 lines 0 comments Download
M ui/views/widget/desktop_aura/desktop_window_tree_host_x11_unittest.cc View 1 2 3 2 chunks +33 lines, -78 lines 0 comments Download
A ui/views/widget/desktop_aura/x11_topmost_window_finder_unittest.cc View 1 2 3 1 chunk +389 lines, -0 lines 0 comments Download

Messages

Total messages: 15 (0 generated)
pkotwicz
Sadrul, PTAL Next up is adding tests for drag and drop
6 years, 7 months ago (2014-05-01 21:01:42 UTC) #1
pkotwicz
https://codereview.chromium.org/264713007/diff/20001/ui/gfx/x/x11_atom_cache.h File ui/gfx/x/x11_atom_cache.h (right): https://codereview.chromium.org/264713007/diff/20001/ui/gfx/x/x11_atom_cache.h#newcode13 ui/gfx/x/x11_atom_cache.h:13: #include "ui/gfx/x/x11_types.h" I made this change because including X11/Xlib.h ...
6 years, 7 months ago (2014-05-01 21:04:18 UTC) #2
sadrul
> This CL also modifies X11TopmostWindowFinder to query the X > server for properties about ...
6 years, 7 months ago (2014-05-02 19:19:37 UTC) #3
pkotwicz
Sadrul, can you please take another look?
6 years, 7 months ago (2014-05-17 00:36:41 UTC) #4
pkotwicz
Ping?
6 years, 7 months ago (2014-05-24 22:42:34 UTC) #5
sadrul
On 2014/05/24 22:42:34, pkotwicz wrote: > Ping? Please include some details about the changes made ...
6 years, 7 months ago (2014-05-26 14:42:52 UTC) #6
pkotwicz
Done, can you please take another look?
6 years, 7 months ago (2014-05-26 14:55:23 UTC) #7
sadrul
Nice! LGTM https://codereview.chromium.org/264713007/diff/70001/ui/views/test/x11_property_change_waiter.h File ui/views/test/x11_property_change_waiter.h (right): https://codereview.chromium.org/264713007/diff/70001/ui/views/test/x11_property_change_waiter.h#newcode31 ui/views/test/x11_property_change_waiter.h:31: // Returns whether the run loop can ...
6 years, 7 months ago (2014-05-27 15:17:18 UTC) #8
pkotwicz
The CQ bit was checked by pkotwicz@chromium.org
6 years, 6 months ago (2014-06-13 14:33:59 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/264713007/190001
6 years, 6 months ago (2014-06-13 14:34:16 UTC) #10
pkotwicz
The CQ bit was unchecked by pkotwicz@chromium.org
6 years, 6 months ago (2014-06-13 17:14:41 UTC) #11
pkotwicz
The CQ bit was checked by pkotwicz@chromium.org
6 years, 6 months ago (2014-06-13 17:15:22 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/264713007/190001
6 years, 6 months ago (2014-06-13 17:16:59 UTC) #13
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win_chromium_rel on tryserver.chromium ...
6 years, 6 months ago (2014-06-13 20:38:36 UTC) #14
commit-bot: I haz the power
6 years, 6 months ago (2014-06-13 22:44:06 UTC) #15
Message was sent while issue was closed.
Change committed as 277116

Powered by Google App Engine
This is Rietveld 408576698