New issue
Advanced search Search tips
Starred by 1 user
Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocking:
issue 699310
issue 701916



Sign in to add a comment
GN target //media/base/android:android should not be copied into multiple libraries
Project Member Reported by jrumm...@chromium.org, Mar 16 2017 Back to list
GN 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
 
Comment 1 by xhw...@chromium.org, Mar 17 2017
Blocking: 699310
Comment 2 by xhw...@chromium.org, Mar 17 2017
Cc: sanfin@chromium.org yucliu@chromium.org
This could also affect Android TV. Adding sanfin@ and yucliu@ FYI.
Blocking: 701916
Comment 4 by xhw...@chromium.org, Mar 18 2017
I tried a non-component build and it works fine. So this is not affecting official builds. 
Project Member Comment 5 by bugdroid1@chromium.org, 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

Comment 6 by xhw...@chromium.org, Apr 10 2017
Is this fixed now?
Status: Fixed
Sign in to add a comment