Add tests for enforcing C++ dependencies in PRESUBMIT

This CL omits tests for directories where dependencies aren't checked:
PRESUBMIT test mocks is missing support for FileSourceFilter. This will
be added to test mocks along with the missing tests in a followup CL.

Bug: 763980
Change-Id: Ibf9265911f32d24f446e690d9ec05ddb7a61f198
Reviewed-on: https://chromium-review.googlesource.com/671812Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505189}

PRESUBMIT_test_mocks.py

@@ -9,6 +9,47 @@ import re
 import subprocess
 import sys
 
+# TODO(dcheng): It's kind of horrible that this is copy and pasted from
+# presubmit_canned_checks.py, but it's far easier than any of the alternatives.
+def _ReportErrorFileAndLine(filename, line_num, dummy_line):
+  """Default error formatter for _FindNewViolationsOfRule."""
+  return '%s:%s' % (filename, line_num)
+
+
+class MockCannedChecks(object):
+  def _FindNewViolationsOfRule(self, callable_rule, input_api,
+                               source_file_filter=None,
+                               error_formatter=_ReportErrorFileAndLine):
+    """Find all newly introduced violations of a per-line rule (a callable).
+
+    Arguments:
+      callable_rule: a callable taking a file extension and line of input and
+        returning True if the rule is satisfied and False if there was a
+        problem.
+      input_api: object to enumerate the affected files.
+      source_file_filter: a filter to be passed to the input api.
+      error_formatter: a callable taking (filename, line_number, line) and
+        returning a formatted error string.
+
+    Returns:
+      A list of the newly-introduced violations reported by the rule.
+    """
+    errors = []
+    for f in input_api.AffectedFiles(include_deletes=False,
+                                     file_filter=source_file_filter):
+      # For speed, we do two passes, checking first the full file.  Shelling out
+      # to the SCM to determine the changed region can be quite expensive on
+      # Win32.  Assuming that most files will be kept problem-free, we can
+      # skip the SCM operations most of the time.
+      extension = str(f.LocalPath()).rsplit('.', 1)[-1]
+      if all(callable_rule(extension, line) for line in f.NewContents()):
+        continue  # No violation found in full text: can skip considering diff.
+
+      for line_num, line in f.ChangedContents():
+        if not callable_rule(extension, line):
+          errors.append(error_formatter(f.LocalPath(), line_num, line))
+
+    return errors
 
 class MockInputApi(object):
   """Mock class for the InputApi class.
@@ -18,6 +59,7 @@ class MockInputApi(object):
   """
 
   def __init__(self):
+    self.canned_checks = MockCannedChecks()
     self.fnmatch = fnmatch
     self.json = json
     self.re = re

third_party/WebKit/PRESUBMIT.py

@@ -108,7 +108,7 @@ def _CheckForForbiddenNamespace(input_api, output_api):
     def source_file_filter(path):
         return input_api.FilterSourceFile(path,
                                           white_list=[r'third_party/WebKit/Source/.*\.(h|cpp)$'],
-                                          black_list=[r'third_party/WebKit/Source/(platform|wtf|web|controller)/'])
+                                          black_list=[r'third_party/WebKit/Source/(platform|controller)/'])
 
     comment_re = input_api.re.compile(r'^\s*//')
     result = []

third_party/WebKit/PRESUBMIT_test.py

@@ -36,8 +36,7 @@ class PresubmitTest(unittest.TestCase):
 
     @mock.patch('subprocess.Popen')
     def testCheckChangeOnUploadWithWebKitAndChromiumFiles(self, _):
-        """
-        This verifies that CheckChangeOnUpload will only call check-webkit-style
+        """This verifies that CheckChangeOnUpload will only call check-webkit-style
         on WebKit files.
         """
         diff_file_webkit_h = ['some diff']
@@ -58,8 +57,7 @@ class PresubmitTest(unittest.TestCase):
 
     @mock.patch('subprocess.Popen')
     def testCheckChangeOnUploadWithEmptyAffectedFileList(self, _):
-        """
-        This verifies that CheckChangeOnUpload will skip calling
+        """This verifies that CheckChangeOnUpload will skip calling
         check-webkit-style if the affected file list is empty.
         """
         diff_file_chromium1_h = ['some diff']
