New issue
Advanced search Search tips

Issue 782184 link

Starred by 2 users
Status: Started
Owner:
brat...@opera.com
Cc:
thomasanderson@chromium.org
cblume@chromium.org
most...@vewd.com
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug

Blocking:
issue 746953



Sign in to add a comment

Wrap X11 headers to prevent widespread macros from interfering with fine code

Project Member Reported by brat...@opera.com, Nov 7 2017 
X11 headers like the main one <X11/Xlib.h> creates macros with common names like None, Status, Success. These collide with common enum values and types and since they are macros, they apply everywhere.

This is a particular problem for jumbo builds because much more code will get to see the X11 headers due to the nature of jumbo (merging files before compilation).

The plan now is to wrap the X11 headers in custom headers that undef the macros and put necessary values into a X11 namespace.
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 10 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf

commit 01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf
Author: Daniel Bratell <bratell@opera.com>
Date: Fri Nov 10 00:41:05 2017

Wrap X11 headers in ui/gfx/x/x11.h

X11 headers define a lot of common symbols and that will interfere
with other code, in particular in jumbo builds where the X11 headers
get wider visibility. This wraps common X11 headers in
ui/gfx/x/x11.h and adds the necessary symbols to the namespace x11
instead.

So instead of writing

XAtom prop_type = None;

you now write

XAtom prop_type = x11::None;

and the same for True, False, Status and a few others.

Bug: 782184
Change-Id: I0d1b83720da615c808a2f7a291ddbf7664b26cd8
Reviewed-on: https://chromium-review.googlesource.com/756711
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Elliot Glaysher <erg@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515373}
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/chrome/browser/extensions/extension_commands_global_registry_apitest.cc
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/base/clipboard/clipboard_aurax11.cc
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/base/cursor/cursor_loader_x11.cc
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/base/cursor/cursor_loader_x11.h
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/base/cursor/cursor_loader_x11_unittest.cc
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/base/dragdrop/os_exchange_data_provider_aurax11.h
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/base/idle/idle_query_x11.cc
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/base/idle/screensaver_window_finder_x11.cc
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/base/ime/input_method_chromeos_unittest.cc
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/base/x/selection_owner.cc
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/base/x/selection_requestor.cc
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/base/x/selection_requestor_unittest.cc
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/base/x/selection_utils.cc
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/base/x/selection_utils.h
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/base/x/x11_util.cc
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/base/x/x11_util_internal.h
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/base/x/x11_window_event_manager.cc
[add] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/gfx/x/x11.h
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/views/controls/menu/menu_controller_unittest.cc
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/views/test/ui_controls_factory_desktop_aurax11.cc
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/views/widget/desktop_aura/desktop_drag_drop_client_aurax11.cc
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/views/widget/desktop_aura/desktop_screen_x11.cc
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/views/widget/desktop_aura/desktop_window_tree_host_x11_interactive_uitest.cc
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/views/widget/desktop_aura/desktop_window_tree_host_x11_unittest.cc
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/views/widget/desktop_aura/x11_desktop_handler.h
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/views/widget/desktop_aura/x11_topmost_window_finder.cc
[modify] https://crrev.com/01b976ce4bf294bccc6c9ed23bf41d525b9c3aaf/ui/views/widget/desktop_aura/x11_topmost_window_finder_interactive_uitest.cc

Comment 2 by brat...@opera.com, Nov 15 2017 

Leaving this open until we've removed the final hack and landed gl support.
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 20 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d1c31388a62e1050b26d0ed2765e8e2b2e691500

commit d1c31388a62e1050b26d0ed2765e8e2b2e691500
Author: Daniel Bratell <bratell@opera.com>
Date: Mon Nov 20 12:10:16 2017

Use the X11 header wrapper in ui/gl

The X11 header wrapper prevents us from polluting the global
namespace with risky macros such as None, True, False, Status.
This is in particular necessary for jumbo builds since they
are more vulnerable to headers doing unexpected things.

