Skip to content

Commit

Permalink
libassistant-dlc: Add feature flag
Browse files Browse the repository at this point in the history
If the libassistant.so is not available by DLC, will disable Assistant.

Bug: b:272370865
Test: Tested manually with flag on/off
Change-Id: I2d24b0cb51af91faa28b9b607d2ad1ea9cc653ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4330571
Commit-Queue: Tao Wu <wutao@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1116020}
  • Loading branch information
Tao Wu authored and Chromium LUCI CQ committed Mar 11, 2023
1 parent d91abf4 commit 176fd82
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 1 deletion.
3 changes: 3 additions & 0 deletions ash/accelerators/accelerator_commands.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1150,6 +1150,9 @@ void ToggleAssistant() {
case AssistantAllowedState::DISALLOWED_BY_KIOSK_MODE:
// No need to show toast in KIOSK mode.
return;
case AssistantAllowedState::DISALLOWED_BY_NO_BINARY:
// No need to show toast.
return;
case AssistantAllowedState::ALLOWED:
// Nothing need to do if allowed.
break;
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/ash/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -5883,6 +5883,7 @@ source_set("unit_tests") {
"//chromeos/ash/components/trash_service",
"//chromeos/ash/components/trash_service/public/cpp",
"//chromeos/ash/components/trash_service/public/mojom",
"//chromeos/ash/services/assistant/public/cpp",
"//chromeos/ash/services/cros_healthd/public/cpp",
"//chromeos/ash/services/cros_healthd/public/mojom",
"//chromeos/ash/services/cros_healthd/public/mojom:mojom_shared",
Expand Down
6 changes: 6 additions & 0 deletions chrome/browser/ash/assistant/assistant_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/identity_manager_factory.h"
#include "chromeos/ash/services/assistant/public/cpp/assistant_prefs.h"
#include "chromeos/ash/services/assistant/public/cpp/features.h"
#include "components/language/core/browser/pref_names.h"
#include "components/prefs/pref_service.h"
#include "components/signin/public/identity_manager/identity_manager.h"
Expand Down Expand Up @@ -132,6 +133,11 @@ bool HasDedicatedAssistantKey() {
namespace assistant {

AssistantAllowedState IsAssistantAllowedForProfile(const Profile* profile) {
// Disabled because the libassistant.so is not available.
if (!ash::assistant::features::IsLibAssistantDLCEnabled()) {
return AssistantAllowedState::DISALLOWED_BY_NO_BINARY;
}

// Primary account might be missing during unittests.
if (!HasPrimaryAccount(profile))
return AssistantAllowedState::DISALLOWED_BY_NONPRIMARY_USER;
Expand Down
27 changes: 27 additions & 0 deletions chrome/browser/ash/assistant/assistant_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@

#include "base/command_line.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/scoped_feature_list.h"
#include "chrome/browser/ash/login/demo_mode/demo_session.h"
#include "chrome/browser/ash/login/users/fake_chrome_user_manager.h"
#include "chrome/browser/signin/identity_test_environment_profile_adaptor.h"
#include "chrome/test/base/testing_browser_process.h"
#include "chrome/test/base/testing_profile.h"
#include "chrome/test/base/testing_profile_manager.h"
#include "chromeos/ash/components/install_attributes/stub_install_attributes.h"
#include "chromeos/ash/services/assistant/public/cpp/features.h"
#include "components/account_id/account_id.h"
#include "components/language/core/browser/pref_names.h"
#include "components/prefs/testing_pref_service.h"
Expand Down Expand Up @@ -256,6 +258,9 @@ class ChromeAssistantUtilTest : public testing::Test {
return GetFakeUserManager()->GetGuestAccountId();
}

protected:
base::test::ScopedFeatureList feature_list_;

private:
content::BrowserTaskEnvironment task_environment_;
base::ScopedTempDir data_dir_;
Expand Down Expand Up @@ -406,4 +411,26 @@ TEST_F(ChromeAssistantUtilTest, IsAssistantAllowedForKiosk_WebKioskApp) {
IsAssistantAllowedForProfile(profile()));
}

TEST_F(ChromeAssistantUtilTest, IsAssistantAllowed_DLCEnabled) {
feature_list_.InitAndEnableFeature(
ash::assistant::features::kEnableLibAssistantDLC);

ScopedLogIn login(GetFakeUserManager(), identity_test_env(),
GetGaiaUserAccountId("user2@googlemail.com", "0123456789"));

EXPECT_EQ(ash::assistant::AssistantAllowedState::ALLOWED,
IsAssistantAllowedForProfile(profile()));
}

TEST_F(ChromeAssistantUtilTest, IsAssistantAllowed_DLCDisabled) {
feature_list_.InitAndDisableFeature(
ash::assistant::features::kEnableLibAssistantDLC);

ScopedLogIn login(GetFakeUserManager(), identity_test_env(),
GetGaiaUserAccountId("user2@googlemail.com", "0123456789"));

EXPECT_EQ(ash::assistant::AssistantAllowedState::DISALLOWED_BY_NO_BINARY,
IsAssistantAllowedForProfile(profile()));
}

} // namespace assistant
4 changes: 3 additions & 1 deletion chromeos/ash/services/assistant/public/cpp/assistant_enums.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@ enum AssistantAllowedState {
DISALLOWED_BY_ACCOUNT_TYPE = 8,
// Disallowed because the device is in Kiosk mode.
DISALLOWED_BY_KIOSK_MODE = 9,
// Disallowed because no libassistant binary available.
DISALLOWED_BY_NO_BINARY = 10,

MAX_VALUE = DISALLOWED_BY_KIOSK_MODE,
MAX_VALUE = DISALLOWED_BY_NO_BINARY,
};

// Enumeration of possible completions for an Assistant interaction.
Expand Down
8 changes: 8 additions & 0 deletions chromeos/ash/services/assistant/public/cpp/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ BASE_FEATURE(kEnableLibAssistantV2,
"LibAssistantV2",
base::FEATURE_ENABLED_BY_DEFAULT);

BASE_FEATURE(kEnableLibAssistantDLC,
"LibAssistantDLC",
base::FEATURE_ENABLED_BY_DEFAULT);

bool IsAppSupportEnabled() {
return base::FeatureList::IsEnabled(
assistant::features::kAssistantAppSupport);
Expand Down Expand Up @@ -100,4 +104,8 @@ bool IsLibAssistantV2Enabled() {
return base::FeatureList::IsEnabled(kEnableLibAssistantV2);
}

bool IsLibAssistantDLCEnabled() {
return base::FeatureList::IsEnabled(kEnableLibAssistantDLC);
}

} // namespace ash::assistant::features
5 changes: 5 additions & 0 deletions chromeos/ash/services/assistant/public/cpp/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ BASE_DECLARE_FEATURE(kEnableLibAssistantBetaBackend);
COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC)
BASE_DECLARE_FEATURE(kEnableLibAssistantV2);

COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC)
BASE_DECLARE_FEATURE(kEnableLibAssistantDLC);

COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC) bool IsAppSupportEnabled();

COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC) bool IsAudioEraserEnabled();
Expand All @@ -75,6 +78,8 @@ bool IsLibAssistantSandboxEnabled();

COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC) bool IsLibAssistantV2Enabled();

COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC) bool IsLibAssistantDLCEnabled();

} // namespace ash::assistant::features

// TODO(b/258750971): remove when internal assistant codes are migrated to
Expand Down

0 comments on commit 176fd82

Please sign in to comment.