@@ -76,5 +74,101 @@ class PresubmitTest(unittest.TestCase):
         subprocess.Popen.assert_not_called()
 
 
+class CxxDependencyTest(unittest.TestCase):
+    allow_list = [
+        'gfx::ColorSpace',
+        'gfx::CubicBezier',
+        'gfx::ICCProfile',
+        'gfx::ScrollOffset',
+    ]
+    disallow_list = [
+        'scoped_refptr<base::SingleThreadTaskRunner>',
+        'base::Callback<void()>',
+        'base::OnceCallback<void()>',
+        'content::RenderFrame',
+        'gfx::Point',
+        'gfx::Rect',
+        'net::IPEndPoint',
+        'ui::Clipboard',
+    ]
+    disallow_message = [
+    ]
+
+    def runCheck(self, filename, file_contents):
+        mock_input_api = MockInputApi()
+        mock_input_api.files = [
+            MockAffectedFile(filename, file_contents),
+        ]
+        # Access to a protected member
+        # pylint: disable=W0212
+        return PRESUBMIT._CheckForForbiddenNamespace(mock_input_api, MockOutputApi())
+
+    # References in comments should never be checked.
+    def testCheckCommentsIgnored(self):
+        filename = 'third_party/WebKit/Source/core/frame/frame.cc'
+        for item in self.allow_list:
+            errors = self.runCheck(filename, ['// %s' % item])
+            self.assertEqual([], errors)
+
+        for item in self.disallow_list:
+            errors = self.runCheck(filename, ['// %s' % item])
+            self.assertEqual([], errors)
+
+    # core, modules, public, et cetera should all have dependency enforcement.
+    def testCheckCoreEnforcement(self):
+        filename = 'third_party/WebKit/Source/core/frame/frame.cc',
+        for item in self.allow_list:
+            errors = self.runCheck(filename, ['%s' % item])
+            self.assertEqual([], errors)
+
+        for item in self.disallow_list:
+            errors = self.runCheck(filename, ['%s' % item])
+            self.assertGreater(len(errors), 0)
+            self.assertRegexpMatches(
+                errors[0].message,
+                '^Do not use Chromium class from namespace [A-Za-z0-9_]+ inside Blink core:')
+            if len(errors) == 2:
+                self.assertRegexpMatches(errors[1].message, '^Do not use Chromium class [A-Za-z0-9_]+ inside Blink core:')
+            else:
+                self.assertEquals(1, len(errors))
+
+    def testCheckModulesEnforcement(self):
+        filename = 'third_party/WebKit/Source/modules/modules_initializer.cc',
+        for item in self.allow_list:
+            errors = self.runCheck(filename, ['%s' % item])
+            self.assertEqual([], errors)
+
+        for item in self.disallow_list:
+            errors = self.runCheck(filename, ['%s' % item])
+            self.assertGreater(len(errors), 0)
+            self.assertRegexpMatches(
+                errors[0].message,
+                '^Do not use Chromium class from namespace [A-Za-z0-9_]+ inside Blink core:')
+            if len(errors) == 2:
+                self.assertRegexpMatches(errors[1].message, '^Do not use Chromium class [A-Za-z0-9_]+ inside Blink core:')
+            else:
+                self.assertEquals(1, len(errors))
+
+    def testCheckPublicEnforcement(self):
+        filename = 'third_party/WebKit/Source/public/platform/WebThread.h',
+        for item in self.allow_list:
+            errors = self.runCheck(filename, ['%s' % item])
+            self.assertEqual([], errors)
+
+        for item in self.disallow_list:
+            errors = self.runCheck(filename, ['%s' % item])
+            self.assertGreater(len(errors), 0)
+            self.assertRegexpMatches(
+                errors[0].message,
+                '^Do not use Chromium class from namespace [A-Za-z0-9_]+ inside Blink core:')
+            if len(errors) == 2:
+                self.assertRegexpMatches(errors[1].message, '^Do not use Chromium class [A-Za-z0-9_]+ inside Blink core:')
+            else:
+                self.assertEquals(1, len(errors))
+
+    # platform and controller should be opted out of enforcement, but aren't
+    # currently checked because the PRESUBMIT test mocks are missing too
+    # much functionality...
+
 if __name__ == '__main__':
     unittest.main()