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'/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.',
+ 'between batched tests. Use HistogramWatcher to check histogram records '
+ 'instead.',
),
False,
excluded_paths=(
# 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.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/webui.js',
'ash/webui/projector_app/resources/annotator/trusted/annotator_browser_proxy.js',
'ash/webui/projector_app/resources/app/trusted/projector_browser_proxy.js',
- 'ash/webui/scanning/resources/scanning_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',
),
),
)
'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 = (
_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'],
+ [
+ # Abseil's benchmarks never linked into chrome.
+ 'third_party/abseil-cpp/.*_benchmark.cc',
+ ],
),
BanRule(
r'/\bstd::sto(i|l|ul|ll|ull)\b',
[_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 is banned. Use scoped_refptr instead.',
'^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/',
# Fuchsia provides C++ libraries that use std::shared_ptr<>.
'^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(
'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,
[
# 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/partition_alloc_unittest\.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'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(
[_THIRD_PARTY_EXCEPT_BLINK], # Not an error in third_party folders.
),
BanRule(
+ 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.',
[
# 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
],
# 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
],
[_THIRD_PARTY_EXCEPT_BLINK], # Not an error in third_party folders.
),
BanRule(
- r'/\babsl::string_view\b',
- (
- 'absl::string_view is a legacy spelling of std::string_view, which is ',
- 'not allowed yet (https://crbug.com/691162). Use base::StringPiece ',
- 'instead, unless std::string_view is needed to use with an external ',
- 'API.',
- ),
- True,
- [_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
- ),
- BanRule(
- r'/\bstd::(u16)?string_view\b',
- (
- 'std::[u16]string_view is not yet allowed (crbug.com/691162). Use ',
- 'base::StringPiece[16] instead, unless std::[u16]string_view is ',
- 'needed to use an external API.',
- ),
- True,
- [
- # Needed to implement and test std::string_view interoperability.
- r'base/strings/string_piece.*',
- # 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'net/quic/.*',
- r'net/spdy/.*',
- r'net/test/embedded_test_server/.*',
- r'net/third_party/quiche/.*',
- r'services/network/web_transport\.cc',
- # This code is in the process of being extracted into an external
- # library, where //base will be unavailable.
- r'net/cert/pki/.*',
- r'net/der/.*',
- # Needed to use APIs from the above.
- r'net/cert/.*',
- # Not an error in third_party folders.
- _THIRD_PARTY_EXCEPT_BLINK
- ],
- ),
- BanRule(
r'/\babsl::(StrSplit|StrJoin|StrCat|StrAppend|Substitute|StrContains)\b',
(
'Abseil string utilities are banned. Use base/strings instead.',
'Abseil\'s time library is banned. Use base/time instead.',
),
True,
- [_THIRD_PARTY_EXCEPT_BLINK], # Not an error in third_party folders.
- ),
- BanRule(
- r'/\bstd::optional\b',
- (
- 'std::optional is not allowed yet (https://crbug.com/1373619). Use ',
- 'absl::optional instead.',
- ),
- True,
- [_THIRD_PARTY_EXCEPT_BLINK], # Not an error in third_party folders.
+ [
+ # 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,
- [_THIRD_PARTY_EXCEPT_BLINK], # Not an error in third_party folders.
+ [
+ # 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>',
True,
(
r'^base/win/scoped_winrt_initializer\.cc$',
+ r'^third_party/abseil-cpp/absl/.*',
),
),
BanRule(
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',
'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',
'lacros-version-skew-roller', 'skylab-test-cros-roller',
'infra-try-recipes-tester', 'lacros-tracking-roller',
'lacros-sdk-version-roller', 'chrome-automated-expectation',
- 'chromium-automated-expectation', 'chrome-branch-day')
+ '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 ('swarming-tasks',)
) | set('%s@fuchsia-infra.iam.gserviceaccount.com' % s
for s in ('global-integration-try-builder',
- 'global-integration-ci-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.')
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 []
r"^chrome/chrome_elf/dll_hash/dll_hash_main\.cc$",
r"^chrome/installer/setup/.*",
r"^chromecast/",
+ 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/"
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))
"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))
)
_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/"
if not input_api.os_path.exists(test_file):
continue
- use_python3 = False
- with open(f.LocalPath(), encoding='utf-8') 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 = []
# 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.
'(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(
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. Use
-@Batch(Batch.PER_CLASS) in most cases. Use @Batch(Batch.UNIT_TESTS) when tests
-have no side-effects. If the 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:
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)
return input_api.FilterSourceFile(
affected_file,
files_to_skip=input_api.DEFAULT_FILES_TO_SKIP +
- (r'^ios/third_party/*', r'^third_party/*'),
+ (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 = []
"""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 []
+ return []
DEPS_FILES = [ 'DEPS', 'buildtools/deps_revisions.gni' ]
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 []