Skip to content

Commit

Permalink
NaCl on ARM64: Remove dependencies on //skia
Browse files Browse the repository at this point in the history
NaCl cannot depend on //skia when building for ARM64, as in that build
some libraries are not present in the 32-bit version that NaCl needs.

Therefore, build some (parts of) files only in builds that are not for
nacl_helper.

Introduce a new buildflag IS_MINIMAL_TOOLCHAIN. This is to limit
dependencies in .h/.cc files if we build nacl_helper for ARM64.
Use nogncheck for such conditional includes as gn does not run the
preprocessor.

Test: Build nacl_helper for Linux ARM64
Bug: 1339021
Change-Id: Id95147bc134e34fbae91af3d540ed30a4e96b512
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3829145
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Commit-Queue: Fabian Sommer <fabiansommer@chromium.org>
Reviewed-by: Derek Schuff <dschuff@chromium.org>
Reviewed-by: Mark Seaborn <mseaborn@chromium.org>
Reviewed-by: vikas soni <vikassoni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1037644}
  • Loading branch information
Fabian-Sommer authored and Chromium LUCI CQ committed Aug 22, 2022
1 parent 76b2362 commit 295fb5d
Show file tree
Hide file tree
Showing 15 changed files with 69 additions and 36 deletions.
6 changes: 5 additions & 1 deletion components/nacl/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import("//build/buildflag_header.gni")
import("//components/nacl/features.gni")
import("//components/nacl/toolchain.gni")
import("//mojo/public/tools/bindings/mojom.gni")

