Skip to content

Commit

Permalink
ozone: Fix the way platform tests link ozone
Browse files Browse the repository at this point in the history
Right now the DRM platform unit tests link ozone as a component, and also
link against the platform classes statically. Linking two copies of the
platform code into the test executable is an ODR violation and undefined
behavior.

The newer wayland platform fixes the issue, but at the cost of exporting
platform internals from ozone. It'd be good to avoid these exports
because it forces us to mark the platform dependency public in GN, which
in turn makes inclusion of platform internals legal from outside ozone
according to GN's dependency checker.

To fix both issues, this creates an new internal //ui/ozone:platform
target which is used by tests to link ozone statically. Only tests will
use it, everyone else will continue to use ozone encapsulated via the
//ui/ozone component.

GYP can do this as well but has a weird quirk: it won't link if there's
no C++ source file in the component target. I added an empty one.

Review URL: https://codereview.chromium.org/1717573002

Cr-Commit-Position: refs/heads/master@{#376823}
  • Loading branch information
mspang authored and Commit bot committed Feb 22, 2016
1 parent c7185a7 commit 4dc16f3
Showing 10 changed files with 48 additions and 20 deletions.
25 changes: 19 additions & 6 deletions ui/ozone/BUILD.gn
Original file line number Diff line number Diff line change
@@ -122,7 +122,7 @@ component("ozone_base") {
]
}

