From 374b302937e59e3396b79ba1d6eb3c812c25b71f Mon Sep 17 00:00:00 2001 From: Pete Miller Date: Mon, 11 Nov 2024 23:59:00 -0800 Subject: [PATCH 1/3] keep an AI Chat conversation alive if it has chat messages and its associated content is still alive This aids recall especially whilst the AIChatHistory feature flag is disabled --- .../ai_chat/core/browser/ai_chat_service.cc | 70 ++++++++++------ .../ai_chat/core/browser/ai_chat_service.h | 1 + .../core/browser/ai_chat_service_unittest.cc | 82 ++++++++++++++++--- .../core/browser/conversation_handler.cc | 10 ++- .../core/browser/conversation_handler.h | 4 + 5 files changed, 126 insertions(+), 41 deletions(-) diff --git a/components/ai_chat/core/browser/ai_chat_service.cc b/components/ai_chat/core/browser/ai_chat_service.cc index b0d0884d9006..3957ec86e774 100644 --- a/components/ai_chat/core/browser/ai_chat_service.cc +++ b/components/ai_chat/core/browser/ai_chat_service.cc @@ -630,36 +630,48 @@ void AIChatService::OnPremiumStatusReceived(GetPremiumStatusCallback callback, void AIChatService::MaybeUnloadConversation( ConversationHandler* conversation_handler) { - if (!conversation_handler->IsAnyClientConnected() && - !conversation_handler->IsRequestInProgress()) { - // Can erase handler because no active UI - bool has_history = conversation_handler->HasAnyHistory(); - auto uuid = conversation_handler->get_conversation_uuid(); - conversation_observations_.RemoveObservation(conversation_handler); - conversation_handlers_.erase(uuid); - DVLOG(1) << "Unloaded conversation (" << uuid << ") from memory. Now have " + // Don't unload if there is active UI for the conversation + if (conversation_handler->IsAnyClientConnected()) { + return; + } + + bool has_history = conversation_handler->HasAnyHistory(); + + // Without the history feature enabled, we can keep the conversation in memory + // until there is no active content and chat history. Without an associated + // content there will be no way to ask for the conversation. If history is + // enabled then we can load from storage if the conversation is requested + // again. + if (!IsAIChatHistoryEnabled() && + conversation_handler->IsAssociatedContentAlive() && has_history) { + return; + } + + // With the history feature enabled, only unload if there is no request in + // progress. + if (IsAIChatHistoryEnabled() && has_history && + conversation_handler->IsRequestInProgress()) { + return; + } + + auto uuid = conversation_handler->get_conversation_uuid(); + conversation_observations_.RemoveObservation(conversation_handler); + conversation_handlers_.erase(uuid); + DVLOG(1) << "Unloaded conversation (" << uuid << ") from memory. Now have " + << conversations_.size() << " Conversation metadata items and " + << conversation_handlers_.size() + << " ConversationHandler instances."; + if (!IsAIChatHistoryEnabled() || !has_history) { + // Can erase because no active UI and no history, so it's + // not a real / persistable conversation + conversations_.erase(uuid); + std::erase_if(content_conversations_, + [&uuid](const auto& kv) { return kv.second == uuid; }); + DVLOG(1) << "Erased conversation (" << uuid << "). Now have " << conversations_.size() << " Conversation metadata items and " << conversation_handlers_.size() << " ConversationHandler instances."; - if (!IsAIChatHistoryEnabled() || !has_history) { - // Can erase because no active UI and no history, so it's - // not a real / persistable conversation - conversations_.erase(uuid); - std::erase_if(content_conversations_, - [&uuid](const auto& kv) { return kv.second == uuid; }); - DVLOG(1) << "Erased conversation (" << uuid << "). Now have " - << conversations_.size() << " Conversation metadata items and " - << conversation_handlers_.size() - << " ConversationHandler instances."; - OnConversationListChanged(); - } - } else { - DVLOG(4) << "Not unloading conversation (" - << conversation_handler->get_conversation_uuid() - << ") from memory. Has active clients: " - << (conversation_handler->IsAnyClientConnected() ? "yes" : "no") - << " Request is in progress: " - << (conversation_handler->IsRequestInProgress() ? "yes" : "no"); + OnConversationListChanged(); } } @@ -821,6 +833,10 @@ void AIChatService::OnConversationTitleChanged(ConversationHandler* handler, } } +void AIChatService::OnAssociatedContentDestroyed(ConversationHandler* handler) { + MaybeUnloadConversation(handler); +} + void AIChatService::GetVisibleConversations( GetVisibleConversationsCallback callback) { LoadConversationsLazy(base::BindOnce( diff --git a/components/ai_chat/core/browser/ai_chat_service.h b/components/ai_chat/core/browser/ai_chat_service.h index b868c17e4e5d..b1599cce2806 100644 --- a/components/ai_chat/core/browser/ai_chat_service.h +++ b/components/ai_chat/core/browser/ai_chat_service.h @@ -95,6 +95,7 @@ class AIChatService : public KeyedService, void OnClientConnectionChanged(ConversationHandler* handler) override; void OnConversationTitleChanged(ConversationHandler* handler, std::string title) override; + void OnAssociatedContentDestroyed(ConversationHandler* handler) override; // Adds new conversation and returns the handler ConversationHandler* CreateConversation(); diff --git a/components/ai_chat/core/browser/ai_chat_service_unittest.cc b/components/ai_chat/core/browser/ai_chat_service_unittest.cc index b1e3cc300ea4..47ffeed64abd 100644 --- a/components/ai_chat/core/browser/ai_chat_service_unittest.cc +++ b/components/ai_chat/core/browser/ai_chat_service_unittest.cc @@ -193,7 +193,11 @@ class MockAssociatedContent void DisassociateWithConversations(std::string archived_text_content, bool archived_is_video) { + std::set> related_conversations; for (auto& conversation : related_conversations_) { + related_conversations.insert(conversation); + } + for (auto& conversation : related_conversations) { if (conversation) { conversation->OnAssociatedContentDestroyed(archived_text_content, archived_is_video); @@ -428,6 +432,61 @@ TEST_P(AIChatServiceUnitTest, ConversationLifecycle_WithMessages) { task_environment_.RunUntilIdle(); } +TEST_P(AIChatServiceUnitTest, ConversationLifecycle_WithContent) { + NiceMock associated_content{}; + ON_CALL(associated_content, GetURL()) + .WillByDefault(testing::Return(GURL("https://example.com"))); + associated_content.SetContentId(1); + ConversationHandler* conversation_with_content_no_messages = + ai_chat_service_->GetOrCreateConversationHandlerForContent( + associated_content.GetContentId(), associated_content.GetWeakPtr()); + EXPECT_TRUE(conversation_with_content_no_messages); + // Asking again for same content ID gets same conversation + EXPECT_EQ( + conversation_with_content_no_messages, + ai_chat_service_->GetOrCreateConversationHandlerForContent( + associated_content.GetContentId(), associated_content.GetWeakPtr())); + // Shouldn't be visible without messages + ExpectVisibleConversationsSize(FROM_HERE, 0u); + EXPECT_EQ(ai_chat_service_->GetInMemoryConversationCountForTesting(), 1u); + // Disconnecting the client should unload the handler and delete the + // conversation. + auto client1 = + CreateConversationClient(conversation_with_content_no_messages); + DisconnectConversationClient(client1.get()); + EXPECT_EQ(ai_chat_service_->GetInMemoryConversationCountForTesting(), 0u); + ExpectVisibleConversationsSize(FROM_HERE, 0u); + + // Create a new conversation for same content, with messages this time + ConversationHandler* conversation_with_content = + ai_chat_service_->GetOrCreateConversationHandlerForContent( + associated_content.GetContentId(), associated_content.GetWeakPtr()); + conversation_with_content->SetChatHistoryForTesting( + CreateSampleChatHistory(1u)); + ExpectVisibleConversationsSize(FROM_HERE, 1u); + EXPECT_EQ(ai_chat_service_->GetInMemoryConversationCountForTesting(), 1u); + auto client2 = CreateConversationClient(conversation_with_content); + DisconnectConversationClient(client2.get()); + + if (IsAIChatHistoryEnabled()) { + // Disconnecting all clients should delete the handler until because it + // can be reloaded from storage. + EXPECT_EQ(ai_chat_service_->GetInMemoryConversationCountForTesting(), 0u); + ExpectVisibleConversationsSize(FROM_HERE, 1u); + associated_content.DisassociateWithConversations("", false); + EXPECT_EQ(ai_chat_service_->GetInMemoryConversationCountForTesting(), 0u); + ExpectVisibleConversationsSize(FROM_HERE, 1u); + } else { + // Disconnecting all clients should keep the handler in memory until + // the content is destroyed. + EXPECT_EQ(ai_chat_service_->GetInMemoryConversationCountForTesting(), 1u); + ExpectVisibleConversationsSize(FROM_HERE, 1u); + associated_content.DisassociateWithConversations("", false); + EXPECT_EQ(ai_chat_service_->GetInMemoryConversationCountForTesting(), 0u); + ExpectVisibleConversationsSize(FROM_HERE, 0u); + } +} + TEST_P(AIChatServiceUnitTest, GetOrCreateConversationHandlerForContent) { ConversationHandler* conversation_without_content = CreateConversation(); @@ -463,27 +522,24 @@ TEST_P(AIChatServiceUnitTest, GetOrCreateConversationHandlerForContent) { // Creating a second conversation with the same associated content should // make the second conversation the default for that content, but leave // the first still associated with the content. - ConversationHandler* conversation_with_content2 = + ConversationHandler* conversation2 = ai_chat_service_->CreateConversationHandlerForContent( associated_content.GetContentId(), associated_content.GetWeakPtr()); - EXPECT_NE(conversation_with_content, conversation_with_content2); + EXPECT_NE(conversation_with_content, conversation2); EXPECT_NE(conversation_with_content->get_conversation_uuid(), - conversation_with_content2->get_conversation_uuid()); - EXPECT_EQ( - conversation_with_content2->GetAssociatedContentDelegateForTesting(), - &associated_content); - EXPECT_EQ( - conversation_with_content->GetAssociatedContentDelegateForTesting(), - conversation_with_content2->GetAssociatedContentDelegateForTesting()); + conversation2->get_conversation_uuid()); + EXPECT_EQ(conversation2->GetAssociatedContentDelegateForTesting(), + &associated_content); + EXPECT_EQ(conversation_with_content->GetAssociatedContentDelegateForTesting(), + conversation2->GetAssociatedContentDelegateForTesting()); // Check the second conversation is the default for that content ID EXPECT_EQ( ai_chat_service_->GetOrCreateConversationHandlerForContent( associated_content.GetContentId(), associated_content.GetWeakPtr()), - conversation_with_content2); + conversation2); // Let the conversation be deleted - std::string conversation2_uuid = - conversation_with_content2->get_conversation_uuid(); - auto client1 = CreateConversationClient(conversation_with_content2); + std::string conversation2_uuid = conversation2->get_conversation_uuid(); + auto client1 = CreateConversationClient(conversation2); DisconnectConversationClient(client1.get()); ConversationHandler* conversation_with_content3 = ai_chat_service_->GetOrCreateConversationHandlerForContent( diff --git a/components/ai_chat/core/browser/conversation_handler.cc b/components/ai_chat/core/browser/conversation_handler.cc index c1ae53bc0195..680eee836545 100644 --- a/components/ai_chat/core/browser/conversation_handler.cc +++ b/components/ai_chat/core/browser/conversation_handler.cc @@ -320,6 +320,10 @@ bool ConversationHandler::IsRequestInProgress() { return is_request_in_progress_; } +bool ConversationHandler::IsAssociatedContentAlive() { + return associated_content_delegate_ && !archive_content_; +} + void ConversationHandler::OnConversationDeleted() { for (auto& client : conversation_ui_handlers_) { client->OnConversationDeleted(); @@ -386,7 +390,7 @@ void ConversationHandler::OnAssociatedContentDestroyed( // fetch. It may be populated later, e.g. through back navigation. // If this conversation is allowed to be associated with content, we can keep // using our current cached content. - associated_content_delegate_ = nullptr; + DisassociateContentDelegate(); if (!chat_history_.empty() && should_send_page_contents_ && metadata_->associated_content && metadata_->associated_content->is_content_association_possible) { @@ -397,6 +401,10 @@ void ConversationHandler::OnAssociatedContentDestroyed( SetArchiveContent(std::move(last_text_content), is_video); } OnAssociatedContentInfoChanged(); + // Notify observers + for (auto& observer : observers_) { + observer.OnAssociatedContentDestroyed(this); + } } void ConversationHandler::SetArchiveContent(std::string text_content, diff --git a/components/ai_chat/core/browser/conversation_handler.h b/components/ai_chat/core/browser/conversation_handler.h index c76050a50826..ac1fe209a63f 100644 --- a/components/ai_chat/core/browser/conversation_handler.h +++ b/components/ai_chat/core/browser/conversation_handler.h @@ -169,6 +169,7 @@ class ConversationHandler : public mojom::ConversationHandler, virtual void OnSelectedLanguageChanged( ConversationHandler* handler, const std::string& selected_language) {} + virtual void OnAssociatedContentDestroyed(ConversationHandler* handler) {} }; ConversationHandler( @@ -207,6 +208,9 @@ class ConversationHandler : public mojom::ConversationHandler, bool HasAnyHistory(); bool IsRequestInProgress(); + // Returns true if the conversation has associated content that is non-archive + bool IsAssociatedContentAlive(); + // Called when the associated content is destroyed or navigated away. If // it's a navigation, the AssociatedContentDelegate will set itself to a new // ConversationHandler. From 58de2e7ad4986aa18fa1ca28e02e29658cceef7d Mon Sep 17 00:00:00 2001 From: Pete Miller Date: Mon, 25 Nov 2024 15:04:55 -0800 Subject: [PATCH 2/3] Keep conversations in memory if content is alive even when AIChatHistory is enabled since we don't have an async GetOrCreateConverationForContent --- .../ai_chat/core/browser/ai_chat_service.cc | 25 +++++++---------- .../ai_chat/core/browser/ai_chat_service.h | 3 ++- .../core/browser/ai_chat_service_unittest.cc | 15 ++++------- .../core/browser/associated_content_driver.cc | 27 ++++++++++++------- .../core/browser/associated_content_driver.h | 6 +++++ .../core/browser/conversation_handler.cc | 5 +++- .../core/browser/conversation_handler.h | 3 ++- 7 files changed, 46 insertions(+), 38 deletions(-) diff --git a/components/ai_chat/core/browser/ai_chat_service.cc b/components/ai_chat/core/browser/ai_chat_service.cc index 3957ec86e774..e45a02b043c8 100644 --- a/components/ai_chat/core/browser/ai_chat_service.cc +++ b/components/ai_chat/core/browser/ai_chat_service.cc @@ -637,20 +637,13 @@ void AIChatService::MaybeUnloadConversation( bool has_history = conversation_handler->HasAnyHistory(); - // Without the history feature enabled, we can keep the conversation in memory - // until there is no active content and chat history. Without an associated - // content there will be no way to ask for the conversation. If history is - // enabled then we can load from storage if the conversation is requested - // again. - if (!IsAIChatHistoryEnabled() && - conversation_handler->IsAssociatedContentAlive() && has_history) { - return; - } - - // With the history feature enabled, only unload if there is no request in - // progress. - if (IsAIChatHistoryEnabled() && has_history && - conversation_handler->IsRequestInProgress()) { + // We can keep a conversation with history in memory until there is no active + // content. + // TODO(petemill): With the history feature enabled, we should unload (if + // there is no request in progress). However, we can only do this when + // GetOrCreateConversationHandlerForContent allows a callback so that it + // can provide an answer after loading the conversation content from storage. + if (conversation_handler->IsAssociatedContentAlive() && has_history) { return; } @@ -833,7 +826,9 @@ void AIChatService::OnConversationTitleChanged(ConversationHandler* handler, } } -void AIChatService::OnAssociatedContentDestroyed(ConversationHandler* handler) { +void AIChatService::OnAssociatedContentDestroyed(ConversationHandler* handler, + int content_id) { + content_conversations_.erase(content_id); MaybeUnloadConversation(handler); } diff --git a/components/ai_chat/core/browser/ai_chat_service.h b/components/ai_chat/core/browser/ai_chat_service.h index b1599cce2806..c452ee04e794 100644 --- a/components/ai_chat/core/browser/ai_chat_service.h +++ b/components/ai_chat/core/browser/ai_chat_service.h @@ -95,7 +95,8 @@ class AIChatService : public KeyedService, void OnClientConnectionChanged(ConversationHandler* handler) override; void OnConversationTitleChanged(ConversationHandler* handler, std::string title) override; - void OnAssociatedContentDestroyed(ConversationHandler* handler) override; + void OnAssociatedContentDestroyed(ConversationHandler* handler, + int content_id) override; // Adds new conversation and returns the handler ConversationHandler* CreateConversation(); diff --git a/components/ai_chat/core/browser/ai_chat_service_unittest.cc b/components/ai_chat/core/browser/ai_chat_service_unittest.cc index 47ffeed64abd..a8c5a3bec468 100644 --- a/components/ai_chat/core/browser/ai_chat_service_unittest.cc +++ b/components/ai_chat/core/browser/ai_chat_service_unittest.cc @@ -467,21 +467,16 @@ TEST_P(AIChatServiceUnitTest, ConversationLifecycle_WithContent) { EXPECT_EQ(ai_chat_service_->GetInMemoryConversationCountForTesting(), 1u); auto client2 = CreateConversationClient(conversation_with_content); DisconnectConversationClient(client2.get()); + // Disconnecting all clients should keep the handler in memory until + // the content is destroyed. + EXPECT_EQ(ai_chat_service_->GetInMemoryConversationCountForTesting(), 1u); + ExpectVisibleConversationsSize(FROM_HERE, 1u); + associated_content.DisassociateWithConversations("", false); if (IsAIChatHistoryEnabled()) { - // Disconnecting all clients should delete the handler until because it - // can be reloaded from storage. - EXPECT_EQ(ai_chat_service_->GetInMemoryConversationCountForTesting(), 0u); - ExpectVisibleConversationsSize(FROM_HERE, 1u); - associated_content.DisassociateWithConversations("", false); EXPECT_EQ(ai_chat_service_->GetInMemoryConversationCountForTesting(), 0u); ExpectVisibleConversationsSize(FROM_HERE, 1u); } else { - // Disconnecting all clients should keep the handler in memory until - // the content is destroyed. - EXPECT_EQ(ai_chat_service_->GetInMemoryConversationCountForTesting(), 1u); - ExpectVisibleConversationsSize(FROM_HERE, 1u); - associated_content.DisassociateWithConversations("", false); EXPECT_EQ(ai_chat_service_->GetInMemoryConversationCountForTesting(), 0u); ExpectVisibleConversationsSize(FROM_HERE, 0u); } diff --git a/components/ai_chat/core/browser/associated_content_driver.cc b/components/ai_chat/core/browser/associated_content_driver.cc index 536aca02495d..8d54b9b4bcd9 100644 --- a/components/ai_chat/core/browser/associated_content_driver.cc +++ b/components/ai_chat/core/browser/associated_content_driver.cc @@ -67,12 +67,7 @@ AssociatedContentDriver::AssociatedContentDriver( : url_loader_factory_(url_loader_factory) {} AssociatedContentDriver::~AssociatedContentDriver() { - for (auto& conversation : associated_conversations_) { - if (conversation) { - conversation->OnAssociatedContentDestroyed(cached_text_content_, - is_video_); - } - } + DisassociateWithConversations(); } void AssociatedContentDriver::AddRelatedConversation( @@ -279,10 +274,10 @@ void AssociatedContentDriver::OnFaviconImageDataChanged() { } void AssociatedContentDriver::OnNewPage(int64_t navigation_id) { - // Tell the associated_conversations_ that we're breaking up - for (auto& conversation : associated_conversations_) { - conversation->OnAssociatedContentDestroyed(cached_text_content_, is_video_); - } + // This instance will now be used for different content so existing + // conversations need to be disassociated. + DisassociateWithConversations(); + // Tell the observer how to find the next conversation for (auto& observer : observers_) { observer.OnAssociatedContentNavigated(navigation_id); @@ -298,4 +293,16 @@ void AssociatedContentDriver::OnNewPage(int64_t navigation_id) { ConversationHandler::AssociatedContentDelegate::OnNewPage(navigation_id); } +void AssociatedContentDriver::DisassociateWithConversations() { + // Iterator might be invalidated by destruction, so copy the items + std::vector conversations{ + associated_conversations_.begin(), associated_conversations_.end()}; + for (auto& conversation : conversations) { + if (conversation) { + conversation->OnAssociatedContentDestroyed(cached_text_content_, + is_video_); + } + } +} + } // namespace ai_chat diff --git a/components/ai_chat/core/browser/associated_content_driver.h b/components/ai_chat/core/browser/associated_content_driver.h index 602bfd2727ee..9f95dc73462a 100644 --- a/components/ai_chat/core/browser/associated_content_driver.h +++ b/components/ai_chat/core/browser/associated_content_driver.h @@ -145,6 +145,12 @@ class AssociatedContentDriver ConversationHandler::GetStagedEntriesCallback callback, int64_t navigation_id, api_request_helper::APIRequestResult result); + + // Let all conversations using this content know that the content + // has been destroyed or changed to represent different content (e.g. a + // navigation). + void DisassociateWithConversations(); + static std::optional> ParseSearchQuerySummaryResponse(const base::Value& value); diff --git a/components/ai_chat/core/browser/conversation_handler.cc b/components/ai_chat/core/browser/conversation_handler.cc index 680eee836545..b72b43c3c276 100644 --- a/components/ai_chat/core/browser/conversation_handler.cc +++ b/components/ai_chat/core/browser/conversation_handler.cc @@ -390,6 +390,9 @@ void ConversationHandler::OnAssociatedContentDestroyed( // fetch. It may be populated later, e.g. through back navigation. // If this conversation is allowed to be associated with content, we can keep // using our current cached content. + auto content_id = associated_content_delegate_ + ? associated_content_delegate_->GetContentId() + : -1; DisassociateContentDelegate(); if (!chat_history_.empty() && should_send_page_contents_ && metadata_->associated_content && @@ -403,7 +406,7 @@ void ConversationHandler::OnAssociatedContentDestroyed( OnAssociatedContentInfoChanged(); // Notify observers for (auto& observer : observers_) { - observer.OnAssociatedContentDestroyed(this); + observer.OnAssociatedContentDestroyed(this, content_id); } } diff --git a/components/ai_chat/core/browser/conversation_handler.h b/components/ai_chat/core/browser/conversation_handler.h index ac1fe209a63f..8036f52fdee0 100644 --- a/components/ai_chat/core/browser/conversation_handler.h +++ b/components/ai_chat/core/browser/conversation_handler.h @@ -169,7 +169,8 @@ class ConversationHandler : public mojom::ConversationHandler, virtual void OnSelectedLanguageChanged( ConversationHandler* handler, const std::string& selected_language) {} - virtual void OnAssociatedContentDestroyed(ConversationHandler* handler) {} + virtual void OnAssociatedContentDestroyed(ConversationHandler* handler, + int content_id) {} }; ConversationHandler( From 57499b04b5963c09024e9149da6437baaad27534 Mon Sep 17 00:00:00 2001 From: Pete Miller Date: Sun, 8 Dec 2024 14:17:36 -0800 Subject: [PATCH 3/3] fix DanglingPtr in unit test --- .../ai_chat/core/browser/ai_chat_service_unittest.cc | 10 +++------- .../ai_chat/core/browser/conversation_handler.cc | 5 +---- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/components/ai_chat/core/browser/ai_chat_service_unittest.cc b/components/ai_chat/core/browser/ai_chat_service_unittest.cc index a8c5a3bec468..12f4c2af8a57 100644 --- a/components/ai_chat/core/browser/ai_chat_service_unittest.cc +++ b/components/ai_chat/core/browser/ai_chat_service_unittest.cc @@ -104,7 +104,6 @@ class MockConversationHandlerClient : public mojom::ConversationUI { explicit MockConversationHandlerClient(ConversationHandler* driver) { driver->Bind(conversation_handler_remote_.BindNewPipeAndPassReceiver(), conversation_ui_receiver_.BindNewPipeAndPassRemote()); - conversation_handler_ = driver; } ~MockConversationHandlerClient() override = default; @@ -114,9 +113,6 @@ class MockConversationHandlerClient : public mojom::ConversationUI { conversation_ui_receiver_.reset(); } - ConversationHandler* GetConversationHandler() { - return conversation_handler_; - } MOCK_METHOD(void, OnConversationHistoryUpdate, (), (override)); @@ -147,7 +143,6 @@ class MockConversationHandlerClient : public mojom::ConversationUI { private: mojo::Receiver conversation_ui_receiver_{this}; mojo::Remote conversation_handler_remote_; - raw_ptr conversation_handler_; }; class MockAssociatedContent @@ -193,10 +188,11 @@ class MockAssociatedContent void DisassociateWithConversations(std::string archived_text_content, bool archived_is_video) { - std::set> related_conversations; + std::vector> related_conversations; for (auto& conversation : related_conversations_) { - related_conversations.insert(conversation); + related_conversations.push_back(conversation->GetWeakPtr()); } + for (auto& conversation : related_conversations) { if (conversation) { conversation->OnAssociatedContentDestroyed(archived_text_content, diff --git a/components/ai_chat/core/browser/conversation_handler.cc b/components/ai_chat/core/browser/conversation_handler.cc index b72b43c3c276..62bf21009328 100644 --- a/components/ai_chat/core/browser/conversation_handler.cc +++ b/components/ai_chat/core/browser/conversation_handler.cc @@ -386,10 +386,7 @@ void ConversationHandler::InitEngine() { void ConversationHandler::OnAssociatedContentDestroyed( std::string last_text_content, bool is_video) { - // The associated content delegate is destroyed, so we should not try to - // fetch. It may be populated later, e.g. through back navigation. - // If this conversation is allowed to be associated with content, we can keep - // using our current cached content. + // The associated content delegate is already or about to be destroyed. auto content_id = associated_content_delegate_ ? associated_content_delegate_->GetContentId() : -1;