From 41db27fe9a3066295612e1eca7665d88dd74456f Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Tue, 17 Mar 2026 13:18:16 +0900 Subject: [PATCH] Fixed top separator is visible in fullscreen (#34744) Resolves brave/brave-browser#53555 - GetTopSeparatorType(): return kNone when toolbar and bookmark bar are not visible so no separator is drawn at the top of the contents view in fullscreen. - chromium_src: define SetDelegateForTesting (declared but not defined upstream) so tests can inject a layout delegate. TEST=BraveBrowserViewTabbedLayoutImplTest.GetTopSeparatorTypeNoneWhenNoVisibleTopUI --- browser/ui/views/frame/BUILD.gn | 12 +++ .../brave_browser_view_tabbed_layout_impl.cc | 7 ++ ...rowser_view_tabbed_layout_impl_unittest.cc | 101 ++++++++++++++++++ .../views/frame/layout/browser_view_layout.cc | 7 ++ test/BUILD.gn | 1 + 5 files changed, 128 insertions(+) create mode 100644 browser/ui/views/frame/layout/brave_browser_view_tabbed_layout_impl_unittest.cc diff --git a/browser/ui/views/frame/BUILD.gn b/browser/ui/views/frame/BUILD.gn index 044a8282499..78c5325d25e 100644 --- a/browser/ui/views/frame/BUILD.gn +++ b/browser/ui/views/frame/BUILD.gn @@ -31,6 +31,18 @@ source_set("frame") { ] } +source_set("unit_tests") { + testonly = true + sources = [ "layout/brave_browser_view_tabbed_layout_impl_unittest.cc" ] + deps = [ + ":frame", + "//base", + "//chrome/browser/ui", + "//testing/gmock", + "//testing/gtest", + ] +} + source_set("browser_tests") { testonly = true defines = [ "HAS_OUT_OF_PROC_TEST_RUNNER" ] diff --git a/browser/ui/views/frame/layout/brave_browser_view_tabbed_layout_impl.cc b/browser/ui/views/frame/layout/brave_browser_view_tabbed_layout_impl.cc index 14dd85a3b80..d925e75f002 100644 --- a/browser/ui/views/frame/layout/brave_browser_view_tabbed_layout_impl.cc +++ b/browser/ui/views/frame/layout/brave_browser_view_tabbed_layout_impl.cc @@ -240,6 +240,13 @@ void BraveBrowserViewTabbedLayoutImpl::DoPostLayoutVisualAdjustments( BrowserViewTabbedLayoutImpl::TopSeparatorType BraveBrowserViewTabbedLayoutImpl::GetTopSeparatorType() const { + // Return kNone when there is no visible top UI (toolbar and bookmark bar). + // This fixes a 1px visible separator at the top of the contents view when in + // browser fullscreen, where the top chrome is hidden. + if (!delegate().IsToolbarVisible() && !delegate().IsBookmarkBarVisible()) { + return TopSeparatorType::kNone; + } + // Get the upstream separator type as a starting point. The top separator is // a visual line that divides the browser's top UI (toolbar, tabs) from the // main content area below it. diff --git a/browser/ui/views/frame/layout/brave_browser_view_tabbed_layout_impl_unittest.cc b/browser/ui/views/frame/layout/brave_browser_view_tabbed_layout_impl_unittest.cc new file mode 100644 index 00000000000..ccaace0e6bf --- /dev/null +++ b/browser/ui/views/frame/layout/brave_browser_view_tabbed_layout_impl_unittest.cc @@ -0,0 +1,101 @@ +/* Copyright (c) 2026 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#include "brave/browser/ui/views/frame/layout/brave_browser_view_tabbed_layout_impl.h" + +#include + +#include "chrome/browser/ui/views/frame/layout/browser_view_layout.h" +#include "chrome/browser/ui/views/frame/layout/browser_view_layout_delegate.h" +#include "chrome/browser/ui/views/frame/layout/browser_view_layout_params.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +// Minimal fake for BrowserViewLayoutDelegate methods that are not exercised by +// GetTopSeparatorType() on the "no top UI" path (Brave returns early). +class FakeBrowserViewLayoutDelegate : public BrowserViewLayoutDelegate { + public: + FakeBrowserViewLayoutDelegate() = default; + ~FakeBrowserViewLayoutDelegate() override = default; + + bool ShouldDrawTabStrip() const override { return false; } + bool ShouldUseTouchableTabstrip() const override { return false; } + bool ShouldDrawVerticalTabStrip() const override { return false; } + bool IsVerticalTabStripCollapsed() const override { return false; } + bool ShouldDrawWebAppFrameToolbar() const override { return false; } + bool GetBorderlessModeEnabled() const override { return false; } + BrowserLayoutParams GetBrowserLayoutParams(bool) const override { + return BrowserLayoutParams(); + } + WindowState GetBrowserWindowState() const override { + return WindowState::kNormal; + } + views::LayoutAlignment GetWindowTitleAlignment() const override { + return views::LayoutAlignment::kStart; + } + bool IsToolbarVisible() const override { return false; } + bool IsBookmarkBarVisible() const override { return false; } + bool IsInfobarVisible() const override { return false; } + bool IsContentsSeparatorEnabled() const override { return false; } + bool IsActiveTabSplit() const override { return false; } + bool IsActiveTabAtLeadingWindowEdge() const override { return false; } + const ImmersiveModeController* GetImmersiveModeController() const override { + return nullptr; + } + ExclusiveAccessBubbleViews* GetExclusiveAccessBubble() const override { + return nullptr; + } + bool IsTopControlsSlideBehaviorEnabled() const override { return false; } + float GetTopControlsSlideBehaviorShownRatio() const override { return 0.0f; } + gfx::NativeView GetHostViewForAnchoring() const override { + return gfx::NativeView(); + } + bool HasFindBarController() const override { return false; } + void MoveWindowForFindBarIfNecessary() const override {} + bool IsWindowControlsOverlayEnabled() const override { return false; } + void UpdateWindowControlsOverlay(const gfx::Rect&) override {} + bool ShouldLayoutTabStrip() const override { return false; } + int GetExtraInfobarOffset() const override { return 0; } + + bool ShouldShowVerticalTabs() const override { return false; } + bool IsVerticalTabOnRight() const override { return false; } + bool ShouldUseBraveWebViewRoundedCornersForContents() const override { + return false; + } + int GetRoundedCornersWebViewMargin() const override { return 0; } + bool IsBookmarkBarOnByPref() const override { return false; } + bool IsContentTypeSidePanelVisible() const override { return false; } + bool IsFullscreenForBrowser() const override { return false; } + bool IsFullscreenForTab() const override { return false; } + bool IsFullscreen() const override { return false; } +}; + +// Only the two visibility hooks matter for GetTopSeparatorType(); gmock makes +// the test spell that out with EXPECT_CALL. +class MockBrowserViewLayoutDelegate : public FakeBrowserViewLayoutDelegate { + public: + MOCK_METHOD(bool, IsToolbarVisible, (), (const, override)); + MOCK_METHOD(bool, IsBookmarkBarVisible, (), (const, override)); +}; + +} // namespace + +TEST(BraveBrowserViewTabbedLayoutImplTest, + GetTopSeparatorTypeNoneWhenNoVisibleTopUI) { + auto mock = + std::make_unique>(); + EXPECT_CALL(*mock, IsToolbarVisible()).WillOnce(::testing::Return(false)); + EXPECT_CALL(*mock, IsBookmarkBarVisible()).WillOnce(::testing::Return(false)); + + BrowserViewLayoutViews views; + auto layout = std::make_unique( + std::move(mock), nullptr, std::move(views)); + + // TopSeparatorType::kNone (0) - no top separator when top UI is hidden. + // TopSeparatorType is private member. + EXPECT_EQ(0, static_cast(layout->GetTopSeparatorType())); +} diff --git a/chromium_src/chrome/browser/ui/views/frame/layout/browser_view_layout.cc b/chromium_src/chrome/browser/ui/views/frame/layout/browser_view_layout.cc index fe0ff971df3..725526d28b8 100644 --- a/chromium_src/chrome/browser/ui/views/frame/layout/browser_view_layout.cc +++ b/chromium_src/chrome/browser/ui/views/frame/layout/browser_view_layout.cc @@ -12,3 +12,10 @@ #include #undef BrowserViewTabbedLayoutImpl + +// It's upstream method. Declared in header but not defined in upstream. +// Define it here so we can use it in tests. +void BrowserViewLayout::SetDelegateForTesting( + std::unique_ptr delegate) { + delegate_ = std::move(delegate); +} diff --git a/test/BUILD.gn b/test/BUILD.gn index 827b7d6b85f..fcf4f056acd 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -485,6 +485,7 @@ test("brave_unit_tests") { "//brave/browser/ui/tabs/test:unit_tests", "//brave/browser/ui/toolbar:brave_app_menu_unit_test", "//brave/browser/ui/views/download/bubble:unit_tests", + "//brave/browser/ui/views/frame:unit_tests", "//brave/browser/ui/views/infobars:unit_tests", "//brave/browser/ui/views/page_action:unit_tests", "//brave/browser/ui/views/tabs:unit_tests",