[Vertical Tabs] Add TabCollectionNode support for existing components.
This integrates TabCollectionNode with VerticalTabStripRegionView,
VerticalTabStripView, VerticalPinnedTabContainerView, and
VerticalUnpinnedTabContainerView.
Bug: 441086907, 441087017, 441087251, 441086217
Change-Id: I8a1b25199ace475ead5cf70fb6939759234aa202
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6907294
Reviewed-by: David Pennington <dpenning@chromium.org>
Commit-Queue: Caroline Rising <corising@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1511590}
diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn
index 3461ae3..77cacb5 100644
--- a/chrome/browser/ui/BUILD.gn
+++ b/chrome/browser/ui/BUILD.gn
@@ -4623,6 +4623,7 @@
"//chrome/browser/ui/sharing_hub",
"//chrome/browser/ui/sharing_hub:impl",
"//chrome/browser/ui/side_search",
+ "//chrome/browser/ui/tabs/tab_strip_api",
"//chrome/browser/ui/views",
"//chrome/browser/ui/views/bubble",
"//chrome/browser/ui/views/download",
diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc
index 43fe605..b91025c 100644
--- a/chrome/browser/ui/views/frame/browser_view.cc
+++ b/chrome/browser/ui/views/frame/browser_view.cc
@@ -98,6 +98,7 @@
#include "chrome/browser/ui/tabs/features.h"
#include "chrome/browser/ui/tabs/saved_tab_groups/collaboration_messaging_tab_data.h"
#include "chrome/browser/ui/tabs/tab_enums.h"
+#include "chrome/browser/ui/tabs/tab_strip_api/tab_strip_service_feature.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/browser/ui/tabs/tab_utils.h"
#include "chrome/browser/ui/tabs/vertical_tab_strip_state_controller.h"
@@ -1011,6 +1012,9 @@
if (tabs::AreVerticalTabsEnabled()) {
auto vertical_tab_strip_container =
std::make_unique<VerticalTabStripRegionView>(
+ browser_->GetFeatures()
+ .tab_strip_service_feature()
+ ->GetTabStripService(),
browser_->GetFeatures().vertical_tab_strip_state_controller());
vertical_tab_strip_container_ =
AddChildView(std::move(vertical_tab_strip_container));
diff --git a/chrome/browser/ui/views/frame/browser_view.h b/chrome/browser/ui/views/frame/browser_view.h
index 4d2a7b3..702ac11 100644
--- a/chrome/browser/ui/views/frame/browser_view.h
+++ b/chrome/browser/ui/views/frame/browser_view.h
@@ -90,6 +90,7 @@
class TopContainerView;
class TopControlsSlideController;
class TopControlsSlideControllerTest;
+class VerticalTabStripRegionView;
class WebAppFrameToolbarView;
class WebUITabStripContainerView;
@@ -255,6 +256,10 @@
return tab_strip_region_view_;
}
+ VerticalTabStripRegionView* vertical_tab_strip_region_view() const {
+ return vertical_tab_strip_container_;
+ }
+
// Accessor for the TabStrip.
TabStrip* tabstrip() { return tab_strip_region_view_->tab_strip(); }
@@ -1228,7 +1233,7 @@
raw_ptr<ContentsContainerView> contents_container_view_ = nullptr;
// The view responsible for housing the contents of the vertical tab strip.
- raw_ptr<views::View> vertical_tab_strip_container_ = nullptr;
+ raw_ptr<VerticalTabStripRegionView> vertical_tab_strip_container_ = nullptr;
// The side panel aligned to the left or the right side of the browser window
// depending on the kSidePanelHorizontalAlignment pref's value.
diff --git a/chrome/browser/ui/views/frame/vertical_tab_strip_region_view.cc b/chrome/browser/ui/views/frame/vertical_tab_strip_region_view.cc
index 03063c6..1ab1890 100644
--- a/chrome/browser/ui/views/frame/vertical_tab_strip_region_view.cc
+++ b/chrome/browser/ui/views/frame/vertical_tab_strip_region_view.cc
@@ -9,7 +9,9 @@
#include "chrome/browser/ui/browser_element_identifiers.h"
#include "chrome/browser/ui/color/chrome_color_id.h"
#include "chrome/browser/ui/layout_constants.h"
+#include "chrome/browser/ui/tabs/tab_strip_api/tab_strip_service.h"
#include "chrome/browser/ui/tabs/vertical_tab_strip_state_controller.h"
+#include "chrome/browser/ui/views/tabs/vertical/root_tab_collection_node.h"
#include "chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_view.h"
#include "ui/base/metadata/metadata_impl_macros.h"
#include "ui/color/color_id.h"
@@ -22,6 +24,7 @@
#include "ui/views/layout/layout_types.h"
#include "ui/views/view.h"
#include "ui/views/view_class_properties.h"
+#include "ui/views/view_utils.h"
namespace {
constexpr gfx::Insets kRegionInteriorMargins = gfx::Insets::VH(8, 0);
@@ -30,7 +33,9 @@
} // namespace
VerticalTabStripRegionView::VerticalTabStripRegionView(
- tabs::VerticalTabStripStateController* state_controller) {
+ tabs_api::TabStripService* service_register,
+ tabs::VerticalTabStripStateController* state_controller)
+ : state_controller_(state_controller) {
SetBackground(views::CreateSolidBackground(ui::kColorFrameActive));
SetLayoutManager(std::make_unique<views::FlexLayout>())
->SetOrientation(views::LayoutOrientation::kVertical)
@@ -51,18 +56,6 @@
top_button_separator_ = AddChildView(std::make_unique<views::Separator>());
top_button_separator_->SetColorId(kColorTabDividerFrameActive);
- tab_strip_view_ = AddChildView(std::make_unique<VerticalTabStripView>());
- tab_strip_view_->SetCollapsedState(state_controller->IsCollapsed());
- gfx::Insets tab_container_margins = gfx::Insets::TLBR(
- kRegionVerticalPadding,
- GetLayoutConstant(VERTICAL_TAB_STRIP_HORIZONTAL_PADDING),
- kRegionVerticalPadding, 0);
- tab_strip_view_->SetProperty(
- views::kFlexBehaviorKey,
- views::FlexSpecification(views::MinimumFlexSizeRule::kScaleToZero,
- views::MaximumFlexSizeRule::kUnbounded));
- tab_strip_view_->SetProperty(views::kMarginsKey, tab_container_margins);
-
segmented_button_ = AddChildView(std::make_unique<views::View>());
gemini_button_ = AddChildView(std::make_unique<views::View>());
@@ -76,6 +69,11 @@
base::Unretained(this)));
SetProperty(views::kElementIdentifierKey, kVerticalTabStripRegionElementId);
+
+ root_node_ = std::make_unique<RootTabCollectionNode>(
+ service_register, this,
+ base::BindRepeating(&VerticalTabStripRegionView::SetTabStripView,
+ base::Unretained(this)));
}
VerticalTabStripRegionView::~VerticalTabStripRegionView() = default;
@@ -92,6 +90,27 @@
void VerticalTabStripRegionView::OnResize(int resize_amount,
bool done_resizing) {}
+views::View* VerticalTabStripRegionView::SetTabStripView(
+ std::unique_ptr<views::View> view) {
+ CHECK(views::IsViewClass<VerticalTabStripView>(view.get()));
+ tab_strip_view_ =
+ static_cast<VerticalTabStripView*>(AddChildView(std::move(view)));
+ tab_strip_view_->SetCollapsedState(state_controller_->IsCollapsed());
+ gfx::Insets tab_container_margins = gfx::Insets::TLBR(
+ kRegionVerticalPadding,
+ GetLayoutConstant(VERTICAL_TAB_STRIP_HORIZONTAL_PADDING),
+ kRegionVerticalPadding, 0);
+ tab_strip_view_->SetProperty(
+ views::kFlexBehaviorKey,
+ views::FlexSpecification(views::MinimumFlexSizeRule::kScaleToZero,
+ views::MaximumFlexSizeRule::kUnbounded));
+ tab_strip_view_->SetProperty(views::kMarginsKey, tab_container_margins);
+ std::optional<size_t> separator_index = GetIndexOf(top_button_separator_);
+ CHECK(separator_index.has_value());
+ ReorderChildView(tab_strip_view_, separator_index.value() + 1);
+ return tab_strip_view_;
+}
+
void VerticalTabStripRegionView::OnCollapsedStateChanged(
tabs::VerticalTabStripStateController* state_controller) {
tab_strip_view_->SetCollapsedState(state_controller->IsCollapsed());
diff --git a/chrome/browser/ui/views/frame/vertical_tab_strip_region_view.h b/chrome/browser/ui/views/frame/vertical_tab_strip_region_view.h
index c80f7c7f..6d9ce2e 100644
--- a/chrome/browser/ui/views/frame/vertical_tab_strip_region_view.h
+++ b/chrome/browser/ui/views/frame/vertical_tab_strip_region_view.h
@@ -12,6 +12,7 @@
#include "ui/views/accessible_pane_view.h"
#include "ui/views/controls/resize_area_delegate.h"
+class RootTabCollectionNode;
class VerticalUnpinnedTabContainerView;
class VerticalPinnedTabContainerView;
@@ -19,6 +20,10 @@
class VerticalTabStripStateController;
} // namespace tabs
+namespace tabs_api {
+class TabStripService;
+}
+
namespace views {
class ResizeArea;
class Separator;
@@ -35,6 +40,7 @@
static constexpr int kResizeAreaWidth = 6;
explicit VerticalTabStripRegionView(
+ tabs_api::TabStripService* service_register,
tabs::VerticalTabStripStateController* state_controller);
VerticalTabStripRegionView(const VerticalTabStripRegionView&) = delete;
VerticalTabStripRegionView& operator=(const VerticalTabStripRegionView&) =
@@ -46,10 +52,10 @@
}
views::ResizeArea* resize_area_for_testing() { return resize_area_; }
VerticalPinnedTabContainerView* pinned_tabs_container_for_testing() {
- return tab_strip_view_->pinned_tabs_container_for_testing();
+ return tab_strip_view_->GetPinnedTabsContainerForTesting();
}
VerticalUnpinnedTabContainerView* unpinned_tabs_container_for_testing() {
- return tab_strip_view_->unpinned_tabs_container_for_testing();
+ return tab_strip_view_->GetUnpinnedTabsContainerForTesting();
}
// views::View:
@@ -59,6 +65,8 @@
void OnResize(int resize_amount, bool done_resizing) override;
private:
+ views::View* SetTabStripView(std::unique_ptr<views::View> view);
+
void OnCollapsedStateChanged(
tabs::VerticalTabStripStateController* state_controller);
@@ -68,7 +76,9 @@
raw_ptr<views::View> segmented_button_ = nullptr;
raw_ptr<views::View> gemini_button_ = nullptr;
raw_ptr<views::ResizeArea> resize_area_ = nullptr;
+ std::unique_ptr<RootTabCollectionNode> root_node_;
+ raw_ptr<tabs::VerticalTabStripStateController> state_controller_;
base::CallbackListSubscription collapsed_state_changed_subscription_;
};
diff --git a/chrome/browser/ui/views/frame/vertical_tab_strip_region_view_unittest.cc b/chrome/browser/ui/views/frame/vertical_tab_strip_region_view_browsertest.cc
similarity index 67%
rename from chrome/browser/ui/views/frame/vertical_tab_strip_region_view_unittest.cc
rename to chrome/browser/ui/views/frame/vertical_tab_strip_region_view_browsertest.cc
index c890a26..6dac34c1 100644
--- a/chrome/browser/ui/views/frame/vertical_tab_strip_region_view_unittest.cc
+++ b/chrome/browser/ui/views/frame/vertical_tab_strip_region_view_browsertest.cc
@@ -4,52 +4,47 @@
#include "chrome/browser/ui/views/frame/vertical_tab_strip_region_view.h"
+#include "base/test/scoped_feature_list.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_window/public/browser_window_features.h"
+#include "chrome/browser/ui/tabs/features.h"
#include "chrome/browser/ui/tabs/vertical_tab_strip_state.h"
#include "chrome/browser/ui/tabs/vertical_tab_strip_state_controller.h"
+#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/tabs/vertical/vertical_pinned_tab_container_view.h"
#include "chrome/browser/ui/views/tabs/vertical/vertical_unpinned_tab_container_view.h"
-#include "chrome/common/pref_names.h"
-#include "components/pref_registry/pref_registry_syncable.h"
-#include "components/sync_preferences/testing_pref_service_syncable.h"
-#include "testing/gmock/include/gmock/gmock.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "content/public/test/browser_test.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/views/controls/resize_area.h"
#include "ui/views/controls/separator.h"
-class VerticalTabStripRegionViewTest : public testing::Test {
+class VerticalTabStripRegionViewTest : public InProcessBrowserTest {
public:
VerticalTabStripRegionViewTest() = default;
~VerticalTabStripRegionViewTest() override = default;
void SetUp() override {
- testing::Test::SetUp();
- pref_service_.registry()->RegisterBooleanPref(
- prefs::kVerticalTabsEnabled, true,
- user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
- controller_ =
- std::make_unique<tabs::VerticalTabStripStateController>(&pref_service_);
- region_view_ =
- std::make_unique<VerticalTabStripRegionView>(controller_.get());
+ scoped_feature_list_.InitAndEnableFeature(tabs::kVerticalTabs);
+ InProcessBrowserTest::SetUp();
}
- void TearDown() override {
- controller_.reset();
- testing::Test::TearDown();
+ VerticalTabStripRegionView* region_view() {
+ return BrowserView::GetBrowserViewForBrowser(browser())
+ ->vertical_tab_strip_region_view();
}
-
- VerticalTabStripRegionView* region_view() { return region_view_.get(); }
tabs::VerticalTabStripStateController* controller() {
- return controller_.get();
+ return browser()
+ ->browser_window_features()
+ ->vertical_tab_strip_state_controller();
}
private:
- std::unique_ptr<tabs::VerticalTabStripStateController> controller_;
- std::unique_ptr<VerticalTabStripRegionView> region_view_;
- sync_preferences::TestingPrefServiceSyncable pref_service_;
+ base::test::ScopedFeatureList scoped_feature_list_;
};
-TEST_F(VerticalTabStripRegionViewTest,
- SeparatorVisibilityChangesWithCollapsedState) {
+IN_PROC_BROWSER_TEST_F(VerticalTabStripRegionViewTest,
+ SeparatorVisibilityChangesWithCollapsedState) {
controller()->SetCollapsed(true);
EXPECT_TRUE(controller()->IsCollapsed());
EXPECT_TRUE(region_view()->tabs_separator_for_testing()->GetVisible());
@@ -59,7 +54,7 @@
EXPECT_FALSE(region_view()->tabs_separator_for_testing()->GetVisible());
}
-TEST_F(VerticalTabStripRegionViewTest, ResizeAreaBounds) {
+IN_PROC_BROWSER_TEST_F(VerticalTabStripRegionViewTest, ResizeAreaBounds) {
region_view()->SetBounds(0, 0, 200, 600);
// Verify resize area is on the right side of the VerticalTabStripRegionView.
EXPECT_EQ(region_view()->bounds().right(),
@@ -75,7 +70,8 @@
// Verify that the pinned tabs container will never be larger than the unpinned
// tabs area.
-TEST_F(VerticalTabStripRegionViewTest, PinnedTabsAreaSmallerThanUnpinned) {
+IN_PROC_BROWSER_TEST_F(VerticalTabStripRegionViewTest,
+ PinnedTabsAreaSmallerThanUnpinned) {
region_view()->SetBounds(0, 0, 200, 600);
region_view()->pinned_tabs_container_for_testing()->SetPreferredSize(
gfx::Size(100, 500));
diff --git a/chrome/browser/ui/views/tabs/vertical/tab_collection_node.cc b/chrome/browser/ui/views/tabs/vertical/tab_collection_node.cc
index 486624d..108aa9e 100644
--- a/chrome/browser/ui/views/tabs/vertical/tab_collection_node.cc
+++ b/chrome/browser/ui/views/tabs/vertical/tab_collection_node.cc
@@ -4,10 +4,16 @@
#include "chrome/browser/ui/views/tabs/vertical/tab_collection_node.h"
+#include <vector>
+
#include "base/functional/bind.h"
#include "base/no_destructor.h"
#include "chrome/browser/ui/tabs/tab_strip_api/tab_strip_api_types.mojom.h"
+#include "chrome/browser/ui/views/tabs/vertical/vertical_pinned_tab_container_view.h"
+#include "chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_view.h"
+#include "chrome/browser/ui/views/tabs/vertical/vertical_unpinned_tab_container_view.h"
#include "ui/views/view.h"
+#include "vertical_unpinned_tab_container_view.h"
namespace {
@@ -30,6 +36,11 @@
} // anonymous namespace
+base::CallbackListSubscription TabCollectionNode::RegisterWillDestroyCallback(
+ base::OnceClosure callback) {
+ return on_will_destroy_callback_list_.Add(std::move(callback));
+}
+
// static
void TabCollectionNode::SetViewFactoryForTesting(ViewFactory factory) {
GetViewFactory() = std::move(factory);
@@ -41,6 +52,23 @@
if (GetViewFactory()) {
return GetViewFactory().Run(node_for_view);
}
+ switch (node_for_view->GetType()) {
+ case Type::kTabStrip:
+ return std::make_unique<VerticalTabStripView>(node_for_view);
+ case Type::kPinnedTabs:
+ return std::make_unique<VerticalPinnedTabContainerView>(node_for_view);
+ case Type::kUnpinnedTabs:
+ return std::make_unique<VerticalUnpinnedTabContainerView>(node_for_view);
+ case Type::kSplitTab:
+ // TODO(crbug.com/442568605): support split tabs.
+ break;
+ case Type::kTabGroup:
+ // TODO(crbug.com/442567916): support tab groups.
+ break;
+ case Type::kTab:
+ // TODO(crbug.com/442567140): support tabs.
+ break;
+ }
return std::make_unique<CollectionTestViewImpl>(node_for_view);
}
@@ -50,7 +78,9 @@
CustomAddChildView add_node_to_parent_callback)
: add_node_to_parent_(std::move(add_node_to_parent_callback)) {}
-TabCollectionNode::~TabCollectionNode() = default;
+TabCollectionNode::~TabCollectionNode() {
+ on_will_destroy_callback_list_.Notify();
+}
void TabCollectionNode::Initialize(
tabs_api::mojom::ContainerPtr container,
diff --git a/chrome/browser/ui/views/tabs/vertical/tab_collection_node.h b/chrome/browser/ui/views/tabs/vertical/tab_collection_node.h
index 51a534e8..d6a80153 100644
--- a/chrome/browser/ui/views/tabs/vertical/tab_collection_node.h
+++ b/chrome/browser/ui/views/tabs/vertical/tab_collection_node.h
@@ -8,6 +8,7 @@
#include <memory>
#include <vector>
+#include "base/callback_list.h"
#include "base/functional/callback_forward.h"
#include "base/memory/raw_ptr.h"
#include "chrome/browser/ui/tabs/tab_strip_api/tab_strip_api.mojom.h"
@@ -49,12 +50,18 @@
add_child_to_node_ = std::move(add_child_to_node);
}
+ base::CallbackListSubscription RegisterWillDestroyCallback(
+ base::OnceClosure callback);
+
static void SetViewFactoryForTesting(ViewFactory factory);
+ views::View* get_view_for_testing() { return node_view_; }
protected:
static std::unique_ptr<views::View> CreateViewForNode(
TabCollectionNode* node_for_view);
+ base::OnceClosureList on_will_destroy_callback_list_;
+
// the current collection_data object. provided by snapshot and updated
// through TabObserver.
tabs_api::mojom::DataPtr data_;
diff --git a/chrome/browser/ui/views/tabs/vertical/tab_collection_node_browsertest.cc b/chrome/browser/ui/views/tabs/vertical/tab_collection_node_browsertest.cc
index 2f6ff64d..6b41889 100644
--- a/chrome/browser/ui/views/tabs/vertical/tab_collection_node_browsertest.cc
+++ b/chrome/browser/ui/views/tabs/vertical/tab_collection_node_browsertest.cc
@@ -16,9 +16,13 @@
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/tabs/browser_tab_strip_controller.h"
#include "chrome/browser/ui/views/tabs/vertical/root_tab_collection_node.h"
+#include "chrome/browser/ui/views/tabs/vertical/vertical_pinned_tab_container_view.h"
+#include "chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_view.h"
+#include "chrome/browser/ui/views/tabs/vertical/vertical_unpinned_tab_container_view.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "content/public/test/browser_test.h"
#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/views/controls/scroll_view.h"
#include "ui/views/view.h"
class TabCollectionNodeBrowserTest : public InProcessBrowserTest {
@@ -362,6 +366,74 @@
TabCollectionNode::Type::kTab);
}
+IN_PROC_BROWSER_TEST_F(TabCollectionNodeWithSplitTabBrowserTest,
+ RootNodePopulatesWithTabs_ViewClasses) {
+ AppendPinnedTab();
+ AppendTabToNewGroup();
+ AppendSplitTab();
+ auto parent_view = std::make_unique<views::View>();
+
+ RootTabCollectionNode root_node(
+ browser()
+ ->GetFeatures()
+ .tab_strip_service_feature()
+ ->GetTabStripService(),
+ parent_view.get(),
+ base::BindRepeating(static_cast<views::View* (
+ views::View::*)(std::unique_ptr<views::View>)>(
+ &views::View::AddChildView),
+ base::Unretained(parent_view.get())));
+
+ ASSERT_TRUE(
+ base::test::RunUntil([&]() { return !root_node.children().empty(); }));
+
+ // The root node should contain two nodes: one for pinned
+ // (VerticalPinnedTabContainerView), one for unpinned
+ // (VerticalUnpinnedTabContainerView).
+ ASSERT_EQ(root_node.children().size(), 2u);
+ EXPECT_TRUE(views::IsViewClass<VerticalTabStripView>(
+ root_node.get_view_for_testing()));
+ const auto& pinned_node = root_node.children()[0];
+ const auto& unpinned_node = root_node.children()[1];
+ EXPECT_EQ(pinned_node->GetType(), TabCollectionNode::Type::kPinnedTabs);
+ EXPECT_TRUE(views::IsViewClass<VerticalPinnedTabContainerView>(
+ pinned_node->get_view_for_testing()));
+ EXPECT_EQ(unpinned_node->GetType(), TabCollectionNode::Type::kUnpinnedTabs);
+ EXPECT_TRUE(views::IsViewClass<VerticalUnpinnedTabContainerView>(
+ unpinned_node->get_view_for_testing()));
+
+ // The pinned Node should be have one tab.
+ ASSERT_EQ(pinned_node->children().size(), 1u);
+ EXPECT_EQ(pinned_node->children()[0]->GetType(),
+ TabCollectionNode::Type::kTab);
+ // TODO(crbug.com/442567140): Verify tab view is created.
+
+ // The unpinned Node should contain a tab, a tab group, and a split tab.
+ ASSERT_EQ(unpinned_node->children().size(), 3u);
+ EXPECT_EQ(unpinned_node->children()[0]->GetType(),
+ TabCollectionNode::Type::kTab);
+ // TODO(crbug.com/442567140): Verify tab view is created.
+
+ const auto& group_node = unpinned_node->children()[1];
+ EXPECT_EQ(group_node->GetType(), TabCollectionNode::Type::kTabGroup);
+ // TODO(crbug.com/442567916): Verify tab group view is created.
+ ASSERT_EQ(group_node->children().size(), 1u);
+ EXPECT_EQ(group_node->children()[0]->GetType(),
+ TabCollectionNode::Type::kTab);
+ // TODO(crbug.com/442567140): Verify tab view is created.
+
+ const auto& split_node = unpinned_node->children()[2];
+ EXPECT_EQ(split_node->GetType(), TabCollectionNode::Type::kSplitTab);
+ // TODO(crbug.com/442568605): Verify split tab view is created.
+ ASSERT_EQ(split_node->children().size(), 2u);
+ EXPECT_EQ(split_node->children()[0]->GetType(),
+ TabCollectionNode::Type::kTab);
+ // TODO(crbug.com/442567140): Verify tab view is created.
+ EXPECT_EQ(split_node->children()[1]->GetType(),
+ TabCollectionNode::Type::kTab);
+ // TODO(crbug.com/442567140): Verify tab view is created.
+}
+
IN_PROC_BROWSER_TEST_F(TabCollectionNodeBrowserTest,
RootNodePopulatesWithTabs_ViewHierarchy) {
AppendTab();
@@ -385,16 +457,30 @@
ASSERT_EQ(parent_view->children().size(), 1u);
const auto root_node_view = parent_view->children()[0];
- // The root_node_view should have two children, the pinned and unpinned views.
- ASSERT_EQ(root_node_view->children().size(), 2u);
- const auto pinned_node_view = root_node_view->children()[0];
- const auto unpinned_node_view = root_node_view->children()[1];
+ // The root_node_view should have three children, the pinned and unpinned
+ // views and a separator.
+ ASSERT_EQ(root_node_view->children().size(), 3u);
+ const auto pinned_node_scroll_view = root_node_view->children()[0];
+ ASSERT_TRUE(
+ views::IsViewClass<views::Separator>(root_node_view->children()[1]));
+ const auto unpinned_node_scroll_view = root_node_view->children()[2];
- // The pinned_node_view should have no children.
- ASSERT_EQ(pinned_node_view->children().size(), 0u);
+ // The pinned_node_scroll_view's contents should have no children.
+ ASSERT_TRUE(views::IsViewClass<views::ScrollView>(pinned_node_scroll_view));
+ ASSERT_EQ(static_cast<views::ScrollView*>(pinned_node_scroll_view)
+ ->contents()
+ ->children()
+ .size(),
+ 0u);
- // The unpinned_node_view should have two children, the two tab views.
- ASSERT_EQ(unpinned_node_view->children().size(), 2u);
+ // The unpinned_node_scroll_view's contents should have two children, the two
+ // tab views.
+ ASSERT_TRUE(views::IsViewClass<views::ScrollView>(unpinned_node_scroll_view));
+ ASSERT_EQ(static_cast<views::ScrollView*>(unpinned_node_scroll_view)
+ ->contents()
+ ->children()
+ .size(),
+ 2u);
}
namespace {
@@ -461,9 +547,23 @@
ASSERT_EQ(unpinned_node_view->children().size(), 2u);
}
+namespace {
+
+std::unique_ptr<views::View> CreateView(TabCollectionNode* node) {
+ auto view = std::make_unique<views::View>();
+ node->set_add_child_to_node(base::BindRepeating(
+ static_cast<views::View* (views::View::*)(std::unique_ptr<views::View>)>(
+ &views::View::AddChildView),
+ base::Unretained(view.get())));
+ return view;
+}
+
+} // namespace
+
IN_PROC_BROWSER_TEST_F(TabCollectionNodeBrowserTest, GetDirectChildren) {
AppendTab();
auto parent_view = std::make_unique<views::View>();
+ TabCollectionNode::SetViewFactoryForTesting(base::BindRepeating(&CreateView));
RootTabCollectionNode root_node(
browser()
@@ -499,6 +599,7 @@
CollectionReturnsOnlyCollectionItems) {
AppendTab();
auto parent_view = std::make_unique<views::View>();
+ TabCollectionNode::SetViewFactoryForTesting(base::BindRepeating(&CreateView));
views::View* non_collection_view =
parent_view->AddChildView(std::make_unique<views::View>());
diff --git a/chrome/browser/ui/views/tabs/vertical/vertical_pinned_tab_container_view.cc b/chrome/browser/ui/views/tabs/vertical/vertical_pinned_tab_container_view.cc
index becf02f..1430fc8 100644
--- a/chrome/browser/ui/views/tabs/vertical/vertical_pinned_tab_container_view.cc
+++ b/chrome/browser/ui/views/tabs/vertical/vertical_pinned_tab_container_view.cc
@@ -4,7 +4,9 @@
#include "chrome/browser/ui/views/tabs/vertical/vertical_pinned_tab_container_view.h"
+#include "base/functional/callback_forward.h"
#include "chrome/browser/ui/layout_constants.h"
+#include "chrome/browser/ui/views/tabs/vertical/tab_collection_node.h"
#include "ui/base/metadata/metadata_impl_macros.h"
#include "ui/color/color_id.h"
#include "ui/gfx/geometry/rect.h"
@@ -17,8 +19,13 @@
constexpr int kTabVerticalPadding = 4;
} // namespace
-VerticalPinnedTabContainerView::VerticalPinnedTabContainerView() {
+VerticalPinnedTabContainerView::VerticalPinnedTabContainerView(
+ TabCollectionNode* collection_node)
+ : collection_node_(collection_node) {
SetLayoutManager(std::make_unique<views::DelegatingLayoutManager>(this));
+ node_destroyed_subscription_ = collection_node_->RegisterWillDestroyCallback(
+ base::BindOnce(&VerticalPinnedTabContainerView::ResetCollectionNode,
+ base::Unretained(this)));
}
VerticalPinnedTabContainerView::~VerticalPinnedTabContainerView() = default;
@@ -29,15 +36,15 @@
int total_width = 0;
int total_height = 0;
+ const auto children = collection_node_->GetDirectChildren();
+
int x = 0;
int y = 0;
- // TODO(crbug.com/441086907): Update all |children()| callers in this method
- // to instead read from the TabCollectionNode.
- int children_on_row = children().size();
+ int children_on_row = children.size();
// Child width will be uniform and match the largest child's width.
int child_width = 0;
- for (views::View* child : children()) {
+ for (auto* child : children) {
// TODO(corising): look into caching this value and only recomputing if the
// children change.
child_width = std::max(child_width, child->GetPreferredSize().width());
@@ -59,7 +66,7 @@
}
int row_index = 0;
- for (views::View* child : children()) {
+ for (auto* child : children) {
gfx::Rect bounds = gfx::Rect(child->GetPreferredSize());
bounds.set_width(child_width);
if (row_index != 0) {
@@ -82,5 +89,9 @@
return layouts;
}
+void VerticalPinnedTabContainerView::ResetCollectionNode() {
+ collection_node_ = nullptr;
+}
+
BEGIN_METADATA(VerticalPinnedTabContainerView)
END_METADATA
diff --git a/chrome/browser/ui/views/tabs/vertical/vertical_pinned_tab_container_view.h b/chrome/browser/ui/views/tabs/vertical/vertical_pinned_tab_container_view.h
index 1cbcfac..67f5c60 100644
--- a/chrome/browser/ui/views/tabs/vertical/vertical_pinned_tab_container_view.h
+++ b/chrome/browser/ui/views/tabs/vertical/vertical_pinned_tab_container_view.h
@@ -5,17 +5,20 @@
#ifndef CHROME_BROWSER_UI_VIEWS_TABS_VERTICAL_VERTICAL_PINNED_TAB_CONTAINER_VIEW_H_
#define CHROME_BROWSER_UI_VIEWS_TABS_VERTICAL_VERTICAL_PINNED_TAB_CONTAINER_VIEW_H_
+#include "base/callback_list.h"
#include "ui/base/metadata/metadata_header_macros.h"
#include "ui/views/layout/delegating_layout_manager.h"
#include "ui/views/view.h"
+class TabCollectionNode;
+
// Container for the vertical tabstrip's pinned tabs.
class VerticalPinnedTabContainerView : public views::View,
public views::LayoutDelegate {
METADATA_HEADER(VerticalPinnedTabContainerView, views::View)
public:
- VerticalPinnedTabContainerView();
+ explicit VerticalPinnedTabContainerView(TabCollectionNode* collection_node);
VerticalPinnedTabContainerView(const VerticalPinnedTabContainerView&) =
delete;
VerticalPinnedTabContainerView& operator=(
@@ -25,6 +28,13 @@
// LayoutDelegate:
views::ProposedLayout CalculateProposedLayout(
const views::SizeBounds& size_bounds) const override;
+
+ private:
+ void ResetCollectionNode();
+
+ raw_ptr<TabCollectionNode> collection_node_;
+
+ base::CallbackListSubscription node_destroyed_subscription_;
};
#endif // CHROME_BROWSER_UI_VIEWS_TABS_VERTICAL_VERTICAL_PINNED_TAB_CONTAINER_VIEW_H_
diff --git a/chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_view.cc b/chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_view.cc
index 3f25be0d..0c1ef88a 100644
--- a/chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_view.cc
+++ b/chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_view.cc
@@ -8,6 +8,7 @@
#include "base/functional/bind.h"
#include "chrome/browser/ui/color/chrome_color_id.h"
#include "chrome/browser/ui/layout_constants.h"
+#include "chrome/browser/ui/views/tabs/vertical/tab_collection_node.h"
#include "chrome/browser/ui/views/tabs/vertical/vertical_pinned_tab_container_view.h"
#include "chrome/browser/ui/views/tabs/vertical/vertical_unpinned_tab_container_view.h"
#include "ui/base/metadata/metadata_impl_macros.h"
@@ -33,24 +34,23 @@
}
} // namespace
-VerticalTabStripView::VerticalTabStripView() {
+VerticalTabStripView::VerticalTabStripView(TabCollectionNode* collection_node) {
SetLayoutManager(std::make_unique<views::DelegatingLayoutManager>(this));
- views::ScrollView* pinned_tabs_scroll_view =
+ pinned_tabs_scroll_view_ =
AddChildView(std::make_unique<views::ScrollView>());
- SetScrollViewProperties(pinned_tabs_scroll_view);
- pinned_tabs_container_ = pinned_tabs_scroll_view->SetContents(
- std::make_unique<VerticalPinnedTabContainerView>());
+ SetScrollViewProperties(pinned_tabs_scroll_view_);
auto tabs_separator = std::make_unique<views::Separator>();
tabs_separator->SetColorId(kColorTabDividerFrameActive);
tabs_separator_ = AddChildView(std::move(tabs_separator));
- views::ScrollView* unpinned_tabs_scroll_view =
+ unpinned_tabs_scroll_view_ =
AddChildView(std::make_unique<views::ScrollView>());
- SetScrollViewProperties(unpinned_tabs_scroll_view);
- unpinned_tabs_container_ = unpinned_tabs_scroll_view->SetContents(
- std::make_unique<VerticalUnpinnedTabContainerView>());
+ SetScrollViewProperties(unpinned_tabs_scroll_view_);
+
+ collection_node->set_add_child_to_node(base::BindRepeating(
+ &VerticalTabStripView::AddScrollViewContents, base::Unretained(this)));
}
VerticalTabStripView::~VerticalTabStripView() = default;
@@ -77,16 +77,14 @@
}
// Place the pinned container.
- views::ScrollView* pinned_tabs_scroll_view =
- views::ScrollView::GetScrollViewForContents(pinned_tabs_container_.get());
gfx::Rect pinned_container_bounds(
0, y, tab_container_size_bounds.width().value(),
- pinned_tabs_scroll_view->GetPreferredSize(tab_container_size_bounds)
+ pinned_tabs_scroll_view_->GetPreferredSize(tab_container_size_bounds)
.height());
pinned_container_bounds.set_height(
std::min(pinned_container_bounds.height(), (remaining_height / 2)));
- layouts.child_layouts.emplace_back(pinned_tabs_scroll_view,
- pinned_tabs_scroll_view->GetVisible(),
+ layouts.child_layouts.emplace_back(pinned_tabs_scroll_view_.get(),
+ pinned_tabs_scroll_view_->GetVisible(),
pinned_container_bounds);
remaining_height -= pinned_container_bounds.height();
@@ -108,11 +106,8 @@
// Place the unpinned container.
gfx::Rect unpinned_container_bounds(
0, y, tab_container_size_bounds.width().value(), remaining_height);
- views::ScrollView* unpinned_tabs_scroll_view =
- views::ScrollView::GetScrollViewForContents(
- unpinned_tabs_container_.get());
- layouts.child_layouts.emplace_back(unpinned_tabs_scroll_view,
- unpinned_tabs_scroll_view->GetVisible(),
+ layouts.child_layouts.emplace_back(unpinned_tabs_scroll_view_.get(),
+ unpinned_tabs_scroll_view_->GetVisible(),
unpinned_container_bounds);
layouts.host_size =
@@ -120,11 +115,38 @@
return layouts;
}
+VerticalPinnedTabContainerView*
+VerticalTabStripView::GetPinnedTabsContainerForTesting() {
+ if (views::View* contents = pinned_tabs_scroll_view_->contents()) {
+ return static_cast<VerticalPinnedTabContainerView*>(contents);
+ }
+ return nullptr;
+}
+
+VerticalUnpinnedTabContainerView*
+VerticalTabStripView::GetUnpinnedTabsContainerForTesting() {
+ if (views::View* contents = unpinned_tabs_scroll_view_->contents()) {
+ return static_cast<VerticalUnpinnedTabContainerView*>(contents);
+ }
+ return nullptr;
+}
+
void VerticalTabStripView::SetCollapsedState(bool is_collapsed) {
if (is_collapsed != tabs_separator_->GetVisible()) {
tabs_separator_->SetVisible(is_collapsed);
}
}
+views::View* VerticalTabStripView::AddScrollViewContents(
+ std::unique_ptr<views::View> view) {
+ if (views::IsViewClass<VerticalUnpinnedTabContainerView>(view.get())) {
+ return unpinned_tabs_scroll_view_->SetContents(std::move(view));
+ }
+ // |view| should only ever be VerticalUnpinnedTabContainerView or
+ // VerticalPinnedTabContainerView.
+ CHECK(views::IsViewClass<VerticalPinnedTabContainerView>(view.get()));
+ return pinned_tabs_scroll_view_->SetContents(std::move(view));
+}
+
BEGIN_METADATA(VerticalTabStripView)
END_METADATA
diff --git a/chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_view.h b/chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_view.h
index bb04e47..ce125596 100644
--- a/chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_view.h
+++ b/chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_view.h
@@ -10,10 +10,12 @@
#include "ui/views/layout/delegating_layout_manager.h"
#include "ui/views/view.h"
+class TabCollectionNode;
class VerticalPinnedTabContainerView;
class VerticalUnpinnedTabContainerView;
namespace views {
+class ScrollView;
class Separator;
class View;
} // namespace views
@@ -24,18 +26,14 @@
METADATA_HEADER(VerticalTabStripView, views::View)
public:
- VerticalTabStripView();
+ explicit VerticalTabStripView(TabCollectionNode* collection_node);
VerticalTabStripView(const VerticalTabStripView&) = delete;
VerticalTabStripView& operator=(const VerticalTabStripView&) = delete;
~VerticalTabStripView() override;
views::Separator* tabs_separator_for_testing() { return tabs_separator_; }
- VerticalPinnedTabContainerView* pinned_tabs_container_for_testing() {
- return pinned_tabs_container_;
- }
- VerticalUnpinnedTabContainerView* unpinned_tabs_container_for_testing() {
- return unpinned_tabs_container_;
- }
+ VerticalPinnedTabContainerView* GetPinnedTabsContainerForTesting();
+ VerticalUnpinnedTabContainerView* GetUnpinnedTabsContainerForTesting();
void SetCollapsedState(bool is_collapsed);
@@ -44,9 +42,11 @@
const views::SizeBounds& size_bounds) const override;
private:
- raw_ptr<VerticalPinnedTabContainerView> pinned_tabs_container_ = nullptr;
+ views::View* AddScrollViewContents(std::unique_ptr<views::View> view);
+
+ raw_ptr<views::ScrollView> pinned_tabs_scroll_view_ = nullptr;
raw_ptr<views::Separator> tabs_separator_ = nullptr;
- raw_ptr<VerticalUnpinnedTabContainerView> unpinned_tabs_container_ = nullptr;
+ raw_ptr<views::ScrollView> unpinned_tabs_scroll_view_ = nullptr;
};
#endif // CHROME_BROWSER_UI_VIEWS_TABS_VERTICAL_VERTICAL_TAB_STRIP_VIEW_H_
diff --git a/chrome/browser/ui/views/tabs/vertical/vertical_unpinned_tab_container_view.cc b/chrome/browser/ui/views/tabs/vertical/vertical_unpinned_tab_container_view.cc
index 3cd68d4..85304b1 100644
--- a/chrome/browser/ui/views/tabs/vertical/vertical_unpinned_tab_container_view.cc
+++ b/chrome/browser/ui/views/tabs/vertical/vertical_unpinned_tab_container_view.cc
@@ -4,7 +4,9 @@
#include "chrome/browser/ui/views/tabs/vertical/vertical_unpinned_tab_container_view.h"
+#include "base/functional/callback_forward.h"
#include "chrome/browser/ui/layout_constants.h"
+#include "chrome/browser/ui/views/tabs/vertical/tab_collection_node.h"
#include "ui/base/metadata/metadata_impl_macros.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/views/layout/delegating_layout_manager.h"
@@ -16,8 +18,13 @@
constexpr int kTabVerticalPadding = 2;
} // namespace
-VerticalUnpinnedTabContainerView::VerticalUnpinnedTabContainerView() {
+VerticalUnpinnedTabContainerView::VerticalUnpinnedTabContainerView(
+ TabCollectionNode* collection_node)
+ : collection_node_(collection_node) {
SetLayoutManager(std::make_unique<views::DelegatingLayoutManager>(this));
+ node_destroyed_subscription_ = collection_node_->RegisterWillDestroyCallback(
+ base::BindOnce(&VerticalUnpinnedTabContainerView::ResetCollectionNode,
+ base::Unretained(this)));
}
VerticalUnpinnedTabContainerView::~VerticalUnpinnedTabContainerView() = default;
@@ -30,9 +37,11 @@
int horizontal_padding =
GetLayoutConstant(VERTICAL_TAB_STRIP_HORIZONTAL_PADDING);
+ const auto children = collection_node_->GetDirectChildren();
+
// Layout children in order. Children will have their preferred height and
// fill available width.
- for (const auto& child : children()) {
+ for (auto* child : children) {
gfx::Rect bounds = gfx::Rect(child->GetPreferredSize());
bounds.set_y(height);
// If fully bounded, child views should respect width constraints and take
@@ -45,7 +54,7 @@
width = std::max(width, bounds.width() + horizontal_padding);
}
// Remove excess padding if needed.
- if (!children().empty()) {
+ if (!children.empty()) {
height -= kTabVerticalPadding;
}
@@ -53,5 +62,9 @@
return layouts;
}
+void VerticalUnpinnedTabContainerView::ResetCollectionNode() {
+ collection_node_ = nullptr;
+}
+
BEGIN_METADATA(VerticalUnpinnedTabContainerView)
END_METADATA
diff --git a/chrome/browser/ui/views/tabs/vertical/vertical_unpinned_tab_container_view.h b/chrome/browser/ui/views/tabs/vertical/vertical_unpinned_tab_container_view.h
index e55b8ef..6f579fd 100644
--- a/chrome/browser/ui/views/tabs/vertical/vertical_unpinned_tab_container_view.h
+++ b/chrome/browser/ui/views/tabs/vertical/vertical_unpinned_tab_container_view.h
@@ -5,18 +5,21 @@
#ifndef CHROME_BROWSER_UI_VIEWS_TABS_VERTICAL_VERTICAL_UNPINNED_TAB_CONTAINER_VIEW_H_
#define CHROME_BROWSER_UI_VIEWS_TABS_VERTICAL_VERTICAL_UNPINNED_TAB_CONTAINER_VIEW_H_
+#include "base/callback_list.h"
#include "base/memory/raw_ptr.h"
#include "ui/base/metadata/metadata_header_macros.h"
#include "ui/views/layout/delegating_layout_manager.h"
#include "ui/views/view.h"
+class TabCollectionNode;
+
// Container for the vertical tabstrip's unpinned tabs.
class VerticalUnpinnedTabContainerView : public views::View,
public views::LayoutDelegate {
METADATA_HEADER(VerticalUnpinnedTabContainerView, views::View)
public:
- VerticalUnpinnedTabContainerView();
+ explicit VerticalUnpinnedTabContainerView(TabCollectionNode* collection_node);
VerticalUnpinnedTabContainerView(const VerticalUnpinnedTabContainerView&) =
delete;
VerticalUnpinnedTabContainerView& operator=(
@@ -26,6 +29,13 @@
// LayoutDelegate:
views::ProposedLayout CalculateProposedLayout(
const views::SizeBounds& size_bounds) const override;
+
+ private:
+ void ResetCollectionNode();
+
+ raw_ptr<TabCollectionNode> collection_node_;
+
+ base::CallbackListSubscription node_destroyed_subscription_;
};
#endif // CHROME_BROWSER_UI_VIEWS_TABS_VERTICAL_VERTICAL_UNPINNED_TAB_CONTAINER_VIEW_H_
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index 39013354..981da97c 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -3631,6 +3631,7 @@
"../browser/ui/views/frame/multi_contents_view_drop_target_controller_browsertest.cc",
"../browser/ui/views/frame/system_menu_model_builder_browsertest.cc",
"../browser/ui/views/frame/tab_strip_region_view_browsertest.cc",
+ "../browser/ui/views/frame/vertical_tab_strip_region_view_browsertest.cc",
"../browser/ui/views/hats/hats_browsertest.cc",
"../browser/ui/views/incognito_clear_browsing_data_dialog_browsertest.cc",
"../browser/ui/views/infobars/confirm_infobar_custom_layout_browsertest.cc",
@@ -10463,7 +10464,6 @@
"../browser/ui/views/frame/browser_view_unittest.cc",
"../browser/ui/views/frame/multi_contents_drop_target_view_unittest.cc",
"../browser/ui/views/frame/multi_contents_view_drop_target_controller_unittest.cc",
- "../browser/ui/views/frame/vertical_tab_strip_region_view_unittest.cc",
"../browser/ui/views/frame/web_contents_close_handler_unittest.cc",
"../browser/ui/views/global_media_controls/cast_device_footer_view_unittest.cc",
"../browser/ui/views/global_media_controls/cast_device_selector_view_unittest.cc",
