New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 747379 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug

Blocking:
issue 747229



Sign in to add a comment

Why isn't media/ part of the deps of //media/renderers.BUILD.gn?

Project Member Reported by gsennton@chromium.org, Jul 21 2017

Issue description

This question stems from crbug.com/747229#c7

media/renderers/audio_renderer_impl.cc includes media/base/media_switches.h which in turn includes ppapi/features/features.h.

ppapi/features/features.h is a generated file that is generated by the buildstep //ppapi/features:features, so if that buildstep is not run before we parse a file include ppapi/features/features.h, then it will look like ppapi/features/features.h is missing. media/renderers/renderers does not seem to have a dependency (i.e. gn deps) on either media/base, or ppapi/features, to me this seems wrong.

Dale, is media/renderers supposed to depend on media/base (or is it actually depending on media/base, and I just missed the dependency) or am I missing something?
 
Yes, media/renderers should depend on media/base. There's been work to split these  BUILD.gn files out so it's probably just gotten lost in the shuffle.
Cc: dalecur...@chromium.org
Owner: c.pa...@samsung.com

Comment 3 by awdf@chromium.org, Jul 27 2017

Blocking: 747229

Comment 4 by awdf@chromium.org, Jul 27 2017

Components: Internals>Media
Labels: -Pri-3 ReleaseBlock-Dev M-61 OS-Android Pri-1

Comment 5 by c.pa...@samsung.com, Jul 27 2017

Owner: a.suchit@chromium.org
I think a.suchit@samsung.com is currently working on splitting of //media/BUILD.gn. May be I was assigned this issue by dalecurtis@chromium.org by mistake. Assigning this back to it's appropriate owner.
Status: Started (was: Assigned)
Yes, sorry for the incorrect assignment!
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5b3329fdca90d0449fc8d4419bee11320e6c88c1

commit 5b3329fdca90d0449fc8d4419bee11320e6c88c1
Author: Suchit Agrawal <a.suchit@samsung.com>
Date: Fri Jul 28 12:34:12 2017

//media/renderers should depend on //media/base.

media/renderers/audio_renderer_impl.cc includes media/base/media_switches.h
which in turn includes ppapi/features/features.h and it is generates in
target //ppapi/features. //media/base is dependent on the //ppapi/features
so //media/base added as dependency in //media/renderers.

Bug= 747379 

Signed-off-by: Suchit Agrawal <a.suchit@samsung.com>
Change-Id: I9eb8e3f82d1e9425b5a324c129faa58d5c665ad8
Reviewed-on: https://chromium-review.googlesource.com/589467
Commit-Queue: SUCHIT AGRAWAL <a.suchit@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490369}
[modify] https://crrev.com/5b3329fdca90d0449fc8d4419bee11320e6c88c1/media/base/BUILD.gn
[modify] https://crrev.com/5b3329fdca90d0449fc8d4419bee11320e6c88c1/media/renderers/BUILD.gn

Status: Fixed (was: Started)

Sign in to add a comment