base tracing: Make trace_event includes a bypassable presubmit error

Base code should include base_tracing.h instead of trace_event impl
headers, so that base can build with enable_base_tracing = false.

Previously, a presubmit check emitted a warning if a trace_event
implementation header was included outside trace_event implementation
directories. This modifies the warning into an error, which can be
bypassed via a source code comment.

TBR: rsesek@chromium.org
Bug: 1093406
Change-Id: Ic82f399390ededd12c04ad6005de8f68929bae87
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2264555Reviewed-by: Eric Seckler <eseckler@chromium.org>
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Eric Seckler <eseckler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782901}

base/PRESUBMIT.py

@@ -40,20 +40,18 @@ def _CheckNoTraceEventInclude(input_api, output_api):
   to maintain compatibility with the gn flag "enable_base_tracing = false".
   """
   discouraged_includes = [
-    r'^#include "base/trace_event/blame_context.h"$',
-    r'^#include "base/trace_event/memory_allocator_dump_guid.h"$',
-    r'^#include "base/trace_event/memory_dump_provider.h"$',
-    r'^#include "base/trace_event/trace_event.h"$',
-    r'^#include "base/trace_event/traced_value.h"$',
+    r'^#include "base/trace_event/(?!base_tracing\.h)',
   ]
 
   white_list = [
     r".*\.(h|cc|mm)$",
   ]
   black_list = [
+    r".*[\\/]test[\\/].*",
     r".*[\\/]trace_event[\\/].*",
     r".*[\\/]tracing[\\/].*",
   ]
+  no_presubmit = r"// no-presubmit-check"
 
   def FilterFile(affected_file):
     return input_api.FilterSourceFile(
@@ -65,16 +63,19 @@ def _CheckNoTraceEventInclude(input_api, output_api):
   for f in input_api.AffectedSourceFiles(FilterFile):
     for line_num, line in f.ChangedContents():
       for include in discouraged_includes:
-        if input_api.re.search(include, line):
+        if (input_api.re.search(include, line) and
+            not input_api.re.search(no_presubmit, line)):
           locations.append("    %s:%d" % (f.LocalPath(), line_num))
           break
 
   if locations:
-    return [ output_api.PresubmitPromptWarning(
-        'Consider replacing includes to trace_event implementation headers\n' +
-        'in //base with "base/trace_event/base_tracing.h" and/or verify\n' +
-        'that base_unittests still passes with gn arg\n' +
-        'enable_base_tracing = false.\n' + '\n'.join(locations)) ]
+    return [ output_api.PresubmitError(
+        'Base code should include "base/trace_event/base_tracing.h" instead\n' +
+        'of trace_event implementation headers. If you need to include an\n' +
+        'implementation header, verify that base_unittests still passes\n' +
+        'with gn arg "enable_base_tracing = false" and add\n' +
+        '"// no-presubmit-check" after the include. \n' +
+        '\n'.join(locations)) ]
   return []
 
 

base/android/orderfile/orderfile_call_graph_instrumentation.cc

@@ -30,8 +30,8 @@
 
 #include "base/command_line.h"
 #include "base/time/time.h"
-#include "base/trace_event/memory_dump_manager.h"
-#include "base/trace_event/memory_dump_provider.h"
+#include "base/trace_event/memory_dump_manager.h"   // no-presubmit-check
+#include "base/trace_event/memory_dump_provider.h"  // no-presubmit-check
 #endif  // BUILDFLAG(DEVTOOLS_INSTRUMENTATION_DUMPING)
 
 #if !BUILDFLAG(SUPPORTS_CODE_ORDERING)

base/android/orderfile/orderfile_instrumentation.cc

@@ -28,8 +28,8 @@
 
 #include "base/command_line.h"
 #include "base/time/time.h"
-#include "base/trace_event/memory_dump_manager.h"
-#include "base/trace_event/memory_dump_provider.h"
+#include "base/trace_event/memory_dump_manager.h"   // no-presubmit-check
+#include "base/trace_event/memory_dump_provider.h"  // no-presubmit-check
 #endif  // BUILDFLAG(DEVTOOLS_INSTRUMENTATION_DUMPING)
 
 #if !BUILDFLAG(SUPPORTS_CODE_ORDERING)

base/android/trace_event_binding.cc

@@ -14,7 +14,7 @@
 #include "base/tracing_buildflags.h"
 
 #if BUILDFLAG(ENABLE_BASE_TRACING)
-#include "base/trace_event/trace_event_impl.h"
+#include "base/trace_event/trace_event_impl.h"  // no-presubmit-check
 #endif  // BUILDFLAG(ENABLE_BASE_TRACING)
 
 namespace base {

base/containers/mru_cache_unittest.cc

@@ -11,7 +11,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 #if BUILDFLAG(ENABLE_BASE_TRACING)
-#include "base/trace_event/memory_usage_estimator.h"
+#include "base/trace_event/memory_usage_estimator.h"  // no-presubmit-check
 #endif  // BUILDFLAG(ENABLE_BASE_TRACING)
 
 namespace base {

base/memory/discardable_shared_memory.cc

@@ -41,8 +41,8 @@
 #endif
 
 #if BUILDFLAG(ENABLE_BASE_TRACING)
-#include "base/trace_event/memory_allocator_dump.h"
-#include "base/trace_event/process_memory_dump.h"
+#include "base/trace_event/memory_allocator_dump.h"  // no-presubmit-check
+#include "base/trace_event/process_memory_dump.h"    // no-presubmit-check
 #endif  // BUILDFLAG(ENABLE_BASE_TRACING)
 
 namespace base {

base/memory/discardable_shared_memory_unittest.cc

@@ -14,8 +14,8 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 #if BUILDFLAG(ENABLE_BASE_TRACING)
-#include "base/trace_event/memory_allocator_dump.h"
-#include "base/trace_event/process_memory_dump.h"
+#include "base/trace_event/memory_allocator_dump.h"  // no-presubmit-check
+#include "base/trace_event/process_memory_dump.h"    // no-presubmit-check
 #endif  // BUILDFLAG(ENABLE_BASE_TRACING)
 
 namespace base {

base/memory/madv_free_discardable_memory_allocator_posix.cc

@@ -13,7 +13,7 @@
 #include "base/tracing_buildflags.h"
 
 #if BUILDFLAG(ENABLE_BASE_TRACING)
-#include "base/trace_event/memory_dump_manager.h"
+#include "base/trace_event/memory_dump_manager.h"  // no-presubmit-check
 #endif  // BUILDFLAG(ENABLE_BASE_TRACING)
 
 namespace base {

base/memory/madv_free_discardable_memory_allocator_posix_unittest.cc

@@ -16,8 +16,8 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 #if BUILDFLAG(ENABLE_BASE_TRACING)
-#include "base/trace_event/memory_allocator_dump.h"
-#include "base/trace_event/process_memory_dump.h"
+#include "base/trace_event/memory_allocator_dump.h"  // no-presubmit-check
+#include "base/trace_event/process_memory_dump.h"    // no-presubmit-check
 #endif  // BUILDFLAG(ENABLE_BASE_TRACING)
 
 #define SUCCEED_IF_MADV_FREE_UNSUPPORTED()                                  \

base/memory/madv_free_discardable_memory_posix.cc

@@ -22,8 +22,8 @@
 #include "base/tracing_buildflags.h"
 
 #if BUILDFLAG(ENABLE_BASE_TRACING)
-#include "base/trace_event/memory_allocator_dump.h"
-#include "base/trace_event/memory_dump_manager.h"
+#include "base/trace_event/memory_allocator_dump.h"  // no-presubmit-check
+#include "base/trace_event/memory_dump_manager.h"    // no-presubmit-check
 #endif  // BUILDFLAG(ENABLE_BASE_TRACING)
 
 #if defined(ADDRESS_SANITIZER)

base/memory/shared_memory_tracker.cc

@@ -11,8 +11,8 @@
 #include "base/tracing_buildflags.h"
 
 #if BUILDFLAG(ENABLE_BASE_TRACING)
-#include "base/trace_event/memory_dump_manager.h"
-#include "base/trace_event/process_memory_dump.h"
+#include "base/trace_event/memory_dump_manager.h"  // no-presubmit-check
+#include "base/trace_event/process_memory_dump.h"  // no-presubmit-check
 #endif  // BUILDFLAG(ENABLE_BASE_TRACING)
 
 namespace base {

base/sampling_heap_profiler/sampling_heap_profiler.cc

@@ -18,7 +18,7 @@
 #include "base/partition_alloc_buildflags.h"
 #include "base/sampling_heap_profiler/lock_free_address_hash_set.h"
 #include "base/threading/thread_local_storage.h"
-#include "base/trace_event/heap_profiler_allocation_context_tracker.h"
+#include "base/trace_event/heap_profiler_allocation_context_tracker.h"  // no-presubmit-check
 #include "build/build_config.h"
 
 #if defined(OS_MACOSX)
@@ -31,7 +31,7 @@
 
 #if defined(OS_ANDROID) && BUILDFLAG(CAN_UNWIND_WITH_CFI_TABLE) && \
     defined(OFFICIAL_BUILD)
-#include "base/trace_event/cfi_backtrace_android.h"
+#include "base/trace_event/cfi_backtrace_android.h"  // no-presubmit-check
 #endif
 
 namespace base {

base/threading/thread_id_name_manager.cc

@@ -13,7 +13,7 @@
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/threading/thread_local.h"
-#include "base/trace_event/heap_profiler_allocation_context_tracker.h"
+#include "base/trace_event/heap_profiler_allocation_context_tracker.h"  // no-presubmit-check
 
 namespace base {
 namespace {

base/values.cc

@@ -24,7 +24,7 @@
 #include "base/tracing_buildflags.h"
 
 #if BUILDFLAG(ENABLE_BASE_TRACING)
-#include "base/trace_event/memory_usage_estimator.h"
+#include "base/trace_event/memory_usage_estimator.h"  // no-presubmit-check
 #endif  // BUILDFLAG(ENABLE_BASE_TRACING)
 
 namespace base {