Disable Glic in response to policy
In https://crrev.com/c/6142513 we added an enterprise policy to allow
admins to disable Glic. That CL added the policy and mapped it to a
pref but didn't connect the pref to any behavior.
This CL implements the pref's behavior by disabling Glic in any profile
that's disabled by this policy. It also changes the background mode
manager to only enter background mode if there's any profile loaded that
doesn't have a disabling policy.
Bug: 382722218
Change-Id: I06b947bb363791c407d39524fed65d819e82a4e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6141955
Reviewed-by: Erik Chen <[email protected]>
Reviewed-by: Carlos Knippschild <[email protected]>
Commit-Queue: David Bokan <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1404912}
diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index 7f47cc35..375bf3c9 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -471,6 +471,8 @@
tearing_down_ = true;
DCHECK(IsShuttingDown());
+ features_->Shutdown();
+
// TODO(https://crbug.com/388906971): fix dead code below.
#if BUILDFLAG(IS_ANDROID)
accessibility_prefs_controller_.reset();
diff --git a/chrome/browser/glic/BUILD.gn b/chrome/browser/glic/BUILD.gn
index e5407d7a..babf5ff8 100644
--- a/chrome/browser/glic/BUILD.gn
+++ b/chrome/browser/glic/BUILD.gn
@@ -57,6 +57,7 @@
":glic",
"//chrome/browser:browser_process",
"//chrome/browser:global_features",
+ "//chrome/browser/glic/launcher",
"//chrome/browser/media/webrtc",
"//chrome/browser/ui:browser_element_identifiers",
"//chrome/browser/ui:browser_list",
@@ -79,7 +80,11 @@
"glic_enabling.cc",
"glic_enabling.h",
]
- deps = [ "//chrome/browser/profiles:profile" ]
+ deps = [
+ ":glic",
+ "//chrome/browser/profiles:profile",
+ "//components/prefs",
+ ]
public_deps = [
"//base",
"//chrome/common:chrome_features",
@@ -118,7 +123,9 @@
":glic",
"//chrome/browser",
"//chrome/browser:browser_process",
+ "//chrome/browser:global_features",
"//chrome/browser/extensions:extensions",
+ "//chrome/browser/glic/launcher",
"//chrome/browser/policy:test_support",
"//chrome/browser/ui",
"//chrome/common:constants",
diff --git a/chrome/browser/glic/glic_enabling.cc b/chrome/browser/glic/glic_enabling.cc
index 12d7f6d..c78bbc7e 100644
--- a/chrome/browser/glic/glic_enabling.cc
+++ b/chrome/browser/glic/glic_enabling.cc
@@ -4,8 +4,10 @@
#include "chrome/browser/glic/glic_enabling.h"
+#include "chrome/browser/glic/glic_pref_names.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/chrome_features.h"
+#include "components/prefs/pref_service.h"
bool GlicEnabling::IsEnabledByFlags() {
return CheckEnabling() == glic::GlicEnabledStatus::kEnabled;
@@ -18,14 +20,13 @@
return false;
}
- // Glic is not supported from incognito or guest mode.
- if (profile->IsOffTheRecord()) {
+ // Glic is supported only in regular profiles, i.e. disable in incognito,
+ // guest, system profile, etc.
+ if (!profile->IsRegularProfile()) {
return false;
}
- // TODO(crbug.com/382722218): Enterprise policy may disable Glic for certain
- // user profiles.
- return true;
+ return profile->GetPrefs()->GetBoolean(glic::prefs::kGlicEnabledByPolicy);
}
glic::GlicEnabledStatus GlicEnabling::CheckEnabling() {
diff --git a/chrome/browser/glic/glic_policy_browsertest.cc b/chrome/browser/glic/glic_policy_browsertest.cc
index 8fb0a84..1829135 100644
--- a/chrome/browser/glic/glic_policy_browsertest.cc
+++ b/chrome/browser/glic/glic_policy_browsertest.cc
@@ -2,10 +2,23 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include "chrome/browser/browser_process.h"
#include "chrome/browser/glic/glic_pref_names.h"
+#include "chrome/browser/glic/launcher/glic_background_mode_manager.h"
+#include "chrome/browser/global_features.h"
#include "chrome/browser/policy/policy_test_utils.h"
+#include "chrome/browser/policy/profile_policy_connector.h"
+#include "chrome/browser/policy/profile_policy_connector_builder.h"
#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/profiles/profile_test_util.h"
#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/frame/tab_strip_region_view.h"
+#include "chrome/browser/ui/views/tabs/tab_strip_action_container.h"
+#include "chrome/common/chrome_features.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/test/base/ui_test_utils.h"
#include "components/policy/core/common/policy_types.h"
#include "components/policy/policy_constants.h"
#include "components/prefs/pref_service.h"
@@ -13,9 +26,84 @@
using glic::prefs::kGlicEnabledByPolicy;
+namespace glic {
+class GlicButton;
+}
+
namespace policy {
-class GlicPolicyTest : public PolicyTest {};
+class GlicPolicyTest : public PolicyTest {
+ public:
+ GlicPolicyTest() {
+ scoped_feature_list_.InitWithFeatures(
+ /*enabled_features=*/{features::kGlic, features::kTabstripComboButton},
+ /*disabled_features=*/{});
+ }
+ GlicPolicyTest(const GlicPolicyTest&) = delete;
+ GlicPolicyTest& operator=(const GlicPolicyTest&) = delete;
+
+ ~GlicPolicyTest() override = default;
+
+ void SetUpCommandLine(base::CommandLine* command_line) override {
+ PolicyTest::SetUpCommandLine(command_line);
+
+ // Load blank page in glic guest view
+ command_line->AppendSwitchASCII(::switches::kGlicGuestURL, "about:blank");
+ }
+
+ void SetUpOnMainThread() override {
+ PolicyTest::SetUpOnMainThread();
+
+ g_browser_process->local_state()->SetBoolean(
+ glic::prefs::kGlicLauncherEnabled, true);
+
+ profile_1_ = browser()->profile();
+
+ {
+ policy_for_profile_2_.SetDefaultReturns(
+ /*is_initialization_complete_return=*/true,
+ /*is_first_policy_load_complete_return=*/true);
+ policy::PushProfilePolicyConnectorProviderForTesting(
+ &policy_for_profile_2_);
+
+ ProfileManager* profile_manager = g_browser_process->profile_manager();
+ base::FilePath new_path =
+ profile_manager->GenerateNextProfileDirectoryPath();
+ profile_2_ =
+ &profiles::testing::CreateProfileSync(profile_manager, new_path);
+ }
+ }
+
+ void TearDownOnMainThread() override {
+ if (glic::GlicBackgroundModeManager* background_mode_manager =
+ g_browser_process->GetFeatures()->glic_background_mode_manager()) {
+ background_mode_manager->TerminateForTesting();
+ }
+ profile_1_ = nullptr;
+ profile_2_ = nullptr;
+ }
+
+ glic::GlicButton* GetGlicButtonForBrowser(Browser* browser) {
+ TabStripActionContainer* container =
+ BrowserView::GetBrowserViewForBrowser(browser)
+ ->tab_strip_region_view()
+ ->GetTabStripActionContainer();
+ CHECK(container);
+ return container->GetGlicButton();
+ }
+
+ protected:
+ // The first profile.
+ raw_ptr<Profile> profile_1_;
+ // The second profile.
+ raw_ptr<Profile> profile_2_;
+
+ testing::NiceMock<policy::MockConfigurationPolicyProvider>
+ policy_for_profile_2_;
+
+ private:
+ base::test::ScopedFeatureList scoped_feature_list_;
+};
IN_PROC_BROWSER_TEST_F(GlicPolicyTest, PrefDisabledByPolicy) {
// By default the pref should start off unmanaged and defaulted to enabled.
@@ -36,4 +124,108 @@
EXPECT_FALSE(prefs->GetBoolean(kGlicEnabledByPolicy));
}
+// Ensure that when policy disables Glic, a browser window doesn't show the Glic
+// button.
+IN_PROC_BROWSER_TEST_F(GlicPolicyTest, PolicyDisablesGlicButton) {
+ ASSERT_EQ(browser()->profile(), profile_1_);
+ ASSERT_NE(profile_1_, profile_2_);
+
+ // The pref defaults to enabled. Ensure the button was created.
+ PrefService* prefs = browser()->profile()->GetPrefs();
+ ASSERT_TRUE(prefs->GetBoolean(kGlicEnabledByPolicy));
+ EXPECT_TRUE(GetGlicButtonForBrowser(browser()));
+
+ // Disable the policy in the default profile.
+ PolicyMap policies;
+ policies.Set(key::kGlicEnabled, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+ POLICY_SOURCE_ENTERPRISE_DEFAULT, base::Value(false), nullptr);
+ UpdateProviderPolicy(policies);
+ ASSERT_FALSE(prefs->GetBoolean(kGlicEnabledByPolicy));
+
+ {
+ // A new window in profile 1 shouldn't have the Glic button.
+ Browser* new_window_profile_1 = CreateBrowser(profile_1_);
+ EXPECT_FALSE(GetGlicButtonForBrowser(new_window_profile_1));
+
+ // A new window in profile 2 should continue to have the Glic button since
+ // only profile 1 disabled Glic.
+ Browser* new_window_profile_2 = CreateBrowser(profile_2_);
+ EXPECT_TRUE(GetGlicButtonForBrowser(new_window_profile_2));
+ }
+
+ // TODO(crbug.com/382722218): The button should be removed from and added to
+ // existing windows but this isn't implemented yet.
+
+ // Re-enable the policy. Ensure a new window gets the button again.
+ policies.Set(key::kGlicEnabled, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+ POLICY_SOURCE_ENTERPRISE_DEFAULT, base::Value(true), nullptr);
+ UpdateProviderPolicy(policies);
+ ASSERT_TRUE(prefs->GetBoolean(kGlicEnabledByPolicy));
+
+ {
+ // A new window in profile 1 should again get the Glic button now that the
+ // policy is re-enabled.
+ Browser* new_window_profile_1 = CreateBrowser(profile_1_);
+ EXPECT_TRUE(GetGlicButtonForBrowser(new_window_profile_1));
+ }
+}
+
+// Ensure that background mode is entered if and only if a profile with the
+// policy enabled is loaded.
+IN_PROC_BROWSER_TEST_F(GlicPolicyTest, PolicyDisablesBackgroundMode) {
+ ASSERT_EQ(browser()->profile(), profile_1_);
+ ASSERT_NE(profile_1_, profile_2_);
+
+ Browser* new_window_profile_2 = CreateBrowser(profile_2_);
+ ASSERT_TRUE(new_window_profile_2);
+
+ // The pref defaults to enabled.
+ ASSERT_TRUE(profile_1_->GetPrefs()->GetBoolean(kGlicEnabledByPolicy));
+ ASSERT_TRUE(profile_2_->GetPrefs()->GetBoolean(kGlicEnabledByPolicy));
+
+ glic::GlicBackgroundModeManager* background_mode_manager =
+ g_browser_process->GetFeatures()->glic_background_mode_manager();
+ EXPECT_TRUE(background_mode_manager->IsInBackgroundModeForTesting());
+
+ // Disable the policy in the default profile.
+ {
+ PolicyMap policies;
+ policies.Set(key::kGlicEnabled, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+ POLICY_SOURCE_ENTERPRISE_DEFAULT, base::Value(false), nullptr);
+ UpdateProviderPolicy(policies);
+ ASSERT_FALSE(profile_1_->GetPrefs()->GetBoolean(kGlicEnabledByPolicy));
+ ASSERT_TRUE(profile_2_->GetPrefs()->GetBoolean(kGlicEnabledByPolicy));
+ }
+
+ // Background mode should remain active since profile_2_ still has it enabled.
+ EXPECT_TRUE(background_mode_manager->IsInBackgroundModeForTesting());
+
+ // Disable the policy in the second profile.
+ {
+ PolicyMap policies;
+ policies.Set(key::kGlicEnabled, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+ POLICY_SOURCE_ENTERPRISE_DEFAULT, base::Value(false), nullptr);
+ policy_for_profile_2_.UpdateChromePolicy(policies);
+ ASSERT_FALSE(profile_1_->GetPrefs()->GetBoolean(kGlicEnabledByPolicy));
+ ASSERT_FALSE(profile_2_->GetPrefs()->GetBoolean(kGlicEnabledByPolicy));
+ }
+
+ // Background mode should be exited since none of the loaded profiles enable
+ // Glic.
+ EXPECT_FALSE(background_mode_manager->IsInBackgroundModeForTesting());
+
+ // Enable the policy in the default profile again.
+ {
+ PolicyMap policies;
+ policies.Set(key::kGlicEnabled, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+ POLICY_SOURCE_ENTERPRISE_DEFAULT, base::Value(true), nullptr);
+ UpdateProviderPolicy(policies);
+ ASSERT_TRUE(profile_1_->GetPrefs()->GetBoolean(kGlicEnabledByPolicy));
+ ASSERT_FALSE(profile_2_->GetPrefs()->GetBoolean(kGlicEnabledByPolicy));
+ }
+
+ // Background mode should be reentered since the first profile is enabled.
+ EXPECT_TRUE(background_mode_manager->IsInBackgroundModeForTesting());
+}
+
} // namespace policy
diff --git a/chrome/browser/glic/glic_profile_configuration.cc b/chrome/browser/glic/glic_profile_configuration.cc
index 837e0335..22338839 100644
--- a/chrome/browser/glic/glic_profile_configuration.cc
+++ b/chrome/browser/glic/glic_profile_configuration.cc
@@ -7,6 +7,7 @@
#include "base/values.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/glic/glic_pref_names.h"
+#include "chrome/browser/glic/launcher/glic_background_mode_manager.h"
#include "chrome/browser/profiles/profile.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h"
@@ -43,8 +44,7 @@
void GlicProfileConfiguration::OnEnabledByPolicyChanged() {
// TODO(crbug.com/382722218): Update UI in each window to remove/add Glic
// button.
- // TODO(crbug.com/382722218): Update background mode in response to changed
- // policy.
+ GlicBackgroundModeManager::GetInstance()->OnPolicyChanged();
}
} // namespace glic
diff --git a/chrome/browser/glic/glic_profile_manager.cc b/chrome/browser/glic/glic_profile_manager.cc
index 4c5b36aa..c016d0e 100644
--- a/chrome/browser/glic/glic_profile_manager.cc
+++ b/chrome/browser/glic/glic_profile_manager.cc
@@ -40,6 +40,8 @@
Profile* GlicProfileManager::GetProfileForLaunch() {
// TODO(https://crbug.com/379165457): Implement profile choice logic.
+ // TODO(crbug.com/382722218): This needs to avoid using a profile that's been
+ // disabled via enterprise policy.
return ProfileManager::GetLastUsedProfileAllowedByPolicy();
}
diff --git a/chrome/browser/glic/launcher/BUILD.gn b/chrome/browser/glic/launcher/BUILD.gn
index f4afc1c..77b3ee4 100644
--- a/chrome/browser/glic/launcher/BUILD.gn
+++ b/chrome/browser/glic/launcher/BUILD.gn
@@ -11,6 +11,7 @@
deps = [
"//base",
+ "//chrome/browser/profiles:profile",
"//components/prefs",
"//ui/base",
"//ui/base/accelerators/global_accelerator_listener",
@@ -34,7 +35,10 @@
"//chrome/app:generated_resources",
"//chrome/app/vector_icons",
"//chrome/browser:browser_process",
+ "//chrome/browser:global_features",
"//chrome/browser/glic",
+ "//chrome/browser/glic:enabling",
+ "//chrome/browser/profiles:profile",
"//chrome/browser/ui",
"//chrome/browser/ui:browser_element_identifiers",
"//components/keep_alive_registry",
diff --git a/chrome/browser/glic/launcher/glic_background_mode_manager.cc b/chrome/browser/glic/launcher/glic_background_mode_manager.cc
index b3f8a08..254247e9 100644
--- a/chrome/browser/glic/launcher/glic_background_mode_manager.cc
+++ b/chrome/browser/glic/launcher/glic_background_mode_manager.cc
@@ -8,35 +8,51 @@
#include "base/check.h"
#include "chrome/browser/browser_process.h"
+#include "chrome/browser/glic/glic_enabling.h"
#include "chrome/browser/glic/launcher/glic_controller.h"
#include "chrome/browser/glic/launcher/glic_launcher_configuration.h"
#include "chrome/browser/glic/launcher/glic_status_icon.h"
+#include "chrome/browser/global_features.h"
+#include "chrome/browser/profiles/profile_manager.h"
#include "components/keep_alive_registry/keep_alive_types.h"
#include "components/keep_alive_registry/scoped_keep_alive.h"
#include "ui/base/accelerators/accelerator.h"
#include "ui/base/accelerators/global_accelerator_listener/global_accelerator_listener.h"
+namespace {
+bool IsEnabledInAnyLoadedProfile() {
+ return base::ranges::any_of(
+ g_browser_process->profile_manager()->GetLoadedProfiles(),
+ GlicEnabling::IsEnabledForProfile);
+}
+} // namespace
+
namespace glic {
GlicBackgroundModeManager::GlicBackgroundModeManager(StatusTray* status_tray)
: configuration_(std::make_unique<GlicLauncherConfiguration>(this)),
controller_(std::make_unique<GlicController>()),
status_tray_(status_tray),
- enabled_(configuration_->IsEnabled()),
+ enabled_pref_(configuration_->IsEnabled()),
expected_registered_hotkey_(configuration_->GetGlobalHotkey()) {
UpdateState();
+ g_browser_process->profile_manager()->AddObserver(this);
}
GlicBackgroundModeManager::~GlicBackgroundModeManager() = default;
+GlicBackgroundModeManager* GlicBackgroundModeManager::GetInstance() {
+ return g_browser_process->GetFeatures()->glic_background_mode_manager();
+}
+
void GlicBackgroundModeManager::OnEnabledChanged(bool enabled) {
- if (enabled_ == enabled) {
+ if (enabled_pref_ == enabled) {
return;
}
- enabled_ = enabled;
+ enabled_pref_ = enabled;
UpdateState();
- EnableLaunchOnStartup(enabled_);
+ EnableLaunchOnStartup(enabled_pref_);
}
void GlicBackgroundModeManager::OnGlobalHotkeyChanged(ui::Accelerator hotkey) {
@@ -61,6 +77,26 @@
// TODO(crbug.com/385194502): Handle Linux.
}
+void GlicBackgroundModeManager::OnProfileAdded(Profile* profile) {
+ // If a profile is added when not in background mode, check if it can now be
+ // entered.
+ if (!status_icon_) {
+ CHECK(!keep_alive_);
+ UpdateState();
+ }
+}
+
+void GlicBackgroundModeManager::OnPolicyChanged() {
+ // Recompute whether the background launcher should change state based on the
+ // updated policy.
+ UpdateState();
+}
+
+void GlicBackgroundModeManager::Shutdown() {
+ CHECK(g_browser_process->profile_manager());
+ g_browser_process->profile_manager()->RemoveObserver(this);
+}
+
void GlicBackgroundModeManager::EnterBackgroundMode() {
if (!keep_alive_) {
keep_alive_ = std::make_unique<ScopedKeepAlive>(
@@ -104,7 +140,9 @@
void GlicBackgroundModeManager::UpdateState() {
UnregisterHotkey();
- if (enabled_) {
+
+ bool background_mode_enabled = enabled_pref_ && IsEnabledInAnyLoadedProfile();
+ if (background_mode_enabled) {
EnterBackgroundMode();
if (!expected_registered_hotkey_.IsEmpty()) {
RegisterHotkey(expected_registered_hotkey_);
diff --git a/chrome/browser/glic/launcher/glic_background_mode_manager.h b/chrome/browser/glic/launcher/glic_background_mode_manager.h
index 65fee5c..4e1c581f 100644
--- a/chrome/browser/glic/launcher/glic_background_mode_manager.h
+++ b/chrome/browser/glic/launcher/glic_background_mode_manager.h
@@ -9,6 +9,7 @@
#include "base/memory/raw_ptr.h"
#include "chrome/browser/glic/launcher/glic_launcher_configuration.h"
+#include "chrome/browser/profiles/profile_manager_observer.h"
#include "ui/base/accelerators/global_accelerator_listener/global_accelerator_listener.h"
class GlicController;
@@ -28,11 +29,14 @@
// listen to a global hotkey, and provide a status icon for triggering the UI.
class GlicBackgroundModeManager
: public GlicLauncherConfiguration::Observer,
- public ui::GlobalAcceleratorListener::Observer {
+ public ui::GlobalAcceleratorListener::Observer,
+ public ProfileManagerObserver {
public:
explicit GlicBackgroundModeManager(StatusTray* status_tray);
~GlicBackgroundModeManager() override;
+ static GlicBackgroundModeManager* GetInstance();
+
// GlicConfiguration::Observer
void OnEnabledChanged(bool enabled) override;
void OnGlobalHotkeyChanged(ui::Accelerator hotkey) override;
@@ -42,10 +46,27 @@
void ExecuteCommand(const std::string& accelerator_group_id,
const std::string& command_id) override;
+ // ProfileManagerObserver:
+ void OnProfileAdded(Profile* profile) override;
+
+ // Called when the enterprise policy-linked pref has changed for any profile.
+ void OnPolicyChanged();
+
+ void Shutdown();
+
ui::Accelerator RegisteredHotkeyForTesting() {
return actual_registered_hotkey_;
}
+ bool IsInBackgroundModeForTesting() {
+ CHECK_EQ(static_cast<bool>(keep_alive_), static_cast<bool>(status_icon_));
+ return keep_alive_ != nullptr;
+ }
+
+ // Tests need a way to manually exit background mode so that the test can
+ // complete.
+ void TerminateForTesting() { ExitBackgroundMode(); }
+
private:
void EnterBackgroundMode();
void ExitBackgroundMode();
@@ -69,7 +90,12 @@
// mode is enabled.
std::unique_ptr<GlicStatusIcon> status_icon_;
- bool enabled_ = false;
+ // The current state of the launcher_enabled pref. Note that the pref is a
+ // local state and is thus per-installation. Each profile also has an
+ // "enabled_by_policy". Background mode is entered only if `enabled_pref` is
+ // true AND at least one loaded profile is enabled by policy.
+ bool enabled_pref_ = false;
+
// The actual registered hotkey may be different from the expected hotkey
// because the Glic launcher may be disabled or registration fails which
// results in no hotkey being registered and is represented with an empty
diff --git a/chrome/browser/global_features.cc b/chrome/browser/global_features.cc
index a2c7e61..da4e1b3 100644
--- a/chrome/browser/global_features.cc
+++ b/chrome/browser/global_features.cc
@@ -74,6 +74,14 @@
#endif
}
+void GlobalFeatures::Shutdown() {
+#if BUILDFLAG(ENABLE_GLIC)
+ if (glic_background_mode_manager_) {
+ glic_background_mode_manager_->Shutdown();
+ }
+#endif
+}
+
std::unique_ptr<system_permission_settings::PlatformHandle>
GlobalFeatures::CreateSystemPermissionsPlatformHandle() {
return system_permission_settings::PlatformHandle::Create();
diff --git a/chrome/browser/global_features.h b/chrome/browser/global_features.h
index 7a7ee16..068eb0a 100644
--- a/chrome/browser/global_features.h
+++ b/chrome/browser/global_features.h
@@ -45,6 +45,9 @@
// Called exactly once to initialize features.
void Init();
+ // Called exactly once when the browser starts to shutdown.
+ void Shutdown();
+
// Public accessors for features, e.g.
// FooFeature* foo_feature() { return foo_feature_.get(); }