From 5d05b50d9e704fdfd7f9eeb506595668203ff5bf Mon Sep 17 00:00:00 2001
From: cdesouza-chromium <cdesouza@igalia.com>
Date: Tue, 28 Feb 2023 16:49:23 +0000
Subject: [PATCH] DCHECK tests using EXPECT_DEATH (#17411)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* DCHECK tests using EXPECT_DEATH

This change updates the only DCHECK test we have to rely on
EXPECT_DEATH, as `logging::ScopedLogAssertHandler` will not be able to
prevent termination in coming upstream updates.

Chromium change:
https://chromium.googlesource.com/chromium/src/+/2cc59dab0e60c52aa7e1aca45d97992fdfc5a6dd

commit 2cc59dab0e60c52aa7e1aca45d97992fdfc5a6dd
Author: Peter Boström <pbos@chromium.org>
Date:   Sat Feb 18 08:13:24 2023 +0000

    (Partial) Make sure CHECK() failures crash

    This partial land excludes iOS and Windows where two tests currently
    rely on aborting a LOG(FATAL). This is going in to not backslide in
    platform-agnostic code.

    This is trying to make sure that CHECK failures effectively don't
    return before LOG(FATAL) is properly [[noreturn]]. A follow-up could
    be to split out CheckError from DcheckError to make sure the
    destructor can be marked [[noreturn]].

    Note that this doesn't apply to the optimized version of CHECK which is
    already a [[noreturn]] function call, but also doesn't involve
    LOG(FATAL).

    Bug: 1409729

* Add another test to check both DCHECKs.

---------

Co-authored-by: Aleksey Khoroshilov <akhoroshilov@brave.com>
---
 base/feature_override_unittest.cc | 47 ++++++++++++++++---------------
 1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/base/feature_override_unittest.cc b/base/feature_override_unittest.cc
index d9bf870c722..b6e8e0f4f5f 100644
--- a/base/feature_override_unittest.cc
+++ b/base/feature_override_unittest.cc
@@ -4,6 +4,7 @@
  * You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 #include "base/feature_override.h"
+#include "base/debug/debugging_buildflags.h"
 #include "base/feature_list.h"
 #include "base/logging.h"
 #include "base/test/mock_callback.h"
@@ -71,38 +72,40 @@ TEST(FeatureOverrideTest, OverridesTest) {
   }
 }
 
-#if DCHECK_IS_ON()
+#if DCHECK_IS_ON() && !BUILDFLAG(DCHECK_IS_CONFIGURABLE)
 TEST(FeatureOverrideTest, FeatureDuplicateDChecks) {
   // Check any feature to make sure overridden features are finalized (moved
   // from an unsorted vector to a sorted flat_map).
   ASSERT_FALSE(base::FeatureList::IsEnabled(kTestEnabledButOverridenFeature));
 
-  base::MockCallback<logging::LogAssertHandlerFunction> mock_log_handler;
-  logging::ScopedLogAssertHandler scoped_log_handler(mock_log_handler.Get());
-  EXPECT_CALL(
-      mock_log_handler,
-      Run(_, _,
-          testing::HasSubstr("TestEnabledButOverridenFeature is duplicated"),
-          _));
-  EXPECT_CALL(
-      mock_log_handler,
-      Run(_, _,
-          testing::HasSubstr(
-              "TestEnabledButOverridenFeature has already been overridden"),
-          _))
-      .Times(2);
-
   // This will add a feature to an unsorted vector of overrides.
   internal::FeatureDefaultStateOverrider init_overrides{{
       {kTestEnabledButOverridenFeature, FEATURE_DISABLED_BY_DEFAULT},
   }};
 
-  // This should trigger DCHECKs.
-  internal::FeatureDefaultStateOverrider test_overrider{{
-      {kTestEnabledButOverridenFeature, FEATURE_DISABLED_BY_DEFAULT},
-      {kTestEnabledButOverridenFeature, FEATURE_DISABLED_BY_DEFAULT},
-  }};
+  // This should trigger DCHECK.
+  EXPECT_DEATH_IF_SUPPORTED(
+      internal::FeatureDefaultStateOverrider({
+          {kTestEnabledButOverridenFeature, FEATURE_DISABLED_BY_DEFAULT},
+      }),
+      testing::HasSubstr("Feature TestEnabledButOverridenFeature has already "
+                         "been overridden"));
 }
-#endif  // DCHECK_IS_ON()
+
+TEST(FeatureOverrideTest, FeatureDuplicateInSameMacroDChecks) {
+  // Check any feature to make sure overridden features are finalized (moved
+  // from an unsorted vector to a sorted flat_map).
+  ASSERT_FALSE(base::FeatureList::IsEnabled(kTestEnabledButOverridenFeature));
+
+  // This should trigger DCHECK.
+  EXPECT_DEATH_IF_SUPPORTED(
+      internal::FeatureDefaultStateOverrider({
+          {kTestEnabledButOverridenFeature, FEATURE_DISABLED_BY_DEFAULT},
+          {kTestEnabledButOverridenFeature, FEATURE_DISABLED_BY_DEFAULT},
+      }),
+      testing::HasSubstr("Feature TestEnabledButOverridenFeature is duplicated "
+                         "in the current override macros"));
+}
+#endif  // DCHECK_IS_ON() && !BUILDFLAG(DCHECK_IS_CONFIGURABLE)
 
 }  // namespace base