Skip to content

Commit

Permalink
global state cleanup attempt based on review
Browse files Browse the repository at this point in the history
  • Loading branch information
petemill committed Nov 26, 2024
1 parent 06ee62b commit 31f796e
Show file tree
Hide file tree
Showing 11 changed files with 159 additions and 159 deletions.
67 changes: 37 additions & 30 deletions components/ai_chat/core/browser/ai_chat_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -362,10 +362,7 @@ void AIChatService::MaybeInitStorage() {
weak_ptr_factory_.GetWeakPtr()));
}
}
// notify clients
for (auto& remote : observer_remotes_) {
remote->OnStoragePrefChanged(IsAIChatHistoryEnabled());
}
OnStateChanged();
}

void AIChatService::OnOsCryptAsyncReady(os_crypt_async::Encryptor encryptor,
Expand Down Expand Up @@ -548,28 +545,6 @@ void AIChatService::MaybeAssociateContentWithConversation(
associated_content_id, conversation->get_conversation_uuid());
}

void AIChatService::GetNoticesState(GetNoticesStateCallback callback) {
bool has_user_dismissed_storage_notice =
profile_prefs_->GetBoolean(prefs::kUserDismissedStorageNotice);
base::Time last_accepted_disclaimer =
profile_prefs_->GetTime(ai_chat::prefs::kLastAcceptedDisclaimer);

bool is_user_opted_in = !last_accepted_disclaimer.is_null();

// Premium prompt is only shown conditionally (e.g. the user hasn't dismissed
// it and it's been some time since the user started using the feature).
bool can_show_premium_prompt =
!profile_prefs_->GetBoolean(prefs::kUserDismissedPremiumPrompt) &&
!last_accepted_disclaimer.is_null() &&
last_accepted_disclaimer < base::Time::Now() - base::Days(1);

bool is_storage_enabled = profile_prefs_->GetBoolean(prefs::kStorageEnabled);

std::move(callback).Run(is_user_opted_in, is_storage_enabled,
has_user_dismissed_storage_notice,
can_show_premium_prompt);
}

void AIChatService::MarkAgreementAccepted() {
SetUserOptedIn(profile_prefs_, true);
}
Expand Down Expand Up @@ -684,6 +659,38 @@ void AIChatService::MaybeUnloadConversation(
}
}

mojom::ServiceStatePtr AIChatService::BuildState() {
bool has_user_dismissed_storage_notice =
profile_prefs_->GetBoolean(prefs::kUserDismissedStorageNotice);
base::Time last_accepted_disclaimer =
profile_prefs_->GetTime(ai_chat::prefs::kLastAcceptedDisclaimer);

bool is_user_opted_in = !last_accepted_disclaimer.is_null();

// Premium prompt is only shown conditionally (e.g. the user hasn't dismissed
// it and it's been some time since the user started using the feature).
bool can_show_premium_prompt =
!profile_prefs_->GetBoolean(prefs::kUserDismissedPremiumPrompt) &&
!last_accepted_disclaimer.is_null() &&
last_accepted_disclaimer < base::Time::Now() - base::Days(1);

bool is_storage_enabled = profile_prefs_->GetBoolean(prefs::kStorageEnabled);

mojom::ServiceStatePtr state = mojom::ServiceState::New();
state->has_accepted_agreement = is_user_opted_in;
state->is_storage_pref_enabled = is_storage_enabled;
state->is_storage_notice_dismissed = has_user_dismissed_storage_notice;
state->can_show_premium_prompt = can_show_premium_prompt;
return state;
}

void AIChatService::OnStateChanged() {
mojom::ServiceStatePtr state = BuildState();
for (auto& remote : observer_remotes_) {
remote->OnStateChanged(state.Clone());
}
}

