New issue
Advanced search Search tips

Issue 725808 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: ----



Sign in to add a comment

generate_build_files failing on chromium.chrome/Google Chrome Win

Project Member Reported by vitaliii@chromium.org, May 24 2017

Issue description

generate_build_files failing on chromium.chrome/Google Chrome Win

Builders failed on: 
- Google Chrome Win: 
  https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Win



 
In all 3 failing builds (starting from https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Win/builds/18366) the error message is the same:
=== LOG STARTS ===
C:\b\c\b\win_chrome\src\buildtools\win\gn.exe gen //out/Release --check
  -> returned 1
ERROR at //build/split_static_library.gni:27:7: Dependency not allowed.
      static_library(current_name) {
      ^-----------------------------
The item //content/renderer:renderer_0
can not depend on //media/mojo/clients:clients
because it is not in //media/mojo/clients:clients's visibility list: [
  //chromecast/*
  //content/renderer:renderer
  //media/mojo:*
  //media/mojo/services:*
  //media/test/*
  //content/gpu:*
  //content/test/*
]
=== LOG ENDS ===
Cc: xhw...@chromium.org
Only https://codereview.chromium.org/2900773004 touches Mojo in the first failing build.
Project Member

Comment 4 by bugdroid1@chromium.org, May 24 2017

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

commit 94274172ed5f2f5f5b2ead35f07e5e011a73e3b4
Author: vitaliii <vitaliii@chromium.org>
Date: Wed May 24 07:55:01 2017