Bug: 782184
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Icd39a959c24a6b8f5a74e044e3fd1e6b959c6280
Reviewed-on: https://chromium-review.googlesource.com/771195
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#517799}
[modify] https://crrev.com/d1c31388a62e1050b26d0ed2765e8e2b2e691500/gpu/tools/compositor_model_bench/compositor_model_bench.cc
[modify] https://crrev.com/d1c31388a62e1050b26d0ed2765e8e2b2e691500/ui/PRESUBMIT.py
[modify] https://crrev.com/d1c31388a62e1050b26d0ed2765e8e2b2e691500/ui/gfx/x/x11.h
[modify] https://crrev.com/d1c31388a62e1050b26d0ed2765e8e2b2e691500/ui/gl/gl_bindings.cc
[modify] https://crrev.com/d1c31388a62e1050b26d0ed2765e8e2b2e691500/ui/gl/gl_bindings.h
[modify] https://crrev.com/d1c31388a62e1050b26d0ed2765e8e2b2e691500/ui/gl/gl_context_egl.cc
[modify] https://crrev.com/d1c31388a62e1050b26d0ed2765e8e2b2e691500/ui/gl/gl_context_glx.cc
[modify] https://crrev.com/d1c31388a62e1050b26d0ed2765e8e2b2e691500/ui/gl/gl_context_glx_unittest.cc
[modify] https://crrev.com/d1c31388a62e1050b26d0ed2765e8e2b2e691500/ui/gl/gl_image_glx.cc
[modify] https://crrev.com/d1c31388a62e1050b26d0ed2765e8e2b2e691500/ui/gl/gl_surface_egl.cc
[modify] https://crrev.com/d1c31388a62e1050b26d0ed2765e8e2b2e691500/ui/gl/gl_surface_egl_x11.cc
[modify] https://crrev.com/d1c31388a62e1050b26d0ed2765e8e2b2e691500/ui/gl/gl_surface_glx.cc
[modify] https://crrev.com/d1c31388a62e1050b26d0ed2765e8e2b2e691500/ui/gl/gl_surface_glx_x11.cc
[modify] https://crrev.com/d1c31388a62e1050b26d0ed2765e8e2b2e691500/ui/gl/gl_surface_osmesa_x11.cc
[modify] https://crrev.com/d1c31388a62e1050b26d0ed2765e8e2b2e691500/ui/gl/gl_surface_osmesa_x11.h
[modify] https://crrev.com/d1c31388a62e1050b26d0ed2765e8e2b2e691500/ui/gl/gl_visual_picker_glx.h
[modify] https://crrev.com/d1c31388a62e1050b26d0ed2765e8e2b2e691500/ui/gl/init/gl_initializer_x11.cc
[modify] https://crrev.com/d1c31388a62e1050b26d0ed2765e8e2b2e691500/ui/gl/test/gl_surface_test_support.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 21 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6dc91b0aaff3b55fd88638ddb0623919d1084490

commit 6dc91b0aaff3b55fd88638ddb0623919d1084490
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Nov 21 19:33:15 2017

Use "gfx/x/x11.h" instead of system headers in ui/events

By using x11.h instead of X11 system headers we prevent spreading
some common macros named None, Status, True, False, ... in the
global namespace. These macros forced various workarounds in
innocent code and made jumbo builds harder.

This moves a lot of X11 configuration and defines from ui/event
headers into ui/gfx/x/x11.h to make sure no code has to manually
include X11 headers.

Bug: 782184
Change-Id: Iab069db82d71cd27df395afefa34a54c252b874d
Reviewed-on: https://chromium-review.googlesource.com/779144
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518334}
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/PRESUBMIT.py
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/aura/test/ui_controls_factory_aurax11.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/aura/test/x11_event_sender.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/aura/window_tree_host_x11.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/base/x/x11_pointer_grab.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/blink/web_input_event.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/blink/web_input_event_unittest.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/devices/x11/device_data_manager_x11.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/devices/x11/device_data_manager_x11.h
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/devices/x11/device_list_cache_x11.h
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/devices/x11/touch_factory_x11.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/event.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/event_unittest.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/keycodes/DEPS
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/keycodes/keyboard_code_conversion_x.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/keycodes/keysym_to_unicode.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/keycodes/xkb_keysym.h
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/platform/x11/x11_event_source.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/platform/x11/x11_event_source_glib.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/platform/x11/x11_event_source_libevent.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/platform/x11/x11_hotplug_event_handler.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/test/event_generator.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/test/events_test_utils_x11.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/x/events_x.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/x/events_x_unittest.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/events/x/events_x_utils.cc
[modify] https://crrev.com/6dc91b0aaff3b55fd88638ddb0623919d1084490/ui/gfx/x/x11.h
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 22 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fe53b35af5daadd32605a4590240590075ddd956

commit fe53b35af5daadd32605a4590240590075ddd956
Author: Daniel Bratell <bratell@opera.com>
Date: Wed Nov 22 16:14:22 2017

Replace system header includes with gfx/x/x11.h in ui/views