bool AIChatService::IsAIChatHistoryEnabled() {
return (features::IsAIChatHistoryEnabled() &&
profile_prefs_->GetBoolean(prefs::kStorageEnabled));
Expand Down Expand Up @@ -846,8 +853,10 @@ void AIChatService::BindConversation(
}

void AIChatService::BindObserver(
mojo::PendingRemote<mojom::ServiceObserver> observer) {
mojo::PendingRemote<mojom::ServiceObserver> observer,
BindObserverCallback callback) {
observer_remotes_.Add(std::move(observer));
std::move(callback).Run(BuildState());
}

bool AIChatService::HasUserOptedIn() {
Expand Down Expand Up @@ -876,9 +885,7 @@ void AIChatService::OnUserOptedIn() {
for (auto& kv : conversation_handlers_) {
kv.second->OnUserOptedIn();
}
for (auto& remote : observer_remotes_) {
remote->OnAgreementAccepted();
}
OnStateChanged();
if (ai_chat_metrics_ != nullptr) {
ai_chat_metrics_->RecordEnabled(true, true, {});
}
Expand Down
6 changes: 4 additions & 2 deletions components/ai_chat/core/browser/ai_chat_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ class AIChatService : public KeyedService,
base::OnceClosure open_ai_chat);

// mojom::Service
void GetNoticesState(GetNoticesStateCallback callback) override;
void MarkAgreementAccepted() override;
void EnableStoragePref() override;
void DismissStorageNotice() override;
Expand All @@ -153,7 +152,8 @@ class AIChatService : public KeyedService,
mojo::PendingReceiver<mojom::ConversationHandler> receiver,
mojo::PendingRemote<mojom::ConversationUI> conversation_ui_handler)
override;
void BindObserver(mojo::PendingRemote<mojom::ServiceObserver> ui) override;
void BindObserver(mojo::PendingRemote<mojom::ServiceObserver> ui,
BindObserverCallback callback) override;

bool HasUserOptedIn();
bool IsPremiumStatus();
Expand Down Expand Up @@ -209,6 +209,8 @@ class AIChatService : public KeyedService,
mojom::PremiumStatus status,
mojom::PremiumInfoPtr info);
void OnDataDeletedForDisabledStorage(bool success);
mojom::ServiceStatePtr BuildState();
void OnStateChanged();

bool IsAIChatHistoryEnabled();

Expand Down
7 changes: 4 additions & 3 deletions components/ai_chat/core/browser/ai_chat_service_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "base/files/scoped_temp_dir.h"
#include "base/functional/bind.h"
#include "base/functional/callback.h"
#include "base/functional/callback_helpers.h"
#include "base/functional/overloaded.h"
#include "base/memory/scoped_refptr.h"
#include "base/run_loop.h"
Expand Down Expand Up @@ -72,8 +73,8 @@ class MockAIChatCredentialManager : public AIChatCredentialManager {
class MockServiceClient : public mojom::ServiceObserver {
public:
explicit MockServiceClient(AIChatService* service) {
service->BindObserver(
service_observer_receiver_.BindNewPipeAndPassRemote());
service->BindObserver(service_observer_receiver_.BindNewPipeAndPassRemote(),
base::DoNothing());
service->Bind(service_remote_.BindNewPipeAndPassReceiver());
}

Expand All @@ -91,7 +92,7 @@ class MockServiceClient : public mojom::ServiceObserver {
(std::vector<mojom::ConversationPtr>),
(override));

MOCK_METHOD(void, OnAgreementAccepted, (), (override));
MOCK_METHOD(void, OnStateChanged, (mojom::ServiceState), (override));

private:
mojo::Receiver<mojom::ServiceObserver> service_observer_receiver_{this};
Expand Down
24 changes: 12 additions & 12 deletions components/ai_chat/core/common/mojom/ai_chat.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -297,15 +297,16 @@ struct ActionGroup {
array<ActionEntry> entries;
};

interface Service {
// Gets the current state of any agreements or notices that can or need to be
// shown to the user.
GetNoticesState() =>
(bool has_accepted_agreement,
bool is_storage_pref_enabled,
bool is_storage_notice_dismissed,
bool can_show_premium_prompt);
// This does not cover more specific data that the Service owns, such as the
// conversation list, but does cover status of preferences and notices.
struct ServiceState {
bool has_accepted_agreement;
bool is_storage_pref_enabled;
bool is_storage_notice_dismissed;
bool can_show_premium_prompt;
};

interface Service {
// Profile-level acknowledgements
MarkAgreementAccepted();
EnableStoragePref();
Expand All @@ -324,8 +325,8 @@ interface Service {
DeleteConversation(string id);
RenameConversation(string id, string new_name);

// Send events to the UI
BindObserver(pending_remote<ServiceObserver> ui);
// Bind ability to send events to the UI and receive current state
BindObserver(pending_remote<ServiceObserver> ui) => (ServiceState state);

// Bind specified Conversation for 2-way communication
BindConversation(
Expand All @@ -336,8 +337,7 @@ interface Service {

interface ServiceObserver {
OnConversationListChanged(array<Conversation> conversations);
OnStoragePrefChanged(bool is_storage_pref_enabled);
OnAgreementAccepted();
OnStateChanged(ServiceState state);
};

// Browser-side handler for general AI Chat UI functions, implemented
Expand Down
Loading

0 comments on commit 31f796e

Please sign in to comment.