Skip to content

Commit

Permalink
Block usage of handle<shared_buffer> in Chrome mojom files.
Browse files Browse the repository at this point in the history
Chrome Mojo interfaces should prefer one of the higher-level primitives
built on top of the native Mojo handle<shared_buffer>:

- mojo_base.mojom.ReadOnlySharedMemoryRegion
- mojo_base.mojom.WritableSharedMemoryRegion
- mojo_base.mojom.UnsafeSharedMemoryRegion

Fixed: 1132241
Change-Id: I082d512594db6cedb9146928e40f7387a492bbe2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3526187
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#981728}
  • Loading branch information
zetafunction authored and Chromium LUCI CQ committed Mar 16, 2022
1 parent c1920ef commit 92c15e3
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 6 deletions.
26 changes: 20 additions & 6 deletions PRESUBMIT.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ class BanRule:
excluded_paths: Optional[Sequence[str]] = None


# Format: Sequence of BanRule:
_BANNED_JAVA_IMPORTS : Sequence[BanRule] = (
BanRule(
'import java.net.URI;',
Expand Down Expand Up @@ -175,7 +174,6 @@ class BanRule:
),
)

# Format: Sequence of BanRule:
_BANNED_JAVA_FUNCTIONS : Sequence[BanRule] = (
BanRule(
'StrictMode.allowThreadDiskReads()',
Expand Down Expand Up @@ -203,7 +201,6 @@ class BanRule:
),
)

# Format: Sequence of BanRule:
_BANNED_OBJC_FUNCTIONS : Sequence[BanRule] = (
BanRule(
'addTrackingRect:',
Expand Down Expand Up @@ -305,7 +302,6 @@ class BanRule:
),
)

# Format: Sequence of BanRule:
_BANNED_IOS_OBJC_FUNCTIONS = (
BanRule(
r'/\bTEST[(]',
Expand All @@ -328,7 +324,6 @@ class BanRule:
),
)

# Format: Sequence of BanRule:
_BANNED_IOS_EGTEST_FUNCTIONS : Sequence[BanRule] = (
BanRule(
r'/\bEXPECT_OCMOCK_VERIFY\b',
Expand All @@ -340,7 +335,6 @@ class BanRule:
),
)

# Format: Sequence of BanRule:
_BANNED_CPP_FUNCTIONS : Sequence[BanRule] = (
BanRule(
r'/\busing namespace ',
Expand Down Expand Up @@ -1008,6 +1002,19 @@ class BanRule:
),
)

_BANNED_MOJOM_PATTERNS : Sequence[BanRule] = (
BanRule(
'handle<shared_buffer>',
(
'Please use one of the more specific shared memory types instead:',
' mojo_base.mojom.ReadOnlySharedMemoryRegion',
' mojo_base.mojom.WritableSharedMemoryRegion',
' mojo_base.mojom.UnsafeSharedMemoryRegion',
),
True,
),
)

# Format: Sequence of tuples containing:
# * String pattern or, if starting with a slash, a regular expression.
# * Sequence of strings to show when the pattern matches.
Expand Down Expand Up @@ -1683,6 +1690,13 @@ def CheckForMatch(affected_file, line_num: int, line: str,
for ban_rule in _BANNED_CPP_FUNCTIONS:
CheckForMatch(f, line_num, line, ban_rule)

file_filter = lambda f: f.LocalPath().endswith(('.mojom'))
for f in input_api.AffectedFiles(file_filter=file_filter):
for line_num, line in f.ChangedContents():
for ban_rule in _BANNED_MOJOM_PATTERNS:
CheckForMatch(f, line_num, line, ban_rule)


result = []
if (warnings):
result.append(
Expand Down
22 changes: 22 additions & 0 deletions PRESUBMIT_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2591,6 +2591,28 @@ def testBannedMojoFunctions(self):
self.assertTrue('third_party/blink/ok/file3.cc' not in results[0].message)
self.assertTrue('content/renderer/ok/file3.cc' not in results[0].message)

def testBannedMojomPatterns(self):
input_api = MockInputApi()
input_api.files = [
MockFile('bad.mojom',
['struct Bad {',
' handle<shared_buffer> buffer;',
'};']),
MockFile('good.mojom',
['struct Good {',
' mojo_base.mojom.ReadOnlySharedMemoryRegion region1;',
' mojo_base.mojom.WritableSharedMemoryRegion region2;',
' mojo_base.mojom.UnsafeSharedMemoryRegion region3;',
'};']),
]

results = PRESUBMIT.CheckNoBannedFunctions(input_api, MockOutputApi())

# warnings are results[0], errors are results[1]
self.assertEqual(1, len(results))
self.assertTrue('bad.mojom' in results[0].message)
self.assertTrue('good.mojom' not in results[0].message)

def testDeprecatedMojoTypes(self):
ok_paths = ['components/arc']
warning_paths = ['some/cpp']
Expand Down

0 comments on commit 92c15e3

Please sign in to comment.