if (enable_nacl) {
Expand Down Expand Up @@ -146,5 +147,8 @@ static_library("switches") {

buildflag_header("buildflags") {
header = "buildflags.h"
flags = [ "ENABLE_NACL=$enable_nacl" ]
flags = [
"ENABLE_NACL=$enable_nacl",
"IS_MINIMAL_TOOLCHAIN=$is_minimal_toolchain",
]
}
5 changes: 4 additions & 1 deletion components/nacl/loader/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,10 @@ if ((is_linux || is_chromeos) && current_cpu != "arm64") {
]

if (is_minimal_toolchain) {
assert_no_deps += [ "//net" ]
assert_no_deps += [
"//net",
"//skia",
]
}
}

Expand Down
9 changes: 6 additions & 3 deletions gpu/command_buffer/client/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

import("//components/nacl/toolchain.gni")
import("//ui/gl/features.gni")

if (!is_nacl) {
if (!is_nacl && !is_minimal_toolchain) {
import("//skia/features.gni")
}

Expand Down Expand Up @@ -95,7 +96,7 @@ source_set("client_sources") {
]

# These files now rely on Skia which isn't allowed as a dependency in nacl builds.
if (!is_nacl) {
if (!is_nacl && !is_minimal_toolchain) {
sources += [
"shared_image_interface.cc",
"shared_image_interface.h",
Expand Down Expand Up @@ -235,13 +236,14 @@ component("gles2_implementation") {
":gles2_cmd_helper",
":gles2_interface",
"//base",
"//components/nacl/common:buildflags",
"//gpu/command_buffer/common",
"//gpu/command_buffer/common:gles2",
"//gpu/command_buffer/common:gles2_utils",
"//ui/gfx/geometry",
]

if (!is_nacl) {
if (!is_nacl && !is_minimal_toolchain) {
deps += [
"//components/viz/common:resource_format",
"//ui/gfx:color_space",
Expand Down Expand Up @@ -338,6 +340,7 @@ component("gles2_implementation_no_check") {
":gles2_cmd_helper",
":gles2_interface",
"//base",
"//components/nacl/common:buildflags",
"//gpu/command_buffer/common",
"//gpu/command_buffer/common:gles2",
"//gpu/command_buffer/common:gles2_utils",
Expand Down
1 change: 1 addition & 0 deletions gpu/command_buffer/client/DEPS
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
include_rules = [
"+ui/latency",
"+cc/paint",
"+components/nacl/common/buildflags.h",
"+third_party/skia",
"+components/viz/common/resources/resource_format.h",
"+components/viz/common/resources/resource_format_utils.h",
Expand Down
12 changes: 7 additions & 5 deletions gpu/command_buffer/client/gles2_implementation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "base/trace_event/process_memory_dump.h"
#include "base/trace_event/trace_event.h"
#include "build/build_config.h"
#include "components/nacl/common/buildflags.h"
#include "gpu/command_buffer/client/buffer_tracker.h"
#include "gpu/command_buffer/client/gles2_cmd_helper.h"
#include "gpu/command_buffer/client/gpu_control.h"
Expand All @@ -56,9 +57,9 @@
#include "ui/gfx/geometry/rect_f.h"
#include "ui/gl/gpu_preference.h"

#if !defined(__native_client__)
#include "ui/gfx/color_space.h"
#include "ui/gfx/ipc/color/gfx_param_traits.h"
#if !defined(__native_client__) && !BUILDFLAG(IS_MINIMAL_TOOLCHAIN)
#include "ui/gfx/color_space.h" // nogncheck
#include "ui/gfx/ipc/color/gfx_param_traits.h" // nogncheck
#endif

#if defined(GPU_CLIENT_DEBUG)
Expand All @@ -82,7 +83,8 @@
//
// If it was up to us we'd just always write to the destination but the OpenGL
// spec defines the behavior of OpenGL functions, not us. :-(
#if defined(__native_client__) || defined(GLES2_CONFORMANCE_TESTS)
#if defined(__native_client__) || defined(GLES2_CONFORMANCE_TESTS) || \
BUILDFLAG(IS_MINIMAL_TOOLCHAIN)
#define GPU_CLIENT_VALIDATE_DESTINATION_INITALIZATION_ASSERT(v)
#define GPU_CLIENT_DCHECK(v)
#elif defined(GPU_DCHECK)
Expand Down Expand Up @@ -5924,7 +5926,7 @@ void GLES2Implementation::ResizeCHROMIUM(GLuint width,
<< height << ", " << scale_factor << ", " << alpha << ")");
// Including gfx::ColorSpace would bring Skia and a lot of other code into
// NaCl's IRT, so just leave the color space unspecified.
#if !defined(__native_client__)
#if !defined(__native_client__) && !BUILDFLAG(IS_MINIMAL_TOOLCHAIN)
if (gl_color_space) {
gfx::ColorSpace gfx_color_space =
*reinterpret_cast<const gfx::ColorSpace*>(gl_color_space);
Expand Down
5 changes: 3 additions & 2 deletions gpu/ipc/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# found in the LICENSE file.

import("//build/config/ui.gni")
import("//components/nacl/toolchain.gni")
import("//gpu/vulkan/features.gni")
import("//mojo/public/tools/bindings/mojom.gni")

Expand Down Expand Up @@ -52,7 +53,7 @@ source_set("command_buffer_traits_sources") {
"//ui/gfx/ipc/geometry",
]

if (!is_nacl) {
if (!is_nacl && !is_minimal_toolchain) {
deps += [ "//ui/gfx/ipc/skia" ]
}
}
Expand Down Expand Up @@ -158,7 +159,7 @@ source_set("ipc_common_sources") {
frameworks = [ "IOSurface.framework" ]
}

if (!is_nacl) {
if (!is_nacl && !is_minimal_toolchain) {
deps += [ "//ui/gfx/ipc/skia" ]
}

Expand Down
6 changes: 4 additions & 2 deletions ppapi/proxy/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# found in the LICENSE file.

import("//build/config/nacl/config.gni")
import("//components/nacl/toolchain.gni")
import("//ppapi/buildflags/buildflags.gni")

assert(enable_ppapi)
Expand Down Expand Up @@ -229,6 +230,7 @@ component("proxy") {
deps = [
":common",
"//base",
"//components/nacl/common:buildflags",
"//device/base/synchronization",
"//device/gamepad/public/cpp:shared_with_blink",
"//gpu/command_buffer/client:client",
Expand All @@ -245,7 +247,7 @@ component("proxy") {
"//ui/gfx/ipc/geometry",
]

if (!is_nacl) {
if (!is_nacl && !is_minimal_toolchain) {
deps += [
"//base/third_party/dynamic_annotations",
"//gin",
Expand Down Expand Up @@ -320,7 +322,7 @@ source_set("ipc_sources") {

public_deps = [ "//ipc" ]

if (!is_nacl) {
if (!is_nacl && !is_minimal_toolchain) {
deps += [ "//skia" ]
}
}
Expand Down
1 change: 1 addition & 0 deletions ppapi/proxy/DEPS
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
include_rules = [
"+base",
"+components/nacl/common/buildflags.h",
"+device",
"+gin",
"+gpu",
Expand Down
19 changes: 10 additions & 9 deletions ppapi/proxy/ppb_image_data_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "base/memory/weak_ptr.h"
#include "base/time/time.h"
#include "build/build_config.h"
#include "components/nacl/common/buildflags.h"
#include "ppapi/c/pp_completion_callback.h"
#include "ppapi/c/pp_errors.h"
#include "ppapi/c/pp_resource.h"
Expand All @@ -32,9 +33,9 @@
#include "ppapi/thunk/enter.h"
#include "ppapi/thunk/thunk.h"

#if !BUILDFLAG(IS_NACL)
#include "skia/ext/platform_canvas.h"
#include "ui/surface/transport_dib.h"
#if !BUILDFLAG(IS_NACL) && !BUILDFLAG(IS_MINIMAL_TOOLCHAIN)
#include "skia/ext/platform_canvas.h" //nogncheck
#include "ui/surface/transport_dib.h" //nogncheck
#endif

using ppapi::thunk::PPB_ImageData_API;
Expand Down Expand Up @@ -371,7 +372,7 @@ void ImageData::RecycleToPlugin(bool zero_contents) {

// PlatformImageData -----------------------------------------------------------

#if !BUILDFLAG(IS_NACL)
#if !BUILDFLAG(IS_NACL) && !BUILDFLAG(IS_MINIMAL_TOOLCHAIN)
PlatformImageData::PlatformImageData(
const HostResource& resource,
const PP_ImageDataDesc& desc,
Expand Down Expand Up @@ -411,7 +412,7 @@ void PlatformImageData::Unmap() {
SkCanvas* PlatformImageData::GetCanvas() {
return mapped_canvas_.get();
}
#endif // !BUILDFLAG(IS_NACL)
#endif // !BUILDFLAG(IS_NACL) && !BUILDFLAG(IS_MINIMAL_TOOLCHAIN)

// SimpleImageData -------------------------------------------------------------

Expand Down Expand Up @@ -490,7 +491,7 @@ PP_Resource PPB_ImageData_Proxy::CreateProxyResource(
break;
}
case PPB_ImageData_Shared::PLATFORM: {
#if !BUILDFLAG(IS_NACL)
#if !BUILDFLAG(IS_NACL) && !BUILDFLAG(IS_MINIMAL_TOOLCHAIN)
ppapi::proxy::SerializedHandle image_handle;
dispatcher->Send(new PpapiHostMsg_PPBImageData_CreatePlatform(
kApiID, instance, format, size, init_to_zero, &result, &desc,
Expand Down Expand Up @@ -518,7 +519,7 @@ PP_Resource PPB_ImageData_Proxy::CreateProxyResource(
bool PPB_ImageData_Proxy::OnMessageReceived(const IPC::Message& msg) {
bool handled = true;
IPC_BEGIN_MESSAGE_MAP(PPB_ImageData_Proxy, msg)
#if !BUILDFLAG(IS_NACL)
#if !BUILDFLAG(IS_NACL) && !BUILDFLAG(IS_MINIMAL_TOOLCHAIN)
IPC_MESSAGE_HANDLER(PpapiHostMsg_PPBImageData_CreatePlatform,
OnHostMsgCreatePlatform)
IPC_MESSAGE_HANDLER(PpapiHostMsg_PPBImageData_CreateSimple,
Expand All @@ -532,7 +533,7 @@ bool PPB_ImageData_Proxy::OnMessageReceived(const IPC::Message& msg) {
return handled;
}

#if !BUILDFLAG(IS_NACL)
#if !BUILDFLAG(IS_NACL) && !BUILDFLAG(IS_MINIMAL_TOOLCHAIN)
// static
PP_Resource PPB_ImageData_Proxy::CreateImageData(
PP_Instance instance,
Expand Down Expand Up @@ -640,7 +641,7 @@ void PPB_ImageData_Proxy::OnHostMsgCreateSimple(
result_image_handle->set_null_shmem_region();
}
}
#endif // !BUILDFLAG(IS_NACL)
#endif // !BUILDFLAG(IS_NACL) && !BUILDFLAG(IS_MINIMAL_TOOLCHAIN)

void PPB_ImageData_Proxy::OnPluginMsgNotifyUnusedImageData(
const HostResource& old_image_data) {
Expand Down
11 changes: 6 additions & 5 deletions ppapi/proxy/ppb_image_data_proxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

#include "base/memory/unsafe_shared_memory_region.h"
#include "build/build_config.h"
#include "components/nacl/common/buildflags.h"
#include "ipc/ipc_platform_file.h"
#include "ppapi/c/pp_bool.h"
#include "ppapi/c/pp_completion_callback.h"
Expand All @@ -27,9 +28,9 @@
#include "ppapi/shared_impl/resource.h"
#include "ppapi/thunk/ppb_image_data_api.h"

#if !BUILDFLAG(IS_NACL)
#include "third_party/skia/include/core/SkRefCnt.h"
#endif // !BUILDFLAG(IS_NACL)
#if !BUILDFLAG(IS_NACL) && !BUILDFLAG(IS_MINIMAL_TOOLCHAIN)
#include "third_party/skia/include/core/SkRefCnt.h" //nogncheck
#endif // !BUILDFLAG(IS_NACL) && !BUILDFLAG(IS_MINIMAL_TOOLCHAIN)

class TransportDIB;

Expand Down Expand Up @@ -83,7 +84,7 @@ class PPAPI_PROXY_EXPORT ImageData : public ppapi::Resource,
// PlatformImageData is a full featured image data resource which can access
// the underlying platform-specific canvas and |image_region|. This can't be
// used by NaCl apps.
#if !BUILDFLAG(IS_NACL)
#if !BUILDFLAG(IS_NACL) && !BUILDFLAG(IS_MINIMAL_TOOLCHAIN)
class PPAPI_PROXY_EXPORT PlatformImageData : public ImageData {
public:
PlatformImageData(const ppapi::HostResource& resource,
Expand All @@ -106,7 +107,7 @@ class PPAPI_PROXY_EXPORT PlatformImageData : public ImageData {
// Null when the image isn't mapped.
std::unique_ptr<SkCanvas> mapped_canvas_;
};
#endif // !BUILDFLAG(IS_NACL)
#endif // !BUILDFLAG(IS_NACL) && !BUILDFLAG(IS_MINIMAL_TOOLCHAIN)

// SimpleImageData is a simple, platform-independent image data resource which
// can be used by NaCl. It can also be used by trusted apps when access to the
Expand Down
4 changes: 3 additions & 1 deletion ppapi/shared_impl/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,14 @@ source_set("common") {
"//base",
"//base:i18n",
"//build:chromeos_buildflags",
"//components/nacl/common:buildflags",
"//ppapi/c",
"//ppapi/thunk:headers",
"//third_party/icu:icuuc",
"//url",
]

if (!is_nacl) {
if (!is_nacl && !is_minimal_toolchain) {
deps += [ "//skia" ]
}
}
Expand Down Expand Up @@ -222,6 +223,7 @@ component("shared_impl") {
deps = [
"//base/third_party/dynamic_annotations",
"//build:chromeos_buildflags",
"//components/nacl/common:buildflags",
"//device/gamepad/public/cpp:shared_with_blink",
"//gpu/command_buffer/client",
"//gpu/command_buffer/client:gles2_cmd_helper",
Expand Down
1 change: 1 addition & 0 deletions ppapi/shared_impl/DEPS
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
include_rules = [
"+base",
"+components/nacl/common/buildflags.h",
"+device/gamepad/public/cpp",
"+gpu",
"+media/audio",
Expand Down
8 changes: 5 additions & 3 deletions ppapi/shared_impl/ppb_image_data_shared.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@

#include "base/notreached.h"
#include "build/build_config.h"
#include "components/nacl/common/buildflags.h"

#if !BUILDFLAG(IS_NACL) && !defined(NACL_WIN64)
#include "third_party/skia/include/core/SkTypes.h"
#if !BUILDFLAG(IS_NACL) && !defined(NACL_WIN64) && \
!BUILDFLAG(IS_MINIMAL_TOOLCHAIN)
#include "third_party/skia/include/core/SkTypes.h" //nogncheck
#endif

namespace ppapi {
Expand All @@ -20,7 +22,7 @@ PP_ImageDataFormat PPB_ImageData_Shared::GetNativeImageDataFormat() {
// later.
// TODO(dmichael): Really proxy this.
return PP_IMAGEDATAFORMAT_BGRA_PREMUL;
#elif defined(NACL_WIN64)
#elif defined(NACL_WIN64) || BUILDFLAG(IS_MINIMAL_TOOLCHAIN)
// In the NaCl Win64 helper, this shouldn't be called. If we start building
// Chrome on Windows 64 for realz, we should really implement this.
NOTIMPLEMENTED();
Expand Down
8 changes: 5 additions & 3 deletions ppapi/shared_impl/private/net_address_private_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/check.h"
#include "base/strings/stringprintf.h"
#include "build/build_config.h"
#include "components/nacl/common/buildflags.h"
#include "ppapi/c/pp_var.h"
#include "ppapi/c/private/ppb_net_address_private.h"
#include "ppapi/shared_impl/proxy_lock.h"
Expand All @@ -22,7 +23,8 @@
#include <windows.h>
#include <winsock2.h>
#include <ws2tcpip.h>
#elif BUILDFLAG(IS_POSIX) && !BUILDFLAG(IS_NACL)
#elif BUILDFLAG(IS_POSIX) && !BUILDFLAG(IS_NACL) && \
!BUILDFLAG(IS_MINIMAL_TOOLCHAIN)
#include <arpa/inet.h>
#include <netinet/in.h>
#include <sys/socket.h>
Expand Down Expand Up @@ -389,7 +391,7 @@ GetPPB_NetAddress_Private_1_1_Thunk() {
} // namespace thunk

// For the NaCl target, all we need are the API functions and the thunk.
#if !BUILDFLAG(IS_NACL)
#if !BUILDFLAG(IS_NACL) && !BUILDFLAG(IS_MINIMAL_TOOLCHAIN)

// static
bool NetAddressPrivateImpl::ValidateNetAddress(
Expand Down Expand Up @@ -484,7 +486,7 @@ bool NetAddressPrivateImpl::NetAddressToIPEndPoint(
address->Assign(net_addr->address, address_size);
return true;
}
#endif // !BUILDFLAG(IS_NACL)
#endif // !BUILDFLAG(IS_NACL) && !BUILDFLAG(IS_MINIMAL_TOOLCHAIN)

// static
std::string NetAddressPrivateImpl::DescribeNetAddress(
Expand Down
Loading

0 comments on commit 295fb5d

Please sign in to comment.