From 036c7f280badc78c658c5cc9252acd6ae73be6a6 Mon Sep 17 00:00:00 2001 From: Nikhil Date: Wed, 1 Apr 2026 15:06:41 -0700 Subject: [PATCH] fix: tab-grouping cdp crash (#635) * fix: tab group crash + history fix * fix: tab group crash + history fix --- .../devtools/protocol/browser_handler.cc | 26 +++- .../protocol/devtools_protocol_browsertest.cc | 123 ++++++++++++++++++ .../devtools/protocol/history_handler.cc | 4 +- 3 files changed, 144 insertions(+), 9 deletions(-) create mode 100644 packages/browseros/chromium_patches/chrome/browser/devtools/protocol/devtools_protocol_browsertest.cc diff --git a/packages/browseros/chromium_patches/chrome/browser/devtools/protocol/browser_handler.cc b/packages/browseros/chromium_patches/chrome/browser/devtools/protocol/browser_handler.cc index 92053f9e7..e6a0971c4 100644 --- a/packages/browseros/chromium_patches/chrome/browser/devtools/protocol/browser_handler.cc +++ b/packages/browseros/chromium_patches/chrome/browser/devtools/protocol/browser_handler.cc @@ -1,8 +1,13 @@ diff --git a/chrome/browser/devtools/protocol/browser_handler.cc b/chrome/browser/devtools/protocol/browser_handler.cc -index 30bd52d09c3fc..33c7d6d8455fc 100644 +index 30bd52d09c3fc..dd9ef4e3b7cbb 100644 --- a/chrome/browser/devtools/protocol/browser_handler.cc +++ b/chrome/browser/devtools/protocol/browser_handler.cc -@@ -8,19 +8,32 @@ +@@ -4,23 +4,37 @@ + + #include "chrome/browser/devtools/protocol/browser_handler.h" + ++#include + #include #include #include "base/functional/bind.h" @@ -35,7 +40,7 @@ index 30bd52d09c3fc..33c7d6d8455fc 100644 #include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/devtools_agent_host.h" -@@ -30,10 +43,21 @@ +@@ -30,10 +44,21 @@ #include "ui/gfx/image/image.h" #include "ui/gfx/image/image_png_rep.h" @@ -57,7 +62,7 @@ index 30bd52d09c3fc..33c7d6d8455fc 100644 BrowserWindow* GetBrowserWindow(int window_id) { BrowserWindow* result = nullptr; ForEachCurrentBrowserWindowInterfaceOrderedByActivation( -@@ -72,17 +96,411 @@ std::unique_ptr GetBrowserWindowBounds( +@@ -72,17 +97,419 @@ std::unique_ptr GetBrowserWindowBounds( .Build(); } @@ -437,6 +442,14 @@ index 30bd52d09c3fc..33c7d6d8455fc 100644 + out_indices->push_back(found_index); + } + ++ if (!(*out_bwi)->GetTabStripModel()->SupportsTabGroups()) { ++ return Response::ServerError("Tab grouping not supported for this window"); ++ } ++ ++ std::ranges::sort(*out_indices); ++ out_indices->erase(std::ranges::unique(*out_indices).begin(), ++ out_indices->end()); ++ + return Response::Success(); +} + @@ -471,7 +484,7 @@ index 30bd52d09c3fc..33c7d6d8455fc 100644 Response BrowserHandler::GetWindowForTarget( std::optional target_id, -@@ -120,6 +538,65 @@ Response BrowserHandler::GetWindowForTarget( +@@ -120,6 +547,65 @@ Response BrowserHandler::GetWindowForTarget( return Response::Success(); } @@ -537,7 +550,7 @@ index 30bd52d09c3fc..33c7d6d8455fc 100644 Response BrowserHandler::GetWindowBounds( int window_id, std::unique_ptr* out_bounds) { -@@ -297,3 +774,910 @@ protocol::Response BrowserHandler::AddPrivacySandboxEnrollmentOverride( +@@ -297,3 +783,909 @@ protocol::Response BrowserHandler::AddPrivacySandboxEnrollmentOverride( net::SchemefulSite(url_to_add)); return Response::Success(); } @@ -1447,4 +1460,3 @@ index 30bd52d09c3fc..33c7d6d8455fc 100644 +bool BrowserHandler::IsHiddenWindow(int window_id) const { + return hidden_window_ids_.contains(window_id); +} -+ diff --git a/packages/browseros/chromium_patches/chrome/browser/devtools/protocol/devtools_protocol_browsertest.cc b/packages/browseros/chromium_patches/chrome/browser/devtools/protocol/devtools_protocol_browsertest.cc new file mode 100644 index 000000000..41ed24738 --- /dev/null +++ b/packages/browseros/chromium_patches/chrome/browser/devtools/protocol/devtools_protocol_browsertest.cc @@ -0,0 +1,123 @@ +diff --git a/chrome/browser/devtools/protocol/devtools_protocol_browsertest.cc b/chrome/browser/devtools/protocol/devtools_protocol_browsertest.cc +index e57b0883b725f..58bfa8d8f5412 100644 +--- a/chrome/browser/devtools/protocol/devtools_protocol_browsertest.cc ++++ b/chrome/browser/devtools/protocol/devtools_protocol_browsertest.cc +@@ -20,6 +20,7 @@ + #include "base/test/test_switches.h" + #include "base/test/values_test_util.h" + #include "base/threading/thread_restrictions.h" ++#include "base/time/time.h" + #include "base/values.h" + #include "build/build_config.h" + #include "chrome/browser/apps/app_service/app_service_proxy.h" +@@ -30,6 +31,7 @@ + #include "chrome/browser/data_saver/data_saver.h" + #include "chrome/browser/devtools/devtools_window.h" + #include "chrome/browser/devtools/protocol/devtools_protocol_test_support.h" ++#include "chrome/browser/history/history_service_factory.h" + #include "chrome/browser/preloading/preloading_prefs.h" + #include "chrome/browser/privacy_sandbox/privacy_sandbox_attestations/privacy_sandbox_attestations_mixin.h" + #include "chrome/browser/profiles/profile.h" +@@ -43,6 +45,8 @@ + #include "components/content_settings/core/browser/cookie_settings.h" + #include "components/content_settings/core/common/pref_names.h" + #include "components/custom_handlers/protocol_handler_registry.h" ++#include "components/history/core/browser/history_service.h" ++#include "components/history/core/test/history_service_test_util.h" + #include "components/infobars/content/content_infobar_manager.h" + #include "components/infobars/core/infobar.h" + #include "components/infobars/core/infobar_delegate.h" +@@ -2202,6 +2206,93 @@ IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, + SendCommandSync("Target.getTargets"); + EXPECT_EQ(2u, result()->FindList("targetInfos")->size()); + } ++ ++IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, ++ CreateTabGroupAcceptsUnsortedTabIds) { ++ AttachToBrowserTarget(); ++ ++ ASSERT_EQ(1, browser()->tab_strip_model()->count()); ++ ++ base::DictValue params; ++ params.Set("url", "about:blank"); ++ params.Set("background", true); ++ ASSERT_TRUE(SendCommandSync("Browser.createTab", params.Clone())); ++ ASSERT_TRUE(SendCommandSync("Browser.createTab", std::move(params))); ++ ++ const base::DictValue* tabs_result = SendCommandSync("Browser.getTabs"); ++ ASSERT_TRUE(tabs_result); ++ const base::ListValue* tabs = tabs_result->FindList("tabs"); ++ ASSERT_TRUE(tabs); ++ ASSERT_EQ(3u, tabs->size()); ++ ++ std::vector tab_ids; ++ tab_ids.reserve(tabs->size()); ++ for (const auto& tab : *tabs) { ++ tab_ids.push_back(*tab.GetDict().FindInt("tabId")); ++ } ++ ++ base::ListValue unsorted_tab_ids; ++ unsorted_tab_ids.Append(tab_ids[2]); ++ unsorted_tab_ids.Append(tab_ids[0]); ++ ++ base::DictValue create_group_params; ++ create_group_params.Set("tabIds", std::move(unsorted_tab_ids)); ++ create_group_params.Set("title", "Unsorted"); ++ ++ const base::DictValue* create_group_result = ++ SendCommandSync("Browser.createTabGroup", std::move(create_group_params)); ++ ASSERT_TRUE(create_group_result); ++ ASSERT_FALSE(error()); ++ ++ const base::DictValue* group = create_group_result->FindDict("group"); ++ ASSERT_TRUE(group); ++ const base::ListValue* grouped_tab_ids = group->FindList("tabIds"); ++ ASSERT_TRUE(grouped_tab_ids); ++ ASSERT_EQ(2u, grouped_tab_ids->size()); ++ EXPECT_EQ(tab_ids[0], *grouped_tab_ids->front().GetIfInt()); ++ EXPECT_EQ(tab_ids[2], *grouped_tab_ids->back().GetIfInt()); ++ EXPECT_EQ("Unsorted", *group->FindString("title")); ++} ++ ++IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, HistorySearchUsesVisitTime) { ++ AttachToBrowserTarget(); ++ ++ history::HistoryService* history_service = ++ HistoryServiceFactory::GetForProfile(browser()->profile(), ++ ServiceAccessType::EXPLICIT_ACCESS); ++ ui_test_utils::WaitForHistoryToLoad(history_service); ++ ++ const GURL url("https://history-timestamp-test.example/path"); ++ const base::Time older_visit = base::Time::Now() - base::Days(2); ++ const base::Time newer_visit = base::Time::Now() - base::Hours(1); ++ ++ history_service->AddPage(url, older_visit, history::SOURCE_BROWSED); ++ history_service->AddPage(url, newer_visit, history::SOURCE_BROWSED); ++ history::BlockUntilHistoryProcessesPendingRequests(history_service); ++ ++ base::DictValue search_params; ++ search_params.Set("query", ""); ++ search_params.Set( ++ "startTime", ++ (older_visit - base::Minutes(1)).InMillisecondsFSinceUnixEpoch()); ++ search_params.Set( ++ "endTime", ++ (newer_visit - base::Minutes(1)).InMillisecondsFSinceUnixEpoch()); ++ ++ const base::DictValue* search_result = ++ SendCommandSync("History.search", std::move(search_params)); ++ ASSERT_TRUE(search_result); ++ ASSERT_FALSE(error()); ++ ++ const base::ListValue* entries = search_result->FindList("entries"); ++ ASSERT_TRUE(entries); ++ ASSERT_EQ(1u, entries->size()); ++ ++ const base::DictValue& entry = entries->front().GetDict(); ++ EXPECT_EQ(url.spec(), *entry.FindString("url")); ++ EXPECT_EQ(older_visit.InMillisecondsFSinceUnixEpoch(), ++ *entry.FindDouble("lastVisitTime")); ++} + #endif // !BUILDFLAG(IS_ANDROID) + + #if !BUILDFLAG(IS_ANDROID) diff --git a/packages/browseros/chromium_patches/chrome/browser/devtools/protocol/history_handler.cc b/packages/browseros/chromium_patches/chrome/browser/devtools/protocol/history_handler.cc index dc855ee1f..4378c8334 100644 --- a/packages/browseros/chromium_patches/chrome/browser/devtools/protocol/history_handler.cc +++ b/packages/browseros/chromium_patches/chrome/browser/devtools/protocol/history_handler.cc @@ -1,6 +1,6 @@ diff --git a/chrome/browser/devtools/protocol/history_handler.cc b/chrome/browser/devtools/protocol/history_handler.cc new file mode 100644 -index 0000000000000..689f6e900a968 +index 0000000000000..4087a679a527f --- /dev/null +++ b/chrome/browser/devtools/protocol/history_handler.cc @@ -0,0 +1,188 @@ @@ -36,7 +36,7 @@ index 0000000000000..689f6e900a968 + .SetId(base::NumberToString(result.id())) + .SetUrl(result.url().spec()) + .SetTitle(base::UTF16ToUTF8(result.title())) -+ .SetLastVisitTime(result.last_visit().InMillisecondsFSinceUnixEpoch()) ++ .SetLastVisitTime(result.visit_time().InMillisecondsFSinceUnixEpoch()) + .SetVisitCount(result.visit_count()) + .SetTypedCount(result.typed_count()) + .Build();