Revert of media: Add experimental feature to enable Mojo CDM on desktop Chromium (patchset #5 id:180001 of https://codereview.chromium.org/2900773004/ )

Reason for revert:
This CL seems to break |generate_build_files| step on chromium.chrome/Google Chrome Win. Please see  crbug.com/725808  for details.

Original issue's description:
> media: Add experimental feature to enable Mojo CDM on desktop Chromium
>
> Currently in Chromium, the CDM is hosted by the CDM adapter which is a
> pepper plugin. This CL updates the build flags such that when pepper
> CDMs are enabled, we also enable hosting the CDM using mojo in an
> unsandboxed utility process. By default, pepper CDM will always be
> used and mojo CDM is disabled. A new base::Feature is added such that
> developers and testers can enable mojo CDM for early prototyping and
> testing.
>
> To enable mojo CDM, please add the following command line options:
>
> --enable-features=MojoCdm
>
> Detailed changes in this CL:
> - Updated build flags to enable mojo CDM in utility process when pepper
>   CDMs are enabled.
> - Add a base::Feature media::kMojoCdm to enable mojo CDM at run time.
> - Add CdmAdapterFactory to create CdmAdapter, which hosts the CDM
>   binary.
> - Register MediaService in the utility process to create
>   CdmAdapterFactory when requested.
> - Also copy base::Feature flags on command line for utility process.
>
> Note that Mojo CDM on desktop Chromium is still under development. Some
> features will not work. Also, the CDM is running in an unsandboxed
> utility process since we cannot load the CDM in a sandboxed utility
> process yet. These will be addressed in future CLs.
>
> BUG=403462, 725394 , 561090 
>
> Review-Url: https://codereview.chromium.org/2900773004
> Cr-Commit-Position: refs/heads/master@{#474186}
> Committed: https://chromium.googlesource.com/chromium/src/+/846b429c5175d3637e4183bc06eb417eee09482c

TBR=jrummell@chromium.org,jam@chromium.org,xhwang@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=403462, 725394 , 561090 , 725808 

Review-Url: https://codereview.chromium.org/2902943002
Cr-Commit-Position: refs/heads/master@{#474202}

[modify] https://crrev.com/94274172ed5f2f5f5b2ead35f07e5e011a73e3b4/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/94274172ed5f2f5f5b2ead35f07e5e011a73e3b4/content/browser/media/encrypted_media_browsertest.cc
[modify] https://crrev.com/94274172ed5f2f5f5b2ead35f07e5e011a73e3b4/content/browser/service_manager/service_manager_context.cc
[modify] https://crrev.com/94274172ed5f2f5f5b2ead35f07e5e011a73e3b4/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/94274172ed5f2f5f5b2ead35f07e5e011a73e3b4/content/utility/BUILD.gn
[modify] https://crrev.com/94274172ed5f2f5f5b2ead35f07e5e011a73e3b4/content/utility/utility_service_factory.cc
[modify] https://crrev.com/94274172ed5f2f5f5b2ead35f07e5e011a73e3b4/media/BUILD.gn
[modify] https://crrev.com/94274172ed5f2f5f5b2ead35f07e5e011a73e3b4/media/base/media_switches.cc
[modify] https://crrev.com/94274172ed5f2f5f5b2ead35f07e5e011a73e3b4/media/base/media_switches.h
[delete] https://crrev.com/8da026c9e77facffc5ed46cfb75c2932f09fcc12/media/cdm/cdm_adapter_factory.cc
[delete] https://crrev.com/8da026c9e77facffc5ed46cfb75c2932f09fcc12/media/cdm/cdm_adapter_factory.h
[modify] https://crrev.com/94274172ed5f2f5f5b2ead35f07e5e011a73e3b4/media/cdm/cdm_allocator.h
[modify] https://crrev.com/94274172ed5f2f5f5b2ead35f07e5e011a73e3b4/media/media_options.gni
[modify] https://crrev.com/94274172ed5f2f5f5b2ead35f07e5e011a73e3b4/media/mojo/services/main.cc

The build with the revert (https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Win/builds/18372) just successfully passed |generate_build_files| step.
Labels: Pri-1
Status: Fixed (was: Started)
The builder is green again.
Project Member

Comment 7 by bugdroid1@chromium.org, May 24 2017

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

commit c1878a272a87bd797949717bebdfdbc1e6f0c143
Author: xhwang <xhwang@chromium.org>
Date: Wed May 24 18:48:48 2017

media: Fix //media/mojo/clients visibility

Currently //content/renderer:renderer is in //media/mojo/clients'
visibility list. However when using split_static_library, we could end
up having //content/renderer:renderer_0 target which has all the same
dependencies as //content/renderer:renderer. However,
//content/renderer:renderer_0 isn't in //media/mojo/clients' visibility
list, causing gn check failures.

This is not the first time split_static_library causing similar issues.
For example, see  issue 645621 .

I'll send an email discussing this generic issue. Meanwhile, this CL
relaxes the visibility rule such that all //content/renderer:* targets
are in //media/mojo/clients' visibility list. This is also how issue
645621 was "fixed".

Here's the current gn check log:

------------------------------------------
C:\b\c\b\win_chrome\src\buildtools\win\gn.exe gen //out/Release --check
  -> returned 1
ERROR at //build/split_static_library.gni:27:7: Dependency not allowed.
      static_library(current_name) {
      ^-----------------------------
The item //content/renderer:renderer_0
can not depend on //media/mojo/clients:clients
because it is not in //media/mojo/clients:clients's visibility list: [
  //content/renderer:renderer
  ...
]
------------------------------------------

BUG= 725808 

Review-Url: https://codereview.chromium.org/2901943003
Cr-Commit-Position: refs/heads/master@{#474375}

[modify] https://crrev.com/c1878a272a87bd797949717bebdfdbc1e6f0c143/media/mojo/clients/BUILD.gn

Project Member

Comment 8 by bugdroid1@chromium.org, May 24 2017

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

commit 79fc0eff0e3bf95ad6a9759a49d9c837781440f8
Author: xhwang <xhwang@chromium.org>
Date: Wed May 24 18:52:32 2017

Reland of media: Add experimental feature to enable Mojo CDM on desktop Chromium (patchset #1 id:1 of https://codereview.chromium.org/2902943002/ )

Reason for revert:
The fix on gn visibility has been landed in https://codereview.chromium.org/2901943003/

Original issue's description:
> Revert of media: Add experimental feature to enable Mojo CDM on desktop Chromium (patchset #5 id:180001 of https://codereview.chromium.org/2900773004/ )
>
> Reason for revert:
> This CL seems to break |generate_build_files| step on chromium.chrome/Google Chrome Win. Please see  crbug.com/725808  for details.
>
> Original issue's description:
> > media: Add experimental feature to enable Mojo CDM on desktop Chromium
> >
> > Currently in Chromium, the CDM is hosted by the CDM adapter which is a
> > pepper plugin. This CL updates the build flags such that when pepper
> > CDMs are enabled, we also enable hosting the CDM using mojo in an
> > unsandboxed utility process. By default, pepper CDM will always be
> > used and mojo CDM is disabled. A new base::Feature is added such that
> > developers and testers can enable mojo CDM for early prototyping and
> > testing.
> >
> > To enable mojo CDM, please add the following command line options:
> >
> > --enable-features=MojoCdm
> >
> > Detailed changes in this CL:
> > - Updated build flags to enable mojo CDM in utility process when pepper
> >   CDMs are enabled.
> > - Add a base::Feature media::kMojoCdm to enable mojo CDM at run time.
> > - Add CdmAdapterFactory to create CdmAdapter, which hosts the CDM
> >   binary.
> > - Register MediaService in the utility process to create
> >   CdmAdapterFactory when requested.
> > - Also copy base::Feature flags on command line for utility process.
> >
> > Note that Mojo CDM on desktop Chromium is still under development. Some
> > features will not work. Also, the CDM is running in an unsandboxed
> > utility process since we cannot load the CDM in a sandboxed utility
> > process yet. These will be addressed in future CLs.
> >
> > BUG=403462, 725394 , 561090 
> >
> > Review-Url: https://codereview.chromium.org/2900773004
> > Cr-Commit-Position: refs/heads/master@{#474186}
> > Committed: https://chromium.googlesource.com/chromium/src/+/846b429c5175d3637e4183bc06eb417eee09482c
>
> TBR=jrummell@chromium.org,jam@chromium.org,xhwang@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=403462, 725394 , 561090 , 725808 
>
> Review-Url: https://codereview.chromium.org/2902943002
> Cr-Commit-Position: refs/heads/master@{#474202}
> Committed: https://chromium.googlesource.com/chromium/src/+/94274172ed5f2f5f5b2ead35f07e5e011a73e3b4

TBR=jrummell@chromium.org,jam@chromium.org,vitaliii@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=403462, 725394 , 561090 , 725808 

Review-Url: https://codereview.chromium.org/2905713002
Cr-Commit-Position: refs/heads/master@{#474376}

[modify] https://crrev.com/79fc0eff0e3bf95ad6a9759a49d9c837781440f8/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/79fc0eff0e3bf95ad6a9759a49d9c837781440f8/content/browser/media/encrypted_media_browsertest.cc
[modify] https://crrev.com/79fc0eff0e3bf95ad6a9759a49d9c837781440f8/content/browser/service_manager/service_manager_context.cc
[modify] https://crrev.com/79fc0eff0e3bf95ad6a9759a49d9c837781440f8/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/79fc0eff0e3bf95ad6a9759a49d9c837781440f8/content/utility/BUILD.gn
[modify] https://crrev.com/79fc0eff0e3bf95ad6a9759a49d9c837781440f8/content/utility/utility_service_factory.cc
[modify] https://crrev.com/79fc0eff0e3bf95ad6a9759a49d9c837781440f8/media/BUILD.gn
[modify] https://crrev.com/79fc0eff0e3bf95ad6a9759a49d9c837781440f8/media/base/media_switches.cc
[modify] https://crrev.com/79fc0eff0e3bf95ad6a9759a49d9c837781440f8/media/base/media_switches.h
[add] https://crrev.com/79fc0eff0e3bf95ad6a9759a49d9c837781440f8/media/cdm/cdm_adapter_factory.cc
[add] https://crrev.com/79fc0eff0e3bf95ad6a9759a49d9c837781440f8/media/cdm/cdm_adapter_factory.h
[modify] https://crrev.com/79fc0eff0e3bf95ad6a9759a49d9c837781440f8/media/cdm/cdm_allocator.h
[modify] https://crrev.com/79fc0eff0e3bf95ad6a9759a49d9c837781440f8/media/media_options.gni
[modify] https://crrev.com/79fc0eff0e3bf95ad6a9759a49d9c837781440f8/media/mojo/services/main.cc

Sign in to add a comment