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

Issue 671706 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit 26 days ago
Closed: Dec 2016
EstimatedDays: ----
NextAction: ----
OS: Linux , All
Pri: 3
Type: Bug



Sign in to add a comment

Remove unneeded build flags / defines

Project Member Reported by brettw@chromium.org, Dec 6 2016

Issue description

As announced in this email to chromium-dev:
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/AMyyv2wr7jI/ajZMX-4QBAAJ

The following build flags and corresponding defines can be removed:

  enable_notifications
  enable_web_speech
  enable_task_manager
  enable_configuration_policy
  enable_mac_keystone
  enable_themes
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 7 2016

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

commit 1f92eed58cef4d29db962d41d1055935d4cd2f59
Author: brettw <brettw@chromium.org>
Date: Wed Dec 07 01:12:58 2016

Remove enable_notifications build flag and define

This flag is not overridable and is set everywhere except iOS. All users except
one are in //chrome which isn't compiled on iOS. So in all of these cases we
can remove the conditionals and always compile the code.

For the one case in ui, The conditional is replaced with !is_ios. This was also
done with the .grd file in chrome. This file has a lot of iOS conditionals and
I want to be sure it's not needed on iOS before inlining them unconditionally.
Then we can delete all of the iOS conditionals.

BUG= 671706 

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

