Add support for finch testing of kernel features

Kernel finch is a feature to test kernel changes. Add support to Chrome
to enable features available on the device. debugd is consulted for
querying and enabling features.

Tested by enabling features using features command line args to Chrome.

BUG=b:171811126

Change-Id: I03fe7b2477300cffb0f4f532e6f9667330bbc85e
Signed-off-by: Joel Fernandes <joelaf@google.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2728198
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Brian Geffon <bgeffon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#880953}

base/feature_list.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/feature_list.h"
+#include <string>
 
 // feature_list.h is a widely included header and its size impacts build
 // time. Try not to raise this limit unless necessary. See
@@ -13,9 +14,6 @@
 
 #include <stddef.h>
 
-#include <utility>
-#include <vector>
-
 #include "base/base_paths.h"
 #include "base/base_switches.h"
 #include "base/debug/alias.h"
@@ -533,15 +531,45 @@ bool FeatureList::IsFeatureEnabled(const Feature& feature) {
 
 FieldTrial* FeatureList::GetAssociatedFieldTrial(const Feature& feature) {
   DCHECK(initialized_);
-  DCHECK(IsValidFeatureOrFieldTrialName(feature.name)) << feature.name;
   DCHECK(CheckFeatureIdentity(feature)) << feature.name;
 
-  auto it = overrides_.find(feature.name);
+  return GetAssociatedFieldTrialByFeatureName(feature.name);
+}
+
+const base::FeatureList::OverrideEntry*
+FeatureList::GetOverrideEntryByFeatureName(StringPiece name) {
+  DCHECK(initialized_);
+  DCHECK(IsValidFeatureOrFieldTrialName(std::string(name))) << name;
+
+  auto it = overrides_.find(name);
   if (it != overrides_.end()) {
     const OverrideEntry& entry = it->second;
-    return entry.field_trial;
+    return &entry;
+  }
+  return nullptr;
+}
+
+FieldTrial* FeatureList::GetAssociatedFieldTrialByFeatureName(
+    StringPiece name) {
+  DCHECK(initialized_);
+
+  const base::FeatureList::OverrideEntry* entry =
+      GetOverrideEntryByFeatureName(name);
+  if (entry) {
+    return entry->field_trial;
   }
+  return nullptr;
+}
+
+FieldTrial* FeatureList::GetEnabledFieldTrialByFeatureName(StringPiece name) {
+  DCHECK(initialized_);
 
+  const base::FeatureList::OverrideEntry* entry =
+      GetOverrideEntryByFeatureName(name);
+  if (entry &&
+      entry->overridden_state == base::FeatureList::OVERRIDE_ENABLE_FEATURE) {
+    return entry->field_trial;
+  }
   return nullptr;
 }
 

base/feature_list.h

@@ -225,6 +225,14 @@ class BASE_EXPORT FeatureList {
   void GetCommandLineFeatureOverrides(std::string* enable_overrides,
                                       std::string* disable_overrides);
 
+  // Returns the field trial associated with the given feature |name|. Used for
+  // getting the FieldTrial without requiring a struct Feature.
+  base::FieldTrial* GetAssociatedFieldTrialByFeatureName(StringPiece name);
+
+  // Get associated field trial for the given feature |name| only if override
+  // enables it.
+  FieldTrial* GetEnabledFieldTrialByFeatureName(StringPiece name);
+
   // Returns whether the given |feature| is enabled. Must only be called after
   // the singleton instance has been registered via SetInstance(). Additionally,
   // a feature with a given name must only have a single corresponding Feature
@@ -313,6 +321,11 @@ class BASE_EXPORT FeatureList {
     OverrideEntry(OverrideState overridden_state, FieldTrial* field_trial);
   };
 
+  // Returns the override for the field trial associated with the given feature
+  // |name| or null if the feature is not found.
+  const base::FeatureList::OverrideEntry* GetOverrideEntryByFeatureName(
+      StringPiece name);
+
   // Finalizes the initialization state of the FeatureList, so that no further
   // overrides can be registered. This is called by SetInstance() on the
   // singleton feature list that is being registered.

chrome/browser/ash/system/kernel_feature_manager.cc

@@ -0,0 +1,101 @@
+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ash/system/kernel_feature_manager.h"
+
+#include <stdint.h>
+
+#include <string>
+
+#include "base/bind.h"
+#include "base/command_line.h"
+#include "base/feature_list.h"
+#include "base/metrics/field_trial.h"
+#include "base/metrics/field_trial_params.h"
+#include "base/strings/string_split.h"
+#include "chromeos/dbus/debug_daemon/debug_daemon_client.h"
+#include "third_party/cros_system_api/dbus/debugd/dbus-constants.h"
+
+namespace chromeos {
+
+KernelFeatureManager::KernelFeatureManager(
+    DebugDaemonClient* debug_daemon_client)
+    : debug_daemon_client_(debug_daemon_client) {
+  debug_daemon_client_->WaitForServiceToBeAvailable(
+      base::BindOnce(&KernelFeatureManager::OnDebugDaemonReady,
+                     weak_ptr_factory_.GetWeakPtr()));
+}
+
+KernelFeatureManager::~KernelFeatureManager() = default;
+
+void KernelFeatureManager::OnDebugDaemonReady(bool service_is_ready) {
+  if (!service_is_ready) {
+    LOG(ERROR) << "debugd service not ready";
+    return;
+  }
+
+  // Initialize the system.
+  debug_daemon_ready_ = true;
+  GetKernelFeatureList();
+}
+
+void KernelFeatureManager::GetKernelFeatureList() {
+  debug_daemon_client_->GetKernelFeatureList(
+      base::BindOnce(&KernelFeatureManager::OnKernelFeatureList,
+                     weak_ptr_factory_.GetWeakPtr()));
+}
+
+void KernelFeatureManager::OnKernelFeatureList(bool result,
+                                               const std::string& out) {
+  if (result) {
+    // Split the CSV into a feature list
+    kernel_feature_list_ = base::SplitString(out, ",", base::TRIM_WHITESPACE,
+                                             base::SPLIT_WANT_NONEMPTY);
+    if (!kernel_feature_list_.empty()) {
+      // Find out which of these features requested in Finch
+      EnableKernelFeatures();
+      return;
+    }
+  }
+  LOG(ERROR) << "Failed to get or parse kernel feature list from debugd.";
+}
+
+void KernelFeatureManager::EnableKernelFeatures() {
+  base::FeatureList* feature_list_instance = base::FeatureList::GetInstance();
+  DCHECK(feature_list_instance);
+
+  for (const auto& name : kernel_feature_list_) {
+    VLOG(1) << "Enabling kernel feature via debugd: " << name << std::endl;
+
+    // Was this feature requested in the field trial and also enabled?
+    // Note: We don't support dynamic disabling of kernel features right now.
+    // So any requests to disable a feature are ignored. Disabled is the
+    // default.
+    if (!feature_list_instance->GetEnabledFieldTrialByFeatureName(name)) {
+      continue;
+    }
+
+    debug_daemon_client_->KernelFeatureEnable(
+        name, base::BindOnce(&KernelFeatureManager::OnKernelFeatureEnable,
+                             weak_ptr_factory_.GetWeakPtr()));
+  }
+}
+
+void KernelFeatureManager::OnKernelFeatureEnable(bool result,
+                                                 const std::string& out) {
+  DCHECK(!result ||
+         base::FeatureList::GetInstance()->GetAssociatedFieldTrialByFeatureName(
+             out));
+
+  if (result) {
+    base::FeatureList::GetInstance()
+        ->GetAssociatedFieldTrialByFeatureName(out)
+        ->group();
+    VLOG(1) << "Kernel feature " << out << "activated successfully!";
+    return;
+  }
+  VLOG(1) << "Kernel feature has not been activated: " << out;
+}
+
+}  // namespace chromeos

chrome/browser/ash/system/kernel_feature_manager.h

@@ -0,0 +1,60 @@
+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_CHROMEOS_KERNEL_FEATURES_MANAGER_H_
+#define CHROME_BROWSER_CHROMEOS_KERNEL_FEATURES_MANAGER_H_
+
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "base/memory/weak_ptr.h"
+
+namespace chromeos {
+
+class DebugDaemonClient;
+
+// It is desirable to test kernel patches on a population to measure benefits
+// or problems with application of a kernel patch. For experimentation,
+// a patch or tuning can be applied to all kernels, and finch will enable it on
+// subset of the user population. An experiment has an ‘enablement method’.
+// This method can involve writing to sysfs, procfs entry, or executing
+// other commands, via debugd. This class manages the enabling of these
+// kernel experiments, when Chrome starts, via ChromeOS's debugd. The behavior
+// of the running kernel is changed live without requiring a restart.
+class KernelFeatureManager {
+ public:
+  explicit KernelFeatureManager(DebugDaemonClient* debug_daemon_client);
+  KernelFeatureManager(const KernelFeatureManager&) = delete;
+  KernelFeatureManager& operator=(const KernelFeatureManager&) = delete;
+  ~KernelFeatureManager();
+
+ private:
+  void OnDebugDaemonReady(bool service_is_ready);
+
+  // Get a list of kernel features that debugd can enable. The list of features
+  // are passed to the callback in |out| as a comma separate value string, or
+  // an error string containing the reason for failure. |result| contains true
+  // or false depending on if the request succeeds or fails.
+  void GetKernelFeatureList();
+  void OnKernelFeatureList(bool result, const std::string& out);
+
+  // Enable all kernel features that were requested to be enabled in field trial
+  // experiments. A callback is called for each feature that is enabled, with
+  // |result| containing true or false depending on if the request succeeds or
+  // fails. |out| contains an error string on failure and name of the feature on
+  // success.
+  void EnableKernelFeatures();
+  void OnKernelFeatureEnable(bool result, const std::string& out);
+
+  DebugDaemonClient* const debug_daemon_client_;
+  bool debug_daemon_ready_ = false;
+  std::vector<std::string> kernel_feature_list_;
+
+  base::WeakPtrFactory<KernelFeatureManager> weak_ptr_factory_{this};
+};
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_KERNEL_FEATURES_MANAGER_H_

chrome/browser/ash/system/kernel_feature_manager_unittest.cc

@@ -0,0 +1,113 @@
+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ash/system/kernel_feature_manager.h"
+
+#include "base/test/scoped_feature_list.h"
+#include "base/test/scoped_field_trial_list_resetter.h"
+#include "base/test/task_environment.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "chromeos/dbus/debug_daemon/fake_debug_daemon_client.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+class TestFakeDebugDaemonClient : public chromeos::FakeDebugDaemonClient {
+ public:
+  void GetKernelFeatureList(KernelFeatureListCallback callback) override {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::BindOnce(std::move(callback), true,
+                                  "TrialExample1,TrialExample2,TrialExample3"));
+  }
+
+  void KernelFeatureEnable(const std::string& name,
+                           KernelFeatureListCallback callback) override {
+    bool result = false;
+    std::string out;
+
+    // Hardcode the behavior of different trials.
+    if (name == "TrialExample1") {
+      result = true;
+      out = "TrialExample1";
+    } else if (name == "TrialExample2") {
+      out = "Device does not support";
+    } else if (name == "TrialExample3") {
+      out = "Disable is the default, not doing anything";
+    } else {
+      out = "Unknown feature requested to be enabled";
+    }
+
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::BindOnce(std::move(callback), result, out));
+  }
+};
+
+class KernelFeatureManagerTest : public testing::Test {
+ public:
+  KernelFeatureManagerTest() {
+    // Provide an empty FeatureList to each test by default.
+    scoped_feature_list_.InitWithFeatureList(
+        std::make_unique<base::FeatureList>());
+  }
+  KernelFeatureManagerTest(const KernelFeatureManagerTest&) = delete;
+  KernelFeatureManagerTest& operator=(const KernelFeatureManagerTest&) = delete;
+  ~KernelFeatureManagerTest() override = default;
+
+  TestFakeDebugDaemonClient debug_daemon_client_;
+  base::test::TaskEnvironment task_environment_;
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+TEST_F(KernelFeatureManagerTest, EnableIfDeviceSupports) {
+  base::test::ScopedFieldTrialListResetter resetter;
+  base::FieldTrialList field_trial_list(nullptr);
+  auto feature_list = std::make_unique<base::FeatureList>();
+
+  // Feature that the device supports and we enable.
+  base::FieldTrial* trial1 =
+      base::FieldTrialList::CreateFieldTrial("TrialExample1", "A");
+
+  // Feature that the device lists but does not support and we want to enable.
+  base::FieldTrial* trial2 =
+      base::FieldTrialList::CreateFieldTrial("TrialExample2", "B");
+
+  // Feature that the device supports and we try to disable (note that disable
+  // is the default so this really should not do anything. We test that
+  // the code is not buggy enough to accidentally enable it.
+  base::FieldTrial* trial3 =
+      base::FieldTrialList::CreateFieldTrial("TrialExample3", "C");
+
+  // Feature that the device does not list or support and we want to enable.
+  base::FieldTrial* trial4 =
+      base::FieldTrialList::CreateFieldTrial("TrialExample4", "D");
+
+  feature_list->RegisterFieldTrialOverride(
+      "TrialExample1", base::FeatureList::OVERRIDE_ENABLE_FEATURE, trial1);
+  feature_list->RegisterFieldTrialOverride(
+      "TrialExample2", base::FeatureList::OVERRIDE_ENABLE_FEATURE, trial2);
+  feature_list->RegisterFieldTrialOverride(
+      "TrialExample3", base::FeatureList::OVERRIDE_DISABLE_FEATURE, trial3);
+  feature_list->RegisterFieldTrialOverride(
+      "TrialExample4", base::FeatureList::OVERRIDE_ENABLE_FEATURE, trial4);
+
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitWithFeatureList(std::move(feature_list));
+
+  // Initially, no trial should be active.
+  EXPECT_FALSE(base::FieldTrialList::IsTrialActive(trial1->trial_name()));
+  EXPECT_FALSE(base::FieldTrialList::IsTrialActive(trial2->trial_name()));
+  EXPECT_FALSE(base::FieldTrialList::IsTrialActive(trial3->trial_name()));
+  EXPECT_FALSE(base::FieldTrialList::IsTrialActive(trial4->trial_name()));
+
+  chromeos::KernelFeatureManager manager(&debug_daemon_client_);
+  debug_daemon_client_.SetServiceIsAvailable(true);
+  task_environment_.RunUntilIdle();
+
+  EXPECT_TRUE(base::FieldTrialList::IsTrialActive(trial1->trial_name()));
+  EXPECT_FALSE(base::FieldTrialList::IsTrialActive(trial2->trial_name()));
+  EXPECT_FALSE(base::FieldTrialList::IsTrialActive(trial3->trial_name()));
+  EXPECT_FALSE(base::FieldTrialList::IsTrialActive(trial4->trial_name()));
+}
+
+}  // namespace