component("ozone") {
source_set("platform") {
sources = [
"common/stub_client_native_pixmap_factory.cc",
"common/stub_client_native_pixmap_factory.h",
@@ -161,12 +161,18 @@ component("ozone") {

# TODO(GYP) the GYP version has a way to add additional dependencies via
# build flags.
public_deps += ozone_platform_deps
deps += ozone_platform_deps

# Platforms are always linked into //ui/ozone and can include our headers.
allow_circular_includes_from = ozone_platform_deps
}

component("ozone") {
public_deps = [
":platform",
]
}

# GYP version: ui/ozone/ozone.gyp:generate_ozone_platform_list
action("generate_ozone_platform_list") {
script = "generate_ozone_platform_list.py"
@@ -217,8 +223,15 @@ test("ozone_unittests") {
]

deps = [
"//base/test:test_support",
"//testing/gtest",
"//ui/gfx/geometry",
] + ozone_platform_test_deps
"//base/test:test_support",
"//testing/gtest",
"//ui/gfx/geometry",
]

# Add tests of platform internals.
deps += ozone_platform_test_deps

# Platform tests link ozone statically. Make sure we're not getting a
# 2nd copy of any code via the component.
assert_no_deps = [ "//ui/ozone" ]
}
6 changes: 6 additions & 0 deletions ui/ozone/empty.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// This empty source file exists to force GYP to link the
// ui/ozone/ozone.gyp:ozone target using the C++ compiler.
18 changes: 14 additions & 4 deletions ui/ozone/ozone.gyp
Original file line number Diff line number Diff line change
@@ -83,9 +83,9 @@
],
},
{
# GN version: //ui/ozone
'target_name': 'ozone',
'type': '<(component)',
# GN version: //ui/ozone:platform
'target_name': 'ozone_platform',
'type': 'static_library',
'dependencies': [
'<(DEPTH)/base/base.gyp:base',
'<(DEPTH)/ipc/ipc.gyp:ipc',
@@ -194,14 +194,24 @@
}],
],
},
{
# GN version: //ui/ozone
'target_name': 'ozone',
'type': '<(component)',
'sources': [
'empty.cc',
],
'dependencies': [
'ozone_platform',
],
},
{
'target_name': 'ozone_unittests',
'type': '<(gtest_target_type)',
'sources': [
'run_all_unittests.cc',
],
'dependencies': [
'ozone',
'../../base/base.gyp:base',
'../../base/base.gyp:test_support_base',
'../../testing/gtest.gyp:gtest',
2 changes: 1 addition & 1 deletion ui/ozone/platform/drm/BUILD.gn
Original file line number Diff line number Diff line change
@@ -189,7 +189,7 @@ source_set("gbm_unittests") {
"//skia",
"//testing/gtest",
"//ui/gfx",
"//ui/ozone",
"//ui/ozone:platform",
]

public_configs = [ ":libdrm" ]
2 changes: 1 addition & 1 deletion ui/ozone/platform/drm/gbm.gypi
Original file line number Diff line number Diff line change
@@ -176,7 +176,7 @@
'../../skia/skia.gyp:skia',
'../gfx/gfx.gyp:gfx',
'../gfx/gfx.gyp:gfx_geometry',
'ozone.gyp:ozone',
'ozone.gyp:ozone_platform',
'drm_atomic',
],
'export_dependent_settings': [
3 changes: 2 additions & 1 deletion ui/ozone/platform/wayland/BUILD.gn
Original file line number Diff line number Diff line change
@@ -46,11 +46,12 @@ source_set("wayland_unittests") {
]

deps = [
":wayland",
"//testing/gmock",
"//testing/gtest",
"//third_party/wayland:wayland_server",
"//third_party/wayland-protocols:xdg_shell_protocol",
"//ui/gfx:test_support",
"//ui/ozone",
"//ui/ozone:platform",
]
}
1 change: 1 addition & 0 deletions ui/ozone/platform/wayland/wayland.gypi
Original file line number Diff line number Diff line change
@@ -47,6 +47,7 @@
'target_name': 'ozone_platform_wayland_unittests',
'type': 'none',
'dependencies': [
'ozone.gyp:ozone_platform',
'../../skia/skia.gyp:skia',
'../../testing/gmock.gyp:gmock',
'../../third_party/wayland-protocols/wayland-protocols.gyp:xdg_shell_protocol',
5 changes: 2 additions & 3 deletions ui/ozone/platform/wayland/wayland_display.h
Original file line number Diff line number Diff line change
@@ -10,15 +10,14 @@
#include "base/message_loop/message_pump_libevent.h"
#include "ui/events/platform/platform_event_source.h"
#include "ui/gfx/native_widget_types.h"
#include "ui/ozone/ozone_export.h"
#include "ui/ozone/platform/wayland/wayland_object.h"

namespace ui {

class WaylandWindow;

class OZONE_EXPORT WaylandDisplay : public PlatformEventSource,
public base::MessagePumpLibevent::Watcher {
class WaylandDisplay : public PlatformEventSource,
public base::MessagePumpLibevent::Watcher {
public:
WaylandDisplay();
~WaylandDisplay() override;
3 changes: 1 addition & 2 deletions ui/ozone/platform/wayland/wayland_surface_factory.h
Original file line number Diff line number Diff line change
@@ -5,14 +5,13 @@
#ifndef UI_OZONE_PLATFORM_WAYLAND_WAYLAND_SURFACE_FACTORY_H_
#define UI_OZONE_PLATFORM_WAYLAND_WAYLAND_SURFACE_FACTORY_H_

#include "ui/ozone/ozone_export.h"
#include "ui/ozone/public/surface_factory_ozone.h"

namespace ui {

class WaylandDisplay;

class OZONE_EXPORT WaylandSurfaceFactory : public SurfaceFactoryOzone {
class WaylandSurfaceFactory : public SurfaceFactoryOzone {
public:
explicit WaylandSurfaceFactory(WaylandDisplay* display);
~WaylandSurfaceFactory() override;
3 changes: 1 addition & 2 deletions ui/ozone/platform/wayland/wayland_window.h
Original file line number Diff line number Diff line change
@@ -7,15 +7,14 @@

#include "ui/gfx/geometry/rect.h"
#include "ui/gfx/native_widget_types.h"
#include "ui/ozone/ozone_export.h"
#include "ui/ozone/platform/wayland/wayland_object.h"
#include "ui/platform_window/platform_window.h"

namespace ui {

class WaylandDisplay;

class OZONE_EXPORT WaylandWindow : public PlatformWindow {
class WaylandWindow : public PlatformWindow {
public:
WaylandWindow(PlatformWindowDelegate* delegate,
WaylandDisplay* display,

0 comments on commit 4dc16f3

Please sign in to comment.