X11 system headers define macros that easily clash with other code.
For instance None, True, False. To avoid that, which becomes
critical for jumbo builds, we instead have a x11.h wrapper
that protects the code from the dangerous macros.

Also removes use of the "Time" type in global scope since Time
is an X11 typedef and can't be protected by x11.h.

Bug: 782184
Change-Id: I1a605bddd8c5e4fe2f83c8432509915dc00b3700
Reviewed-on: https://chromium-review.googlesource.com/785170
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#518648}
[modify] https://crrev.com/fe53b35af5daadd32605a4590240590075ddd956/ui/PRESUBMIT.py
[modify] https://crrev.com/fe53b35af5daadd32605a4590240590075ddd956/ui/views/controls/menu/menu_controller.cc
[modify] https://crrev.com/fe53b35af5daadd32605a4590240590075ddd956/ui/views/style/typography_provider.cc
[modify] https://crrev.com/fe53b35af5daadd32605a4590240590075ddd956/ui/views/test/widget_test_aura.cc
[modify] https://crrev.com/fe53b35af5daadd32605a4590240590075ddd956/ui/views/test/x11_property_change_waiter.cc
[modify] https://crrev.com/fe53b35af5daadd32605a4590240590075ddd956/ui/views/widget/desktop_aura/desktop_drag_drop_client_aurax11.cc
[modify] https://crrev.com/fe53b35af5daadd32605a4590240590075ddd956/ui/views/widget/desktop_aura/desktop_drag_drop_client_aurax11.h
[modify] https://crrev.com/fe53b35af5daadd32605a4590240590075ddd956/ui/views/widget/desktop_aura/desktop_drag_drop_client_aurax11_unittest.cc
[modify] https://crrev.com/fe53b35af5daadd32605a4590240590075ddd956/ui/views/widget/desktop_aura/desktop_screen_x11.h
[modify] https://crrev.com/fe53b35af5daadd32605a4590240590075ddd956/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.h
[modify] https://crrev.com/fe53b35af5daadd32605a4590240590075ddd956/ui/views/widget/desktop_aura/x11_desktop_handler.cc
[modify] https://crrev.com/fe53b35af5daadd32605a4590240590075ddd956/ui/views/widget/desktop_aura/x11_desktop_window_move_client.cc
[modify] https://crrev.com/fe53b35af5daadd32605a4590240590075ddd956/ui/views/widget/desktop_aura/x11_desktop_window_move_client.h
[modify] https://crrev.com/fe53b35af5daadd32605a4590240590075ddd956/ui/views/widget/desktop_aura/x11_topmost_window_finder.cc
[modify] https://crrev.com/fe53b35af5daadd32605a4590240590075ddd956/ui/views/widget/desktop_aura/x11_whole_screen_move_loop.cc
[modify] https://crrev.com/fe53b35af5daadd32605a4590240590075ddd956/ui/views/widget/desktop_aura/x11_window_event_filter.cc
[modify] https://crrev.com/fe53b35af5daadd32605a4590240590075ddd956/ui/views/widget/desktop_aura/x11_window_event_filter.h
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 29 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6f2fbd46fb73982fc312c1e84017a7139ef48142

commit 6f2fbd46fb73982fc312c1e84017a7139ef48142
Author: Julien Isorce <julien.isorce@chromium.org>
Date: Wed Nov 29 19:43:39 2017

Fix build for VaapiTFPPicture

Introduced recently by https://chromium-review.googlesource.com/756711
  "Wrap X11 headers in ui/gfx/x/x11.h"

Build error 1:
  media/gpu/vaapi/vaapi_tfp_picture.cc:
    error use of undeclared identifier 'RootWindow'; did you mean 'XRootWindow'?
      x_pixmap_ = XCreatePixmap(x_display_, RootWindow(x_display_, screen),

Build error 2:
  In file included from ../../media/gpu/vaapi_video_decode_accelerator_unittest.cc:14:
  error: expected unqualified-id
    /usr/include/X11/X.h:115:30: note: expanded from macro 'None'

Bug: 782184
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I1b1ecf39baee160e1778e5cdd596426a63e4e054
Reviewed-on: https://chromium-review.googlesource.com/788111
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Julien Isorce <julien.isorce@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520205}
[modify] https://crrev.com/6f2fbd46fb73982fc312c1e84017a7139ef48142/media/gpu/vaapi/vaapi_tfp_picture.cc
[modify] https://crrev.com/6f2fbd46fb73982fc312c1e84017a7139ef48142/media/gpu/vaapi_wrapper.cc
[modify] https://crrev.com/6f2fbd46fb73982fc312c1e84017a7139ef48142/media/gpu/vaapi_wrapper.h
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 30 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/17a05c9d72120bb69a64cf714252db1adad08719

commit 17a05c9d72120bb69a64cf714252db1adad08719
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Nov 30 00:39:30 2017

Clearing content from include <X11/*>

X11 headers leak a lot of macros named things like None, Status, True,
False and they cause problems with normal code. To avoid that we now
wrap all commonly named symbols in an x11 namespace and include
"ui/gfx/x/x11.h" instead of <X11/*>.

Bug: 782184
Change-Id: I37bf100f2d2acc5d51ec62051e90939b56d9f690
Reviewed-on: https://chromium-review.googlesource.com/797034
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#520344}
[modify] https://crrev.com/17a05c9d72120bb69a64cf714252db1adad08719/content/common/cursors/webcursor_aurax11.cc
[modify] https://crrev.com/17a05c9d72120bb69a64cf714252db1adad08719/content/public/test/unittest_test_suite.cc
[modify] https://crrev.com/17a05c9d72120bb69a64cf714252db1adad08719/content/renderer/render_view_browsertest.cc
[modify] https://crrev.com/17a05c9d72120bb69a64cf714252db1adad08719/content/renderer/render_view_impl.cc
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 30 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/75489b190de3ba44d28f04a7cc07429e07a35c0d

commit 75489b190de3ba44d28f04a7cc07429e07a35c0d
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Nov 30 20:50:11 2017

Change X11/* includes to ui/gfx/x/x11.h includes

Includes from the X11 system libraries declare macros with common
names such as None, True, False, Status and those complicate
the life for other code. To avoid that complication we have
moved the common symbol names to the namespace x11 and changed
so that you include "ui/gfx/x/x11.h" instead.

Bug: 782184
Change-Id: I3d2893425bb79cb29d6c5a602512a0e8d3cc30cd
Reviewed-on: https://chromium-review.googlesource.com/800550
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#520671}
[modify] https://crrev.com/75489b190de3ba44d28f04a7cc07429e07a35c0d/chrome/browser/chromeos/input_method/input_method_engine.cc
[modify] https://crrev.com/75489b190de3ba44d28f04a7cc07429e07a35c0d/chrome/browser/extensions/global_shortcut_listener_x11.cc
[modify] https://crrev.com/75489b190de3ba44d28f04a7cc07429e07a35c0d/chrome/browser/extensions/global_shortcut_listener_x11.h
[modify] https://crrev.com/75489b190de3ba44d28f04a7cc07429e07a35c0d/chrome/browser/media/webrtc/window_icon_util_x11.cc
[modify] https://crrev.com/75489b190de3ba44d28f04a7cc07429e07a35c0d/chrome/browser/ui/input_method/input_method_engine_base.cc
[modify] https://crrev.com/75489b190de3ba44d28f04a7cc07429e07a35c0d/chrome/browser/ui/libgtkui/gtk_event_loop.cc
[modify] https://crrev.com/75489b190de3ba44d28f04a7cc07429e07a35c0d/chrome/browser/ui/libgtkui/gtk_key_bindings_handler.cc
[modify] https://crrev.com/75489b190de3ba44d28f04a7cc07429e07a35c0d/chrome/browser/ui/libgtkui/gtk_ui.cc
[modify] https://crrev.com/75489b190de3ba44d28f04a7cc07429e07a35c0d/chrome/browser/ui/libgtkui/select_file_dialog_impl_gtk.cc
[modify] https://crrev.com/75489b190de3ba44d28f04a7cc07429e07a35c0d/chrome/browser/ui/libgtkui/select_file_dialog_impl_kde.cc
[modify] https://crrev.com/75489b190de3ba44d28f04a7cc07429e07a35c0d/chrome/browser/ui/libgtkui/x11_input_method_context_impl_gtk.cc
[modify] https://crrev.com/75489b190de3ba44d28f04a7cc07429e07a35c0d/chrome/test/chromedriver/keycode_text_conversion_x.cc
Project Member

Comment 9 by bugdroid1@chromium.org, Dec 4 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b40fc335e4614161b6d17db5bf6a1b2c5b73fd35

commit b40fc335e4614161b6d17db5bf6a1b2c5b73fd35
Author: Daniel Bratell <bratell@opera.com>
Date: Mon Dec 04 10:28:21 2017

Clear //remoting from direct X11 includes

X11 headers pollute the global namespace. Instead we use a
wrapper header (ui/gfx/x/x11.h) that puts important symbols
in the x11 namespace and removes them from the global namespace.

Bug: 782184
Change-Id: I3173f2dde17e6991cc31bd89ceaee8e651ccaf3f
Reviewed-on: https://chromium-review.googlesource.com/801390
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#521303}
[modify] https://crrev.com/b40fc335e4614161b6d17db5bf6a1b2c5b73fd35/remoting/host/clipboard_x11.cc
[modify] https://crrev.com/b40fc335e4614161b6d17db5bf6a1b2c5b73fd35/remoting/host/curtain_mode_linux.cc
[modify] https://crrev.com/b40fc335e4614161b6d17db5bf6a1b2c5b73fd35/remoting/host/desktop_resizer_x11.cc
[modify] https://crrev.com/b40fc335e4614161b6d17db5bf6a1b2c5b73fd35/remoting/host/input_injector_x11.cc
[modify] https://crrev.com/b40fc335e4614161b6d17db5bf6a1b2c5b73fd35/remoting/host/it2me/it2me_native_messaging_host_main.cc
[modify] https://crrev.com/b40fc335e4614161b6d17db5bf6a1b2c5b73fd35/remoting/host/linux/unicode_to_keysym.cc
[modify] https://crrev.com/b40fc335e4614161b6d17db5bf6a1b2c5b73fd35/remoting/host/linux/x11_keyboard_impl.cc
[modify] https://crrev.com/b40fc335e4614161b6d17db5bf6a1b2c5b73fd35/remoting/host/linux/x11_keyboard_impl.h
[modify] https://crrev.com/b40fc335e4614161b6d17db5bf6a1b2c5b73fd35/remoting/host/linux/x11_util.cc
[modify] https://crrev.com/b40fc335e4614161b6d17db5bf6a1b2c5b73fd35/remoting/host/linux/x11_util.h
[modify] https://crrev.com/b40fc335e4614161b6d17db5bf6a1b2c5b73fd35/remoting/host/linux/x_server_clipboard.cc
[modify] https://crrev.com/b40fc335e4614161b6d17db5bf6a1b2c5b73fd35/remoting/host/linux/x_server_clipboard.h
[modify] https://crrev.com/b40fc335e4614161b6d17db5bf6a1b2c5b73fd35/remoting/host/linux/x_server_clipboard_unittest.cc
[modify] https://crrev.com/b40fc335e4614161b6d17db5bf6a1b2c5b73fd35/remoting/host/local_input_monitor_x11.cc
[modify] https://crrev.com/b40fc335e4614161b6d17db5bf6a1b2c5b73fd35/remoting/host/remoting_me2me_host.cc
[modify] https://crrev.com/b40fc335e4614161b6d17db5bf6a1b2c5b73fd35/remoting/test/it2me_standalone_host_main.cc
[modify] https://crrev.com/b40fc335e4614161b6d17db5bf6a1b2c5b73fd35/ui/gfx/x/x11.h
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 5 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/89ce5e636ea2a1f973b61f0b7225138edc92f329

commit 89ce5e636ea2a1f973b61f0b7225138edc92f329
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Dec 05 10:58:27 2017

Replace X11 includes in //gpu with ui/gfx/x/x11.h

X11 header files are infamous for creating macros with common
names such as None, True, False, Status, Bool, ...

To save oursaves the mess that creates, they are now wrapped
in a header, ui/gfx/x/x11.h, which puts those symbols in the
x11 namespace and removes the global macros.

Bug: 782184
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I10a9acecb8e7adacd170d1080a411bc9d1e49f79
Reviewed-on: https://chromium-review.googlesource.com/806275
Reviewed-by: Victor Miura <vmiura@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#521659}
[modify] https://crrev.com/89ce5e636ea2a1f973b61f0b7225138edc92f329/gpu/ipc/service/gpu_watchdog_thread.cc
[modify] https://crrev.com/89ce5e636ea2a1f973b61f0b7225138edc92f329/gpu/vulkan/tests/native_window_x11.cc
[modify] https://crrev.com/89ce5e636ea2a1f973b61f0b7225138edc92f329/gpu/vulkan/vulkan_platform.h
Project Member

Comment 11 by bugdroid1@chromium.org, Dec 5 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cfe4f098d21bc16e74f9ded6e97fb5c0553271aa

commit cfe4f098d21bc16e74f9ded6e97fb5c0553271aa
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Dec 05 18:07:05 2017

Replace X11 includes in viz with ui/gfx/x/x11.h

X11 header files are infamous for creating macros with common
names such as None, True, False, Status, Bool, ...

To save oursaves the mess that creates, they are now wrapped
in a header, ui/gfx/x/x11.h, which puts those symbols in the
x11 namespace and removes the global macros.

Bug: 782184
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: Ida4d26422f44edda1a0120f52ffbaa4e437ab012
Reviewed-on: https://chromium-review.googlesource.com/805814
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#521740}
[modify] https://crrev.com/cfe4f098d21bc16e74f9ded6e97fb5c0553271aa/components/viz/service/display_embedder/software_output_device_x11.cc
[modify] https://crrev.com/cfe4f098d21bc16e74f9ded6e97fb5c0553271aa/components/viz/service/display_embedder/software_output_device_x11.h
Project Member

Comment 12 by bugdroid1@chromium.org, Dec 5 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c0a7f1670d6fb77b1c07fbe60a9173888fed7d24

commit c0a7f1670d6fb77b1c07fbe60a9173888fed7d24
Author: Maksim Sisov <msisov@igalia.com>
Date: Tue Dec 05 18:39:11 2017

Replace X11 includes in //ui/platform_window with ui/gfx/x/x11.h

X11 header files are infamous for creating macros with common
names such as None, True, False, Status, Bool, ...

Use a wrapper header ui/gfx/x/x11.h to make our life easier.

Bug: 782184
Change-Id: I88e1093b9e3a8195395a23f0fd8291613586972f
Reviewed-on: https://chromium-review.googlesource.com/809047
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Cr-Commit-Position: refs/heads/master@{#521759}
[modify] https://crrev.com/c0a7f1670d6fb77b1c07fbe60a9173888fed7d24/ui/platform_window/x11/x11_cursor_ozone.cc
[modify] https://crrev.com/c0a7f1670d6fb77b1c07fbe60a9173888fed7d24/ui/platform_window/x11/x11_cursor_ozone.h
[modify] https://crrev.com/c0a7f1670d6fb77b1c07fbe60a9173888fed7d24/ui/platform_window/x11/x11_window.cc
[modify] https://crrev.com/c0a7f1670d6fb77b1c07fbe60a9173888fed7d24/ui/platform_window/x11/x11_window_base.cc
[modify] https://crrev.com/c0a7f1670d6fb77b1c07fbe60a9173888fed7d24/ui/platform_window/x11/x11_window_base.h
[modify] https://crrev.com/c0a7f1670d6fb77b1c07fbe60a9173888fed7d24/ui/platform_window/x11/x11_window_ozone.cc
Project Member

Comment 13 by bugdroid1@chromium.org, Dec 5 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5e19a79920924c6d48c85d42f079b0aed31299b7

commit 5e19a79920924c6d48c85d42f079b0aed31299b7
Author: Maksim Sisov <msisov@igalia.com>
Date: Tue Dec 05 19:36:00 2017

Replace X11 includes in //ui/ozone with ui/gfx/x/x11.h

X11 header files are infamous for creating macros with common
names such as None, True, False, Status, Bool, ...

Use a wrapper header ui/gfx/x/x11.h to simplify our work.

Bug: 782184
Change-Id: I25a14236e6d4490a04dee49430929898d4ec124c
Reviewed-on: https://chromium-review.googlesource.com/809124
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Cr-Commit-Position: refs/heads/master@{#521785}
[modify] https://crrev.com/5e19a79920924c6d48c85d42f079b0aed31299b7/ui/ozone/platform/x11/gl_surface_egl_ozone_x11.cc
[modify] https://crrev.com/5e19a79920924c6d48c85d42f079b0aed31299b7/ui/ozone/platform/x11/gl_surface_glx_ozone.cc
[modify] https://crrev.com/5e19a79920924c6d48c85d42f079b0aed31299b7/ui/ozone/platform/x11/ozone_platform_x11.cc
[modify] https://crrev.com/5e19a79920924c6d48c85d42f079b0aed31299b7/ui/ozone/platform/x11/x11_cursor_factory_ozone.h
[modify] https://crrev.com/5e19a79920924c6d48c85d42f079b0aed31299b7/ui/ozone/platform/x11/x11_surface_factory.cc
Project Member

Comment 14 by bugdroid1@chromium.org, Dec 8 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ff8b86fb20a49008edc923f6f670e1d373e24636

commit ff8b86fb20a49008edc923f6f670e1d373e24636
Author: Daniel Bratell <bratell@opera.com>
Date: Fri Dec 08 06:35:53 2017

Replace X11 includes in media with ui/gfx/x/x11.h

X11 header files are infamous for creating macros with common
names such as None, True, False, Status, Bool, ...

To save oursaves the mess that creates, they are now wrapped
in a header, ui/gfx/x/x11.h, which puts those symbols in the
x11 namespace and removes the global macros.

I believe the references to min and max came from an include of
<X11/Xlibint.h>. I can't find such an include anymore and
the one in user_input_monitor_linux.cc doesn't seem needed so
I removed all undefs of min and max and all includes of that
header.

Bug: 782184
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I3a09cccee04eb312ab6b2575be01753df8a1f137
Reviewed-on: https://chromium-review.googlesource.com/806217
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Fredrik Hubinette <hubbe@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#522723}
[modify] https://crrev.com/ff8b86fb20a49008edc923f6f670e1d373e24636/media/audio/cras/audio_manager_cras.cc
[modify] https://crrev.com/ff8b86fb20a49008edc923f6f670e1d373e24636/media/base/user_input_monitor_linux.cc
[modify] https://crrev.com/ff8b86fb20a49008edc923f6f670e1d373e24636/media/cast/test/linux_output_window.h
[add] https://crrev.com/ff8b86fb20a49008edc923f6f670e1d373e24636/ui/gfx/x/OWNERS
Project Member

Comment 15 by bugdroid1@chromium.org, Dec 11 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6a08383e75eaafe07302fc939cee805011311e65

commit 6a08383e75eaafe07302fc939cee805011311e65
Author: Daniel Bratell <bratell@opera.com>
Date: Mon Dec 11 17:28:00 2017

Remove X11 includes and replace with the ui/gfx/x/x11.h wrapper

X11 headers pollute the global namespace with macros with
common words like None, Status, False. To avoid problems
because of that we now wrap X11 includes in ui/gfx/x/x11.h
and move all problematic words to the x11 namespace.

Bug: 782184
Change-Id: I81859c8aabae677c40614d3c26036c5d4d1e715a
Reviewed-on: https://chromium-review.googlesource.com/819731
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#523121}
[modify] https://crrev.com/6a08383e75eaafe07302fc939cee805011311e65/services/ui/service.cc
[modify] https://crrev.com/6a08383e75eaafe07302fc939cee805011311e65/services/ui/ws/gpu_host_unittest.cc
Project Member

Comment 16 by bugdroid1@chromium.org, Dec 11 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d08628bcd03cb902fbc9adcbd3b1c6e4a2f72fde

commit d08628bcd03cb902fbc9adcbd3b1c6e4a2f72fde
Author: Daniel Bratell <bratell@opera.com>
Date: Mon Dec 11 19:08:18 2017

Remove the last X11 includes in //ui outside ui/gfx/x/*

X11 header files are infamous for creating macros with common
names such as None, True, False, Status, Bool, ...

To save oursaves the mess that creates, they are now wrapped
in a header, ui/gfx/x/x11.h, which puts those symbols in the
x11 namespace and removes the global macros.

This means we can remove various undefs in files that have had
problems in the past.

Bug: 782184
Change-Id: Ie48fe619e3101344d53ea837b0b21570516185ca
Reviewed-on: https://chromium-review.googlesource.com/814676
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#523149}
[modify] https://crrev.com/d08628bcd03cb902fbc9adcbd3b1c6e4a2f72fde/ui/PRESUBMIT.py
[modify] https://crrev.com/d08628bcd03cb902fbc9adcbd3b1c6e4a2f72fde/ui/base/dragdrop/os_exchange_data_provider_aurax11_unittest.cc
[modify] https://crrev.com/d08628bcd03cb902fbc9adcbd3b1c6e4a2f72fde/ui/compositor/test/test_compositor_host_x11.cc
[modify] https://crrev.com/d08628bcd03cb902fbc9adcbd3b1c6e4a2f72fde/ui/display/util/x11/edid_parser_x11.cc
[modify] https://crrev.com/d08628bcd03cb902fbc9adcbd3b1c6e4a2f72fde/ui/events/devices/x11/device_data_manager_x11_unittest.cc
[modify] https://crrev.com/d08628bcd03cb902fbc9adcbd3b1c6e4a2f72fde/ui/gfx/path_x11.cc
[modify] https://crrev.com/d08628bcd03cb902fbc9adcbd3b1c6e4a2f72fde/ui/views/controls/scrollbar/base_scroll_bar.cc
Project Member

Comment 17 by bugdroid1@chromium.org, Jan 2 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bb5cf034c050c520420021dc8d07059e21546764

commit bb5cf034c050c520420021dc8d07059e21546764
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Jan 02 20:00:42 2018

Hide DeviceMode from X11 headers

X11 headers create a lot of macros using fairly normal names which
causes collisions in code that was written without X11 headers in
mind.

DeviceMode is also a type in components/policy and if that and
X11 includes end up in the same translation unit, there will be
compilation errors.

This was encountered in non-standard jumbo configurations and will
not affect currently supported code.

Bug: 782184
Change-Id: I84f9874d2858fc60c7e7c0e017d028dc8b710661
Reviewed-on: https://chromium-review.googlesource.com/847577
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#526524}
[modify] https://crrev.com/bb5cf034c050c520420021dc8d07059e21546764/ui/gfx/x/x11.h

Comment 18 by brat...@opera.com, Jan 12 2018 

X11 headers now remain in:

services/device/wake_lock/power_save_blocker/power_save_blocker_x11.cc
(uses X11/extensions/dpms.h which is not in gfx/x/x11.h)

third_party/{khronos, libXNVCtrl, vulkan}

tools/xdisplaycheck/xdisplaycheck.cc

Keeping this open to fix services/device/wake_lock but if someone else want to do it, feel free.
Project Member

Comment 19 by bugdroid1@chromium.org, Feb 3 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f27852bd3c54ffbafd122db9e276a36b9ab6d82d

commit f27852bd3c54ffbafd122db9e276a36b9ab6d82d
Author: Daniel Bratell <bratell@opera.com>
Date: Sat Feb 03 11:50:43 2018

Prevent FocusIn and FocusOut from leaking from x11.h

FocusIn and FocusOut are function names in parts of the code. To
make sure that X11 headers don't cause problems, even in jumbo
builds, this patch moves those constants to the x11 namespace.

Bug: 782184
Change-Id: I6c694557653c7c1119a134960a59a38a2670b103
Reviewed-on: https://chromium-review.googlesource.com/891322
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#534270}
[modify] https://crrev.com/f27852bd3c54ffbafd122db9e276a36b9ab6d82d/ui/gfx/x/x11.h
[modify] https://crrev.com/f27852bd3c54ffbafd122db9e276a36b9ab6d82d/ui/platform_window/x11/x11_window.cc
[modify] https://crrev.com/f27852bd3c54ffbafd122db9e276a36b9ab6d82d/ui/platform_window/x11/x11_window_base.cc
[modify] https://crrev.com/f27852bd3c54ffbafd122db9e276a36b9ab6d82d/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc
Project Member

Comment 20 by bugdroid1@chromium.org, Feb 10 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c4321f0e31fcf30a1adf437dc3427237b17979cf

commit c4321f0e31fcf30a1adf437dc3427237b17979cf
Author: Joone Hur <joone.hur@intel.com>
Date: Sat Feb 10 04:48:08 2018

Call XRootWindow instead of RootWindow

We have to use the X11 header wrapper since r515373.

BUG=782184

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Icda075c7cc9723f9bd8c667d6ab7e8848e48f8b1
Reviewed-on: https://chromium-review.googlesource.com/911626
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Joone Hur <joone.hur@intel.com>
Cr-Commit-Position: refs/heads/master@{#535956}
[modify] https://crrev.com/c4321f0e31fcf30a1adf437dc3427237b17979cf/gpu/vulkan/tests/native_window_x11.cc
Project Member

Comment 21 by bugdroid1@chromium.org, Mar 6 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2394696dc2c167332aacaeff85b1d3abb2708b48

commit 2394696dc2c167332aacaeff85b1d3abb2708b48
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Mar 06 16:39:27 2018

Hide AddToList from X11 headers since dom_distiller uses that name

components/dom_distiller uses the name AddToList which makes it hard
to combine with X11 headers. It doesn't happen in Chromium but it
happens in jumbo builds in Opera (any maybe in Chromium in the
future).

Bug: 782184
Change-Id: I546c254ed85fbfa86cd0ba06ddd294884c8ba6de
Reviewed-on: https://chromium-review.googlesource.com/950780
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#541120}
[modify] https://crrev.com/2394696dc2c167332aacaeff85b1d3abb2708b48/ui/gfx/x/x11.h

Comment 22 by e...@chromium.org, Mar 9 2018

Cc: -e...@chromium.org
Un-cc-ing me from all bugs on my final day.

Comment 23 by cblume@chromium.org, May 31 2018

Cc: cblume@chromium.org
►

Sign in to add a comment