fix: tab-grouping cdp crash (#635)

* fix: tab group crash + history fix

* fix: tab group crash + history fix
This commit is contained in:
Nikhil
2026-04-01 15:06:41 -07:00
committed by GitHub
parent 000429277d
commit 036c7f280b
3 changed files with 144 additions and 9 deletions

View File

@@ -1,8 +1,13 @@
diff --git a/chrome/browser/devtools/protocol/browser_handler.cc b/chrome/browser/devtools/protocol/browser_handler.cc 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 --- a/chrome/browser/devtools/protocol/browser_handler.cc
+++ b/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 <algorithm>
#include <set>
#include <vector> #include <vector>
#include "base/functional/bind.h" #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_task_traits.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "content/public/browser/devtools_agent_host.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.h"
#include "ui/gfx/image/image_png_rep.h" #include "ui/gfx/image/image_png_rep.h"
@@ -57,7 +62,7 @@ index 30bd52d09c3fc..33c7d6d8455fc 100644
BrowserWindow* GetBrowserWindow(int window_id) { BrowserWindow* GetBrowserWindow(int window_id) {
BrowserWindow* result = nullptr; BrowserWindow* result = nullptr;
ForEachCurrentBrowserWindowInterfaceOrderedByActivation( ForEachCurrentBrowserWindowInterfaceOrderedByActivation(
@@ -72,17 +96,411 @@ std::unique_ptr<protocol::Browser::Bounds> GetBrowserWindowBounds( @@ -72,17 +97,419 @@ std::unique_ptr<protocol::Browser::Bounds> GetBrowserWindowBounds(
.Build(); .Build();
} }
@@ -437,6 +442,14 @@ index 30bd52d09c3fc..33c7d6d8455fc 100644
+ out_indices->push_back(found_index); + 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(); + return Response::Success();
+} +}
+ +
@@ -471,7 +484,7 @@ index 30bd52d09c3fc..33c7d6d8455fc 100644
Response BrowserHandler::GetWindowForTarget( Response BrowserHandler::GetWindowForTarget(
std::optional<std::string> target_id, std::optional<std::string> target_id,
@@ -120,6 +538,65 @@ Response BrowserHandler::GetWindowForTarget( @@ -120,6 +547,65 @@ Response BrowserHandler::GetWindowForTarget(
return Response::Success(); return Response::Success();
} }
@@ -537,7 +550,7 @@ index 30bd52d09c3fc..33c7d6d8455fc 100644
Response BrowserHandler::GetWindowBounds( Response BrowserHandler::GetWindowBounds(
int window_id, int window_id,
std::unique_ptr<protocol::Browser::Bounds>* out_bounds) { std::unique_ptr<protocol::Browser::Bounds>* out_bounds) {
@@ -297,3 +774,910 @@ protocol::Response BrowserHandler::AddPrivacySandboxEnrollmentOverride( @@ -297,3 +783,909 @@ protocol::Response BrowserHandler::AddPrivacySandboxEnrollmentOverride(
net::SchemefulSite(url_to_add)); net::SchemefulSite(url_to_add));
return Response::Success(); return Response::Success();
} }
@@ -1447,4 +1460,3 @@ index 30bd52d09c3fc..33c7d6d8455fc 100644
+bool BrowserHandler::IsHiddenWindow(int window_id) const { +bool BrowserHandler::IsHiddenWindow(int window_id) const {
+ return hidden_window_ids_.contains(window_id); + return hidden_window_ids_.contains(window_id);
+} +}
+

View File

@@ -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<int> 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)

View File

@@ -1,6 +1,6 @@
diff --git a/chrome/browser/devtools/protocol/history_handler.cc b/chrome/browser/devtools/protocol/history_handler.cc diff --git a/chrome/browser/devtools/protocol/history_handler.cc b/chrome/browser/devtools/protocol/history_handler.cc
new file mode 100644 new file mode 100644
index 0000000000000..689f6e900a968 index 0000000000000..4087a679a527f
--- /dev/null --- /dev/null
+++ b/chrome/browser/devtools/protocol/history_handler.cc +++ b/chrome/browser/devtools/protocol/history_handler.cc
@@ -0,0 +1,188 @@ @@ -0,0 +1,188 @@
@@ -36,7 +36,7 @@ index 0000000000000..689f6e900a968
+ .SetId(base::NumberToString(result.id())) + .SetId(base::NumberToString(result.id()))
+ .SetUrl(result.url().spec()) + .SetUrl(result.url().spec())
+ .SetTitle(base::UTF16ToUTF8(result.title())) + .SetTitle(base::UTF16ToUTF8(result.title()))
+ .SetLastVisitTime(result.last_visit().InMillisecondsFSinceUnixEpoch()) + .SetLastVisitTime(result.visit_time().InMillisecondsFSinceUnixEpoch())
+ .SetVisitCount(result.visit_count()) + .SetVisitCount(result.visit_count())
+ .SetTypedCount(result.typed_count()) + .SetTypedCount(result.typed_count())
+ .Build(); + .Build();