Commit cc35843b authored by David Black's avatar David Black Committed by Commit Bot

Add profile pref to store time of last Assistant interaction.

This pref will eventually be used to target onboarding features to
users who have not been using Assistant for a long time.

Bug: b:161908220
Change-Id: I336b8193182765054126abf504c786c766ff51fc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2314157
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: default avatarXiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Reviewed-by: default avatarJeroen Dhollander <jeroendh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791546}
parent 3c98d0a3
......@@ -53,6 +53,7 @@ AssistantControllerImpl::~AssistantControllerImpl() {
void AssistantControllerImpl::RegisterProfilePrefs(
PrefRegistrySimple* registry) {
registry->RegisterIntegerPref(prefs::kAssistantNumWarmerWelcomeTriggered, 0);
AssistantInteractionControllerImpl::RegisterProfilePrefs(registry);
}
void AssistantControllerImpl::BindReceiver(
......
......@@ -35,6 +35,7 @@
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "chromeos/services/assistant/public/cpp/features.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "net/base/url_util.h"
......@@ -111,6 +112,13 @@ AssistantInteractionControllerImpl::~AssistantInteractionControllerImpl() {
assistant_->RemoveAssistantInteractionSubscriber(this);
}
// static
void AssistantInteractionControllerImpl::RegisterProfilePrefs(
PrefRegistrySimple* registry) {
registry->RegisterTimePref(prefs::kAssistantTimeOfLastInteraction,
base::Time());
}
void AssistantInteractionControllerImpl::SetAssistant(
chromeos::assistant::Assistant* assistant) {
if (assistant_)
......@@ -127,6 +135,12 @@ const AssistantInteractionModel* AssistantInteractionControllerImpl::GetModel()
return &model_;
}
base::TimeDelta
AssistantInteractionControllerImpl::GetTimeDeltaSinceLastInteraction() const {
return base::Time::Now() -
pref_service()->GetTime(prefs::kAssistantTimeOfLastInteraction);
}
void AssistantInteractionControllerImpl::StartTextInteraction(
const std::string& text,
bool allow_tts,
......@@ -319,6 +333,13 @@ void AssistantInteractionControllerImpl::OnMicStateChanged(MicState mic_state) {
void AssistantInteractionControllerImpl::OnCommittedQueryChanged(
const AssistantQuery& assistant_query) {
// Update the time of the last Assistant interaction so that we can later
// determine how long it has been since a user interacted with the Assistant.
// NOTE: We do this in OnCommittedQueryChanged() to filter out accidental
// interactions that would still have triggered OnInteractionStarted().
pref_service()->SetTime(prefs::kAssistantTimeOfLastInteraction,
base::Time::Now());
std::string query;
switch (assistant_query.type()) {
case AssistantQueryType::kText: {
......
......@@ -26,6 +26,8 @@
#include "chromeos/services/assistant/public/cpp/assistant_service.h"
#include "mojo/public/cpp/bindings/receiver.h"
class PrefRegistrySimple;
namespace ash {
class AssistantControllerImpl;
......@@ -55,11 +57,14 @@ class AssistantInteractionControllerImpl
AssistantControllerImpl* assistant_controller);
~AssistantInteractionControllerImpl() override;
static void RegisterProfilePrefs(PrefRegistrySimple* registry);
// Provides a pointer to the |assistant| owned by AssistantService.
void SetAssistant(chromeos::assistant::Assistant* assistant);
// AssistantInteractionController:
const AssistantInteractionModel* GetModel() const override;
base::TimeDelta GetTimeDeltaSinceLastInteraction() const override;
void StartTextInteraction(const std::string& text,
bool allow_tts,
AssistantQuerySource query_source) override;
......
......@@ -24,6 +24,7 @@
#include "base/test/scoped_feature_list.h"
#include "chromeos/services/assistant/public/cpp/assistant_service.h"
#include "chromeos/services/assistant/public/cpp/features.h"
#include "chromeos/services/assistant/test_support/mock_assistant_interaction_subscriber.h"
#include "testing/gmock/include/gmock/gmock.h"
namespace ash {
......@@ -38,6 +39,8 @@ using chromeos::assistant::AssistantInteractionType;
using chromeos::assistant::AssistantQuerySource;
using chromeos::assistant::AssistantSuggestion;
using chromeos::assistant::AssistantSuggestionType;
using chromeos::assistant::MockAssistantInteractionSubscriber;
using chromeos::assistant::ScopedAssistantInteractionSubscriber;
using ::testing::Invoke;
using ::testing::Mock;
......@@ -250,6 +253,30 @@ TEST_P(AssistantInteractionControllerImplTest, ShouldDisplayGenericErrorOnce) {
EXPECT_EQ(ui_elements.front()->type(), AssistantUiElementType::kError);
}
TEST_P(AssistantInteractionControllerImplTest,
ShouldUpdateTimeOfLastInteraction) {
MockAssistantInteractionSubscriber mock_subscriber;
ScopedAssistantInteractionSubscriber scoped_subscriber{&mock_subscriber};
scoped_subscriber.Add(assistant_service());
base::RunLoop run_loop;
base::Time actual_time_of_last_interaction;
EXPECT_CALL(mock_subscriber, OnInteractionStarted)
.WillOnce(Invoke([&](const AssistantInteractionMetadata& metadata) {
actual_time_of_last_interaction = base::Time::Now();
run_loop.QuitClosure().Run();
}));
ShowAssistantUi();
MockTextInteraction().WithTextResponse("<Any-Text-Response>");
run_loop.Run();
auto actual = interaction_controller()->GetTimeDeltaSinceLastInteraction();
auto expected = base::Time::Now() - actual_time_of_last_interaction;
EXPECT_NEAR(actual.InSeconds(), expected.InSeconds(), 1);
}
// We parameterize all AssistantInteractionControllerImplTests to verify that
// they work for both response processing v1 as well as response processing v2.
INSTANTIATE_TEST_SUITE_P(All,
......
......@@ -452,6 +452,10 @@ const char kDetachableBaseDevices[] = "ash.detachable_base.devices";
const char kAssistantNumWarmerWelcomeTriggered[] =
"ash.assistant.num_warmer_welcome_triggered";
// Pref storing the time of the last Assistant interaction.
const char kAssistantTimeOfLastInteraction[] =
"ash.assistant.time_of_last_interaction";
// Whether the user is allowed to disconnect and configure VPN connections.
const char kVpnConfigAllowed[] = "vpn_config_allowed";
......
......@@ -171,6 +171,7 @@ ASH_PUBLIC_EXPORT extern const char kDetachableBaseDevices[];
ASH_PUBLIC_EXPORT extern const char kCursorMotionBlurEnabled[];
ASH_PUBLIC_EXPORT extern const char kAssistantNumWarmerWelcomeTriggered[];
ASH_PUBLIC_EXPORT extern const char kAssistantTimeOfLastInteraction[];
ASH_PUBLIC_EXPORT extern const char kVpnConfigAllowed[];
......
......@@ -10,6 +10,10 @@
#include "ash/public/cpp/ash_public_export.h"
#include "chromeos/services/assistant/public/cpp/assistant_service.h"
namespace base {
class TimeDelta;
} // namespace base
namespace ash {
class AssistantInteractionModel;
......@@ -23,6 +27,10 @@ class ASH_PUBLIC_EXPORT AssistantInteractionController {
// Returns a pointer to the underlying model.
virtual const AssistantInteractionModel* GetModel() const = 0;
// Returns the TimeDelta since the last Assistant interaction. Note that the
// last interaction may have been performed in a different user session.
virtual base::TimeDelta GetTimeDeltaSinceLastInteraction() const = 0;
// Start Assistant text interaction.
virtual void StartTextInteraction(
const std::string& query,
......
......@@ -140,6 +140,7 @@ source_set("tests") {
testonly = true
deps = [
":lib",
":test_support",
"//ash/public/cpp/assistant/test_support",
"//ash/public/mojom",
"//base",
......@@ -187,8 +188,6 @@ source_set("tests") {
"test_support/fake_platform_api.h",
"test_support/fake_service_context.cc",
"test_support/fake_service_context.h",
"test_support/mock_assistant_interaction_subscriber.cc",
"test_support/mock_assistant_interaction_subscriber.h",
"test_support/mock_media_manager.cc",
"test_support/mock_media_manager.h",
]
......@@ -212,6 +211,8 @@ static_library("test_support") {
sources = [
"test_support/mock_assistant.cc",
"test_support/mock_assistant.h",
"test_support/mock_assistant_interaction_subscriber.cc",
"test_support/mock_assistant_interaction_subscriber.h",
]
deps = [
"//base",
......
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