From 7f3a15f714c8311b1adb7bdce1ac83c7113af598 Mon Sep 17 00:00:00 2001 From: jbudorick Date: Fri, 10 Jun 2016 06:28:40 -0700 Subject: [Android] Add support for standalone PDFium gn build on Android. This pulls in the android NDK and catapult, rolls chromium/src/build/, and pulls in two .gni updates. It also fixes a few miscellaneous compile failures in android-specific code. BUG=pdfium:38 Review-Url: https://codereview.chromium.org/2059553002 --- .gitignore | 2 + BUILD.gn | 8 ++ DEPS | 10 +- build_overrides/build.gni | 18 +++- core/fxge/android/fpf_skiafontmgr.cpp | 17 ++-- core/fxge/android/fpf_skiafontmgr.h | 4 +- core/fxge/android/fpf_skiamodule.h | 2 +- core/fxge/android/fx_android_font.cpp | 1 + core/fxge/android/fx_android_imp.cpp | 2 +- testing/test.gni | 178 ++++++++++++++++++++++++++++++---- 10 files changed, 208 insertions(+), 34 deletions(-) diff --git a/.gitignore b/.gitignore index c8dc4a0a31..7b6ce1d682 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,8 @@ /testing/corpus /testing/gmock /testing/gtest +/third_party/android_ndk +/third_party/catapult /third_party/icu /third_party/llvm /third_party/llvm-build diff --git a/BUILD.gn b/BUILD.gn index ec57d80d2f..59215c1dd2 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -1641,6 +1641,10 @@ test("pdfium_unittests") { ] } configs += [ ":pdfium_core_config" ] + if (is_android) { + ignore_all_data_deps = true + use_raw_android_executable = true + } } test("pdfium_embeddertests") { @@ -1694,6 +1698,10 @@ test("pdfium_embeddertests") { configs += [ "//v8:external_startup_data" ] } configs += [ ":pdfium_core_config" ] + if (is_android) { + ignore_all_data_deps = true + use_raw_android_executable = true + } } if (pdf_is_standalone) { diff --git a/DEPS b/DEPS index 9b7dc4de1a..267e00ac26 100644 --- a/DEPS +++ b/DEPS @@ -4,8 +4,10 @@ vars = { 'chromium_git': 'https://chromium.googlesource.com', 'pdfium_git': 'https://pdfium.googlesource.com', - 'build_revision': '4842479bd8da7b9e5eb027f8c15bee533f9c328f', + 'android_ndk_revision': '5022f40f12953c02b2614c5f7beb981ec5d0e833', + 'build_revision': '9bbfcd5cce0290376e46f3aeaca3475e96a67fdb', 'buildtools_revision': '099f1da55bfe8caa12266371a7eb983698fb1d87', + 'catapult_revision': '8ff23141e1df724317e9f558644ce2a7d8afde54', 'clang_revision': 'b6f620b311665e2d96d0921833f54295b9bbf925', 'cygwin_revision': 'c89e446b273697fadf3a10ff1007a97c0b7de6df', 'gen_library_loader_revision': '916d4acd8b2cde67a390737dfba90b3c37de23a1', @@ -64,6 +66,12 @@ deps = { } deps_os = { + "android": { + "third_party/android_ndk": + Var('chromium_git') + "/android_ndk.git@" + Var('android_ndk_revision'), + "third_party/catapult": + Var('chromium_git') + "/external/github.com/catapult-project/catapult.git@" + Var('catapult_revision'), + }, "win": { "v8/third_party/cygwin": Var('chromium_git') + "/chromium/deps/cygwin@" + Var('cygwin_revision'), diff --git a/build_overrides/build.gni b/build_overrides/build.gni index 9d68f10933..1c0b53fe19 100644 --- a/build_overrides/build.gni +++ b/build_overrides/build.gni @@ -3,6 +3,20 @@ # found in the LICENSE file. # See https://bugs.chromium.org/p/webrtc/issues/detail?id=5453. -# These overrides are needed and should generally track Chromium settings. -mac_deployment_target_build_override = "10.7" +# Some WebRTC targets require the 10.7 deployment version of the Mac SDK +# and a 10.11 min SDK, but those targets are only used in non-Chromium +# builds. We can remove this when Chromium drops 10.6 support and also +# requires 10.7. mac_sdk_min_build_override = "10.10" +mac_deployment_target_build_override = "10.7" + +# Variable that can be used to support multiple build scenarios, like having +# Chromium specific targets in a client project's GN file etc. +build_with_chromium = true + +# Support different NDK locations in non-Chromium builds. +default_android_ndk_root = "//third_party/android_ndk" +default_android_ndk_version = "r11c" + +# Some non-Chromium builds don't support building java targets. +enable_java_templates = false diff --git a/core/fxge/android/fpf_skiafontmgr.cpp b/core/fxge/android/fpf_skiafontmgr.cpp index 8a8cc83fb7..fb5b686a87 100644 --- a/core/fxge/android/fpf_skiafontmgr.cpp +++ b/core/fxge/android/fpf_skiafontmgr.cpp @@ -30,8 +30,7 @@ static unsigned long FPF_SkiaStream_Read(FXFT_Stream stream, return 0; } if (count > 0) { - if (pFileRead->ReadBlock(buffer, (FX_FILESIZE)offset, (size_t)count) != - count) { + if (!pFileRead->ReadBlock(buffer, (FX_FILESIZE)offset, (size_t)count)) { return 0; } } @@ -209,12 +208,12 @@ static FX_BOOL FPF_SkiaIsCJK(uint8_t uCharset) { (uCharset == FXFONT_SHIFTJIS_CHARSET); } static FX_BOOL FPF_SkiaMaybeSymbol(const CFX_ByteStringC& bsFacename) { - CFX_ByteString bsName = bsFacename; + CFX_ByteString bsName(bsFacename); bsName.MakeLower(); return bsName.Find("symbol") > -1; } static FX_BOOL FPF_SkiaMaybeArabic(const CFX_ByteStringC& bsFacename) { - CFX_ByteString bsName = bsFacename; + CFX_ByteString bsName(bsFacename); bsName.MakeLower(); return bsName.Find("arabic") > -1; } @@ -371,7 +370,7 @@ FXFT_Face CFPF_SkiaFontMgr::GetFontFace(const CFX_ByteStringC& bsFile, } FXFT_Open_Args args; args.flags = FT_OPEN_PATHNAME; - args.pathname = static_cast(bsFile.c_str()); + args.pathname = const_cast(bsFile.c_str()); FXFT_Face face; if (FXFT_Open_Face(m_FTLibrary, &args, iFaceIndex, &face)) { return FALSE; @@ -399,7 +398,7 @@ FXFT_Face CFPF_SkiaFontMgr::GetFontFace(const uint8_t* pBuffer, FXFT_Set_Pixel_Sizes(face, 0, 64); return face; } -void CFPF_SkiaFontMgr::ScanPath(const CFX_ByteStringC& path) { +void CFPF_SkiaFontMgr::ScanPath(const CFX_ByteString& path) { void* handle = FX_OpenFolder(path.c_str()); if (!handle) { return; @@ -418,7 +417,7 @@ void CFPF_SkiaFontMgr::ScanPath(const CFX_ByteStringC& path) { continue; } } - CFX_ByteString fullpath = path; + CFX_ByteString fullpath(path); fullpath += "/"; fullpath += filename; if (bFolder) { @@ -429,8 +428,8 @@ void CFPF_SkiaFontMgr::ScanPath(const CFX_ByteStringC& path) { } FX_CloseFolder(handle); } -void CFPF_SkiaFontMgr::ScanFile(const CFX_ByteStringC& file) { - FXFT_Face face = GetFontFace(file); +void CFPF_SkiaFontMgr::ScanFile(const CFX_ByteString& file) { + FXFT_Face face = GetFontFace(file.AsStringC()); if (face) { CFPF_SkiaPathFont* pFontDesc = new CFPF_SkiaPathFont; pFontDesc->SetPath(file.c_str()); diff --git a/core/fxge/android/fpf_skiafontmgr.h b/core/fxge/android/fpf_skiafontmgr.h index ec27a7fb21..6e8ebb6e64 100644 --- a/core/fxge/android/fpf_skiafontmgr.h +++ b/core/fxge/android/fpf_skiafontmgr.h @@ -111,8 +111,8 @@ class CFPF_SkiaFontMgr { int32_t iFaceIndex = 0); protected: - void ScanPath(const CFX_ByteStringC& path); - void ScanFile(const CFX_ByteStringC& file); + void ScanPath(const CFX_ByteString& path); + void ScanFile(const CFX_ByteString& file); void ReportFace(FXFT_Face face, CFPF_SkiaFontDescriptor* pFontDesc); void OutputSystemFonts(); diff --git a/core/fxge/android/fpf_skiamodule.h b/core/fxge/android/fpf_skiamodule.h index 8ad7549cf1..c3d5772fb3 100644 --- a/core/fxge/android/fpf_skiamodule.h +++ b/core/fxge/android/fpf_skiamodule.h @@ -7,7 +7,7 @@ #ifndef CORE_FXGE_ANDROID_FPF_SKIAMODULE_H_ #define CORE_FXGE_ANDROID_FPF_SKIAMODULE_H_ -#include "core/fxcrt/include/fx_system.h'" +#include "core/fxcrt/include/fx_system.h" #if _FX_OS_ == _FX_ANDROID_ diff --git a/core/fxge/android/fx_android_font.cpp b/core/fxge/android/fx_android_font.cpp index 3246eb7f9e..c7e876544b 100644 --- a/core/fxge/android/fx_android_font.cpp +++ b/core/fxge/android/fx_android_font.cpp @@ -13,6 +13,7 @@ #include "core/fxge/android/fx_android_font.h" CFX_AndroidFontInfo::CFX_AndroidFontInfo() : m_pFontMgr(nullptr) {} +CFX_AndroidFontInfo::~CFX_AndroidFontInfo() {} FX_BOOL CFX_AndroidFontInfo::Init(CFPF_SkiaFontMgr* pFontMgr) { if (!pFontMgr) return FALSE; diff --git a/core/fxge/android/fx_android_imp.cpp b/core/fxge/android/fx_android_imp.cpp index 08fdea3d89..e4a830ff65 100644 --- a/core/fxge/android/fx_android_imp.cpp +++ b/core/fxge/android/fx_android_imp.cpp @@ -31,7 +31,7 @@ void CFX_GEModule::InitPlatform() { void CFX_GEModule::DestroyPlatform() { if (m_pPlatformData) - static_cast(m_pPlatformData)->Destroy(); + static_cast(m_pPlatformData)->Destroy(); } #endif // _FX_OS_ == _FX_ANDROID_ diff --git a/testing/test.gni b/testing/test.gni index 13c434238a..3f6f175613 100644 --- a/testing/test.gni +++ b/testing/test.gni @@ -6,14 +6,69 @@ # TEST SETUP # ============================================================================== +template("_gen_isolate") { + testonly = true + _runtime_deps_file = "$target_gen_dir/$target_name.runtime_deps" + group("${target_name}__write_deps") { + forward_variables_from(invoker, + [ + "data", + "data_deps", + "deps", + "public_deps", + ]) + write_runtime_deps = _runtime_deps_file + } + + action(target_name) { + script = "//testing/generate_isolate.py" + inputs = [ + _runtime_deps_file, + ] + outputs = [ + invoker.output, + ] + args = [ + "--output-directory=.", + "--out-file", + rebase_path(invoker.output, root_build_dir), + "--runtime-deps-file", + rebase_path(_runtime_deps_file, root_build_dir), + ] + if (is_android) { + args += [ "--apply-android-filters" ] + } + if (defined(invoker.apply_device_filters) && invoker.apply_device_filters) { + args += [ "--apply-device-filters" ] + } + _assert_no_odd_data = + defined(invoker.assert_no_odd_data) && invoker.assert_no_odd_data + if (_assert_no_odd_data) { + args += [ "--assert-no-odd-data" ] + } + if (defined(invoker.command)) { + _isolate_dir = get_path_info(invoker.output, "dir") + args += [ + "--command", + rebase_path(invoker.command, _isolate_dir), + ] + } + deps = [ + ":${invoker.target_name}__write_deps", + ] + } +} + # Define a test as an executable (or apk on Android) with the "testonly" flag # set. # Variable: # use_raw_android_executable: Use executable() rather than android_apk(). +# use_native_activity: Test implements ANativeActivity_onCreate(). template("test") { if (is_android) { import("//build/config/android/config.gni") import("//build/config/android/rules.gni") + import("//build/config/sanitizers/sanitizers.gni") _use_raw_android_executable = defined(invoker.use_raw_android_executable) && invoker.use_raw_android_executable @@ -25,6 +80,40 @@ template("test") { _output_name = invoker.output_name } + _test_runner_target = "${_output_name}__test_runner_script" + _wrapper_script_vars = [ "shard_timeout" ] + _gen_isolate_vars = [ + "allow_odd_runtime_deps", + "ignore_all_data_deps", + ] + _generate_device_isolate = + !defined(invoker.ignore_all_data_deps) || !invoker.ignore_all_data_deps + + if (_generate_device_isolate) { + _allow_odd_runtime_deps = defined(invoker.allow_odd_runtime_deps) && + invoker.allow_odd_runtime_deps + + # The device isolate is needed at runtime, so it cannot go in + # target_gen_dir, as builder/tester configurations do not include it. + _target_dir_name = get_label_info(":$target_name", "dir") + _device_isolate_path = "$root_out_dir/gen.runtime/$_target_dir_name/$target_name.device.isolate" + _gen_isolate_target_name = "${target_name}__isolate" + _gen_isolate(_gen_isolate_target_name) { + forward_variables_from(invoker, + [ + "data", + "data_deps", + "deps", + "public_deps", + ]) + assert_no_odd_data = !_allow_odd_runtime_deps + output = _device_isolate_path + apply_device_filters = true + } + } + + assert(_use_raw_android_executable || enable_java_templates) + if (_use_raw_android_executable) { _exec_target = "${target_name}__exec" _dist_target = "${target_name}__dist" @@ -35,7 +124,10 @@ template("test") { # Configs will always be defined since we set_defaults in BUILDCONFIG.gn. configs = [] data_deps = [] - forward_variables_from(invoker, "*", [ "extra_dist_files" ]) + forward_variables_from( + invoker, + "*", + _wrapper_script_vars + _gen_isolate_vars + [ "extra_dist_files" ]) testonly = true # Thanks to the set_defaults() for test(), configs are initialized with @@ -70,8 +162,11 @@ template("test") { _apk_specific_vars = [ "android_manifest", "enable_multidex", + "proguard_configs", + "proguard_enabled", "use_default_launcher", "write_asset_list", + "use_native_activity", ] shared_library(_library_target) { # Configs will always be defined since we set_defaults in BUILDCONFIG.gn. @@ -82,10 +177,8 @@ template("test") { deps = [] forward_variables_from(invoker, "*", - _apk_specific_vars + [ - "isolate_file", - "visibility", - ]) + _apk_specific_vars + _wrapper_script_vars + + _gen_isolate_vars + [ "visibility" ]) if (!defined(invoker.use_default_launcher) || invoker.use_default_launcher) { @@ -102,13 +195,29 @@ template("test") { install_script_name = "install_${invoker.output_name}" } deps += [ ":$_library_target" ] + + # TODO(agrieve): Remove this data_dep once bots don't build the _apk + # target (post-GYP). + # It's a bit backwards for the apk to depend on the runner script, since + # the apk is conceptually a runtime_dep of the script. However, it is + # currently necessary because the bots build this _apk target directly + # rather than the group() below. + data_deps = [ + ":$_test_runner_target", + ] } # Incremental test targets work only for .apks. _incremental_test_runner_target = "${_output_name}_incremental__test_runner_script" test_runner_script(_incremental_test_runner_target) { - forward_variables_from(invoker, [ "isolate_file" ]) + forward_variables_from(invoker, _wrapper_script_vars) + if (_generate_device_isolate) { + isolate_file = _device_isolate_path + deps = [ + ":$_gen_isolate_target_name", + ] + } apk_target = ":$_apk_target" test_name = "${_output_name}_incremental" test_type = "gtest" @@ -128,7 +237,14 @@ template("test") { _test_runner_target = "${_output_name}__test_runner_script" test_runner_script(_test_runner_target) { - forward_variables_from(invoker, [ "isolate_file" ]) + forward_variables_from(invoker, _wrapper_script_vars) + if (_generate_device_isolate) { + isolate_file = _device_isolate_path + deps = [ + ":$_gen_isolate_target_name", + ] + } + if (_use_raw_android_executable) { executable_dist_dir = "$root_out_dir/$_dist_target" } else { @@ -151,7 +267,7 @@ template("test") { } } - # TODO(GYP): Delete this after we've converted everything to GN. + # TODO(GYP_GONE): Delete this after we've converted everything to GN. # The _run targets exist only for compatibility w/ GYP. group("${target_name}_apk_run") { testonly = true @@ -178,20 +294,37 @@ template("test") { ] } - app(_test_target) { - # TODO(GYP): Make this configurable and only provide a default - # that can be overridden. - info_plist = "//testing/gtest_ios/unittest-Info.plist" - app_name = target_name - entitlements_path = "//testing/gtest_ios" - code_signing_identity = "" + ios_app_bundle(_test_target) { testonly = true - extra_substitutions = [ "BUNDLE_ID_TEST_NAME=$app_name" ] # See above call. set_sources_assignment_filter([]) + forward_variables_from(invoker, "*", [ "testonly" ]) - forward_variables_from(invoker, "*") + # Provide sensible defaults in case invoker did not define any of those + # required variables. + if (!defined(info_plist) && !defined(info_plist_target)) { + info_plist = "//testing/gtest_ios/unittest-Info.plist" + } + if (!defined(entitlements_path)) { + entitlements_path = "//testing/gtest_ios" + } + if (!defined(code_signing_identity)) { + code_signing_identity = "" + } + + # TODO(crbug.com/603102): remove this once gyp support is dropped and all + # application uses the target name as value for BUNDLE_ID_TEST_NAME. + if (defined(invoker.app_name)) { + app_name = invoker.app_name + } else { + app_name = target_name + } + + if (!defined(extra_substitutions)) { + extra_substitutions = [] + } + extra_substitutions += [ "BUNDLE_ID_TEST_NAME=$app_name" ] if (!defined(deps)) { deps = [] @@ -220,7 +353,7 @@ template("test") { ] } - # TODO(GYP): Delete this after we've converted everything to GN. + # TODO(GYP_GONE): Delete this after we've converted everything to GN. # The _run targets exist only for compatibility with GYP. group("${target_name}_run") { testonly = true @@ -228,5 +361,14 @@ template("test") { ":${invoker.target_name}", ] } + + if (defined(invoker.output_name) && target_name != invoker.output_name) { + group("${invoker.output_name}_run") { + testonly = true + deps = [ + ":${invoker.target_name}", + ] + } + } } } -- cgit v1.2.3