chrome/browser/browser_process_platform_part_chromeos.cc

@@ -22,6 +22,7 @@
 #include "chrome/browser/ash/system/automatic_reboot_manager.h"
 #include "chrome/browser/ash/system/device_disabling_manager.h"
 #include "chrome/browser/ash/system/device_disabling_manager_default_delegate.h"
+#include "chrome/browser/ash/system/kernel_feature_manager.h"
 #include "chrome/browser/ash/system/system_clock.h"
 #include "chrome/browser/ash/system/timezone_resolver_manager.h"
 #include "chrome/browser/ash/system/timezone_util.h"
@@ -263,6 +264,16 @@ void BrowserProcessPlatformPart::ShutdownSchedulerConfigurationManager() {
   scheduler_configuration_manager_.reset();
 }
 
+void BrowserProcessPlatformPart::InitializeKernelFeatureManager() {
+  DCHECK(!kernel_feature_manager_);
+  kernel_feature_manager_ = std::make_unique<chromeos::KernelFeatureManager>(
+      chromeos::DBusThreadManager::Get()->GetDebugDaemonClient());
+}
+
+void BrowserProcessPlatformPart::ShutdownKernelFeatureManager() {
+  kernel_feature_manager_.reset();
+}
+
 void BrowserProcessPlatformPart::InitializePrimaryProfileServices(
     Profile* primary_profile) {
   DCHECK(primary_profile);