[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/build/config/BUILD.gn
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/build/config/features.gni
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/chrome/app/generated_resources.grd
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/chrome/browser/BUILD.gn
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/chrome/browser/about_flags.cc
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/chrome/browser/background/background_contents_service.cc
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/chrome/browser/background/background_contents_service_unittest.cc
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/chrome/browser/browser_process_impl.cc
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/chrome/browser/extensions/extension_system_impl.cc
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/chrome/browser/permissions/permission_context_base_unittest.cc
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/chrome/browser/permissions/permission_infobar_delegate.cc
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/chrome/browser/permissions/permission_request_impl.cc
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/chrome/browser/permissions/permission_util.cc
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/chrome/browser/prefs/browser_prefs.cc
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/chrome/browser/push_messaging/push_messaging_browsertest.cc
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/chrome/browser/push_messaging/push_messaging_service_impl.cc
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/chrome/browser/push_messaging/push_messaging_service_impl.h
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/chrome/test/BUILD.gn
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/chrome/test/base/testing_browser_process.cc
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/tools/grit/grit_rule.gni
[modify] https://crrev.com/1f92eed58cef4d29db962d41d1055935d4cd2f59/ui/message_center/BUILD.gn

Project Member

Comment 2 by bugdroid1@chromium.org, Dec 7 2016

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

commit 24746e51329bde141ba7c4a1a39a94624381de6f
Author: brettw <brettw@chromium.org>
Date: Wed Dec 07 04:26:17 2016

Remove enable_web_speech flag.

This flag is not overridable from the build. Its definition is non-mobile. Now
that iOS is no longer compiled using chrome or content, all web speech
conditions become non-Android and this flag can be removed.

BUG= 671706 

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

[modify] https://crrev.com/24746e51329bde141ba7c4a1a39a94624381de6f/build/config/features.gni
[modify] https://crrev.com/24746e51329bde141ba7c4a1a39a94624381de6f/chrome/test/BUILD.gn
[modify] https://crrev.com/24746e51329bde141ba7c4a1a39a94624381de6f/content/browser/BUILD.gn
[modify] https://crrev.com/24746e51329bde141ba7c4a1a39a94624381de6f/content/test/BUILD.gn

Project Member

Comment 3 by bugdroid1@chromium.org, Dec 8 2016

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

commit ee486bb1c7a7351039d88ba233ad9eabc94887df
Author: brettw <brettw@chromium.org>
Date: Thu Dec 08 17:56:34 2016

Remove the enable_task_manager build flag and define.

These have been replaced by !android checks. The task manager is enabled on all
platforms except Android and iOS, and code in chrome isn't compiled on iOS.

BUG= 671706 

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

[modify] https://crrev.com/ee486bb1c7a7351039d88ba233ad9eabc94887df/build/config/BUILD.gn
[modify] https://crrev.com/ee486bb1c7a7351039d88ba233ad9eabc94887df/build/config/features.gni
[modify] https://crrev.com/ee486bb1c7a7351039d88ba233ad9eabc94887df/chrome/app/generated_resources.grd
[modify] https://crrev.com/ee486bb1c7a7351039d88ba233ad9eabc94887df/chrome/browser/BUILD.gn
[modify] https://crrev.com/ee486bb1c7a7351039d88ba233ad9eabc94887df/chrome/browser/extensions/BUILD.gn
[modify] https://crrev.com/ee486bb1c7a7351039d88ba233ad9eabc94887df/chrome/browser/net/chrome_network_delegate.cc
[modify] https://crrev.com/ee486bb1c7a7351039d88ba233ad9eabc94887df/chrome/browser/prefs/browser_prefs.cc
[modify] https://crrev.com/ee486bb1c7a7351039d88ba233ad9eabc94887df/chrome/browser/prerender/prerender_browsertest.cc
[modify] https://crrev.com/ee486bb1c7a7351039d88ba233ad9eabc94887df/chrome/browser/printing/print_preview_dialog_controller_browsertest.cc
[modify] https://crrev.com/ee486bb1c7a7351039d88ba233ad9eabc94887df/chrome/browser/task_manager/web_contents_tags.cc
[modify] https://crrev.com/ee486bb1c7a7351039d88ba233ad9eabc94887df/chrome/browser/ui/browser_commands.cc
[modify] https://crrev.com/ee486bb1c7a7351039d88ba233ad9eabc94887df/chrome/common/extensions/api/BUILD.gn
[modify] https://crrev.com/ee486bb1c7a7351039d88ba233ad9eabc94887df/chrome/test/BUILD.gn
[modify] https://crrev.com/ee486bb1c7a7351039d88ba233ad9eabc94887df/tools/grit/grit_rule.gni

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 9 2016

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

commit cee2ef7eac5b521699ba2a6c4c494ecb2d91b042
Author: brettw <brettw@chromium.org>
Date: Fri Dec 09 23:25:20 2016

Remove the enable_mac_keystone build flag.

enable_mac_keystone was a global flag in the build directory that's only used
by 4 total checks in 2 BUILD files. I'm trying to clean up the global build
flags.

Changes the checks to just check for mac Chrome branded builds. The previous
code additionally checked for "official" builds, but "official" is actually an
optimization level that I don't think is relevant to whether there are
installer scripts included in the bundle.

BUG= 671706 

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

[modify] https://crrev.com/cee2ef7eac5b521699ba2a6c4c494ecb2d91b042/build/config/features.gni
[modify] https://crrev.com/cee2ef7eac5b521699ba2a6c4c494ecb2d91b042/chrome/BUILD.gn
[modify] https://crrev.com/cee2ef7eac5b521699ba2a6c4c494ecb2d91b042/chrome/installer/mac/BUILD.gn

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 10 2016

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

commit 3865a71a732defdff6b64fd3c0fb2899fa7a05c6
Author: brettw <brettw@chromium.org>
Date: Sat Dec 10 05:59:08 2016

Remove enable_configuration_policy build define.

This build flag is always set for non-iOS platforms. The flag is not
overridable in the build and we never test other settings of it. The policy
code is stable and doesn't change.

This patch removes the flag and replaces it with !ios conditionals.

BUG= 671706 

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

[modify] https://crrev.com/3865a71a732defdff6b64fd3c0fb2899fa7a05c6/BUILD.gn
[modify] https://crrev.com/3865a71a732defdff6b64fd3c0fb2899fa7a05c6/android_webview/BUILD.gn
[modify] https://crrev.com/3865a71a732defdff6b64fd3c0fb2899fa7a05c6/build/config/features.gni
[modify] https://crrev.com/3865a71a732defdff6b64fd3c0fb2899fa7a05c6/chrome/browser/BUILD.gn
[modify] https://crrev.com/3865a71a732defdff6b64fd3c0fb2899fa7a05c6/components/BUILD.gn
[modify] https://crrev.com/3865a71a732defdff6b64fd3c0fb2899fa7a05c6/components/policy/BUILD.gn
[modify] https://crrev.com/3865a71a732defdff6b64fd3c0fb2899fa7a05c6/components/policy/core/browser/BUILD.gn
[modify] https://crrev.com/3865a71a732defdff6b64fd3c0fb2899fa7a05c6/components/policy/core/common/BUILD.gn
[modify] https://crrev.com/3865a71a732defdff6b64fd3c0fb2899fa7a05c6/components/search_engines/BUILD.gn
[modify] https://crrev.com/3865a71a732defdff6b64fd3c0fb2899fa7a05c6/components/sync/BUILD.gn
[modify] https://crrev.com/3865a71a732defdff6b64fd3c0fb2899fa7a05c6/components/sync_preferences/BUILD.gn
[modify] https://crrev.com/3865a71a732defdff6b64fd3c0fb2899fa7a05c6/components/sync_preferences/pref_service_syncable_factory.cc
[modify] https://crrev.com/3865a71a732defdff6b64fd3c0fb2899fa7a05c6/remoting/BUILD.gn
[modify] https://crrev.com/3865a71a732defdff6b64fd3c0fb2899fa7a05c6/remoting/host/BUILD.gn
[modify] https://crrev.com/3865a71a732defdff6b64fd3c0fb2899fa7a05c6/remoting/host/win/BUILD.gn

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 11 2016

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

commit 9b0866fd103e996c5927d183dfa68f0931a76a45
Author: brettw <brettw@chromium.org>
Date: Sun Dec 11 02:34:06 2016

Remove the enable_themes build flag and define.

This is always defined on non-mobile platforms and it is not possible to
override in the build. Since //chrome is no longer build on iOS, most uses
can be easily keyed off of the Android defines. For files not compiled on
Android, the conditionals were removed altogether.

This change additionally coalesces some Android blocks in
chrome/browser/BUILD.gn and chrome/browser/ui/BUILD.gn

BUG= 671706 

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

[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/build/config/BUILD.gn
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/build/config/features.gni
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/chrome/browser/BUILD.gn
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/chrome/browser/search/instant_service.cc
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/chrome/browser/search/instant_service.h
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/chrome/browser/search/instant_service_factory.cc
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/chrome/browser/supervised_user/supervised_user_service.cc
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/chrome/browser/sync/chrome_sync_client.cc
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/chrome/browser/sync/profile_sync_service_factory.cc
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/chrome/browser/ui/browser.cc
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/chrome/browser/ui/prefs/prefs_tab_helper.cc
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/chrome/browser/ui/webui/about_ui.cc
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/chrome/browser/ui/webui/app_launcher_page_ui.cc
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/chrome/browser/ui/webui/md_downloads/md_downloads_ui.cc
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/chrome/browser/ui/webui/ntp/app_resource_cache_factory.cc
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/chrome/browser/ui/webui/ntp/new_tab_ui.cc
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/chrome/browser/ui/webui/ntp/ntp_resource_cache_factory.cc
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/chrome/browser/ui/webui/signin/md_user_manager_ui.cc
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/chrome/browser/ui/webui/version_ui.cc
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/chrome/test/BUILD.gn
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/components/version_ui/resources/about_version.html
[modify] https://crrev.com/9b0866fd103e996c5927d183dfa68f0931a76a45/tools/grit/grit_rule.gni

Comment 7 by brettw@chromium.org, Dec 12 2016

Status: Fixed (was: Started)

Sign in to add a comment