Skip to content

Commit

Permalink
Add presubmits warnings for commonly misused functions.
Browse files Browse the repository at this point in the history
Change-Id: I0b9c673dfcb13e29d4a384c39e1721cdb7c2d231
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5591890
Reviewed-by: Daniel Cheng <[email protected]>
Commit-Queue: Erik Chen <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1309589}
  • Loading branch information
erikchen authored and Chromium LUCI CQ committed Jun 3, 2024
1 parent 7d279a6 commit f58f720
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 0 deletions.
21 changes: 21 additions & 0 deletions PRESUBMIT.py
Original file line number Diff line number Diff line change
Expand Up @@ -1976,6 +1976,27 @@ class BanRule:
),
treat_as_error = False,
),
BanRule(
pattern = 'ProfileManager::GetLastUsedProfile',
explanation = (
'Most code should already be scoped to a Profile. Pass in a Profile* '
'or retreive from an existing entity with a reference to the Profile '
'(e.g. WebContents).'
),
treat_as_error = False,
),
BanRule(
pattern = (
r'/FindBrowserWithUiElementContext|'
r'FindBrowserWithTab|'
r'FindBrowserWithGroup'
),
explanation = (
'Most code should already be scoped to a Browser. Pass in a Browser* '
'or retreive from an existing entity with a reference to the Browser.'
),
treat_as_error = False,
),
)

_DEPRECATED_SYNC_CONSENT_FUNCTION_WARNING = (
Expand Down
3 changes: 3 additions & 0 deletions PRESUBMIT_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2982,6 +2982,8 @@ def testBannedCppFunctions(self):
['params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET']),
MockFile('some/cpp/problematic/file4.cc',
['params.ownership = Widget::InitParams::NATIVE_WIDGET_OWNS_WIDGET']),
MockFile('some/cpp/problematic/file5.cc',
['Browser* browser = chrome::FindBrowserWithTab(web_contents)']),
]

results = PRESUBMIT.CheckNoBannedFunctions(input_api, MockOutputApi())
Expand All @@ -2995,6 +2997,7 @@ def testBannedCppFunctions(self):
self.assertTrue('some/cpp/problematic/file2.cc' in results[0].message)
self.assertTrue('some/cpp/problematic/file3.cc' in results[0].message)
self.assertTrue('some/cpp/problematic/file4.cc' in results[0].message)
self.assertTrue('some/cpp/problematic/file5.cc' in results[0].message)
self.assertFalse('some/cpp/nocheck/file.cc' in results[0].message)
self.assertFalse('some/cpp/nocheck/file.cc' in results[1].message)
self.assertFalse('some/cpp/comment/file.cc' in results[0].message)
Expand Down

0 comments on commit f58f720

Please sign in to comment.