Elevate v8::Isolate::GetCurrent() PRESUBMIT warning to error
V8 offers v8::Isolate::IsCurrent() as of [1]. This leaves no reason to
use v8::Isolate::GetCurrent() anymore, meaning it can be completely
banned from PDFium.
Extend the ban to v8::Isolate::TryGetCurrent().
[1] https://crrev.com/c/v8/v8/+/3251173
Bug: pdfium:1737
Change-Id: Iad6779068327687e84e02eeb7390e342e1125ca0
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/86637
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Daniel Hosseinian <dhoss@chromium.org>
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index fa7241f..606f48d 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -59,12 +59,13 @@
[_THIRD_PARTY],
),
(
- 'v8::Isolate::GetCurrent()',
+ r'/v8::Isolate::(?:|Try)GetCurrent()',
(
- 'Avoid uses of v8::Isolate::GetCurrent(). Prefer holding a pointer to',
- 'the v8::Isolate that was entered.',
+ 'v8::Isolate::GetCurrent() and v8::Isolate::TryGetCurrent() are banned. Hold',
+ 'a pointer to the v8::Isolate that was entered. Use v8::Isolate::IsCurrent()',
+ 'to check whether a given v8::Isolate is entered.',
),
- False,
+ True,
(),
),
)
diff --git a/PRESUBMIT_test.py b/PRESUBMIT_test.py
index 72723ed..41d2a3b 100755
--- a/PRESUBMIT_test.py
+++ b/PRESUBMIT_test.py
@@ -22,11 +22,19 @@
['using namespace std; // nocheck']),
MockFile('some/cpp/comment/file.cc',
[' // A comment about `using namespace std;`']),
- MockFile('some/cpp/v8/file.cc', ['v8::Isolate::GetCurrent()']),
+ MockFile('some/cpp/v8/get-current.cc', ['v8::Isolate::GetCurrent()']),
+ MockFile('some/cpp/v8/try-get-current.cc',
+ ['v8::Isolate::TryGetCurrent()']),
]
results = PRESUBMIT._CheckNoBannedFunctions(input_api, MockOutputApi())
+ # There are no warnings to test, so add an empty warning to keep the test
+ # extendable for the future. This block can be removed once warnings are
+ # added.
+ self.assertEqual(1, len(results))
+ results.insert(0, MockOutputApi().PresubmitPromptWarning(''))
+
# warnings are results[0], errors are results[1]
self.assertEqual(2, len(results))
self.assertTrue('some/cpp/problematic/file.cc' in results[1].message)
@@ -37,7 +45,8 @@
self.assertFalse('some/cpp/nocheck/file.cc' in results[1].message)
self.assertFalse('some/cpp/comment/file.cc' in results[0].message)
self.assertFalse('some/cpp/comment/file.cc' in results[1].message)
- self.assertTrue('some/cpp/v8/file.cc' in results[0].message)
+ self.assertTrue('some/cpp/v8/get-current.cc' in results[1].message)
+ self.assertTrue('some/cpp/v8/try-get-current.cc' in results[1].message)
class CheckChangeOnUploadTest(unittest.TestCase):
