[Rounded Corners]: Fix WebContents shadow in light mode (#25738)

browser/ui/views/frame/brave_browser_view_layout.cc

@@ -67,8 +67,9 @@ void BraveBrowserViewLayout::Layout(views::View* host) {
 }
 
 void BraveBrowserViewLayout::LayoutVerticalTabs() {
-  if (!vertical_tab_strip_host_.get())
+  if (!vertical_tab_strip_host_.get()) {
     return;
+  }
 
   if (!tabs::utils::ShouldShowVerticalTabs(browser_view_->browser())) {
     vertical_tab_strip_host_->SetBorder(nullptr);
@@ -108,10 +109,11 @@ void BraveBrowserViewLayout::LayoutVerticalTabs() {
   insets = AdjustInsetsConsideringFrameBorder(insets);
 #endif
 
-  if (insets.IsEmpty())
+  if (insets.IsEmpty()) {
     vertical_tab_strip_host_->SetBorder(nullptr);
-  else
+  } else {
     vertical_tab_strip_host_->SetBorder(views::CreateEmptyBorder(insets));
+  }
 
   const auto width =
       vertical_tab_strip_host_->GetPreferredSize().width() + insets.width();
@@ -147,8 +149,9 @@ int BraveBrowserViewLayout::LayoutTabStripRegion(int top) {
 
 int BraveBrowserViewLayout::LayoutBookmarkAndInfoBars(int top,
                                                       int browser_view_y) {
-  if (!vertical_tab_strip_host_ || !ShouldPushBookmarkBarForVerticalTabs())
+  if (!vertical_tab_strip_host_ || !ShouldPushBookmarkBarForVerticalTabs()) {
     return BrowserViewLayout::LayoutBookmarkAndInfoBars(top, browser_view_y);
+  }
 
   auto new_rect = vertical_layout_rect_;
   new_rect.Inset(GetInsetsConsideringVerticalTabHost());
@@ -157,8 +160,9 @@ int BraveBrowserViewLayout::LayoutBookmarkAndInfoBars(int top,
 }
 
 int BraveBrowserViewLayout::LayoutInfoBar(int top) {
-  if (!vertical_tab_strip_host_)
+  if (!vertical_tab_strip_host_) {
     return BrowserViewLayout::LayoutInfoBar(top);
+  }
 
   if (ShouldPushBookmarkBarForVerticalTabs()) {
     // Insets are already applied from LayoutBookmarkAndInfoBar().
@@ -204,8 +208,8 @@ void BraveBrowserViewLayout::LayoutSideBar(gfx::Rect& contents_bounds) {
 #endif
 
   gfx::Rect separator_bounds;
-
-  if (sidebar_container_->sidebar_on_left()) {
+  const bool on_left = sidebar_container_->sidebar_on_left();
+  if (on_left) {
     contents_bounds.set_x(contents_bounds.x() + sidebar_bounds.width());
 
     // When vertical tabs and the sidebar are adjacent, add a separator between
@@ -224,10 +228,22 @@ void BraveBrowserViewLayout::LayoutSideBar(gfx::Rect& contents_bounds) {
     sidebar_bounds.set_x(contents_bounds.right());
   }
 
-  // Side panel doesn't need margin as sidebar UI and contents container
-  // will have margins if needed.
   gfx::Insets panel_margins = GetContentsMargins();
-  panel_margins.set_left_right(0, 0);
+  if (BraveBrowser::ShouldUseBraveWebViewRoundedCorners(
+          browser_view_->browser())) {
+    // In rounded mode, there is already a gap between the sidebar and the main
+    // contents view, so we only remove from the margin from that side (we need
+    // to keep it between the sidebar controls and the sidebar content).
+    if (on_left) {
+      panel_margins.set_right(0);
+    } else {
+      panel_margins.set_left(0);
+    }
+  } else {
+    // Side panel doesn't need margin as sidebar UI and contents container
+    // will have margins if needed.
+    panel_margins.set_left_right(0, 0);
+  }
   sidebar_container_->side_panel()->SetProperty(views::kMarginsKey,
                                                 panel_margins);
 
@@ -250,14 +266,20 @@ void BraveBrowserViewLayout::UpdateContentsContainerInsets(
   // Control contents's margin with sidebar & vertical tab state.
   gfx::Insets contents_margins = GetContentsMargins();
 
+  // In rounded corners mode, we need to include a little margin so we have
+  // somewhere to draw the shadow.
+  int contents_margin_for_rounded_corners =
+      BraveContentsViewUtil::GetRoundedCornersWebViewMargin(
+          browser_view_->browser());
+
   // Don't need contents container's left or right margin with vertical tab as
   // vertical tab itself has sufficient padding.
   if (tabs::utils::ShouldShowVerticalTabs(browser_view_->browser()) &&
       !IsFullscreenForBrowser()) {
     if (tabs::utils::IsVerticalTabOnRight(browser_view_->browser())) {
-      contents_margins.set_right(0);
+      contents_margins.set_right(contents_margin_for_rounded_corners);
     } else {
-      contents_margins.set_left(0);
+      contents_margins.set_left(contents_margin_for_rounded_corners);
     }
   }
 
@@ -282,9 +304,9 @@ void BraveBrowserViewLayout::UpdateContentsContainerInsets(
   // If sidebar is shown in left-side, contents container doens't need its
   // left margin.
   if (sidebar_container_->sidebar_on_left()) {
-    contents_margins.set_left(0);
+    contents_margins.set_left(contents_margin_for_rounded_corners);
   } else {
-    contents_margins.set_right(0);
+    contents_margins.set_right(contents_margin_for_rounded_corners);
   }
   contents_container_bounds.Inset(contents_margins);
 }

browser/ui/views/frame/brave_contents_view_util.cc

@@ -5,17 +5,18 @@
 
 #include "brave/browser/ui/views/frame/brave_contents_view_util.h"
 
+#include "brave/browser/ui/brave_browser.h"
+#include "chrome/browser/ui/browser.h"
 #include "ui/compositor/layer.h"
 #include "ui/views/view.h"
 
 namespace {
 
 constexpr ViewShadow::ShadowParameters kShadow{
     .offset_x = 0,
-    .offset_y = 1,
-    .blur_radius = 4,
-    .shadow_color = SkColorSetA(SK_ColorBLACK, 0.07 * 255)};
-
+    .offset_y = 0,
+    .blur_radius = BraveContentsViewUtil::kMarginThickness,
+    .shadow_color = SkColorSetA(SK_ColorBLACK, 0.1 * 255)};
 }  // namespace
 
 std::unique_ptr<ViewShadow> BraveContentsViewUtil::CreateShadow(
@@ -26,3 +27,9 @@ std::unique_ptr<ViewShadow> BraveContentsViewUtil::CreateShadow(
   view->layer()->SetIsFastRoundedCorner(true);
   return shadow;
 }
+
+int BraveContentsViewUtil::GetRoundedCornersWebViewMargin(Browser* browser) {
+  return BraveBrowser::ShouldUseBraveWebViewRoundedCorners(browser)
+             ? BraveContentsViewUtil::kMarginThickness
+             : 0;
+}

browser/ui/views/frame/brave_contents_view_util.h

@@ -10,6 +10,8 @@
 
 #include "brave/browser/ui/views/view_shadow.h"
 
+class Browser;
+
 namespace views {
 class View;
 }
@@ -28,6 +30,10 @@ class BraveContentsViewUtil {
 
   // Creates a drop shadow for the specified content area view.
   static std::unique_ptr<ViewShadow> CreateShadow(views::View* view);
+
+  // If rounded corners are enabled, returns the additional margin required to
+  // get the shadow to display properly. Otherwise 0.
+  static int GetRoundedCornersWebViewMargin(Browser* browser);
 };
 
 #endif  // BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_CONTENTS_VIEW_UTIL_H_

browser/ui/views/frame/vertical_tab_strip_region_view.cc

@@ -11,10 +11,12 @@
 #include <utility>
 #include <vector>
 
+#include "base/functional/bind.h"
 #include "brave/app/vector_icons/vector_icons.h"
 #include "brave/browser/ui/brave_browser.h"
 #include "brave/browser/ui/color/brave_color_id.h"
 #include "brave/browser/ui/tabs/brave_tab_prefs.h"
+#include "brave/browser/ui/views/frame/brave_contents_view_util.h"
 #include "brave/browser/ui/views/tabs/brave_new_tab_button.h"
 #include "brave/browser/ui/views/tabs/brave_tab_search_button.h"
 #include "brave/browser/ui/views/tabs/brave_tab_strip_layout_helper.h"
@@ -656,7 +658,6 @@ VerticalTabStripRegionView::VerticalTabStripRegionView(
       base::BindRepeating(
           &VerticalTabStripRegionView::OnFloatingModePrefChanged,
           base::Unretained(this)));
-  OnFloatingModePrefChanged();
 
 #if BUILDFLAG(IS_MAC)
   show_toolbar_on_fullscreen_pref_.Init(
@@ -667,9 +668,13 @@ VerticalTabStripRegionView::VerticalTabStripRegionView(
 
   vertical_tab_on_right_.Init(
       brave_tabs::kVerticalTabsOnRight, browser()->profile()->GetPrefs(),
-      base::BindRepeating(
-          &VerticalTabStripRegionView::OnVerticalTabPositionChanged,
-          base::Unretained(this)));
+      base::BindRepeating(&VerticalTabStripRegionView::OnBrowserPanelsMoved,
+                          base::Unretained(this)));
+
+  sidebar_side_.Init(
+      prefs::kSidePanelHorizontalAlignment, prefs,
+      base::BindRepeating(&VerticalTabStripRegionView::OnBrowserPanelsMoved,
+                          base::Unretained(this)));
 
   widget_observation_.Observe(browser_view->GetWidget());
 
@@ -680,6 +685,9 @@ VerticalTabStripRegionView::VerticalTabStripRegionView(
          BrowserList::GetInstance()->end())
       << "Browser shouldn't be added at this point.";
   BrowserList::AddObserver(this);
+
+  // Note: This should happen after all the PrefMembers have been initialized.
+  OnFloatingModePrefChanged();
 }
 
 VerticalTabStripRegionView::~VerticalTabStripRegionView() {
@@ -945,7 +953,7 @@ void VerticalTabStripRegionView::OnShowVerticalTabsPrefChanged() {
   UpdateBorder();
 }
 
-void VerticalTabStripRegionView::OnVerticalTabPositionChanged() {
+void VerticalTabStripRegionView::OnBrowserPanelsMoved() {
   UpdateBorder();
   PreferredSizeChanged();
 }
@@ -1091,8 +1099,10 @@ void VerticalTabStripRegionView::OnBoundsChanged(
 #if DCHECK_IS_ON()
   if (auto width = GetContentsBounds().width();
       width && !IsBrowserFullscren()) {
-    CHECK_GE(width, tabs::kVerticalTabMinWidth +
-                        tabs::kMarginForVerticalTabContainers * 2);
+    CHECK_GE(
+        width,
+        tabs::kVerticalTabMinWidth + tabs::kMarginForVerticalTabContainers * 2 -
+            BraveContentsViewUtil::GetRoundedCornersWebViewMargin(browser_));
   }
 #endif
 }
@@ -1213,10 +1223,19 @@ void VerticalTabStripRegionView::UpdateBorder() {
            state_ == State::kFloating;
   };
 
-  gfx::Insets border_insets =
-      (!vertical_tab_on_right_.GetPrefName().empty() && *vertical_tab_on_right_)
-          ? gfx::Insets::TLBR(0, 1, 0, 0)
-          : gfx::Insets::TLBR(0, 0, 0, 1);
+  // If the sidebar is on the same side as the vertical tab strip, we shouldn't
+  // take away the margin on the vertical tabs, because the sidebar will be
+  // between it and the web_contents.
+  bool is_on_right =
+      !vertical_tab_on_right_.GetPrefName().empty() && *vertical_tab_on_right_;
+  bool sidebar_on_same_side = sidebar_side_.GetValue() == is_on_right;
+  int inset =
+      1 -
+      (sidebar_on_same_side
+           ? 0
+           : BraveContentsViewUtil::GetRoundedCornersWebViewMargin(browser_));
+  gfx::Insets border_insets = (is_on_right) ? gfx::Insets::TLBR(0, inset, 0, 0)
+                                            : gfx::Insets::TLBR(0, 0, 0, inset);
 
   if (show_visible_border()) {
     SetBorder(views::CreateSolidSidedBorder(

browser/ui/views/frame/vertical_tab_strip_region_view.h

@@ -144,7 +144,7 @@ class VerticalTabStripRegionView : public views::View,
   void UpdateStateAfterDragAndDropFinished(State original_state);
 
   void OnShowVerticalTabsPrefChanged();
-  void OnVerticalTabPositionChanged();
+  void OnBrowserPanelsMoved();
 
   void UpdateLayout(bool in_destruction = false);
 
@@ -228,6 +228,7 @@ class VerticalTabStripRegionView : public views::View,
       fullscreen_observation_{this};
 
   BooleanPrefMember vertical_tab_on_right_;
+  BooleanPrefMember sidebar_side_;
 
   base::WeakPtrFactory<VerticalTabStripRegionView> weak_factory_{this};
 };

browser/ui/views/sidebar/sidebar_container_view.cc

@@ -531,7 +531,11 @@ void SidebarContainerView::ShowSidebar(bool show_side_panel) {
   animation_start_width_ = width();
   animation_end_width_ = sidebar_control_view_->GetPreferredSize().width();
   if (show_side_panel) {
-    animation_end_width_ += side_panel_->GetPreferredSize().width();
+    // Note: as margins of |side_panel_| are part of |width()| we need to add
+    // them when calculating the ideal width of the contents.
+    animation_end_width_ +=
+        side_panel_->GetPreferredSize().width() +
+        side_panel_->GetProperty(views::kMarginsKey)->width();
   }
 
   // Don't need event detect widget when sidebar gets visible.

browser/ui/views/sidebar/sidebar_control_view.cc

@@ -11,6 +11,7 @@
 #include "brave/browser/ui/sidebar/sidebar_controller.h"
 #include "brave/browser/ui/sidebar/sidebar_service_factory.h"
 #include "brave/browser/ui/sidebar/sidebar_utils.h"
+#include "brave/browser/ui/views/frame/brave_contents_view_util.h"
 #include "brave/browser/ui/views/sidebar/sidebar_item_add_button.h"
 #include "brave/browser/ui/views/sidebar/sidebar_items_scroll_view.h"
 #include "brave/components/l10n/common/localization_util.h"
@@ -19,8 +20,10 @@
 #include "brave/grit/brave_generated_resources.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser_command_controller.h"
+#include "chrome/browser/ui/browser_window/public/browser_window_features.h"
 #include "chrome/browser/ui/color/chrome_color_id.h"
 #include "chrome/browser/ui/singleton_tabs.h"
+#include "chrome/browser/ui/views/side_panel/side_panel_ui.h"
 #include "chrome/common/pref_names.h"
 #include "components/prefs/pref_service.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -86,13 +89,11 @@ void SidebarControlView::UpdateBackgroundAndBorder() {
   if (const ui::ColorProvider* color_provider = GetColorProvider()) {
     SetBackground(
         views::CreateSolidBackground(color_provider->GetColor(kColorToolbar)));
-    if (!BraveBrowser::ShouldUseBraveWebViewRoundedCorners(browser_)) {
-      constexpr int kBorderThickness = 1;
-      SetBorder(views::CreateSolidSidedBorder(
-          gfx::Insets::TLBR(0, sidebar_on_left_ ? 0 : kBorderThickness, 0,
-                            sidebar_on_left_ ? kBorderThickness : 0),
-          color_provider->GetColor(kColorToolbarContentAreaSeparator)));
-    }
+    int border_thickness =
+        1 - BraveContentsViewUtil::GetRoundedCornersWebViewMargin(browser_);
+    SetBorder(views::CreateEmptyBorder(
+        gfx::Insets::TLBR(0, sidebar_on_left_ ? 0 : border_thickness, 0,
+                          sidebar_on_left_ ? border_thickness : 0)));
   }
 }
 

chromium_src/chrome/browser/ui/views/side_panel/side_panel_web_ui_view.cc

@@ -10,10 +10,7 @@
 
 void SidePanelWebUIView::AddedToWidget() {
   if (base::FeatureList::IsEnabled(features::kBraveWebViewRoundedCorners)) {
-    // Side panel WebUI views are positioned at the bottom of the side panel. In
-    // order to maintain rounded corners around the side panel, give the web
-    // contents native view rounded corners on the bottom.
-    constexpr auto radius = BraveContentsViewUtil::kBorderRadius;
-    holder()->SetCornerRadii(gfx::RoundedCornersF(0, 0, radius, radius));
+    constexpr auto kRadius = BraveContentsViewUtil::kBorderRadius;
+    holder()->SetCornerRadii(gfx::RoundedCornersF(kRadius));
   }
 }