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

Issue 597830 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Traveling - Back 2/6
Closed: May 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android
Pri: 3
Type: Bug



Sign in to add a comment

Add blimp to check_targets in .gn

Project Member Reported by shaktisahu@chromium.org, Mar 24 2016

Issue description

There are quite a few errors when running gn gen --check.
Adding blimp to check_targets will have their includes checked for proper dependencies. 

 

Comment 1 by w...@chromium.org, Mar 27 2016

Labels: -Proj-Blimp

Comment 2 by w...@chromium.org, Mar 29 2016

Cc: w...@chromium.org

Comment 3 by w...@chromium.org, Mar 31 2016

Labels: OS-Linux
Status: Available (was: Untriaged)
Cc: nyquist@chromium.org
Cc: -nyquist@chromium.org
Owner: nyquist@chromium.org
Status: Started (was: Available)
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 27 2016

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

commit ae6d61aae5d8b33110c93d60bbe746722f5f966a
Author: nyquist <nyquist@chromium.org>
Date: Wed Apr 27 16:46:38 2016

Add missing public_deps for //content/public/app:xxx GN targets.

The non-component build version of the //content/public/app:both target
provides the public_app_shared_sources as sources, whereas the component
build version of the same target does not.

This causes issues for targets outside //content that need to depend on
//content/public/app/content_main.h and other files in
public_app_shared_sources from //content/public/app/BUILD.gn, because
the non-component build has to be special cased.

This CL fixes the public_deps for //content/public/app:xxx for the
component build to make the dependency on //content a public_deps entry.

This includes the targets:
//content/public/app:browser
//content/public/app:child
//content/public/app:both

BUG= 597830 

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

[modify] https://crrev.com/ae6d61aae5d8b33110c93d60bbe746722f5f966a/content/public/app/BUILD.gn

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 28 2016

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

commit 409b3366f93797658ebba8002564dd175bb49bd8
Author: nyquist <nyquist@chromium.org>
Date: Thu Apr 28 21:10:07 2016

Add blimp to root GN check_targets

//blimp has not been checked for GN issues until now, which has caused
incorrect dependencies to creep in, as well as several missing
dependencies.

This CL adds //blimp/* to the list of targets that should always be
checked by GN, which should prevent further issues.

As part of this change, several parts of //blimp/engine:app had to be
split out into their own source_sets since otherwise there would be
cyclic dependencies between different targets in //blimp/engine/*.

The functionality related to getting the user agent was also moved
from the blimp_content_client.h header into its own file, also to
remove a cyclic dependency.

The blimp_client_export.h file is not needed anymore, since no symbols
ever need to be exported from the client, so it has thus been removed.

Lastly, all deps that used to be //content have now been changed to
more specific ones, such as //content/public/browser. This lead to an
issue with //content/app, since //content/app/content_main_runner.cc
refers to PpapiPluginMain in RunZygote if ENABLE_PLUGINS is defined, a
missing dependency was added to the content_app_deps in
//content/app/BUILD.gn. This should only be added on linux though,
since the code that uses it is only used on linux, and adding it on
Windows when is_multi_dll_chrome is true would lead to both the browser
and the child process to get it, when only the child process should
have it. This also required updating the visibility
of the //content/ppapi:ppapi_plugin_sources target to allow for this.

BUG= 597830 

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

