From f086917d3b27c2296bf527ef5eae24bef6c30429 Mon Sep 17 00:00:00 2001 From: Pete Miller Date: Wed, 11 Dec 2024 17:01:14 -0800 Subject: [PATCH] Don't crash when editing the title of a conversation not in-memory and log database title because of historical bad values due to ref expiring --- .../ai_chat/core/browser/ai_chat_database.cc | 2 ++ .../ai_chat/core/browser/ai_chat_service.cc | 31 +++++++++---------- .../ai_chat/core/browser/ai_chat_service.h | 8 +++-- .../core/browser/conversation_handler.cc | 2 +- .../core/browser/conversation_handler.h | 5 +-- .../mock_conversation_handler_observer.h | 14 ++++----- 6 files changed, 33 insertions(+), 29 deletions(-) diff --git a/components/ai_chat/core/browser/ai_chat_database.cc b/components/ai_chat/core/browser/ai_chat_database.cc index 941588cb8efa..6a4d2602f941 100644 --- a/components/ai_chat/core/browser/ai_chat_database.cc +++ b/components/ai_chat/core/browser/ai_chat_database.cc @@ -654,6 +654,8 @@ bool AIChatDatabase::AddConversationEntry( bool AIChatDatabase::UpdateConversationTitle(std::string_view conversation_uuid, std::string_view title) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + DVLOG(4) << __func__ << " for " << conversation_uuid << " with title " + << title; if (!LazyInit()) { return false; } diff --git a/components/ai_chat/core/browser/ai_chat_service.cc b/components/ai_chat/core/browser/ai_chat_service.cc index 58a3226d90f6..9e0063194098 100644 --- a/components/ai_chat/core/browser/ai_chat_service.cc +++ b/components/ai_chat/core/browser/ai_chat_service.cc @@ -61,7 +61,8 @@ constexpr auto kAllowedSchemes = base::MakeFixedFlatSet( {url::kHttpsScheme, url::kHttpScheme, url::kFileScheme, url::kDataScheme}); std::vector FilterVisibleConversations( - std::map& conversations_map) { + std::map>& + conversations_map) { std::vector conversations; for (const auto& kv : conversations_map) { auto& conversation = kv.second; @@ -598,14 +599,7 @@ void AIChatService::DeleteConversation(const std::string& id) { void AIChatService::RenameConversation(const std::string& id, const std::string& new_name) { - ConversationHandler* conversation_handler = - conversation_handlers_.at(id).get(); - if (!conversation_handler) { - return; - } - - DVLOG(1) << "Renamed conversation " << id << " to '" << new_name << "'"; - OnConversationTitleChanged(conversation_handler, new_name); + OnConversationTitleChanged(id, new_name); } void AIChatService::ConversationExists(const std::string& conversation_uuid, @@ -814,12 +808,17 @@ void AIChatService::OnClientConnectionChanged(ConversationHandler* handler) { MaybeUnloadConversation(handler); } -void AIChatService::OnConversationTitleChanged(ConversationHandler* handler, - std::string title) { - auto conversation_it = conversations_.find(handler->get_conversation_uuid()); - CHECK(conversation_it != conversations_.end()); - auto& conversation = conversation_it->second; - conversation->title = title; +void AIChatService::OnConversationTitleChanged( + const std::string& conversation_uuid, + const std::string& new_title) { + auto conversation_it = conversations_.find(conversation_uuid); + if (conversation_it == conversations_.end()) { + DLOG(ERROR) << "Conversation not found for title change"; + return; + } + + auto& conversation_metadata = conversation_it->second; + conversation_metadata->title = new_title; OnConversationListChanged(); @@ -827,7 +826,7 @@ void AIChatService::OnConversationTitleChanged(ConversationHandler* handler, if (ai_chat_db_) { ai_chat_db_ .AsyncCall(base::IgnoreResult(&AIChatDatabase::UpdateConversationTitle)) - .WithArgs(handler->get_conversation_uuid(), std::move(title)); + .WithArgs(conversation_uuid, new_title); } } diff --git a/components/ai_chat/core/browser/ai_chat_service.h b/components/ai_chat/core/browser/ai_chat_service.h index 80913c0985ba..3515ae66aa53 100644 --- a/components/ai_chat/core/browser/ai_chat_service.h +++ b/components/ai_chat/core/browser/ai_chat_service.h @@ -8,6 +8,7 @@ #include +#include #include #include #include @@ -93,8 +94,8 @@ class AIChatService : public KeyedService, void OnConversationEntryRemoved(ConversationHandler* handler, std::string entry_uuid) override; void OnClientConnectionChanged(ConversationHandler* handler) override; - void OnConversationTitleChanged(ConversationHandler* handler, - std::string title) override; + void OnConversationTitleChanged(const std::string& conversation_uuid, + const std::string& title) override; void OnAssociatedContentDestroyed(ConversationHandler* handler, int content_id) override; @@ -175,7 +176,8 @@ class AIChatService : public KeyedService, private: // Key is uuid - using ConversationMap = std::map; + using ConversationMap = + std::map>; using ConversationMapCallback = base::OnceCallback; void MaybeInitStorage(); diff --git a/components/ai_chat/core/browser/conversation_handler.cc b/components/ai_chat/core/browser/conversation_handler.cc index 898b40671860..bbaf95bbf225 100644 --- a/components/ai_chat/core/browser/conversation_handler.cc +++ b/components/ai_chat/core/browser/conversation_handler.cc @@ -1685,7 +1685,7 @@ void ConversationHandler::OnClientConnectionChanged() { void ConversationHandler::OnConversationTitleChanged(std::string title) { for (auto& observer : observers_) { - observer.OnConversationTitleChanged(this, title); + observer.OnConversationTitleChanged(metadata_->uuid, title); } } diff --git a/components/ai_chat/core/browser/conversation_handler.h b/components/ai_chat/core/browser/conversation_handler.h index 8dfcd8a9d99b..9607c3ba6955 100644 --- a/components/ai_chat/core/browser/conversation_handler.h +++ b/components/ai_chat/core/browser/conversation_handler.h @@ -166,8 +166,9 @@ class ConversationHandler : public mojom::ConversationHandler, // Called when a mojo client connects or disconnects virtual void OnClientConnectionChanged(ConversationHandler* handler) {} - virtual void OnConversationTitleChanged(ConversationHandler* handler, - std::string title) {} + virtual void OnConversationTitleChanged( + const std::string& conversation_uuid, + const std::string& title) {} virtual void OnSelectedLanguageChanged( ConversationHandler* handler, const std::string& selected_language) {} diff --git a/components/ai_chat/core/browser/mock_conversation_handler_observer.h b/components/ai_chat/core/browser/mock_conversation_handler_observer.h index b944d74382f3..a33e2c360188 100644 --- a/components/ai_chat/core/browser/mock_conversation_handler_observer.h +++ b/components/ai_chat/core/browser/mock_conversation_handler_observer.h @@ -25,24 +25,24 @@ class MockConversationHandlerObserver : public ConversationHandler::Observer { MOCK_METHOD(void, OnRequestInProgressChanged, - (ConversationHandler * handler, bool in_progress), + (ConversationHandler*, bool), (override)); MOCK_METHOD(void, OnConversationEntryAdded, - (ConversationHandler * handler, - mojom::ConversationTurnPtr& entry, - std::optional associated_content_value), + (ConversationHandler*, + mojom::ConversationTurnPtr&, + std::optional), (override)); MOCK_METHOD(void, OnConversationEntryRemoved, - (ConversationHandler * handler, std::string turn_uuid), + (ConversationHandler*, std::string), (override)); MOCK_METHOD(void, OnConversationEntryUpdated, - (ConversationHandler * handler, mojom::ConversationTurnPtr entry), + (ConversationHandler*, mojom::ConversationTurnPtr), (override)); MOCK_METHOD(void, @@ -52,7 +52,7 @@ class MockConversationHandlerObserver : public ConversationHandler::Observer { MOCK_METHOD(void, OnConversationTitleChanged, - (ConversationHandler * handler, std::string title), + (const std::string&, const std::string&), (override)); private: