From 90ed98032bc3eeb8a6cc9f0d6283c8a09198b5c8 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 2 Jun 2020 06:00:08 -0700 Subject: [PATCH 1/2] Don't export embedder API in desktop embeddings The embedder.h API layer is an implementation detail of the desktop embeddings, not part of the public API surface, so should not be part of the public symbol list for those libraries. --- shell/platform/common/cpp/BUILD.gn | 2 +- shell/platform/darwin/macos/BUILD.gn | 4 ++-- shell/platform/embedder/BUILD.gn | 15 +++++++++++---- shell/platform/embedder/embedder.cc | 2 ++ shell/platform/glfw/BUILD.gn | 2 +- shell/platform/linux/BUILD.gn | 2 +- shell/platform/windows/BUILD.gn | 2 +- 7 files changed, 19 insertions(+), 10 deletions(-) diff --git a/shell/platform/common/cpp/BUILD.gn b/shell/platform/common/cpp/BUILD.gn index 9f1ad8c6121fe..2027ac55f3de8 100644 --- a/shell/platform/common/cpp/BUILD.gn +++ b/shell/platform/common/cpp/BUILD.gn @@ -67,7 +67,7 @@ source_set("common_cpp") { deps = [ ":common_cpp_library_headers", "//flutter/shell/platform/common/cpp/client_wrapper:client_wrapper", - "//flutter/shell/platform/embedder:embedder_with_symbol_prefix", + "//flutter/shell/platform/embedder:embedder_as_internal_library", ] public_deps = [ diff --git a/shell/platform/darwin/macos/BUILD.gn b/shell/platform/darwin/macos/BUILD.gn index 2e5a9cc6e7423..b22fd110effc8 100644 --- a/shell/platform/darwin/macos/BUILD.gn +++ b/shell/platform/darwin/macos/BUILD.gn @@ -70,7 +70,7 @@ source_set("flutter_framework_source") { deps = [ "//flutter/shell/platform/darwin/common:framework_shared", - "//flutter/shell/platform/embedder:embedder_with_symbol_prefix", + "//flutter/shell/platform/embedder:embedder_as_internal_library", ] public_configs = [ "//flutter:config" ] @@ -115,7 +115,7 @@ executable("flutter_desktop_darwin_unittests") { ":flutter_desktop_darwin_fixtures", ":flutter_framework_source", "//flutter/shell/platform/darwin/common:framework_shared", - "//flutter/shell/platform/embedder:embedder_with_symbol_prefix", + "//flutter/shell/platform/embedder:embedder_as_internal_library", "//flutter/testing", "//flutter/testing:dart", "//flutter/testing:skia", diff --git a/shell/platform/embedder/BUILD.gn b/shell/platform/embedder/BUILD.gn index f4b5be57e13eb..c06f96303535a 100644 --- a/shell/platform/embedder/BUILD.gn +++ b/shell/platform/embedder/BUILD.gn @@ -83,12 +83,19 @@ embedder_source_set("embedder") { public_configs = [] } -embedder_source_set("embedder_with_symbol_prefix") { - public_configs = [ ":embedder_prefix_config" ] +embedder_source_set("embedder_as_internal_library") { + public_configs = [ ":embedder_internal_library_config" ] } -config("embedder_prefix_config") { - defines = [ "FLUTTER_API_SYMBOL_PREFIX=Embedder" ] +# For using the embedder API as internal implementation detail of an +# embedding. +config("embedder_internal_library_config") { + defines = [ + # Use prefixed symbols to avoid collisions with higher level API. + "FLUTTER_API_SYMBOL_PREFIX=Embedder", + # Don't export the embedder.h API surface. + "EMBEDDER_AS_INTERNAL_LIBRARY", + ] } test_fixtures("fixtures") { diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 9091b33bbca2e..8256bf1574522 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -14,11 +14,13 @@ #include "third_party/dart/runtime/bin/elf_loader.h" #include "third_party/dart/runtime/include/dart_native_api.h" +#if !defined(EMBEDDER_AS_INTERNAL_LIBRARY) #if OS_WIN #define FLUTTER_EXPORT __declspec(dllexport) #else // OS_WIN #define FLUTTER_EXPORT __attribute__((visibility("default"))) #endif // OS_WIN +#endif // !EMBEDDER_AS_INTERNAL_LIBRARY extern "C" { #if FLUTTER_RUNTIME_MODE == FLUTTER_RUNTIME_MODE_DEBUG diff --git a/shell/platform/glfw/BUILD.gn b/shell/platform/glfw/BUILD.gn index 7771c67a9ca59..0911a4d053af7 100644 --- a/shell/platform/glfw/BUILD.gn +++ b/shell/platform/glfw/BUILD.gn @@ -55,7 +55,7 @@ source_set("flutter_glfw") { "//flutter/shell/platform/common/cpp:common_cpp", "//flutter/shell/platform/common/cpp:common_cpp_input", "//flutter/shell/platform/common/cpp/client_wrapper:client_wrapper", - "//flutter/shell/platform/embedder:embedder_with_symbol_prefix", + "//flutter/shell/platform/embedder:embedder_as_internal_library", "//flutter/shell/platform/glfw/client_wrapper:client_wrapper_glfw", "//third_party/rapidjson", ] diff --git a/shell/platform/linux/BUILD.gn b/shell/platform/linux/BUILD.gn index 809d5a25d849a..048b1110f4b25 100644 --- a/shell/platform/linux/BUILD.gn +++ b/shell/platform/linux/BUILD.gn @@ -110,7 +110,7 @@ source_set("flutter_linux") { deps = [ "//flutter/shell/platform/common/cpp:common_cpp_input", - "//flutter/shell/platform/embedder:embedder_with_symbol_prefix", + "//flutter/shell/platform/embedder:embedder_as_internal_library", "//third_party/rapidjson", ] } diff --git a/shell/platform/windows/BUILD.gn b/shell/platform/windows/BUILD.gn index 53d335c02fe8c..248d3ccdc24c7 100644 --- a/shell/platform/windows/BUILD.gn +++ b/shell/platform/windows/BUILD.gn @@ -74,7 +74,7 @@ source_set("flutter_windows_source") { "//flutter/shell/platform/common/cpp:common_cpp", "//flutter/shell/platform/common/cpp:common_cpp_input", "//flutter/shell/platform/common/cpp/client_wrapper:client_wrapper", - "//flutter/shell/platform/embedder:embedder_with_symbol_prefix", + "//flutter/shell/platform/embedder:embedder_as_internal_library", "//flutter/shell/platform/windows/client_wrapper:client_wrapper_windows", "//third_party/angle:libEGL_static", # the order of libEGL_static and libGLESv2_static is important.. if reversed, will cause a linker error DllMain already defined in LIBCMTD.lib "//third_party/angle:libGLESv2_static", From 6e178ec567e9daf9acac26afe7eaa72232b1b85d Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 2 Jun 2020 15:33:20 -0700 Subject: [PATCH 2/2] Address review comments, format --- shell/platform/embedder/BUILD.gn | 3 ++- shell/platform/embedder/embedder.cc | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/shell/platform/embedder/BUILD.gn b/shell/platform/embedder/BUILD.gn index d1b1c53aa4995..955c641571614 100644 --- a/shell/platform/embedder/BUILD.gn +++ b/shell/platform/embedder/BUILD.gn @@ -104,8 +104,9 @@ config("embedder_internal_library_config") { defines = [ # Use prefixed symbols to avoid collisions with higher level API. "FLUTTER_API_SYMBOL_PREFIX=Embedder", + # Don't export the embedder.h API surface. - "EMBEDDER_AS_INTERNAL_LIBRARY", + "FLUTTER_NO_EXPORT", ] } diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 8256bf1574522..6e2f5bcf60ca6 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -14,13 +14,13 @@ #include "third_party/dart/runtime/bin/elf_loader.h" #include "third_party/dart/runtime/include/dart_native_api.h" -#if !defined(EMBEDDER_AS_INTERNAL_LIBRARY) +#if !defined(FLUTTER_NO_EXPORT) #if OS_WIN #define FLUTTER_EXPORT __declspec(dllexport) #else // OS_WIN #define FLUTTER_EXPORT __attribute__((visibility("default"))) #endif // OS_WIN -#endif // !EMBEDDER_AS_INTERNAL_LIBRARY +#endif // !FLUTTER_NO_EXPORT extern "C" { #if FLUTTER_RUNTIME_MODE == FLUTTER_RUNTIME_MODE_DEBUG