From e211c3d666b4e5f4ee6d28fa1fa6f2276e50c89b Mon Sep 17 00:00:00 2001 From: Etienne Bergeron <etienneb@chromium.org> Date: Thu, 1 May 2025 06:12:43 -0700 Subject: [PATCH] Deprecate the use of EnableWalModeByDefault experiment This CL is removing the old experiment from: https://chromium-review.googlesource.com/c/chromium/src/+/2095927 The deprecated feature was trying to enable WAL mode on all database at once. The gains of WAL mode is database specific. The WAL mode was shipped on some specifics database and more database will be converted after fixing the exclusive mode issues. Bug: 40549957 Change-Id: I35d70211ebf2f875ccaf8662dd071b43de351472 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6496038 Reviewed-by: Avi Drissman <avi@chromium.org> Reviewed-by: Greg Thompson <grt@chromium.org> Commit-Queue: Etienne Bergeron <etienneb@chromium.org> Cr-Commit-Position: refs/heads/main@{#1454409} --- content/browser/network_service_browsertest.cc | 6 ------ sql/database.h | 5 +---- sql/recovery_unittest.cc | 5 ----- sql/sql_features.cc | 5 ----- sql/sql_features.h | 1 - 5 files changed, 1 insertion(+), 21 deletions(-) diff --git a/content/browser/network_service_browsertest.cc b/content/browser/network_service_browsertest.cc index dc9929db4d9b2..751308376ebb9 100644 --- a/content/browser/network_service_browsertest.cc +++ b/content/browser/network_service_browsertest.cc @@ -77,7 +77,6 @@ #include "services/network/public/mojom/network_service_test.mojom.h" #include "services/network/test/udp_socket_test_util.h" #include "sql/database.h" -#include "sql/sql_features.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -980,11 +979,6 @@ static const base::FilePath::CharType kNetworkSubpath[] = class MAYBE_NetworkServiceDataMigrationBrowserTest : public ContentBrowserTest { public: MAYBE_NetworkServiceDataMigrationBrowserTest() { - // Migration only supports non-WAL sqlite databases. If this feature is - // switched on by default before migration has been completed then the code - // in MaybeGrantSandboxAccessToNetworkContextData will need to be updated. - EXPECT_FALSE( - base::FeatureList::IsEnabled(sql::features::kEnableWALModeByDefault)); #if BUILDFLAG(IS_WIN) // On Windows, the network sandbox needs to be disabled. This is because the // code that performs the migration on Windows DCHECKs if network sandbox is diff --git a/sql/database.h b/sql/database.h index a2e4cce3ff930..660f05a1c20d7 100644 --- a/sql/database.h +++ b/sql/database.h @@ -20,7 +20,6 @@ #include "base/check_op.h" #include "base/component_export.h" #include "base/containers/flat_map.h" -#include "base/feature_list.h" #include "base/files/file_path.h" #include "base/functional/callback.h" #include "base/gtest_prod_util.h" @@ -36,7 +35,6 @@ #include "base/time/time.h" #include "base/types/pass_key.h" #include "sql/internal_api_token.h" -#include "sql/sql_features.h" #include "sql/sql_name_variants.h" #include "sql/sqlite_result_code.h" #include "sql/sqlite_result_code_values.h" @@ -280,8 +278,7 @@ struct COMPONENT_EXPORT(SQL) DatabaseOptions { bool exclusive_locking_ = true; bool exclusive_database_file_lock_ = false; - bool wal_mode_ = - base::FeatureList::IsEnabled(sql::features::kEnableWALModeByDefault); + bool wal_mode_ = false; bool flush_to_media_ = false; int page_size_ = kDefaultPageSize; int cache_size_ = 0; diff --git a/sql/recovery_unittest.cc b/sql/recovery_unittest.cc index d2d21751a44c7..eb34482f38cfb 100644 --- a/sql/recovery_unittest.cc +++ b/sql/recovery_unittest.cc @@ -31,11 +31,9 @@ #include "base/test/bind.h" #include "base/test/gtest_util.h" #include "base/test/metrics/histogram_tester.h" -#include "base/test/scoped_feature_list.h" #include "build/buildflag.h" #include "sql/database.h" #include "sql/meta_table.h" -#include "sql/sql_features.h" #include "sql/sqlite_result_code.h" #include "sql/sqlite_result_code_values.h" #include "sql/statement.h" @@ -70,8 +68,6 @@ class SqlRecoveryTest : public testing::Test, public: SqlRecoveryTest() : db_(DatabaseOptions().set_wal_mode(ShouldEnableWal()), test::kTestTag) { - scoped_feature_list_.InitWithFeatureStates( - {{features::kEnableWALModeByDefault, ShouldEnableWal()}}); } bool ShouldEnableWal() { return GetParam(); } @@ -106,7 +102,6 @@ class SqlRecoveryTest : public testing::Test, } protected: - base::test::ScopedFeatureList scoped_feature_list_; base::ScopedTempDir temp_dir_; base::FilePath db_path_; Database db_; diff --git a/sql/sql_features.cc b/sql/sql_features.cc index c7f56d9f2c537..fc84ffeb2972a 100644 --- a/sql/sql_features.cc +++ b/sql/sql_features.cc @@ -8,11 +8,6 @@ namespace sql::features { -// Enable WAL mode for all SQLite databases. -BASE_FEATURE(kEnableWALModeByDefault, - "EnableWALModeByDefault", - base::FEATURE_DISABLED_BY_DEFAULT); - // Use a fixed memory-map size instead of using the heuristic. BASE_FEATURE(kSqlFixedMmapSize, "SqlFixedMmapSize", diff --git a/sql/sql_features.h b/sql/sql_features.h index fe02159285bdb..55027c8250683 100644 --- a/sql/sql_features.h +++ b/sql/sql_features.h @@ -14,7 +14,6 @@ namespace sql::features { // be documented alongside the definition of their values in the .cc file. // Alphabetical: -COMPONENT_EXPORT(SQL) BASE_DECLARE_FEATURE(kEnableWALModeByDefault); COMPONENT_EXPORT(SQL) BASE_DECLARE_FEATURE(kSqlFixedMmapSize); COMPONENT_EXPORT(SQL) BASE_DECLARE_FEATURE(kUnlockDatabaseOnClose);