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

Issue 2948553002: Decouple X11WindowEventFilter into WindowEventFilter parent class. (Closed)

Created:
3 years, 6 months ago by msisov
Modified:
3 years, 5 months ago
CC:
chromium-reviews, tfarina, tonikitoo, rjkroege
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Decouple X11WindowEventFilter into WindowEventFilter parent class. This CL decouples X11WindowEventFilter into two classes: WindowEventFilter and X11WindowEventFilter. X11WindowEventFilter would inherit from WindowEventFilter and continued instantiated/used by stock Chrome/Chromium Linux/X11 builds, as it is today, so there would be no functionality change here. However, on the other side, downstream forks of Chromium could benefit from the window system agnostic parts of the implementation, in the newly added WindowEventFilter class. For instance, the Chromium Ozone/Mus implementation in [1] would greatly benefit from it in other to provide resize and window dragging functionalities. FWIW, Ozone backends like X11 and Wayland already work with it downstream. BUG=666958 Review-Url: https://codereview.chromium.org/2948553002 Cr-Commit-Position: refs/heads/master@{#483200} Committed: https://chromium.googlesource.com/chromium/src/+/e293efc74b2050a2eb73e5d17009a59a3d57dc51

Patch Set 1 #

Patch Set 2 : remove unnecessary includes #

Patch Set 3 : rebased, added erg to owners #

Total comments: 12

Patch Set 4 : fixing nits #

Patch Set 5 : change function order #

Unified diffs Side-by-side diffs Delta from patch set Stats (+222 lines, -141 lines) Patch
M ui/views/BUILD.gn View 1 2 3 1 chunk +5 lines, -1 line 0 comments Download
M ui/views/widget/desktop_aura/OWNERS View 1 2 1 chunk +1 line, -0 lines 0 comments Download
A ui/views/widget/desktop_aura/window_event_filter.h View 1 2 3 1 chunk +61 lines, -0 lines 0 comments Download
A ui/views/widget/desktop_aura/window_event_filter.cc View 1 2 3 4 1 chunk +139 lines, -0 lines 0 comments Download
M ui/views/widget/desktop_aura/x11_window_event_filter.h View 3 chunks +6 lines, -27 lines 0 comments Download
M ui/views/widget/desktop_aura/x11_window_event_filter.cc View 1 2 3 1 chunk +10 lines, -113 lines 0 comments Download

Messages

Total messages: 39 (28 generated)
msisov
Please review
3 years, 6 months ago (2017-06-19 13:59:51 UTC) #11
msisov
gentle reminder
3 years, 6 months ago (2017-06-21 07:04:42 UTC) #12
msisov
tapted@chromium.org: Please review changes in ui/views/BUILD.gn; egm: gentle reminder
3 years, 6 months ago (2017-06-23 13:15:24 UTC) #14
tapted
I think you meant for erg@ to review, rather than egm@ :) Also, I suggest ...
3 years, 6 months ago (2017-06-24 04:50:32 UTC) #16
msisov
On 2017/06/24 04:50:32, tapted wrote: > I think you meant for erg@ to review, rather ...
3 years, 6 months ago (2017-06-26 06:46:41 UTC) #19
Elliot Glaysher
lgtm
3 years, 5 months ago (2017-06-27 19:28:15 UTC) #26
tapted
stampy lgtm for BUILD.gn
3 years, 5 months ago (2017-06-28 00:00:38 UTC) #27
sadrul
Some nits. lgtm https://codereview.chromium.org/2948553002/diff/40001/ui/views/widget/desktop_aura/window_event_filter.cc File ui/views/widget/desktop_aura/window_event_filter.cc (right): https://codereview.chromium.org/2948553002/diff/40001/ui/views/widget/desktop_aura/window_event_filter.cc#newcode1 ui/views/widget/desktop_aura/window_event_filter.cc:1: // Copyright (c) 2017 The Chromium ...
3 years, 5 months ago (2017-06-28 04:41:58 UTC) #28
msisov
https://codereview.chromium.org/2948553002/diff/40001/ui/views/widget/desktop_aura/window_event_filter.cc File ui/views/widget/desktop_aura/window_event_filter.cc (right): https://codereview.chromium.org/2948553002/diff/40001/ui/views/widget/desktop_aura/window_event_filter.cc#newcode1 ui/views/widget/desktop_aura/window_event_filter.cc:1: // Copyright (c) 2017 The Chromium Authors. All rights ...
3 years, 5 months ago (2017-06-28 20:55:05 UTC) #35
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2948553002/80001
3 years, 5 months ago (2017-06-28 20:55:23 UTC) #36
commit-bot: I haz the power
3 years, 5 months ago (2017-06-28 23:44:39 UTC) #39
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/e293efc74b2050a2eb73e5d17009...

Powered by Google App Engine
This is Rietveld 408576698