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
This commit is contained in:
Simon Hong
2026-03-17 13:18:16 +09:00
committed by GitHub
parent d700d81f46
commit 41db27fe9a
5 changed files with 128 additions and 0 deletions
+12
View File
@@ -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" ]
@@ -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.
@@ -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 <memory>
#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<testing::NiceMock<MockBrowserViewLayoutDelegate>>();
EXPECT_CALL(*mock, IsToolbarVisible()).WillOnce(::testing::Return(false));
EXPECT_CALL(*mock, IsBookmarkBarVisible()).WillOnce(::testing::Return(false));
BrowserViewLayoutViews views;
auto layout = std::make_unique<BraveBrowserViewTabbedLayoutImpl>(
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<int>(layout->GetTopSeparatorType()));
}
@@ -12,3 +12,10 @@
#include <chrome/browser/ui/views/frame/layout/browser_view_layout.cc>
#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<BrowserViewLayoutDelegate> delegate) {
delegate_ = std::move(delegate);
}
+1
View File
@@ -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",