Include explanatory text when adding the MPArch watchlist during PRESUBMIT
We now have a good idea of the average volume of CLs affected by this
PRESUBMIT (0 to 9 per day, typically around 4). It seems like we could
add some guidance here without being too spammy.
Bug: 1190019
Change-Id: Iaea9f969c6af7b651fef88290ca1012dabbcd2d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3457088
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/main@{#970772}
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 9e7eb56..fbe4c339 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -5770,21 +5770,39 @@
concerning_web_contents_methods, concerning_rfh_methods
] for item in sublist) + r')\(')
- uses_concerning_api = False
+ used_apis = set()
for f in input_api.AffectedFiles(include_deletes=False,
file_filter=source_file_filter):
for line_num, line in f.ChangedContents():
- if (concerning_class_pattern.search(line)
- or concerning_method_pattern.search(line)):
- uses_concerning_api = True
- break
- if uses_concerning_api:
- break
+ class_match = concerning_class_pattern.search(line)
+ if class_match:
+ used_apis.add(class_match[0])
+ method_match = concerning_method_pattern.search(line)
+ if method_match:
+ used_apis.add(method_match[1])
- if uses_concerning_api:
- output_api.AppendCC('mparch-reviews+watch@chromium.org')
+ if not used_apis:
+ return []
- 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.')
+ explaination = (
+ 'Please double check whether new code assumes that a WebContents only '
+ 'contains a single page at a time. For example, it is discouraged to '
+ 'reset per-document state in response to the observation of a '
+ 'navigation. 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=explaination)
+ ]
def CheckAssertAshOnlyCode(input_api, output_api):