[modify] https://crrev.com/409b3366f93797658ebba8002564dd175bb49bd8/.gn
[modify] https://crrev.com/409b3366f93797658ebba8002564dd175bb49bd8/blimp/client/BUILD.gn
[modify] https://crrev.com/409b3366f93797658ebba8002564dd175bb49bd8/blimp/client/app/blimp_startup.h
[delete] https://crrev.com/e98a8fbe91dc81f05799389edaca003e29f78d70/blimp/client/blimp_client_export.h
[modify] https://crrev.com/409b3366f93797658ebba8002564dd175bb49bd8/blimp/client/feature/compositor/blimp_compositor.h
[modify] https://crrev.com/409b3366f93797658ebba8002564dd175bb49bd8/blimp/client/feature/ime_feature.h
[modify] https://crrev.com/409b3366f93797658ebba8002564dd175bb49bd8/blimp/client/feature/navigation_feature.h
[modify] https://crrev.com/409b3366f93797658ebba8002564dd175bb49bd8/blimp/client/feature/render_widget_feature.h
[modify] https://crrev.com/409b3366f93797658ebba8002564dd175bb49bd8/blimp/client/feature/settings_feature.h
[modify] https://crrev.com/409b3366f93797658ebba8002564dd175bb49bd8/blimp/client/feature/tab_control_feature.h
[modify] https://crrev.com/409b3366f93797658ebba8002564dd175bb49bd8/blimp/client/session/assignment_source.h
[modify] https://crrev.com/409b3366f93797658ebba8002564dd175bb49bd8/blimp/client/session/blimp_client_session.h
[modify] https://crrev.com/409b3366f93797658ebba8002564dd175bb49bd8/blimp/common/BUILD.gn
[modify] https://crrev.com/409b3366f93797658ebba8002564dd175bb49bd8/blimp/engine/BUILD.gn
[modify] https://crrev.com/409b3366f93797658ebba8002564dd175bb49bd8/blimp/engine/app/blimp_url_request_context_getter.cc
[modify] https://crrev.com/409b3366f93797658ebba8002564dd175bb49bd8/blimp/engine/common/blimp_content_client.cc
[add] https://crrev.com/409b3366f93797658ebba8002564dd175bb49bd8/blimp/engine/common/blimp_user_agent.cc
[add] https://crrev.com/409b3366f93797658ebba8002564dd175bb49bd8/blimp/engine/common/blimp_user_agent.h
[modify] https://crrev.com/409b3366f93797658ebba8002564dd175bb49bd8/blimp/net/BUILD.gn
[modify] https://crrev.com/409b3366f93797658ebba8002564dd175bb49bd8/content/app/BUILD.gn
[modify] https://crrev.com/409b3366f93797658ebba8002564dd175bb49bd8/content/ppapi_plugin/BUILD.gn

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 29 2016

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

commit 170f32cbc553a61d5dc8f13b7fd173f4c430b432
Author: nyquist <nyquist@chromium.org>
Date: Fri Apr 29 17:25:32 2016

Simplify GN target names for //blimp

As part of making the GN configuration for //blimp up to date with
current best practices, this CL changes top-level targets under
//blimp to have the same name as their folder.

As an example, //blimp/common:blimp_common can now be referred to
using only //blimp/common.

The reason why for example blimp_common was previously chosen was
to ensure that the output name of the component did not collide with
other targets in the chromium repository.

This CL ensures that old output names are kept in place by
specifying the output_name GN variable. This is only necessary for
build artifacts, and not for gn templates such as group or source_set.

BUG= 597830 

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

[modify] https://crrev.com/170f32cbc553a61d5dc8f13b7fd173f4c430b432/blimp/BUILD.gn
[modify] https://crrev.com/170f32cbc553a61d5dc8f13b7fd173f4c430b432/blimp/client/BUILD.gn
[modify] https://crrev.com/170f32cbc553a61d5dc8f13b7fd173f4c430b432/blimp/common/BUILD.gn
[modify] https://crrev.com/170f32cbc553a61d5dc8f13b7fd173f4c430b432/blimp/docs/container.md
[modify] https://crrev.com/170f32cbc553a61d5dc8f13b7fd173f4c430b432/blimp/engine/BUILD.gn
[modify] https://crrev.com/170f32cbc553a61d5dc8f13b7fd173f4c430b432/blimp/engine/engine-manifest.txt
[modify] https://crrev.com/170f32cbc553a61d5dc8f13b7fd173f4c430b432/blimp/net/BUILD.gn

Status: Fixed (was: Started)
Labels: Archive-Blimp

Sign in to add a comment