PRESUBMIT_VERSION = '2.0.0'
-# This line is 'magic' in that git-cl looks for it to decide whether to
-# use Python3 instead of Python2 when running the code in this file.
-USE_PYTHON3 = True
_EXCLUDED_PATHS = (
# Generated file
r"tools/perf/page_sets/maps_perf_test.*",
# Test pages for WebRTC telemetry tests.
r"tools/perf/page_sets/webrtc_cases.*",
+ # Test file compared with generated output.
+ r"tools/polymer/tests/html_to_wrapper/.*.html.ts$",
)
_EXCLUDED_SET_NO_PARENT_PATHS = (
),
),
BanRule(
- 'import android.support.test.rule.UiThreadTestRule;',
+ 'import androidx.test.rule.UiThreadTestRule;',
(
'Do not use UiThreadTestRule, just use '
'@org.chromium.base.test.UiThreadTest on test methods that should run '
),
),
BanRule(
- 'import android.support.test.annotation.UiThreadTest;',
- ('Do not use android.support.test.annotation.UiThreadTest, use '
+ 'import androidx.test.annotation.UiThreadTest;',
+ ('Do not use androidx.test.annotation.UiThreadTest, use '
'org.chromium.base.test.UiThreadTest instead. See '
'https://crbug.com/1111893.',
),
),
BanRule(
- 'import android.support.test.rule.ActivityTestRule;',
+ 'import androidx.test.rule.ActivityTestRule;',
(
'Do not use ActivityTestRule, use '
'org.chromium.base.test.BaseActivityTestRule instead.',
'components/cronet/',
),
),
+ BanRule(
+ 'import androidx.vectordrawable.graphics.drawable.VectorDrawableCompat;',
+ (
+ 'Do not use VectorDrawableCompat, use getResources().getDrawable() to '
+ 'avoid extra indirections. Please also add trace event as the call '
+ 'might take more than 20 ms to complete.',
+ ),
+ ),
)
_BANNED_JAVA_FUNCTIONS : Sequence[BanRule] = (
),
False,
),
+ BanRule(
+ r'/(?<!\bsuper\.)(?<!\bIntent )\bregisterReceiver\(',
+ (
+ 'Do not call android.content.Context.registerReceiver (or an override) '
+ 'directly. Use one of the wrapper methods defined in '
+ 'org.chromium.base.ContextUtils, such as '
+ 'registerProtectedBroadcastReceiver, '
+ 'registerExportedBroadcastReceiver, or '
+ 'registerNonExportedBroadcastReceiver. See their documentation for '
+ 'which one to use.',
+ ),
+ True,
+ excluded_paths=(
+ r'.*Test[^a-z]',
+ r'third_party/',
+ 'base/android/java/src/org/chromium/base/ContextUtils.java',
+ 'chromecast/browser/android/apk/src/org/chromium/chromecast/shell/BroadcastReceiverScope.java',
+ ),
+ ),
+ BanRule(
+ r'/(?:extends|new)\s*(?:android.util.)?Property<[A-Za-z.]+,\s*(?:Integer|Float)>',
+ (
+ 'Do not use Property<..., Integer|Float>, but use FloatProperty or '
+ 'IntProperty because it will avoid unnecessary autoboxing of '
+ 'primitives.',
+ ),
+ ),
+ BanRule(
+ 'requestLayout()',
+ (
+ 'Layouts can be expensive. Prefer using ViewUtils.requestLayout(), '
+ 'which emits a trace event with additional information to help with '
+ 'scroll jank investigations. See http://crbug.com/1354176.',
+ ),
+ False,
+ excluded_paths=(
+ 'ui/android/java/src/org/chromium/ui/base/ViewUtils.java',
+ ),
+ ),
+ BanRule(
+ 'Profile.getLastUsedRegularProfile()',
+ (
+ 'Prefer passing in the Profile reference instead of relying on the '
+ 'static getLastUsedRegularProfile() call. Only top level entry points '
+ '(e.g. Activities) should call this method. Otherwise, the Profile '
+ 'should either be passed in explicitly or retreived from an existing '
+ 'entity with a reference to the Profile (e.g. WebContents).',
+ ),
+ False,
+ excluded_paths=(
+ r'.*Test[A-Z]?.*\.java',
+ ),
+ ),
+ BanRule(
+ r'/(ResourcesCompat|getResources\(\))\.getDrawable\(\)',
+ (
+ 'getDrawable() can be expensive. If you have a lot of calls to '
+ 'GetDrawable() or your code may introduce janks, please put your calls '
+ 'inside a trace().',
+ ),
+ False,
+ excluded_paths=(
+ r'.*Test[A-Z]?.*\.java',
+ ),
+ ),
+ BanRule(
+ r'/RecordHistogram\.getHistogram(ValueCount|TotalCount|Samples)ForTesting\(',
+ (
+ 'Raw histogram counts are easy to misuse; for example they don\'t reset '
+ 'between batched tests. Use HistogramWatcher to check histogram records '
+ 'instead.',
+ ),
+ False,
+ excluded_paths=(
+ 'base/android/javatests/src/org/chromium/base/metrics/RecordHistogramTest.java',
+ 'base/test/android/javatests/src/org/chromium/base/test/util/HistogramWatcher.java',
+ ),
+ ),
+)
+
+_BANNED_JAVASCRIPT_FUNCTIONS : Sequence [BanRule] = (
+ BanRule(
+ r'/\bchrome\.send\b',
+ (
+ 'The use of chrome.send is disallowed in Chrome (context: https://chromium.googlesource.com/chromium/src/+/refs/heads/main/docs/security/handling-messages-from-web-content.md).',
+ 'Please use mojo instead for new webuis. https://docs.google.com/document/d/1RF-GSUoveYa37eoyZ9EhwMtaIwoW7Z88pIgNZ9YzQi4/edit#heading=h.gkk22wgk6wff',
+ ),
+ True,
+ (
+ r'^(?!ash\/webui).+',
+ # TODO(crbug.com/1385601): pre-existing violations still need to be
+ # cleaned up.
+ 'ash/webui/common/resources/cr.m.js',
+ 'ash/webui/common/resources/multidevice_setup/multidevice_setup_browser_proxy.js',
+ 'ash/webui/common/resources/quick_unlock/lock_screen_constants.ts',
+ 'ash/webui/common/resources/smb_shares/smb_browser_proxy.js',
+ 'ash/webui/connectivity_diagnostics/resources/connectivity_diagnostics.js',
+ 'ash/webui/diagnostics_ui/resources/diagnostics_browser_proxy.ts',
+ 'ash/webui/multidevice_debug/resources/logs.js',
+ 'ash/webui/multidevice_debug/resources/webui.js',
+ 'ash/webui/projector_app/resources/annotator/trusted/annotator_browser_proxy.js',
+ 'ash/webui/projector_app/resources/app/trusted/projector_browser_proxy.js',
+ # TODO(b/301634378): Remove violation exception once Scanning App
+ # migrated off usage of `chrome.send`.
+ 'ash/webui/scanning/resources/scanning_browser_proxy.ts',
+ ),
+ ),
)
_BANNED_OBJC_FUNCTIONS : Sequence[BanRule] = (
'Please use |SysNSStringToUTF8| instead.',
),
True,
+ excluded_paths = (
+ '^third_party/ocmock/OCMock/',
+ ),
),
BanRule(
r'__unsafe_unretained',
),
True,
),
+ BanRule(
+ 'This file requires ARC support.',
+ (
+ 'ARC compilation is default in Chromium; do not add boilerplate to ',
+ 'files that require ARC.',
+ ),
+ True,
+ ),
)
_BANNED_IOS_OBJC_FUNCTIONS = (
(
'+[UIImage systemImageNamed:] should not be used to create symbols.',
'Instead use a wrapper defined in:',
- 'ios/chrome/browser/ui/icons/chrome_symbol.h'
+ 'ios/chrome/browser/ui/icons/symbol_helpers.h'
),
True,
excluded_paths=(
- 'ios/chrome/browser/ui/icons/chrome_symbol.mm',
+ 'ios/chrome/browser/shared/ui/symbols/symbol_helpers.mm',
+ 'ios/chrome/search_widget_extension/',
),
),
)
_BANNED_CPP_FUNCTIONS : Sequence[BanRule] = (
BanRule(
+ '%#0',
+ (
+ 'Zero-padded values that use "#" to add prefixes don\'t exhibit ',
+ 'consistent behavior, since the prefix is not prepended for zero ',
+ 'values. Use "0x%0..." instead.',
+ ),
+ False,
+ [_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
+ ),
+ BanRule(
r'/\busing namespace ',
(
'Using directives ("using namespace x") are banned by the Google Style',
'base/gtest_prod_util.h and use FRIEND_TEST_ALL_PREFIXES() instead.',
),
False,
- (),
+ excluded_paths = (
+ "base/gtest_prod_util.h",
+ "base/allocator/partition_allocator/src/partition_alloc/partition_alloc_base/gtest_prod_util.h",
+ ),
),
BanRule(
'setMatrixClip',
'deprecated (http://crbug.com/634507). Please avoid converting away',
'from the Time types in Chromium code, especially if any math is',
'being done on time values. For interfacing with platform/library',
- 'APIs, use FromMicroseconds() or InMicroseconds(), or one of the other',
- 'type converter methods instead. For faking TimeXXX values (for unit',
+ 'APIs, use base::Time::(From,To)DeltaSinceWindowsEpoch() or',
+ 'base::{TimeDelta::In}Microseconds(), or one of the other type',
+ 'converter methods instead. For faking TimeXXX values (for unit',
'testing only), use TimeXXX() + Microseconds(N). For',
'other use cases, please contact base/time/OWNERS.',
),
False,
- (),
+ excluded_paths = (
+ "base/time/time.h",
+ "base/allocator/partition_allocator/src/partition_alloc/partition_alloc_base/time/time.h",
+ ),
),
BanRule(
'CallJavascriptFunctionUnsafe',
're2::RE2 instead (crbug.com/755321)',
),
True,
- # Abseil's benchmarks never linked into chrome.
- ['third_party/abseil-cpp/.*_benchmark.cc'],
- ),
- BanRule(
- r'/\bstd::stoi\b',
- (
- 'std::stoi uses exceptions to communicate results. ',
- 'Use base::StringToInt() instead.',
- ),
- True,
- [_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
- ),
- BanRule(
- r'/\bstd::stol\b',
- (
- 'std::stol uses exceptions to communicate results. ',
- 'Use base::StringToInt() instead.',
- ),
- True,
- [_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
- ),
- BanRule(
- r'/\bstd::stoul\b',
- (
- 'std::stoul uses exceptions to communicate results. ',
- 'Use base::StringToUint() instead.',
- ),
- True,
- [_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
- ),
- BanRule(
- r'/\bstd::stoll\b',
- (
- 'std::stoll uses exceptions to communicate results. ',
- 'Use base::StringToInt64() instead.',
- ),
- True,
- [_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
- ),
- BanRule(
- r'/\bstd::stoull\b',
- (
- 'std::stoull uses exceptions to communicate results. ',
- 'Use base::StringToUint64() instead.',
- ),
- True,
- [_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
- ),
- BanRule(
- r'/\bstd::stof\b',
- (
- 'std::stof uses exceptions to communicate results. ',
- 'For locale-independent values, e.g. reading numbers from disk',
- 'profiles, use base::StringToDouble().',
- 'For user-visible values, parse using ICU.',
- ),
- True,
- [_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
+ [
+ # Abseil's benchmarks never linked into chrome.
+ 'third_party/abseil-cpp/.*_benchmark.cc',
+ ],
),
BanRule(
- r'/\bstd::stod\b',
+ r'/\bstd::sto(i|l|ul|ll|ull)\b',
(
- 'std::stod uses exceptions to communicate results. ',
- 'For locale-independent values, e.g. reading numbers from disk',
- 'profiles, use base::StringToDouble().',
- 'For user-visible values, parse using ICU.',
+ 'std::sto{i,l,ul,ll,ull}() use exceptions to communicate results. ',
+ 'Use base::StringTo[U]Int[64]() instead.',
),
True,
[_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
),
BanRule(
- r'/\bstd::stold\b',
+ r'/\bstd::sto(f|d|ld)\b',
(
- 'std::stold uses exceptions to communicate results. ',
+ 'std::sto{f,d,ld}() use exceptions to communicate results. ',
'For locale-independent values, e.g. reading numbers from disk',
'profiles, use base::StringToDouble().',
'For user-visible values, parse using ICU.',
BanRule(
r'/\bstd::to_string\b',
(
- 'std::to_string is locale dependent and slower than alternatives.',
+ 'std::to_string() is locale dependent and slower than alternatives.',
'For locale-independent strings, e.g. writing numbers to disk',
'profiles, use base::NumberToString().',
'For user-visible strings, use base::FormatNumber() and',
[_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
),
BanRule(
+ r'/#include <(cctype|ctype\.h|cwctype|wctype.h)>',
+ (
+ '<cctype>/<ctype.h>/<cwctype>/<wctype.h> are banned. Use',
+ '"third_party/abseil-cpp/absl/strings/ascii.h" instead.',
+ ),
+ True,
+ [_THIRD_PARTY_EXCEPT_BLINK], # Not an error in third_party folders.
+ ),
+ BanRule(
r'/\bstd::shared_ptr\b',
(
- 'std::shared_ptr should not be used. Use scoped_refptr instead.',
+ 'std::shared_ptr is banned. Use scoped_refptr instead.',
),
True,
[
'^third_party/blink/renderer/bindings/core/v8/' +
'v8_wasm_response_extensions.cc',
'^gin/array_buffer\.(cc|h)',
+ '^gin/per_isolate_data\.(cc|h)',
'^chrome/services/sharing/nearby/',
# Needed for interop with third-party library libunwindstack.
'^base/profiler/libunwindstack_unwinder_android\.(cc|h)',
+ '^base/profiler/native_unwinder_android_memory_regions_map_impl.(cc|h)',
+ # Needed for interop with third-party boringssl cert verifier
+ '^third_party/boringssl/',
+ '^net/cert/',
+ '^net/tools/cert_verify_tool/',
+ '^services/cert_verifier/',
+ '^components/certificate_transparency/',
+ '^components/media_router/common/providers/cast/certificate/',
# gRPC provides some C++ libraries that use std::shared_ptr<>.
'^chromeos/ash/services/libassistant/grpc/',
'^chromecast/cast_core/grpc',
'^chromecast/cast_core/runtime/browser',
'^ios/chrome/test/earl_grey/chrome_egtest_plugin_client\.(mm|h)',
# Fuchsia provides C++ libraries that use std::shared_ptr<>.
- '^base/fuchsia/filtered_service_directory\.(cc|h)',
- '^base/fuchsia/service_directory_test_base\.h',
+ '^base/fuchsia/.*\.(cc|h)',
'.*fuchsia.*test\.(cc|h)',
- # Needed for clang plugin tests
- '^tools/clang/plugins/tests/',
+ # Clang plugins have different build config.
+ '^tools/clang/plugins/',
_THIRD_PARTY_EXCEPT_BLINK], # Not an error in third_party folders.
),
BanRule(
r'/\bstd::weak_ptr\b',
(
- 'std::weak_ptr should not be used. Use base::WeakPtr instead.',
+ 'std::weak_ptr is banned. Use base::WeakPtr instead.',
),
True,
[_THIRD_PARTY_EXCEPT_BLINK], # Not an error in third_party folders.
BanRule(
r'/\blong long\b',
(
- 'long long is banned. Use stdint.h if you need a 64 bit number.',
+ 'long long is banned. Use [u]int64_t instead.',
),
False, # Only a warning since it is already used.
[_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
),
BanRule(
- r'\b(absl|std)::any\b',
+ r'/\b(absl|std)::any\b',
(
- 'absl::any / std::any are not safe to use in a component build.',
+ '{absl,std}::any are banned due to incompatibility with the component ',
+ 'build.',
),
True,
# Not an error in third party folders, though it probably should be :)
BanRule(
r'/\bstd::bind\b',
(
- 'std::bind is banned because of lifetime risks.',
- 'Use base::BindOnce or base::BindRepeating instead.',
+ 'std::bind() is banned because of lifetime risks. Use ',
+ 'base::Bind{Once,Repeating}() instead.',
+ ),
+ True,
+ [_THIRD_PARTY_EXCEPT_BLINK], # Not an error in third_party folders.
+ ),
+ BanRule(
+ (
+ r'/\bstd::(?:'
+ r'linear_congruential_engine|mersenne_twister_engine|'
+ r'subtract_with_carry_engine|discard_block_engine|'
+ r'independent_bits_engine|shuffle_order_engine|'
+ r'minstd_rand0?|mt19937(_64)?|ranlux(24|48)(_base)?|knuth_b|'
+ r'default_random_engine|'
+ r'random_device|'
+ r'seed_seq'
+ r')\b'
+ ),
+ (
+ 'STL random number engines and generators are banned. Use the ',
+ 'helpers in base/rand_util.h instead, e.g. base::RandBytes() or ',
+ 'base::RandomBitGenerator.'
+ '',
+ 'Please reach out to cxx@chromium.org if the base APIs are ',
+ 'insufficient for your needs.',
+ ),
+ True,
+ [
+ # Not an error in third_party folders.
+ _THIRD_PARTY_EXCEPT_BLINK,
+ # Various tools which build outside of Chrome.
+ r'testing/libfuzzer',
+ r'tools/android/io_benchmark/',
+ # Fuzzers are allowed to use standard library random number generators
+ # since fuzzing speed + reproducibility is important.
+ r'tools/ipc_fuzzer/',
+ r'.+_fuzzer\.cc$',
+ r'.+_fuzzertest\.cc$',
+ # TODO(https://crbug.com/1380528): These are all unsanctioned uses of
+ # the standard library's random number generators, and should be
+ # migrated to the //base equivalent.
+ r'ash/ambient/model/ambient_topic_queue\.cc',
+ r'base/allocator/partition_allocator/src/partition_alloc/partition_alloc_unittest\.cc',
+ r'base/ranges/algorithm_unittest\.cc',
+ r'base/test/launcher/test_launcher\.cc',
+ r'cc/metrics/video_playback_roughness_reporter_unittest\.cc',
+ r'chrome/browser/apps/app_service/metrics/website_metrics\.cc',
+ r'chrome/browser/ash/power/auto_screen_brightness/monotone_cubic_spline_unittest\.cc',
+ r'chrome/browser/ash/printing/zeroconf_printer_detector_unittest\.cc',
+ r'chrome/browser/nearby_sharing/contacts/nearby_share_contact_manager_impl_unittest\.cc',
+ r'chrome/browser/nearby_sharing/contacts/nearby_share_contacts_sorter_unittest\.cc',
+ r'chrome/browser/privacy_budget/mesa_distribution_unittest\.cc',
+ r'chrome/browser/web_applications/test/web_app_test_utils\.cc',
+ r'chrome/browser/web_applications/test/web_app_test_utils\.cc',
+ r'chrome/browser/win/conflicts/module_blocklist_cache_util_unittest\.cc',
+ r'chrome/chrome_cleaner/logging/detailed_info_sampler\.cc',
+ r'chromeos/ash/components/memory/userspace_swap/swap_storage_unittest\.cc',
+ r'chromeos/ash/components/memory/userspace_swap/userspace_swap\.cc',
+ r'components/metrics/metrics_state_manager\.cc',
+ r'components/omnibox/browser/history_quick_provider_performance_unittest\.cc',
+ r'components/zucchini/disassembler_elf_unittest\.cc',
+ r'content/browser/webid/federated_auth_request_impl\.cc',
+ r'content/browser/webid/federated_auth_request_impl\.cc',
+ r'media/cast/test/utility/udp_proxy\.h',
+ r'sql/recover_module/module_unittest\.cc',
+ r'components/search_engines/template_url_prepopulate_data.cc',
+ # Do not add new entries to this list. If you have a use case which is
+ # not satisfied by the current APIs (i.e. you need an explicitly-seeded
+ # sequence, or stability of some sort is required), please contact
+ # cxx@chromium.org.
+ ],
+ ),
+ BanRule(
+ r'/\b(absl,std)::bind_front\b',
+ (
+ '{absl,std}::bind_front() are banned. Use base::Bind{Once,Repeating}() '
+ 'instead.',
),
True,
[_THIRD_PARTY_EXCEPT_BLINK], # Not an error in third_party folders.
),
BanRule(
- r'/\bstd::optional\b',
+ r'/\bABSL_FLAG\b',
(
- 'std::optional is banned. Use absl::optional instead.',
+ 'ABSL_FLAG is banned. Use base::CommandLine instead.',
),
True,
[_THIRD_PARTY_EXCEPT_BLINK], # Not an error in third_party folders.
),
BanRule(
- r'/\b#include <chrono>\b',
+ r'/\babsl::c_',
(
- '<chrono> overlaps with Time APIs in base. Keep using',
- 'base classes.',
+ 'Abseil container utilities are banned. Use base/ranges/algorithm.h ',
+ 'instead.',
),
True,
[_THIRD_PARTY_EXCEPT_BLINK], # Not an error in third_party folders.
),
BanRule(
- r'/\b#include <exception>\b',
+ r'/\babsl::FixedArray\b',
+ (
+ 'absl::FixedArray is banned. Use base::FixedArray instead.',
+ ),
+ True,
+ [
+ # base::FixedArray provides canonical access.
+ r'^base/types/fixed_array.h',
+ # Not an error in third_party folders.
+ _THIRD_PARTY_EXCEPT_BLINK,
+ ],
+ ),
+ BanRule(
+ r'/\babsl::FunctionRef\b',
+ (
+ 'absl::FunctionRef is banned. Use base::FunctionRef instead.',
+ ),
+ True,
+ [
+ # base::Bind{Once,Repeating} references absl::FunctionRef to disallow
+ # interoperability.
+ r'^base/functional/bind_internal\.h',
+ # base::FunctionRef is implemented on top of absl::FunctionRef.
+ r'^base/functional/function_ref.*\..+',
+ # Not an error in third_party folders.
+ _THIRD_PARTY_EXCEPT_BLINK,
+ ],
+ ),
+ BanRule(
+ r'/\babsl::(Insecure)?BitGen\b',
+ (
+ 'absl random number generators are banned. Use the helpers in '
+ 'base/rand_util.h instead, e.g. base::RandBytes() or ',
+ 'base::RandomBitGenerator.'
+ ),
+ True,
+ [_THIRD_PARTY_EXCEPT_BLINK], # Not an error in third_party folders.
+ ),
+ BanRule(
+ r'/(\babsl::Span\b|#include <span>)',
+ (
+ 'absl::Span is banned and <span> is not allowed yet ',
+ '(https://crbug.com/1414652). Use base::span instead.',
+ ),
+ True,
+ [
+ # Needed to use QUICHE API.
+ r'services/network/web_transport\.cc',
+ r'chrome/browser/ip_protection/.*',
+ # Not an error in third_party folders.
+ _THIRD_PARTY_EXCEPT_BLINK
+ ],
+ ),
+ BanRule(
+ r'/\babsl::StatusOr\b',
+ (
+ 'absl::StatusOr is banned. Use base::expected instead.',
+ ),
+ True,
+ [
+ # Needed to use liburlpattern API.
+ r'third_party/blink/renderer/core/url_pattern/.*',
+ r'third_party/blink/renderer/modules/manifest/manifest_parser\.cc',
+ # Needed to use QUICHE API.
+ r'chrome/browser/ip_protection/.*',
+ # Needed to use MediaPipe API.
+ r'components/media_effects/.*\.cc',
+ # Not an error in third_party folders.
+ _THIRD_PARTY_EXCEPT_BLINK
+ ],
+ ),
+ BanRule(
+ r'/\babsl::StrFormat\b',
+ (
+ 'absl::StrFormat() is not allowed yet (https://crbug.com/1371963). ',
+ 'Use base::StringPrintf() instead.',
+ ),
+ True,
+ [_THIRD_PARTY_EXCEPT_BLINK], # Not an error in third_party folders.
+ ),
+ BanRule(
+ r'/\babsl::(StrSplit|StrJoin|StrCat|StrAppend|Substitute|StrContains)\b',
+ (
+ 'Abseil string utilities are banned. Use base/strings instead.',
+ ),
+ True,
+ [_THIRD_PARTY_EXCEPT_BLINK], # Not an error in third_party folders.
+ ),
+ BanRule(
+ r'/\babsl::(Mutex|CondVar|Notification|Barrier|BlockingCounter)\b',
+ (
+ 'Abseil synchronization primitives are banned. Use',
+ 'base/synchronization instead.',
+ ),
+ True,
+ [_THIRD_PARTY_EXCEPT_BLINK], # Not an error in third_party folders.
+ ),
+ BanRule(
+ r'/\babsl::(Duration|Time|TimeZone|CivilDay)\b',
+ (
+ 'Abseil\'s time library is banned. Use base/time instead.',
+ ),
+ True,
+ [
+ # Needed to use QUICHE API.
+ r'chrome/browser/ip_protection/.*',
+ r'services/network/web_transport.*',
+ _THIRD_PARTY_EXCEPT_BLINK # Not an error in third_party folders.
+ ],
+ ),
+ BanRule(
+ r'/#include <chrono>',
+ (
+ '<chrono> is banned. Use base/time instead.',
+ ),
+ True,
+ [
+ # Not an error in third_party folders:
+ _THIRD_PARTY_EXCEPT_BLINK,
+ # PartitionAlloc's starscan, doesn't depend on base/. It can't use
+ # base::ConditionalVariable::TimedWait(..).
+ "base/allocator/partition_allocator/src/partition_alloc/starscan/pcscan_internal.cc",
+ ]
+ ),
+ BanRule(
+ r'/#include <exception>',
(
'Exceptions are banned and disabled in Chromium.',
),
BanRule(
r'/\bstd::function\b',
(
- 'std::function is banned. Instead use base::OnceCallback or ',
- 'base::RepeatingCallback, which directly support Chromium\'s weak ',
- 'pointers, ref counting and more.',
+ 'std::function is banned. Use base::{Once,Repeating}Callback instead.',
),
True,
[
# Required for interop with the third-party webrtc library.
'third_party/blink/renderer/modules/peerconnection/mock_peer_connection_impl\.cc',
'third_party/blink/renderer/modules/peerconnection/mock_peer_connection_impl\.h',
-
+ # This code is in the process of being extracted into a third-party library.
+ # See https://crbug.com/1322914
+ '^net/cert/pki/path_builder_unittest\.cc',
# TODO(https://crbug.com/1364577): Various uses that should be
# migrated to something else.
# Should use base::OnceCallback or base::RepeatingCallback.
],
),
BanRule(
- r'/\b#include <random>\b',
+ r'/#include <X11/',
(
- 'Do not use any random number engines from <random>. Instead',
- 'use base::RandomBitGenerator.',
+ 'Do not use Xlib. Use xproto (from //ui/gfx/x:xproto) instead.',
),
True,
[_THIRD_PARTY_EXCEPT_BLINK], # Not an error in third_party folders.
),
BanRule(
- r'/\b#include <X11/',
+ r'/\bstd::ratio\b',
(
- 'Do not use Xlib. Use xproto (from //ui/gfx/x:xproto) instead.',
+ 'std::ratio is banned by the Google Style Guide.',
),
True,
[_THIRD_PARTY_EXCEPT_BLINK], # Not an error in third_party folders.
),
BanRule(
- r'/\bstd::ratio\b',
+ r'/\bstd::aligned_alloc\b',
(
- 'std::ratio is banned by the Google Style Guide.',
+ 'std::aligned_alloc() is not yet allowed (crbug.com/1412818). Use ',
+ 'base::AlignedAlloc() instead.',
),
True,
- [_THIRD_PARTY_EXCEPT_BLINK], # Not an error in third_party folders.
+ [_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
),
BanRule(
- ('base::ThreadRestrictions::ScopedAllowIO'),
+ r'/#include <(barrier|latch|semaphore|stop_token)>',
(
- 'ScopedAllowIO is deprecated, use ScopedAllowBlocking instead.',
+ 'The thread support library is banned. Use base/synchronization '
+ 'instead.',
),
- False,
- (),
+ True,
+ [_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
),
BanRule(
- r'/\bRunMessageLoop\b',
+ r'/\bstd::(c8rtomb|mbrtoc8)\b',
(
- 'RunMessageLoop is deprecated, use RunLoop instead.',
+ 'std::c8rtomb() and std::mbrtoc8() are banned.',
),
- False,
- (),
+ True,
+ [_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
+ ),
+ BanRule(
+ r'/\bchar8_t|std::u8string\b',
+ (
+ 'char8_t and std::u8string are not yet allowed. Can you use [unsigned]',
+ ' char and std::string instead?',
+ ),
+ True,
+ [
+ # The demangler does not use this type but needs to know about it.
+ 'base/third_party/symbolize/demangle\.cc',
+ # Don't warn in third_party folders.
+ _THIRD_PARTY_EXCEPT_BLINK
+ ],
+ ),
+ BanRule(
+ r'/(\b(co_await|co_return|co_yield)\b|#include <coroutine>)',
+ (
+ 'Coroutines are not yet allowed (https://crbug.com/1403840).',
+ ),
+ True,
+ [_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
+ ),
+ BanRule(
+ r'/^\s*(export\s|import\s+["<:\w]|module(;|\s+[:\w]))',
+ (
+ 'Modules are disallowed for now due to lack of toolchain support.',
+ ),
+ True,
+ [_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
+ ),
+ BanRule(
+ r'/\[\[(un)?likely\]\]',
+ (
+ '[[likely]] and [[unlikely]] are not yet allowed ',
+ '(https://crbug.com/1414620). Use [UN]LIKELY instead.',
+ ),
+ True,
+ [_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
+ ),
+ BanRule(
+ r'/#include <format>',
+ (
+ '<format> is not yet allowed. Use base::StringPrintf() instead.',
+ ),
+ True,
+ [_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
+ ),
+ BanRule(
+ r'/#include <ranges>',
+ (
+ '<ranges> is not yet allowed. Use base/ranges/algorithm.h instead.',
+ ),
+ True,
+ [_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
+ ),
+ BanRule(
+ r'/#include <source_location>',
+ (
+ '<source_location> is not yet allowed. Use base/location.h instead.',
+ ),
+ True,
+ [_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
),
BanRule(
- 'RunThisRunLoop',
+ r'/#include <syncstream>',
(
- 'RunThisRunLoop is deprecated, use RunLoop directly instead.',
+ '<syncstream> is banned.',
+ ),
+ True,
+ [_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
+ ),
+ BanRule(
+ r'/\bRunMessageLoop\b',
+ (
+ 'RunMessageLoop is deprecated, use RunLoop instead.',
),
False,
(),
"GetDeferredQuitTaskForRunLoop shouldn't be needed, please contact",
"gab@ if you found a use case where this is the only solution.",
),
- False,
- (),
+ False,
+ (),
+ ),
+ BanRule(
+ 'sqlite3_initialize(',
+ (
+ 'Instead of calling sqlite3_initialize(), depend on //sql, ',
+ '#include "sql/initialize.h" and use sql::EnsureSqliteInitialized().',
+ ),
+ True,
+ (
+ r'^sql/initialization\.(cc|h)$',
+ r'^third_party/sqlite/.*\.(c|cc|h)$',
+ ),
+ ),
+ BanRule(
+ 'CREATE VIEW',
+ (
+ 'SQL views are disabled in Chromium feature code',
+ 'https://chromium.googlesource.com/chromium/src/+/HEAD/sql#no-views',
+ ),
+ True,
+ (
+ _THIRD_PARTY_EXCEPT_BLINK,
+ # sql/ itself uses views when using memory-mapped IO.
+ r'^sql/.*',
+ # Various performance tools that do not build as part of Chrome.
+ r'^infra/.*',
+ r'^tools/perf.*',
+ r'.*perfetto.*',
+ ),
),
BanRule(
- 'sqlite3_initialize(',
+ 'CREATE VIRTUAL TABLE',
(
- 'Instead of calling sqlite3_initialize(), depend on //sql, ',
- '#include "sql/initialize.h" and use sql::EnsureSqliteInitialized().',
+ 'SQL virtual tables are disabled in Chromium feature code',
+ 'https://chromium.googlesource.com/chromium/src/+/HEAD/sql#no-virtual-tables',
),
True,
(
- r'^sql/initialization\.(cc|h)$',
- r'^third_party/sqlite/.*\.(c|cc|h)$',
+ _THIRD_PARTY_EXCEPT_BLINK,
+ # sql/ itself uses virtual tables in the recovery module and tests.
+ r'^sql/.*',
+ # TODO(https://crbug.com/695592): Remove once WebSQL is deprecated.
+ r'third_party/blink/web_tests/storage/websql/.*'
+ # Various performance tools that do not build as part of Chrome.
+ r'^tools/perf.*',
+ r'.*perfetto.*',
),
),
BanRule(
True,
(
r'^base/win/scoped_winrt_initializer\.cc$',
+ r'^third_party/abseil-cpp/absl/.*',
),
),
BanRule(
),
),
BanRule(
- r'/\babsl::FunctionRef\b',
+ r'base::Feature k',
(
- 'absl::FunctionRef is banned. Use base::FunctionRef instead.',
+ 'Please use BASE_DECLARE_FEATURE() or BASE_FEATURE() instead of ',
+ 'directly declaring/defining features.'
),
True,
[
- # base::Bind{Once,Repeating} references absl::FunctionRef to disallow
- # interoperability.
- r'^base/functional/bind_internal\.h',
- # base::FunctionRef is implemented on top of absl::FunctionRef.
- r'^base/functional/function_ref.*\..+',
- # Not an error in third_party folders.
_THIRD_PARTY_EXCEPT_BLINK,
],
),
BanRule(
- r'base::Feature k',
+ r'/\bchartorune\b',
(
- 'Please use BASE_DECLARE_FEATURE() or BASE_FEATURE() instead of ',
- 'directly declaring/defining features.'
+ 'chartorune is not memory-safe, unless you can guarantee the input ',
+ 'string is always null-terminated. Otherwise, please use charntorune ',
+ 'from libphonenumber instead.'
),
True,
[
_THIRD_PARTY_EXCEPT_BLINK,
+ # Exceptions to this rule should have a fuzzer.
],
),
+ BanRule(
+ r'/\b#include "base/atomicops\.h"\b',
+ (
+ 'Do not use base::subtle atomics, but std::atomic, which are simpler '
+ 'to use, have better understood, clearer and richer semantics, and are '
+ 'harder to mis-use. See details in base/atomicops.h.',
+ ),
+ False,
+ [_THIRD_PARTY_EXCEPT_BLINK], # Not an error in third_party folders.
+ ),
+ BanRule(
+ r'CrossThreadPersistent<',
+ (
+ 'Do not use blink::CrossThreadPersistent, but '
+ 'blink::CrossThreadHandle. It is harder to mis-use.',
+ 'More info: '
+ 'https://docs.google.com/document/d/1GIT0ysdQ84sGhIo1r9EscF_fFt93lmNVM_q4vvHj2FQ/edit#heading=h.3e4d6y61tgs',
+ 'Please contact platform-architecture-dev@ before adding new instances.'
+ ),
+ False,
+ []
+ ),
+ BanRule(
+ r'CrossThreadWeakPersistent<',
+ (
+ 'Do not use blink::CrossThreadWeakPersistent, but '
+ 'blink::CrossThreadWeakHandle. It is harder to mis-use.',
+ 'More info: '
+ 'https://docs.google.com/document/d/1GIT0ysdQ84sGhIo1r9EscF_fFt93lmNVM_q4vvHj2FQ/edit#heading=h.3e4d6y61tgs',
+ 'Please contact platform-architecture-dev@ before adding new instances.'
+ ),
+ False,
+ []
+ ),
+ BanRule(
+ r'objc/objc.h',
+ (
+ 'Do not include <objc/objc.h>. It defines away ARC lifetime '
+ 'annotations, and is thus dangerous.',
+ 'Please use the pimpl pattern; search for `ObjCStorage` for examples.',
+ 'For further reading on how to safely mix C++ and Obj-C, see',
+ 'https://chromium.googlesource.com/chromium/src/+/main/docs/mac/mixing_cpp_and_objc.md'
+ ),
+ True,
+ []
+ ),
+ BanRule(
+ r'/#include <filesystem>',
+ (
+ 'libc++ <filesystem> is banned per the Google C++ styleguide.',
+ ),
+ True,
+ # This fuzzing framework is a standalone open source project and
+ # cannot rely on Chromium base.
+ (r'third_party/centipede'),
+ ),
+ BanRule(
+ r'TopDocument()',
+ (
+ 'TopDocument() does not work correctly with out-of-process iframes. '
+ 'Please do not introduce new uses.',
+ ),
+ True,
+ (
+ # TODO(crbug.com/617677): Remove all remaining uses.
+ r'^third_party/blink/renderer/core/dom/document\.cc',
+ r'^third_party/blink/renderer/core/dom/document\.h',
+ r'^third_party/blink/renderer/core/dom/element\.cc',
+ r'^third_party/blink/renderer/core/exported/web_disallow_transition_scope_test\.cc',
+ r'^third_party/blink/renderer/core/exported/web_document_test\.cc',
+ r'^third_party/blink/renderer/core/html/html_anchor_element\.cc',
+ r'^third_party/blink/renderer/core/html/html_dialog_element\.cc',
+ r'^third_party/blink/renderer/core/html/html_element\.cc',
+ r'^third_party/blink/renderer/core/html/html_frame_owner_element\.cc',
+ r'^third_party/blink/renderer/core/html/media/video_wake_lock\.cc',
+ r'^third_party/blink/renderer/core/loader/anchor_element_interaction_tracker\.cc',
+ r'^third_party/blink/renderer/core/page/scrolling/root_scroller_controller\.cc',
+ r'^third_party/blink/renderer/core/page/scrolling/top_document_root_scroller_controller\.cc',
+ r'^third_party/blink/renderer/core/page/scrolling/top_document_root_scroller_controller\.h',
+ r'^third_party/blink/renderer/core/script/classic_pending_script\.cc',
+ r'^third_party/blink/renderer/core/script/script_loader\.cc',
+ ),
+ ),
+ BanRule(
+ pattern = r'base::raw_ptr<',
+ explanation = (
+ 'Do not use base::raw_ptr, use raw_ptr.',
+ ),
+ treat_as_error = True,
+ excluded_paths = (
+ '^base/',
+ '^tools/',
+ ),
+ ),
+ BanRule(
+ pattern = r'base:raw_ref<',
+ explanation = (
+ 'Do not use base::raw_ref, use raw_ref.',
+ ),
+ treat_as_error = True,
+ excluded_paths = (
+ '^base/',
+ '^tools/',
+ ),
+ ),
+ BanRule(
+ pattern = r'/raw_ptr<[^;}]*\w{};',
+ explanation = (
+ 'Do not use {} for raw_ptr initialization, use = nullptr instead.',
+ ),
+ treat_as_error = True,
+ excluded_paths = (
+ '^base/',
+ '^tools/',
+ ),
+ ),
+ BanRule(
+ pattern = r'/#include "base/allocator/.*/raw_'
+ r'(ptr|ptr_cast|ptr_exclusion|ref).h"',
+ explanation = (
+ 'Please include the corresponding facade headers:',
+ '- #include "base/memory/raw_ptr.h"',
+ '- #include "base/memory/raw_ptr_cast.h"',
+ '- #include "base/memory/raw_ptr_exclusion.h"',
+ '- #include "base/memory/raw_ref.h"',
+ ),
+ treat_as_error = True,
+ excluded_paths = (
+ '^base/',
+ '^tools/',
+ ),
+ ),
+ BanRule(
+ pattern = r'ContentSettingsType::COOKIES',
+ explanation = (
+ 'Do not use ContentSettingsType::COOKIES to check whether cookies are '
+ 'supported in the provided context. Instead rely on the '
+ 'content_settings::CookieSettings API. If you are using '
+ 'ContentSettingsType::COOKIES to check the user preference setting '
+ 'specifically, disregard this warning.',
+ ),
+ treat_as_error = False,
+ excluded_paths = (
+ '^chrome/browser/ui/content_settings/',
+ '^components/content_settings/',
+ '^services/network/cookie_settings.cc',
+ '.*test.cc',
+ ),
+ ),
+ BanRule(
+ pattern = r'\bg_signal_connect',
+ explanation = (
+ 'Use ScopedGSignal instead of g_signal_connect*()',
+ ),
+ treat_as_error = True,
+ excluded_paths = (
+ '^ui/base/glib/scoped_gsignal.h',
+ ),
+ ),
+ BanRule(
+ pattern = r'features::kIsolatedWebApps',
+ explanation = (
+ 'Do not use `features::kIsolatedWebApps` directly to guard Isolated ',
+ 'Web App code. ',
+ 'Use `content::IsolatedWebAppsPolicy::AreIsolatedWebAppsEnabled()` in ',
+ 'the browser process or check the `kEnableIsolatedWebAppsInRenderer` ',
+ 'command line flag in the renderer process.',
+ ),
+ treat_as_error = True,
+ excluded_paths = _TEST_CODE_EXCLUDED_PATHS + (
+ '^chrome/browser/about_flags.cc',
+ '^chrome/browser/chrome_content_browser_client.cc',
+ '^chrome/browser/ui/startup/bad_flags_prompt.cc',
+ '^content/shell/browser/shell_content_browser_client.cc'
+ )
+ ),
)
_BANNED_MOJOM_PATTERNS : Sequence[BanRule] = (
r'testing/buildbot/',
r'^components/policy/resources/policy_templates\.json$',
r'^third_party/protobuf/',
- r'^third_party/blink/perf_tests/speedometer/resources/todomvc/learn.json',
+ r'^third_party/blink/perf_tests/speedometer.*/resources/todomvc/learn\.json',
r'^third_party/blink/renderer/devtools/protocol\.json$',
r'^third_party/blink/web_tests/external/wpt/',
r'^tools/perf/',
_GENERIC_PYDEPS_FILES = [
'android_webview/test/components/run_webview_component_smoketest.pydeps',
'android_webview/tools/run_cts.pydeps',
- 'base/android/jni_generator/jni_generator.pydeps',
- 'base/android/jni_generator/jni_registration_generator.pydeps',
'build/android/apk_operations.pydeps',
'build/android/devil_chromium.pydeps',
'build/android/gyp/aar.pydeps',
'build/android/gyp/allot_native_libraries.pydeps',
'build/android/gyp/apkbuilder.pydeps',
'build/android/gyp/assert_static_initializers.pydeps',
+ 'build/android/gyp/binary_baseline_profile.pydeps',
'build/android/gyp/bytecode_processor.pydeps',
'build/android/gyp/bytecode_rewriter.pydeps',
'build/android/gyp/check_flag_expectations.pydeps',
'build/android/gyp/compile_java.pydeps',
+ 'build/android/gyp/compile_kt.pydeps',
'build/android/gyp/compile_resources.pydeps',
'build/android/gyp/copy_ex.pydeps',
'build/android/gyp/create_apk_operations_script.pydeps',
'build/android/gyp/create_test_apk_wrapper_script.pydeps',
'build/android/gyp/create_ui_locale_resources.pydeps',
'build/android/gyp/dex.pydeps',
- 'build/android/gyp/dex_jdk_libs.pydeps',
- 'build/android/gyp/dexsplitter.pydeps',
'build/android/gyp/dist_aar.pydeps',
'build/android/gyp/filter_zip.pydeps',
'build/android/gyp/flatc_java.pydeps',
'third_party/blink/renderer/bindings/scripts/check_generated_file_list.pydeps',
'third_party/blink/renderer/bindings/scripts/collect_idl_files.pydeps',
'third_party/blink/renderer/bindings/scripts/generate_bindings.pydeps',
+ 'third_party/blink/renderer/bindings/scripts/generate_event_interface_names.pydeps',
'third_party/blink/renderer/bindings/scripts/validate_web_idl.pydeps',
'third_party/blink/tools/blinkpy/web_tests/merge_results.pydeps',
'third_party/blink/tools/merge_web_test_results.pydeps',
'wpt-autoroller', 'chrome-weblayer-builder',
'lacros-version-skew-roller', 'skylab-test-cros-roller',
'infra-try-recipes-tester', 'lacros-tracking-roller',
- 'lacros-sdk-version-roller')
+ 'lacros-sdk-version-roller', 'chrome-automated-expectation',
+ 'chromium-automated-expectation', 'chrome-branch-day',
+ 'chromium-autosharder')
) | set('%s@skia-public.iam.gserviceaccount.com' % s
for s in ('chromium-autoroll', 'chromium-release-autoroll')
) | set('%s@skia-corp.google.com.iam.gserviceaccount.com' % s
for s in ('chromium-internal-autoroll',)
) | set('%s@owners-cleanup-prod.google.com.iam.gserviceaccount.com' % s
- for s in ('swarming-tasks',))
+ for s in ('swarming-tasks',)
+ ) | set('%s@fuchsia-infra.iam.gserviceaccount.com' % s
+ for s in ('global-integration-try-builder',
+ 'global-integration-ci-builder')
+ ) | set('%s@prod.google.com' % s
+ for s in ('chops-security-borg',
+ 'chops-security-cronjobs-cpesuggest'))
_INVALID_GRD_FILE_LINE = [
(r'<file lang=.* path=.*', 'Path should come before lang in GRD files.')
return ext in ('.grd', '.xml')
+def _IsMojomFile(input_api, file_path):
+ return input_api.os_path.splitext(file_path)[1] == ".mojom"
+
+
def CheckNoUpstreamDepsOnClank(input_api, output_api):
"""Prevent additions of dependencies from the upstream repo on //clank."""
# clank can depend on clank
inclusion_re = input_api.re.compile(r'(%s)\s*\(' % name_pattern)
# Ignore definitions. (Comments are ignored separately.)
exclusion_re = input_api.re.compile(r'(%s)[^;]+\{' % name_pattern)
+ allowlist_re = input_api.re.compile(r'// IN-TEST$')
problems = []
sources = lambda x: input_api.FilterSourceFile(
continue
if (inclusion_re.search(line) and not comment_re.search(line)
and not annotation_re.search(line)
+ and not allowlist_re.search(line)
and not exclusion_re.search(line)):
problems.append('%s:%d\n %s' %
(local_path, line_number, line.strip()))
return []
+def _CheckNoUNIT_TESTInSourceFiles(input_api, f):
+ problems = []
+
+ unit_test_macro = input_api.re.compile(
+ '^\s*#.*(?:ifn?def\s+UNIT_TEST|defined\s*\(?\s*UNIT_TEST\s*\)?)(?:$|\s+)')
+ for line_num, line in f.ChangedContents():
+ if unit_test_macro.match(line):
+ problems.append(' %s:%d' % (f.LocalPath(), line_num))
+
+ return problems
+
+
def CheckNoUNIT_TESTInSourceFiles(input_api, output_api):
"""Checks to make sure no source files use UNIT_TEST."""
problems = []
for f in input_api.AffectedFiles():
if (not f.LocalPath().endswith(('.cc', '.mm'))):
continue
-
- for line_num, line in f.ChangedContents():
- if 'UNIT_TEST ' in line or line.endswith('UNIT_TEST'):
- problems.append(' %s:%d' % (f.LocalPath(), line_num))
+ problems.extend(
+ _CheckNoUNIT_TESTInSourceFiles(input_api, f))
if not problems:
return []
return []
+def CheckCrosApiNeedBrowserTest(input_api, output_api):
+ """Check new crosapi should add browser test."""
+ has_new_crosapi = False
+ has_browser_test = False
+ for f in input_api.AffectedFiles():
+ path = f.LocalPath()
+ if (path.startswith('chromeos/crosapi/mojom') and
+ _IsMojomFile(input_api, path) and f.Action() == 'A'):
+ has_new_crosapi = True
+ if path.endswith('browsertest.cc') or path.endswith('browser_test.cc'):
+ has_browser_test = True
+ if has_new_crosapi and not has_browser_test:
+ return [
+ output_api.PresubmitPromptWarning(
+ 'You are adding a new crosapi, but there is no file ends with '
+ 'browsertest.cc file being added or modified. It is important '
+ 'to add crosapi browser test coverage to avoid version '
+ ' skew issues.\n'
+ 'Check //docs/lacros/test_instructions.md for more information.'
+ )
+ ]
+ return []
+
+
def CheckValidHostsInDEPSOnUpload(input_api, output_api):
"""Checks that DEPS file deps are from allowed_hosts."""
# Run only if DEPS file has been modified to annoy fewer bystanders.
for ban_rule in _BANNED_JAVA_FUNCTIONS:
CheckForMatch(f, line_num, line, ban_rule)
+ file_filter = lambda f: f.LocalPath().endswith(('.js', '.ts'))
+ for f in input_api.AffectedFiles(file_filter=file_filter):
+ for line_num, line in f.ChangedContents():
+ for ban_rule in _BANNED_JAVASCRIPT_FUNCTIONS:
+ CheckForMatch(f, line_num, line, ban_rule)
+
file_filter = lambda f: f.LocalPath().endswith(('.mm', '.m', '.h'))
for f in input_api.AffectedFiles(file_filter=file_filter):
for line_num, line in f.ChangedContents():
return results
-# TODO: add unit tests.
def CheckNoAbbreviationInPngFileName(input_api, output_api):
"""Makes sure there are no abbreviations in the name of PNG files.
The native_client_sdk directory is excluded because it has auto-generated PNG
files for documentation.
"""
errors = []
- files_to_check = [r'.*_[a-z]_.*\.png$|.*_[a-z]\.png$']
+ files_to_check = [r'.*\.png$']
files_to_skip = [r'^native_client_sdk/',
r'^services/test/',
r'^third_party/blink/web_tests/',
]
file_filter = lambda f: input_api.FilterSourceFile(
f, files_to_check=files_to_check, files_to_skip=files_to_skip)
+ abbreviation = input_api.re.compile('.+_[a-z]\.png|.+_[a-z]_.*\.png')
for f in input_api.AffectedFiles(include_deletes=False,
file_filter=file_filter):
- errors.append(' %s' % f.LocalPath())
+ file_name = input_api.os_path.split(f.LocalPath())[1]
+ if abbreviation.search(file_name):
+ errors.append(' %s' % f.LocalPath())
results = []
if errors:
files_to_skip = (
_EXCLUDED_PATHS + _TEST_CODE_EXCLUDED_PATHS +
input_api.DEFAULT_FILES_TO_SKIP + (
+ r"^base/fuchsia/scoped_fx_logger\.cc$",
r"^base/logging\.h$",
r"^base/logging\.cc$",
r"^base/task/thread_pool/task_tracker\.cc$",
r"^chrome/app/chrome_main_delegate\.cc$",
+ r"^chrome/browser/ash/arc/enterprise/cert_store/arc_cert_installer\.cc$",
+ r"^chrome/browser/ash/policy/remote_commands/user_command_arc_job\.cc$",
r"^chrome/browser/chrome_browser_main\.cc$",
r"^chrome/browser/ui/startup/startup_browser_creator\.cc$",
r"^chrome/browser/browser_switcher/bho/.*",
r"^chrome/chrome_elf/dll_hash/dll_hash_main\.cc$",
r"^chrome/installer/setup/.*",
r"^chromecast/",
- r"^components/browser_watcher/dump_stability_report_main_win\.cc$",
+ r"^components/cast",
r"^components/media_control/renderer/media_playback_options\.cc$",
+ r"^components/policy/core/common/policy_logger\.cc$",
r"^components/viz/service/display/"
r"overlay_strategy_underlay_cast\.cc$",
r"^components/zucchini/.*",
r"^extensions/renderer/logging_native_handler\.cc$",
r"^fuchsia_web/common/init_logging\.cc$",
r"^fuchsia_web/runners/common/web_component\.cc$",
- r"^fuchsia_web/shell/.*_shell\.cc$",
+ r"^fuchsia_web/shell/.*\.cc$",
r"^headless/app/headless_shell\.cc$",
r"^ipc/ipc_logging\.cc$",
r"^native_client_sdk/",
# Loads contents in tools/metrics/actions/actions.xml to memory. It's
# loaded only once.
if not current_actions:
- with open(
- 'tools/metrics/actions/actions.xml') as actions_f:
+ with open('tools/metrics/actions/actions.xml',
+ encoding='utf-8') as actions_f:
current_actions = actions_f.read()
# Search for the matched user action name in |current_actions|.
for action_name in match.groups():
# Restore sys.path to what it was before.
sys.path = original_sys_path
- return checkstyle.RunCheckstyle(
+ return checkstyle.run_presubmit(
input_api,
output_api,
- 'tools/android/checkstyle/chromium-style-5.0.xml',
files_to_skip=_EXCLUDED_PATHS + input_api.DEFAULT_FILES_TO_SKIP)
allowed_owners_files_file = 'build/OWNERS.setnoparent'
allowed_owners_files = set()
- with open(allowed_owners_files_file, 'r') as f:
+ with open(allowed_owners_files_file, 'r', encoding='utf-8') as f:
for line in f:
line = line.strip()
if not line or line.startswith('#'):
results.append(
output_api.PresubmitError(
'Annotations in android.test.suitebuilder.annotation have been'
- ' deprecated since API level 24. Please use android.support.test.filters'
- ' from //third_party/android_support_test_runner:runner_java instead.'
+ ' deprecated since API level 24. Please use androidx.test.filters'
+ ' from //third_party/androidx:androidx_test_runner_java instead.'
' Contact yolandyan@chromium.org if you have any questions.',
errors))
return results
def GetError(self):
"""Returns an error message, or None."""
import difflib
+ new_contents = self._process.stdout.read().splitlines()[2:]
if self._process.wait() != 0:
# STDERR should already be printed.
return 'Command failed: ' + self._cmd
- new_contents = self._process.stdout.read().splitlines()[2:]
if self._old_contents != new_contents:
diff = '\n'.join(
difflib.context_diff(self._old_contents, new_contents))
return []
+def CheckGnRebasePath(input_api, output_api):
+ """Checks that target_gen_dir is not used wtih "//" in rebase_path().
+
+ Developers should use root_build_dir instead of "//" when using target_gen_dir because
+ Chromium is sometimes built outside of the source tree.
+ """
+
+ def gn_files(f):
+ return input_api.FilterSourceFile(f, files_to_check=(r'.+\.gn', ))
+
+ rebase_path_regex = input_api.re.compile(r'rebase_path\(("\$target_gen_dir"|target_gen_dir), ("/"|"//")\)')
+ problems = []
+ for f in input_api.AffectedSourceFiles(gn_files):
+ for line_num, line in f.ChangedContents():
+ if rebase_path_regex.search(line):
+ problems.append(
+ 'Absolute path in rebase_path() in %s:%d' %
+ (f.LocalPath(), line_num))
+
+ if problems:
+ return [
+ output_api.PresubmitPromptWarning(
+ 'Using an absolute path in rebase_path()',
+ items=sorted(problems),
+ long_text=(
+ 'rebase_path() should use root_build_dir instead of "/" ',
+ 'since builds can be initiated from outside of the source ',
+ 'root.'))
+ ]
+ return []
def CheckGnGlobForward(input_api, output_api):
"""Checks that forward_variables_from(invoker, "*") follows best practices.
]
return []
-
def CheckNewHeaderWithoutGnChangeOnUpload(input_api, output_api):
"""Checks that newly added header files have corresponding GN changes.
Note that this is only a heuristic. To be precise, run script:
"correct_name": "Chrome",
"incorrect_name": "Chromium",
}, {
+ "filename_postfix": "google_chrome_strings.grd",
+ "correct_name": "Chrome",
+ "incorrect_name": "Chrome for Testing",
+ }, {
"filename_postfix": "chromium_strings.grd",
"correct_name": "Chromium",
"incorrect_name": "Chrome",
if "<message" in line or "<!--" in line or "-->" in line:
continue
if test_case["incorrect_name"] in line:
+ # Chrome for Testing is a special edge case: https://goo.gle/chrome-for-testing#bookmark=id.n1rat320av91
+ if (test_case["correct_name"] == "Chromium" and line.count("Chrome") == line.count("Chrome for Testing")):
+ continue
problems.append("Incorrect product name in %s:%d" %
(f.LocalPath(), line_num))
# to set the limit too low, but the upper limit for "normal" large
# files seems to be 1-2 MB, with a handful around 5-8 MB, so
# anything over 20 MB is exceptional.
- TOO_LARGE_FILE_SIZE_LIMIT = 20 * 1024 * 1024 # 10 MB
+ TOO_LARGE_FILE_SIZE_LIMIT = 20 * 1024 * 1024
too_large_files = []
for f in input_api.AffectedFiles():
)
_ACCESSIBILITY_TREE_TEST_PATH = (
- r"^content/test/data/accessibility/accname/.*\.html",
- r"^content/test/data/accessibility/aria/.*\.html",
- r"^content/test/data/accessibility/css/.*\.html",
- r"^content/test/data/accessibility/html/.*\.html",
+ r"^content/test/data/accessibility/accname/"
+ ".*-expected-(mac|win|uia-win|auralinux).txt",
+ r"^content/test/data/accessibility/aria/"
+ ".*-expected-(mac|win|uia-win|auralinux).txt",
+ r"^content/test/data/accessibility/css/"
+ ".*-expected-(mac|win|uia-win|auralinux).txt",
+ r"^content/test/data/accessibility/event/"
+ ".*-expected-(mac|win|uia-win|auralinux).txt",
+ r"^content/test/data/accessibility/html/"
+ ".*-expected-(mac|win|uia-win|auralinux).txt",
)
_ACCESSIBILITY_ANDROID_EVENTS_TEST_PATH = (
message = []
for f in input_api.AffectedFiles(include_deletes=True,
file_filter=FilePathFilter):
- if f.Action() == 'A' or f.Action() == 'D':
+ if f.Action() == 'A':
message = (
- "It appears that you are adding, renaming or deleting"
- "\na dump_accessibility_events* test, but have not included"
+ "It appears that you are adding platform expectations for a"
+ "\ndump_accessibility_events* test, but have not included"
"\na corresponding change for Android."
- "\nPlease include (or remove) the test from:"
+ "\nPlease include the test from:"
"\n content/public/android/javatests/src/org/chromium/"
"content/browser/accessibility/"
"WebContentsAccessibilityEventsTest.java"
message = []
for f in input_api.AffectedFiles(include_deletes=True,
file_filter=FilePathFilter):
- if f.Action() == 'A' or f.Action() == 'D':
+ if f.Action() == 'A':
message = (
- "It appears that you are adding, renaming or deleting"
- "\na dump_accessibility_tree* test, but have not included"
+ "It appears that you are adding platform expectations for a"
+ "\ndump_accessibility_tree* test, but have not included"
"\na corresponding change for Android."
"\nPlease include (or remove) the test from:"
"\n content/public/android/javatests/src/org/chromium/"
return [output_api.PresubmitPromptWarning(message)]
+def CheckEsLintConfigChanges(input_api, output_api):
+ """Suggest using "git cl presubmit --files" when .eslintrc.js files are
+ modified. This is important because enabling an error in .eslintrc.js can
+ trigger errors in any .js or .ts files in its directory, leading to hidden
+ presubmit errors."""
+ results = []
+ eslint_filter = lambda f: input_api.FilterSourceFile(
+ f, files_to_check=[r'.*\.eslintrc\.js$'])
+ for f in input_api.AffectedFiles(include_deletes=False,
+ file_filter=eslint_filter):
+ local_dir = input_api.os_path.dirname(f.LocalPath())
+ # Use / characters so that the commands printed work on any OS.
+ local_dir = local_dir.replace(input_api.os_path.sep, '/')
+ if local_dir:
+ local_dir += '/'
+ results.append(
+ output_api.PresubmitNotifyResult(
+ '%(file)s modified. Consider running \'git cl presubmit --files '
+ '"%(dir)s*.js;%(dir)s*.ts"\' in order to check and fix the affected '
+ 'files before landing this change.' %
+ { 'file' : f.LocalPath(), 'dir' : local_dir}))
+ return results
+
+
# string pattern, sequence of strings to show when pattern matches,
# error flag. True if match is a presubmit error, otherwise it's a warning.
_NON_INCLUSIVE_TERMS = (
if not input_api.os_path.exists(test_file):
continue
- use_python3 = False
- with open(f.LocalPath()) as fp:
- use_python3 = any(
- line.startswith('USE_PYTHON3 = True')
- for line in fp.readlines())
-
results.extend(
input_api.canned_checks.RunUnitTestsInDirectory(
input_api,
output_api,
full_path,
- files_to_check=[r'^PRESUBMIT_test\.py$'],
- run_on_python2=not use_python3,
- run_on_python3=use_python3,
- skip_shebang_check=True))
+ files_to_check=[r'^PRESUBMIT_test\.py$']))
return results
# - If the CL contains removed messages in grd files but the corresponding
# .sha1 files aren't removed, warn the developer to remove them.
unnecessary_screenshots = []
+ invalid_sha1 = []
missing_sha1 = []
+ missing_sha1_modified = []
unnecessary_sha1_files = []
# This checks verifies that the ICU syntax of messages this CL touched is
# break message extraction for translation, hence would block Chromium
# translations until they are fixed.
icu_syntax_errors = []
+ sha1_pattern = input_api.re.compile(r'^[a-fA-F0-9]{40}$',
+ input_api.re.MULTILINE)
def _CheckScreenshotAdded(screenshots_dir, message_id):
sha1_path = input_api.os_path.join(screenshots_dir,
message_id + '.png.sha1')
if sha1_path not in new_or_added_paths:
missing_sha1.append(sha1_path)
+ elif not _CheckValidSha1(sha1_path):
+ invalid_sha1.append(sha1_path)
+
+ def _CheckScreenshotModified(screenshots_dir, message_id):
+ sha1_path = input_api.os_path.join(screenshots_dir,
+ message_id + '.png.sha1')
+ if sha1_path not in new_or_added_paths:
+ missing_sha1_modified.append(sha1_path)
+ elif not _CheckValidSha1(sha1_path):
+ invalid_sha1.append(sha1_path)
+
+ def _CheckValidSha1(sha1_path):
+ return sha1_pattern.search(
+ next("\n".join(f.NewContents()) for f in input_api.AffectedFiles()
+ if f.LocalPath() == sha1_path))
def _CheckScreenshotRemoved(screenshots_dir, message_id):
sha1_path = input_api.os_path.join(screenshots_dir,
modified_ids.add(key)
elif old_id_to_msg_map[key].attrs['meaning'] != \
new_id_to_msg_map[key].attrs['meaning']:
- # The message meaning changed. Ensure there is a screenshot for it.
- sha1_path = input_api.os_path.join(screenshots_dir,
- key + '.png.sha1')
- if sha1_path not in new_or_added_paths and not \
- input_api.os_path.exists(sha1_path):
- # There is neither a previous screenshot nor is a new one added now.
- # Require a screenshot.
- modified_ids.add(key)
+ # The message meaning changed. We later check for a screenshot.
+ modified_ids.add(key)
if run_screenshot_check:
# Check the screenshot directory for .png files. Warn if there is any.
_CheckScreenshotAdded(screenshots_dir, added_id)
for modified_id in modified_ids:
- _CheckScreenshotAdded(screenshots_dir, modified_id)
+ _CheckScreenshotModified(screenshots_dir, modified_id)
for removed_id in removed_ids:
_CheckScreenshotRemoved(screenshots_dir, removed_id)
if missing_sha1:
results.append(
output_api.PresubmitError(
- 'You are adding or modifying UI strings.\n'
+ 'You are adding UI strings.\n'
'To ensure the best translations, take screenshots of the relevant UI '
'(https://g.co/chrome/translation) and add these files to your '
'changelist:', sorted(missing_sha1)))
+ if invalid_sha1:
+ results.append(
+ output_api.PresubmitError(
+ 'The following files do not seem to contain valid sha1 hashes. '
+ 'Make sure they contain hashes created by '
+ 'tools/translate/upload_screenshots.py:', sorted(invalid_sha1)))
+
+ if missing_sha1_modified:
+ results.append(
+ output_api.PresubmitError(
+ 'You are modifying UI strings or their meanings.\n'
+ 'To ensure the best translations, take screenshots of the relevant UI '
+ '(https://g.co/chrome/translation) and add these files to your '
+ 'changelist:', sorted(missing_sha1_modified)))
+
if unnecessary_sha1_files:
results.append(
output_api.PresubmitError(
return errors
-def CheckMPArchApiUsage(input_api, output_api):
- """CC the MPArch watchlist if the CL uses an API that is ambiguous in the
- presence of MPArch features such as bfcache, prerendering, and fenced frames.
- """
-
- # Only consider top-level directories that (1) can use content APIs or
- # problematic blink APIs, (2) apply to desktop or android chrome, and (3)
- # are known to have a significant number of uses of the APIs of concern.
- files_to_check = (
- r'^(chrome|components|content|extensions|third_party/blink/renderer)/.+%s' %
- _IMPLEMENTATION_EXTENSIONS,
- r'^(chrome|components|content|extensions|third_party/blink/renderer)/.+%s' %
- _HEADER_EXTENSIONS,
- )
- files_to_skip = (_EXCLUDED_PATHS + _TEST_CODE_EXCLUDED_PATHS +
- input_api.DEFAULT_FILES_TO_SKIP)
- source_file_filter = lambda f: input_api.FilterSourceFile(
- f, files_to_check=files_to_check, files_to_skip=files_to_skip)
-
- # Here we list the classes/methods we're monitoring. For the "fyi" cases,
- # we add the CL to the watchlist, but we don't omit a warning or have it be
- # included in the triage rotation.
- # Note that since these are are just regular expressions and we don't have
- # the compiler's AST, we could have spurious matches (e.g. an unrelated class
- # could have a method named IsInMainFrame).
- fyi_concerning_class_pattern = input_api.re.compile(
- r'WebContentsObserver|WebContentsUserData')
- # A subset of WebContentsObserver overrides where there's particular risk for
- # confusing tab and page level operations and data (e.g. incorrectly
- # resetting page state in DidFinishNavigation).
- fyi_concerning_wco_methods = [
- 'DidStartNavigation',
- 'ReadyToCommitNavigation',
- 'DidFinishNavigation',
- 'RenderViewReady',
- 'RenderViewDeleted',
- 'RenderViewHostChanged',
- 'DOMContentLoaded',
- 'DidFinishLoad',
- ]
- concerning_nav_handle_methods = [
- 'IsInMainFrame',
- ]
- concerning_web_contents_methods = [
- 'FromRenderFrameHost',
- 'FromRenderViewHost',
- ]
- fyi_concerning_web_contents_methods = [
- 'GetRenderViewHost',
- ]
- concerning_rfh_methods = [
- 'GetParent',
- 'GetMainFrame',
- ]
- fyi_concerning_rfh_methods = [
- 'GetFrameTreeNodeId',
- ]
- concerning_rfhi_methods = [
- 'is_main_frame',
- ]
- concerning_ftn_methods = [
- 'IsMainFrame',
- ]
- concerning_blink_frame_methods = [
- 'IsCrossOriginToNearestMainFrame',
- ]
- concerning_method_pattern = input_api.re.compile(r'(' + r'|'.join(
- item for sublist in [
- concerning_nav_handle_methods,
- concerning_web_contents_methods, concerning_rfh_methods,
- concerning_rfhi_methods, concerning_ftn_methods,
- concerning_blink_frame_methods,
- ] for item in sublist) + r')\(')
- fyi_concerning_method_pattern = input_api.re.compile(r'(' + r'|'.join(
- item for sublist in [
- fyi_concerning_wco_methods, fyi_concerning_web_contents_methods,
- fyi_concerning_rfh_methods,
- ] for item in sublist) + r')\(')
-
- used_apis = set()
- used_fyi_methods = False
- for f in input_api.AffectedFiles(include_deletes=False,
- file_filter=source_file_filter):
- for line_num, line in f.ChangedContents():
- fyi_class_match = fyi_concerning_class_pattern.search(line)
- if fyi_class_match:
- used_fyi_methods = True
- fyi_method_match = fyi_concerning_method_pattern.search(line)
- if fyi_method_match:
- used_fyi_methods = True
- method_match = concerning_method_pattern.search(line)
- if method_match:
- used_apis.add(method_match[1])
-
- if not used_apis:
- if used_fyi_methods:
- output_api.AppendCC('mparch-reviews+watchfyi@chromium.org')
-
- return []
-
- output_api.AppendCC('mparch-reviews+watch@chromium.org')
- message = ('This change uses API(s) that are ambiguous in the presence of '
- 'MPArch features such as bfcache, prerendering, and fenced '
- 'frames.')
- explanation = (
- 'Please double check whether new code assumes that a WebContents only '
- 'contains a single page at a time. Notably, checking whether a frame '
- 'is the \"main frame\" is not specific enough to determine whether it '
- 'corresponds to the document reflected in the omnibox. A WebContents '
- 'may have additional main frames for prerendered pages, bfcached '
- 'pages, fenced frames, etc. '
- 'See this doc [1] and the comments on the individual APIs '
- 'for guidance and this doc [2] for context. The MPArch review '
- 'watchlist has been CC\'d on this change to help identify any issues.\n'
- '[1] https://docs.google.com/document/d/13l16rWTal3o5wce4i0RwdpMP5ESELLKr439Faj2BBRo/edit?usp=sharing\n'
- '[2] https://docs.google.com/document/d/1NginQ8k0w3znuwTiJ5qjYmBKgZDekvEPC22q0I4swxQ/edit?usp=sharing'
- )
- return [
- output_api.PresubmitNotifyResult(message,
- items=list(used_apis),
- long_text=explanation)
- ]
-
-
def CheckAssertAshOnlyCode(input_api, output_api):
"""Errors if a BUILD.gn file in an ash/ directory doesn't include
assert(is_chromeos_ash).
return errors
-def _IsRendererOnlyCppFile(input_api, affected_file):
+def _IsMiraclePtrDisallowed(input_api, affected_file):
path = affected_file.LocalPath()
if not _IsCPlusPlusFile(input_api, path):
return False
- # Any code under a "renderer" subdirectory is assumed to be Renderer-only.
- if "/renderer/" in path:
+ # Renderer code is generally allowed to use MiraclePtr.
+ # These directories, however, are specifically disallowed.
+ if ("third_party/blink/renderer/core/" in path
+ or "third_party/blink/renderer/platform/heap/" in path
+ or "third_party/blink/renderer/platform/wtf/" in path):
return True
# Blink's public/web API is only used/included by Renderer-only code. Note
# The regex below matches "raw_ptr<" following a word boundary, but not in a
# C++ comment.
raw_ptr_matcher = input_api.re.compile(r'^((?!//).)*\braw_ptr<')
- file_filter = lambda f: _IsRendererOnlyCppFile(input_api, f)
+ file_filter = lambda f: _IsMiraclePtrDisallowed(input_api, f)
for f, line_num, line in input_api.RightHandSideLines(file_filter):
if raw_ptr_matcher.search(line):
errors.append(
output_api.PresubmitError(
'Problem on {path}:{line} - '\
- 'raw_ptr<T> should not be used in Renderer-only code '\
+ 'raw_ptr<T> should not be used in this renderer code '\
'(as documented in the "Pointers to unprotected memory" '\
'section in //base/memory/raw_ptr.md)'.format(
path=f.LocalPath(), line=line_num)))
return errors
+def CheckAdvancedMemorySafetyChecksUsage(input_api, output_api):
+ """Checks that ADVANCED_MEMORY_SAFETY_CHECKS() macro is neither added nor
+ removed as it is managed by the memory safety team internally.
+ Do not add / remove it manually."""
+ paths = set([])
+ # The regex below matches "ADVANCED_MEMORY_SAFETY_CHECKS(" following a word
+ # boundary, but not in a C++ comment.
+ macro_matcher = input_api.re.compile(
+ r'^((?!//).)*\bADVANCED_MEMORY_SAFETY_CHECKS\(', input_api.re.MULTILINE)
+ for f in input_api.AffectedFiles():
+ if not _IsCPlusPlusFile(input_api, f.LocalPath()):
+ continue
+ if macro_matcher.search(f.GenerateScmDiff()):
+ paths.add(f.LocalPath())
+ if not paths:
+ return []
+ return [output_api.PresubmitPromptWarning(
+ 'ADVANCED_MEMORY_SAFETY_CHECKS() macro is managed by ' \
+ 'the memory safety team (chrome-memory-safety@). ' \
+ 'Please contact us to add/delete the uses of the macro.',
+ paths)]
def CheckPythonShebang(input_api, output_api):
"""Checks that python scripts use #!/usr/bin/env instead of hardcoding a
robolectric_test = input_api.re.compile(r'[rR]obolectric')
test_class_declaration = input_api.re.compile(r'^\s*public\sclass.*Test')
uiautomator_test = input_api.re.compile(r'[uU]i[aA]utomator')
+ test_annotation_declaration = input_api.re.compile(r'^\s*public\s@interface\s.*{')
missing_annotation_errors = []
extra_annotation_errors = []
batch_matched = None
do_not_batch_matched = None
is_instrumentation_test = True
+ test_annotation_declaration_matched = None
for line in f.NewContents():
if robolectric_test.search(line) or uiautomator_test.search(line):
# Skip Robolectric and UiAutomator tests.
do_not_batch_matched = do_not_batch_annotation.search(line)
test_class_declaration_matched = test_class_declaration.search(
line)
- if test_class_declaration_matched:
+ test_annotation_declaration_matched = test_annotation_declaration.search(line)
+ if test_class_declaration_matched or test_annotation_declaration_matched:
break
+ if test_annotation_declaration_matched:
+ continue
if (is_instrumentation_test and
not batch_matched and
not do_not_batch_matched):
results.append(
output_api.PresubmitPromptWarning(
"""
-Instrumentation tests should use either @Batch or @DoNotBatch. If tests are not
-safe to run in batch, please use @DoNotBatch with reasons.
+A change was made to an on-device test that has neither been annotated with
+@Batch nor @DoNotBatch. If this is a new test, please add the annotation. If
+this is an existing test, please consider adding it if you are sufficiently
+familiar with the test (but do so as a separate change).
+
+See https://source.chromium.org/chromium/chromium/src/+/main:docs/testing/batching_instrumentation_tests.md
""", missing_annotation_errors))
if extra_annotation_errors:
results.append(
continue
if mock_annotation.search(line):
- next_line_is_annotated = True
+ field_type_search = field_type.search(line)
+ if field_type_search:
+ fully_qualified_class = _DoClassLookup(
+ field_type_search.group(1), fully_qualified_class_map,
+ package)
+ mocked_by_annotation_classes.add(fully_qualified_class)
+ else:
+ next_line_is_annotated = True
continue
m = mock_function_regex.search(line)
""", error_locations))
return results
+
+def CheckNoJsInIos(input_api, output_api):
+ """Checks to make sure that JavaScript files are not used on iOS."""
+
+ def _FilterFile(affected_file):
+ return input_api.FilterSourceFile(
+ affected_file,
+ files_to_skip=input_api.DEFAULT_FILES_TO_SKIP +
+ (r'^ios/third_party/*', r'^ios/tools/*', r'^third_party/*',
+ r'^components/autofill/ios/form_util/resources/*'),
+ files_to_check=[r'^ios/.*\.js$', r'.*/ios/.*\.js$'])
+
+ deleted_files = []
+
+ # Collect filenames of all removed JS files.
+ for f in input_api.AffectedSourceFiles(_FilterFile):
+ local_path = f.LocalPath()
+
+ if input_api.os_path.splitext(local_path)[1] == '.js' and f.Action() == 'D':
+ deleted_files.append(input_api.os_path.basename(local_path))
+
+ error_paths = []
+ moved_paths = []
+ warning_paths = []
+
+ for f in input_api.AffectedSourceFiles(_FilterFile):
+ local_path = f.LocalPath()
+
+ if input_api.os_path.splitext(local_path)[1] == '.js':
+ if f.Action() == 'A':
+ if input_api.os_path.basename(local_path) in deleted_files:
+ # This script was probably moved rather than newly created.
+ # Present a warning instead of an error for these cases.
+ moved_paths.append(local_path)
+ else:
+ error_paths.append(local_path)
+ elif f.Action() != 'D':
+ warning_paths.append(local_path)
+
+ results = []
+
+ if warning_paths:
+ results.append(output_api.PresubmitPromptWarning(
+ 'TypeScript is now fully supported for iOS feature scripts. '
+ 'Consider converting JavaScript files to TypeScript. See '
+ '//ios/web/public/js_messaging/README.md for more details.',
+ warning_paths))
+
+ if moved_paths:
+ results.append(output_api.PresubmitPromptWarning(
+ 'Do not use JavaScript on iOS for new files as TypeScript is '
+ 'fully supported. (If this is a moved file, you may leave the '
+ 'script unconverted.) See //ios/web/public/js_messaging/README.md '
+ 'for help using scripts on iOS.', moved_paths))
+
+ if error_paths:
+ results.append(output_api.PresubmitError(
+ 'Do not use JavaScript on iOS as TypeScript is fully supported. '
+ 'See //ios/web/public/js_messaging/README.md for help using '
+ 'scripts on iOS.', error_paths))
+
+ return results
+
+def CheckLibcxxRevisionsMatch(input_api, output_api):
+ """Check to make sure the libc++ version matches across deps files."""
+ # Disable check for changes to sub-repositories.
+ if input_api.PresubmitLocalPath() != input_api.change.RepositoryRoot():
+ return []
+
+ DEPS_FILES = [ 'DEPS', 'buildtools/deps_revisions.gni' ]
+
+ file_filter = lambda f: f.LocalPath().replace(
+ input_api.os_path.sep, '/') in DEPS_FILES
+ changed_deps_files = input_api.AffectedFiles(file_filter=file_filter)
+ if not changed_deps_files:
+ return []
+
+ def LibcxxRevision(file):
+ file = input_api.os_path.join(input_api.PresubmitLocalPath(),
+ *file.split('/'))
+ return input_api.re.search(
+ r'libcxx_revision.*[:=].*[\'"](\w+)[\'"]',
+ input_api.ReadFile(file)).group(1)
+
+ if len(set([LibcxxRevision(f) for f in DEPS_FILES])) == 1:
+ return []
+
+ return [output_api.PresubmitError(
+ 'libcxx_revision not equal across %s' % ', '.join(DEPS_FILES),
+ changed_deps_files)]
+
+
+def CheckDanglingUntriaged(input_api, output_api):
+ """Warn developers adding DanglingUntriaged raw_ptr."""
+
+ # Ignore during git presubmit --all.
+ #
+ # This would be too costly, because this would check every lines of every
+ # C++ files. Check from _BANNED_CPP_FUNCTIONS are also reading the whole
+ # source code, but only once to apply every checks. It seems the bots like
+ # `win-presubmit` are particularly sensitive to reading the files. Adding
+ # this check caused the bot to run 2x longer. See https://crbug.com/1486612.
+ if input_api.no_diffs:
+ return []
+
+ def FilterFile(file):
+ return input_api.FilterSourceFile(
+ file,
+ files_to_check=[r".*\.(h|cc|cpp|cxx|m|mm)$"],
+ files_to_skip=[r"^base/allocator.*"],
+ )
+
+ count = 0
+ for f in input_api.AffectedSourceFiles(FilterFile):
+ count -= f.OldContents().count("DanglingUntriaged")
+ count += f.NewContents().count("DanglingUntriaged")
+
+ # Most likely, nothing changed:
+ if count == 0:
+ return []
+
+ # Congrats developers for improving it:
+ if count < 0:
+ message = (
+ f"DanglingUntriaged pointers removed: {-count}",
+ f"Thank you!",
+ )
+ return [output_api.PresubmitNotifyResult(message)]
+
+ # Check for 'DanglingUntriaged-notes' in the description:
+ notes_regex = input_api.re.compile("DanglingUntriaged-notes[:=]")
+ if any(
+ notes_regex.match(line)
+ for line in input_api.change.DescriptionText().splitlines()):
+ return []
+
+ # Check for DanglingUntriaged-notes in the git footer:
+ if input_api.change.GitFootersFromDescription().get(
+ "DanglingUntriaged-notes", []):
+ return []
+
+ message = (
+ "Unexpected new occurrences of `DanglingUntriaged` detected. Please",
+ "avoid adding new ones",
+ "",
+ "See documentation:",
+ "https://chromium.googlesource.com/chromium/src/+/main/docs/dangling_ptr.md",
+ "",
+ "See also the guide to fix dangling pointers:",
+ "https://chromium.googlesource.com/chromium/src/+/main/docs/dangling_ptr_guide.md",
+ "",
+ "To disable this warning, please add in the commit description:",
+ "DanglingUntriaged-notes: <rational for new untriaged dangling "
+ "pointers>",
+ )
+ return [output_api.PresubmitPromptWarning(message)]
+
+def CheckInlineConstexprDefinitionsInHeaders(input_api, output_api):
+ """Checks that non-static constexpr definitions in headers are inline."""
+ # In a properly formatted file, constexpr definitions inside classes or
+ # structs will have additional whitespace at the beginning of the line.
+ # The pattern looks for variables initialized as constexpr kVar = ...; or
+ # constexpr kVar{...};
+ # The pattern does not match expressions that have braces in kVar to avoid
+ # matching constexpr functions.
+ pattern = input_api.re.compile(r'^constexpr (?!inline )[^\(\)]*[={]')
+ attribute_pattern = input_api.re.compile(r'(\[\[[a-zA-Z_:]+\]\]|[A-Z]+[A-Z_]+) ')
+ problems = []
+ for f in input_api.AffectedFiles():
+ if not _IsCPlusPlusHeaderFile(input_api, f.LocalPath()):
+ continue
+
+ for line_number, line in f.ChangedContents():
+ line = attribute_pattern.sub('', line)
+ if pattern.search(line):
+ problems.append(
+ f"{f.LocalPath()}: {line_number}\n {line}")
+
+ if problems:
+ return [
+ output_api.PresubmitPromptWarning(
+ 'Consider inlining constexpr variable definitions in headers '
+ 'outside of classes to avoid unnecessary copies of the '
+ 'constant. See https://abseil.io/tips/168 for more details.',
+ problems)
+ ]
+ else:
+ return []
projects / platform / framework / web / chromium-efl.git / blobdiff