[cobrowse] Do not assume turn id is populated for aim threads There are some scenarios where aim has a thread ID but no turn ID or a non-standard turn ID. The most common case for this is share links generated for a thread where "mstk" is replaced by "smstk". When refreshing the page after loading one of these links, the URL generation logic for the embedded page would expect a turn ID to be available when instead there was nullopt, the bad optional access would crash the browser. This patch makes it acceptable for a thread to be maintained without a turn ID. For these threads, the next input will produce an mstk. (cherry picked from commit e11e38529ee69912dc3e4cd9a011aa5701b88930) Bug: 502651552 Change-Id: I6af6a35ef47b90ee9f3741014e73541ee87b2163 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7766129 Reviewed-by: Duncan Mercer <mercerd@google.com> Commit-Queue: Matthew Jones <mdjones@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#1615900} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7782653 Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com> Owners-Override: Srinivas Sista <srinivassista@chromium.org> Cr-Commit-Position: refs/branch-heads/7727_111@{#3} Cr-Branched-From: d1ead9fc1b005c1458ac6bfb40d3f6b9404ec4f7-refs/branch-heads/7727@{#3353} Cr-Branched-From: ce01102937348db7b88c8a4257ee4b3ac702eb1a-refs/heads/main@{#1596535}
diff --git a/components/contextual_tasks/internal/contextual_tasks_service_impl.cc b/components/contextual_tasks/internal/contextual_tasks_service_impl.cc index 2b4d4c52..b4f29ea 100644 --- a/components/contextual_tasks/internal/contextual_tasks_service_impl.cc +++ b/components/contextual_tasks/internal/contextual_tasks_service_impl.cc
@@ -569,9 +569,14 @@ // URL. A query parameter needs to be present, but its // value is not used for continued threads. url = net::AppendQueryParameter(url, "q", thread->title); - DCHECK(thread->conversation_turn_id.has_value()); - url = net::AppendQueryParameter( - url, "mstk", thread->conversation_turn_id.value()); + + // There are rare cases where the mstk may not be present before + // this url is requested. If that is the case, do not attempt to + // include it in the url. + if (thread->conversation_turn_id) { + url = net::AppendQueryParameter( + url, "mstk", thread->conversation_turn_id.value()); + } url = net::AppendQueryParameter(url, "mtid", thread->server_id); } else if (thread->type == ThreadType::kGemini) {
diff --git a/components/contextual_tasks/internal/contextual_tasks_service_impl_unittest.cc b/components/contextual_tasks/internal/contextual_tasks_service_impl_unittest.cc index a48e617..998a658 100644 --- a/components/contextual_tasks/internal/contextual_tasks_service_impl_unittest.cc +++ b/components/contextual_tasks/internal/contextual_tasks_service_impl_unittest.cc
@@ -2019,6 +2019,38 @@ run_loop.Run(); } +// It's possible for aim to get into a state where there is a thread ID but no +// turn ID (mstk). The most common case is shared links where a "share" turn ID +// (smstk) is used instead. +TEST_F(ContextualTasksServiceImplTest, GetThreadUrlFromTaskId_Aim_NoTurnId) { + ContextualTask task = service_->CreateTask(); + + const std::string server_id = "1234"; + const std::string title = "title"; + service_->UpdateThreadForTask(task.GetTaskId(), ThreadType::kAiMode, + server_id, std::nullopt, title); + + base::RunLoop run_loop; + service_->GetThreadUrlFromTaskId( + task.GetTaskId(), "en-us", + omnibox::ChromeAimEntryPoint::UNKNOWN_AIM_ENTRY_POINT, + base::BindOnce( + [](const std::string& server_id, GURL url) { + ASSERT_TRUE(base::StartsWith(url.host(), "www.google.com")); + ASSERT_EQ("/search", url.path()); + + std::string mstk; + ASSERT_FALSE(net::GetValueForKeyInQuery(url, "mstk", &mstk)); + + std::string mtid; + net::GetValueForKeyInQuery(url, "mtid", &mtid); + ASSERT_EQ(mtid, server_id); + }, + server_id) + .Then(run_loop.QuitClosure())); + run_loop.Run(); +} + TEST_F(ContextualTasksServiceImplTest, GetThreadUrlFromTaskId_Gemini) { ContextualTask task = service_->CreateTask();
diff --git a/components/contextual_tasks/public/contextual_task.cc b/components/contextual_tasks/public/contextual_task.cc index b72cb0f..54087ee 100644 --- a/components/contextual_tasks/public/contextual_task.cc +++ b/components/contextual_tasks/public/contextual_task.cc
@@ -23,7 +23,6 @@ last_turn_time(base::Time::FromMillisecondsSinceUnixEpoch( last_turn_time_unix_epoch_millis)), conversation_turn_id(conversation_turn_id) { - DCHECK(type != ThreadType::kAiMode || conversation_turn_id); } Thread::Thread(const Thread& other) = default; Thread::~Thread() = default;