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

Issue 720159 link

Starred by 1 user
Status: Assigned
Owner:
wychen@chromium.org
Cc:
tfarina@chromium.org
dbeam@chromium.org
thakis@chromium.org
█ brettw@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

Some ChromeOS headers are used in non-ChromeOS build

Project Member Reported by wychen@chromium.org, May 9 2017 
Chrome Version: M59
OS: Linux

The actual header usage should match what's specified in GN build files. If not, it should be fixed and tracked in issue 661774. OS-specific files are filtered by default, and generally should only be used to build for that particular OS.

Typically there are two kinds of solutions:
1. If these headers are actually used cross-OS, we can use set_sources_assignment_filter() in build files to bypass the filter.
2. If they are not used, the inclusion should be guarded by ifdefs in the source.

The following cases can be naively solved by using the former solution, but they have the potential to be cleaned up and use the later.
* certificate_provider in chrome/browser/certificate_manager_model.cc
* cros_settings in chrome/browser/extensions/api/settings_private/settings_private_event_router.h

If the clean up turns up to be not worth it, using the former solution is also fine.
Project Member

Comment 1 by bugdroid1@chromium.org, May 10 2017 

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

commit e120c2fd1e0c07b721caddef640aabf835d75840
Author: wychen <wychen@chromium.org>
Date: Wed May 10 07:37:07 2017

Clean up most of the ChromeOS headers in non-ChromeOS build

I ran check_gn_headers.py on linux and android full builds, merged the
missing header file list, and manually fixed ChromeOS-related ones.

Decreased the number of missing header files by 13.
Android build is now free of ChromeOS headers.

BUG=661774, 720159

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

[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/ash/shell.h
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/certificate_manager_model.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/cryptauth/chrome_cryptauth_service.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/diagnostics/diagnostics_controller_unittest.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/diagnostics/sqlite_diagnostics.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/extensions/api/page_capture/page_capture_apitest.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/extensions/api/settings_private/settings_private_event_router.h
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/extensions/api/web_request/web_request_apitest.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/extensions/api/web_request/web_request_permissions_unittest.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/net/predictor_tab_helper.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/profiles/profile_manager_unittest.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/ssl/security_state_tab_helper.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/ui/input_method/input_method_engine_base.h
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/extensions/browser/api/web_request/web_request_api.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/extensions/browser/api/web_request/web_request_permissions.cc
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 23 

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

commit b429f54e6c9be6a420d01e82e07c93a6912cbe9f
Author: Pavol Marko <pmarko@chromium.org>
Date: Thu Aug 23 06:08:19 2018

certificate_manager_model: Properly support policy and extension provided certs

Refactor CertificateManagerModel to internally use one or more "CertsSource"s
to retrieve certificates to be listed.
The main "CertsSource" is tied to the user's NSSCertDatabase.
On Chrome OS, there are additionaly PolicyCertsSource and ExtensionCertsSource,
which list certificates provided by user policy and extensions, respectively.

There is a pre-defined priority handling:
PolicyCertsSource > PlatformCertsSourceNSS > ExtensionCertsSource.
This means that if e.g. a CA certificate is provided by policy and is also
present in the user's NSS Database, the certificate manager will display
the policy version (e.g. the cert will not be deletable, trust settings can
not be changed).

BUG:  787602 ,720159
Test: unit_tests --gtest_filter=*CertificateManagerModel*

Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I73ff91070b1362afee37bff2413fb56cf4bc06ae
Reviewed-on: https://chromium-review.googlesource.com/916681
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Matt Mueller <mattm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585411}
[modify] https://crrev.com/b429f54e6c9be6a420d01e82e07c93a6912cbe9f/chrome/browser/certificate_manager_model.cc
[modify] https://crrev.com/b429f54e6c9be6a420d01e82e07c93a6912cbe9f/chrome/browser/certificate_manager_model.h
[add] https://crrev.com/b429f54e6c9be6a420d01e82e07c93a6912cbe9f/chrome/browser/certificate_manager_model_unittest.cc
[modify] https://crrev.com/b429f54e6c9be6a420d01e82e07c93a6912cbe9f/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/b429f54e6c9be6a420d01e82e07c93a6912cbe9f/chrome/browser/chromeos/policy/network_configuration_updater_unittest.cc
[modify] https://crrev.com/b429f54e6c9be6a420d01e82e07c93a6912cbe9f/chrome/browser/chromeos/policy/policy_cert_service.h
[modify] https://crrev.com/b429f54e6c9be6a420d01e82e07c93a6912cbe9f/chrome/browser/chromeos/policy/policy_cert_service_factory.cc
[add] https://crrev.com/b429f54e6c9be6a420d01e82e07c93a6912cbe9f/chrome/browser/chromeos/policy/policy_certificate_provider.h
[modify] https://crrev.com/b429f54e6c9be6a420d01e82e07c93a6912cbe9f/chrome/browser/chromeos/policy/user_network_configuration_updater.cc
[modify] https://crrev.com/b429f54e6c9be6a420d01e82e07c93a6912cbe9f/chrome/browser/chromeos/policy/user_network_configuration_updater.h
[modify] https://crrev.com/b429f54e6c9be6a420d01e82e07c93a6912cbe9f/chrome/browser/chromeos/policy/user_network_configuration_updater_factory.cc
[modify] https://crrev.com/b429f54e6c9be6a420d01e82e07c93a6912cbe9f/chrome/browser/chromeos/policy/user_network_configuration_updater_factory.h
[modify] https://crrev.com/b429f54e6c9be6a420d01e82e07c93a6912cbe9f/chrome/browser/chromeos/policy/user_network_configuration_updater_factory_browsertest.cc
[modify] https://crrev.com/b429f54e6c9be6a420d01e82e07c93a6912cbe9f/chrome/browser/ui/webui/certificates_handler.cc
[modify] https://crrev.com/b429f54e6c9be6a420d01e82e07c93a6912cbe9f/chrome/browser/ui/webui/certificates_handler.h
[modify] https://crrev.com/b429f54e6c9be6a420d01e82e07c93a6912cbe9f/chrome/test/BUILD.gn
[modify] https://crrev.com/b429f54e6c9be6a420d01e82e07c93a6912cbe9f/net/cert/nss_cert_database.cc
[modify] https://crrev.com/b429f54e6c9be6a420d01e82e07c93a6912cbe9f/net/cert/nss_cert_database.h
[modify] https://crrev.com/b429f54e6c9be6a420d01e82e07c93a6912cbe9f/ui/webui/resources/cr_components/certificate_manager/certificates_browser_proxy.js
►

Sign in to add a comment