GN target //media/base/android:android should not be copied into multiple libraries |
||||
Issue descriptionGN target //media/base/android:android is a source_set(), yet it is referenced in multiple places [1] [2]. This means when making a component build multiple copies of the same files get included in various different dlls. I noticed this when debugging issues 699310 and 701916. There ended up being multiple copies of static MediaClientAndroid* g_media_client = nullptr; and the actual code that needed to use it couldn't find the client. //media/base/android:android should have limited visibility (only //media/base), and everything else should only depend of //media. [1] https://cs.chromium.org/chromium/src/android_webview/native/BUILD.gn?l=22 [2] https://cs.chromium.org/chromium/src/content/gpu/BUILD.gn?l=76
,
Mar 17 2017
This could also affect Android TV. Adding sanfin@ and yucliu@ FYI.
,
Mar 17 2017
,
Mar 18 2017
I tried a non-component build and it works fine. So this is not affecting official builds.
,
Mar 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/397905b7040fa1c0329d3675bcfd1120c04d7867 commit 397905b7040fa1c0329d3675bcfd1120c04d7867 Author: jrummell <jrummell@chromium.org> Date: Mon Mar 20 22:36:25 2017 Restrict use of GN target //media/base/android:android As //media/base/android:android is a source_set, multiple targets referencing it using DEPS result in multiple copies of the code being used when is_component_build = true. Adding a visibility restriction to the target, and changing all users to use //media (which should be the only target including the code). BUG= 702438 TEST=gn check passes 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 Review-Url: https://codereview.chromium.org/2757463004 Cr-Commit-Position: refs/heads/master@{#458207} [modify] https://crrev.com/397905b7040fa1c0329d3675bcfd1120c04d7867/android_webview/native/BUILD.gn [modify] https://crrev.com/397905b7040fa1c0329d3675bcfd1120c04d7867/chromecast/common/media/BUILD.gn [modify] https://crrev.com/397905b7040fa1c0329d3675bcfd1120c04d7867/chromecast/media/cdm/BUILD.gn [modify] https://crrev.com/397905b7040fa1c0329d3675bcfd1120c04d7867/content/gpu/BUILD.gn [modify] https://crrev.com/397905b7040fa1c0329d3675bcfd1120c04d7867/content/shell/android/BUILD.gn [modify] https://crrev.com/397905b7040fa1c0329d3675bcfd1120c04d7867/media/BUILD.gn [modify] https://crrev.com/397905b7040fa1c0329d3675bcfd1120c04d7867/media/base/android/BUILD.gn [modify] https://crrev.com/397905b7040fa1c0329d3675bcfd1120c04d7867/media/blink/BUILD.gn [modify] https://crrev.com/397905b7040fa1c0329d3675bcfd1120c04d7867/media/gpu/BUILD.gn
,
Apr 10 2017
Is this fixed now?
,
Apr 10 2017
|
||||
►
Sign in to add a comment |
||||
Comment 1 by xhw...@chromium.org
, Mar 17 2017