Commit b394a207 authored by yileili's avatar yileili Committed by Commit Bot

Reland "Enable warmer welcome during Assistant launch."

This is a reland of 29dca9fe

Remove an unnecessary include to fix the compile break:

  ../../chromeos/services/assistant/assistant_manager_service_impl.cc:27:10: fatal error: 'chromeos/services/assistant/assistant_warmer_welcome_log.pb.h' file not found
  #include "chromeos/services/assistant/assistant_warmer_welcome_log.pb.h"
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  1 error generated.

Bug: None.
Test: Compiled

Original change's description:
> Enable warmer welcome during Assistant launch.
>
> 1. It is guarded by a finch flag AssistantWarmerWelcome.
> 2. WW only shows 3 times.
>
> Bug: b:112495005
> Test: Manual
>
> Change-Id: I80cbcfd4104c17172939d36b986be65077bf0bb3
> Reviewed-on: https://chromium-review.googlesource.com/c/1355174
> Reviewed-by: Will Harris <wfh@chromium.org>
> Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Commit-Queue: Yilei Li <yileili@google.com>
> Cr-Commit-Position: refs/heads/master@{#614553}

Bug: b:112495005
Change-Id: I9c67fd0c85b216e17408056c6d7df35e6f205c6f
Reviewed-on: https://chromium-review.googlesource.com/c/1367185Reviewed-by: default avatarWill Harris <wfh@chromium.org>
Reviewed-by: default avatarXiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Yilei Li <yileili@google.com>
Cr-Commit-Position: refs/heads/master@{#614756}
parent 817c2bd0
......@@ -1334,6 +1334,7 @@ component("ash") {
"//chromeos:power_manager_proto",
"//chromeos/assistant:buildflags",
"//chromeos/components/proximity_auth/logging",
"//chromeos/services/assistant/public:feature_flags",
"//chromeos/services/assistant/public/mojom",
"//chromeos/services/multidevice_setup/public/mojom",
"//chromeos/strings",
......
......@@ -17,12 +17,14 @@
#include "ash/assistant/assistant_ui_controller.h"
#include "ash/assistant/util/deep_link_util.h"
#include "ash/new_window_controller.h"
#include "ash/public/cpp/ash_pref_names.h"
#include "ash/session/session_controller.h"
#include "ash/shell.h"
#include "ash/utility/screenshot_controller.h"
#include "ash/voice_interaction/voice_interaction_controller.h"
#include "base/bind.h"
#include "base/memory/scoped_refptr.h"
#include "components/prefs/pref_registry_simple.h"
#include "services/content/public/mojom/constants.mojom.h"
#include "services/service_manager/public/cpp/connector.h"
......@@ -58,6 +60,11 @@ AssistantController::~AssistantController() {
RemoveObserver(this);
}
// static
void AssistantController::RegisterProfilePrefs(PrefRegistrySimple* registry) {
registry->RegisterIntegerPref(prefs::kAssistantNumWarmerWelcomeTriggered, 0);
}
void AssistantController::BindRequest(
mojom::AssistantControllerRequest request) {
assistant_controller_bindings_.AddBinding(this, std::move(request));
......
......@@ -28,6 +28,8 @@
#include "mojo/public/cpp/bindings/interface_ptr_set.h"
#include "services/content/public/mojom/navigable_contents_factory.mojom.h"
class PrefRegistrySimple;
namespace ash {
class AssistantCacheController;
......@@ -48,6 +50,8 @@ class ASH_EXPORT AssistantController
AssistantController();
~AssistantController() override;
static void RegisterProfilePrefs(PrefRegistrySimple* registry);
void BindRequest(mojom::AssistantControllerRequest request);
void BindRequest(mojom::AssistantVolumeControlRequest request);
......
......@@ -17,19 +17,25 @@
#include "ash/assistant/ui/assistant_ui_constants.h"
#include "ash/assistant/util/deep_link_util.h"
#include "ash/assistant/util/histogram_util.h"
#include "ash/public/cpp/ash_pref_names.h"
#include "ash/public/interfaces/voice_interaction_controller.mojom.h"
#include "ash/session/session_controller.h"
#include "ash/shell.h"
#include "ash/strings/grit/ash_strings.h"
#include "ash/voice_interaction/voice_interaction_controller.h"
#include "ash/wm/tablet_mode/tablet_mode_controller.h"
#include "base/optional.h"
#include "base/strings/utf_string_conversions.h"
#include "chromeos/services/assistant/public/features.h"
#include "components/prefs/pref_service.h"
#include "ui/base/l10n/l10n_util.h"
namespace ash {
namespace {
constexpr int kWarmerWelcomesMaxTimesTriggered = 3;
// Helpers ---------------------------------------------------------------------
// Returns true if device is in tablet mode, false otherwise.
......@@ -598,20 +604,50 @@ void AssistantInteractionController::OnUiVisible(
// Assistant UI will immediately start a voice interaction.
const bool launch_with_mic_open =
Shell::Get()->voice_interaction_controller()->launch_with_mic_open();
if (launch_with_mic_open || IsTabletMode())
if (launch_with_mic_open || IsTabletMode()) {
StartVoiceInteraction();
// TODO(yileili): Currently WW is only allowed when user logins and the
// entrypoint does not automatically start a user interaction, e.g.
// stylus or hotword, to avoid WW interrupts the user interaction.
// Need further UX design if to attempt WW after the first interaction.
should_attempt_warmer_welcome_ = false;
}
break;
}
case AssistantEntryPoint::kStylus:
model_.SetInputModality(InputModality::kStylus);
break;
case AssistantEntryPoint::kUnspecified:
FALLTHROUGH;
case AssistantEntryPoint::kDeepLink:
case AssistantEntryPoint::kHotword:
should_attempt_warmer_welcome_ = false;
break;
case AssistantEntryPoint::kUnspecified:
case AssistantEntryPoint::kSetup:
// No action necessary.
break;
}
if (should_attempt_warmer_welcome_) {
if (chromeos::assistant::features::IsWarmerWelcomeEnabled()) {
auto* pref_service =
Shell::Get()->session_controller()->GetLastActiveUserPrefService();
DCHECK(pref_service);
auto num_warmer_welcome_triggered =
pref_service->GetInteger(prefs::kAssistantNumWarmerWelcomeTriggered);
if (num_warmer_welcome_triggered < kWarmerWelcomesMaxTimesTriggered) {
// TODO(b/120242181): If allow_tts is true, Assistant response updates
// mic and starts a new voice interaction automatically. A void voice
// input could return a communication error or empty response.
assistant_->StartWarmerWelcomeInteraction(num_warmer_welcome_triggered,
/*allow_tts=*/false);
pref_service->SetInteger(prefs::kAssistantNumWarmerWelcomeTriggered,
++num_warmer_welcome_triggered);
}
}
should_attempt_warmer_welcome_ = false;
}
}
void AssistantInteractionController::StartMetalayerInteraction(
......
......@@ -135,6 +135,8 @@ class AssistantInteractionController
AssistantInteractionModel model_;
bool should_attempt_warmer_welcome_ = true;
base::WeakPtrFactory<AssistantInteractionController> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(AssistantInteractionController);
......
......@@ -330,6 +330,10 @@ const char kQuickUnlockPinSalt[] = "quick_unlock.pin.salt";
// bases - for exmaple the last used base per user.
const char kDetachableBaseDevices[] = "ash.detachable_base.devices";
// Integer pref storing the number of Assistant warmer welcome triggered times.
const char kAssistantNumWarmerWelcomeTriggered[] =
"ash.assistant.num_warmer_welcome_triggered";
// NOTE: New prefs should start with the "ash." prefix. Existing prefs moved
// into this file should not be renamed, since they may be synced.
......
......@@ -128,6 +128,8 @@ ASH_PUBLIC_EXPORT extern const char kDetachableBaseDevices[];
ASH_PUBLIC_EXPORT extern const char kCursorMotionBlurEnabled[];
ASH_PUBLIC_EXPORT extern const char kAssistantNumWarmerWelcomeTriggered[];
} // namespace prefs
} // namespace ash
......
......@@ -241,6 +241,7 @@ class AshVisibilityController : public ::wm::VisibilityController {
// Registers prefs whose default values are same in user and signin prefs.
void RegisterProfilePrefs(PrefRegistrySimple* registry, bool for_test) {
AccessibilityController::RegisterProfilePrefs(registry, for_test);
AssistantController::RegisterProfilePrefs(registry);
BluetoothPowerController::RegisterProfilePrefs(registry);
DockedMagnifierController::RegisterProfilePrefs(registry, for_test);
LoginScreenController::RegisterProfilePrefs(registry, for_test);
......
......@@ -21,6 +21,7 @@
#include "build/util/webkit_version.h"
#include "chromeos/assistant/internal/internal_constants.h"
#include "chromeos/assistant/internal/internal_util.h"
#include "chromeos/assistant/internal/proto/google3/assistant/api/client_input/warmer_welcome_input.pb.h"
#include "chromeos/assistant/internal/proto/google3/assistant/api/client_op/device_args.pb.h"
#include "chromeos/dbus/util/version_loader.h"
#include "chromeos/services/assistant/public/features.h"
......@@ -342,6 +343,25 @@ void AssistantManagerServiceImpl::StopActiveInteraction(
cancel_conversation);
}
void AssistantManagerServiceImpl::StartWarmerWelcomeInteraction(
int num_warmer_welcome_triggered,
bool allow_tts) {
DCHECK(assistant_manager_internal_ != nullptr);
const std::string interaction =
CreateWarmerWelcomeInteraction(num_warmer_welcome_triggered);
assistant_client::VoicelessOptions options;
options.is_user_initiated = true;
options.modality =
allow_tts ? assistant_client::VoicelessOptions::Modality::VOICE_MODALITY
: assistant_client::VoicelessOptions::Modality::TYPING_MODALITY;
assistant_manager_internal_->SendVoicelessInteraction(
interaction, /*description=*/"warmer_welcome_trigger", options,
[](auto) {});
}
void AssistantManagerServiceImpl::StartCachedScreenContextInteraction() {
if (!IsScreenContextAllowed(service_->assistant_state()))
return;
......
......@@ -118,6 +118,8 @@ class AssistantManagerServiceImpl
void StartMetalayerInteraction(const gfx::Rect& region) override;
void StartTextInteraction(const std::string& query, bool allow_tts) override;
void StartVoiceInteraction() override;
void StartWarmerWelcomeInteraction(int num_warmer_welcome_triggered,
bool allow_tts) override;
void StopActiveInteraction(bool cancel_conversation) override;
void AddAssistantInteractionSubscriber(
mojom::AssistantInteractionSubscriberPtr subscriber) override;
......
......@@ -67,6 +67,10 @@ void FakeAssistantManagerServiceImpl::StartTextInteraction(
void FakeAssistantManagerServiceImpl::StartVoiceInteraction() {}
void FakeAssistantManagerServiceImpl::StartWarmerWelcomeInteraction(
int num_warmer_welcome_triggered,
bool allow_tts) {}
void FakeAssistantManagerServiceImpl::StopActiveInteraction(
bool cancel_conversation) {}
......
......@@ -50,6 +50,8 @@ class FakeAssistantManagerServiceImpl : public AssistantManagerService {
void StartMetalayerInteraction(const gfx::Rect& region) override;
void StartTextInteraction(const std::string& query, bool allow_tts) override;
void StartVoiceInteraction() override;
void StartWarmerWelcomeInteraction(int num_warmer_welcome_triggered,
bool allow_tts) override;
void StopActiveInteraction(bool cancel_conversation) override;
void AddAssistantInteractionSubscriber(
mojom::AssistantInteractionSubscriberPtr subscriber) override;
......
......@@ -13,6 +13,9 @@ namespace features {
const base::Feature kAssistantVoiceMatch{"AssistantVoiceMatch",
base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kAssistantWarmerWelcomeFeature{
"AssistantWarmerWelcome", base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kEnableDspHotword{"EnableDspHotword",
base::FEATURE_DISABLED_BY_DEFAULT};
......@@ -27,6 +30,10 @@ bool IsStereoAudioInputEnabled() {
return base::FeatureList::IsEnabled(kEnableStereoAudioInput);
}
bool IsWarmerWelcomeEnabled() {
return base::FeatureList::IsEnabled(kAssistantWarmerWelcomeFeature);
}
} // namespace features
} // namespace assistant
} // namespace chromeos
......@@ -14,6 +14,9 @@ namespace features {
// Enables Assistant voice match enrollment.
extern const base::Feature kAssistantVoiceMatch;
// Enable Assistant warmer welcome.
extern const base::Feature kAssistantWarmerWelcomeFeature;
// Enables DSP for hotword detection.
extern const base::Feature kEnableDspHotword;
......@@ -24,6 +27,8 @@ bool IsDspHotwordEnabled();
bool IsStereoAudioInputEnabled();
bool IsWarmerWelcomeEnabled();
} // namespace features
} // namespace assistant
} // namespace chromeos
......
......@@ -32,6 +32,13 @@ interface Assistant {
// Starts a new Assistant voice interaction.
StartVoiceInteraction();
// Starts a warmer welcome interaction for Assistant launch.
// |num_warmer_welcome_triggered| is the count of warmer welcomes
// already triggered. If |allow_tts| is true, the result may contain TTS.
// Otherwise TTS will not be present in the generated server response.
StartWarmerWelcomeInteraction(int32 num_warmer_welcome_triggered,
bool allow_tts);
// Stops the active Assistant interaction and cancel the conversation if
// |cancel_conversation|. If there is no active interaction, this method
// is a no-op.
......
......@@ -29,6 +29,8 @@ class MockAssistant : public mojom::Assistant {
MOCK_METHOD0(StartVoiceInteraction, void());
MOCK_METHOD2(StartWarmerWelcomeInteraction, void(int, bool));
MOCK_METHOD1(StopActiveInteraction, void(bool));
MOCK_METHOD1(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment