Add horizontal margin to LocationBar, centering on Toolbar where poss…

…ible

Can be disabled via user preference under Appearance section of settings page. Takes the opportunity to create a Brave whitelist for the settingsPrivate javascript API without adding items through patching chromium directly. This is achieved via subclassing `extensions::PrefsUtil`.

Whilst a target margin is determined according to window's width and a maximum LocationBar width, such as 3%, 5%, 7% or more consider we determined that it's a nicer effect if the LocationBar is centered on the Toolbar, and not within the space between items on the left and right side of the Toolbar, which can be varying widths, especially with many extensions. Therefore space is allocated from the right margin to the left in order to 'shim' the center point of the LocationBar towards the center point of the Toolbar. Similarly space can also be allocated from the left margin to the right for the same purpose, but not so much that it encroaches more than 25% in to the right margin, so not to get too close to extension Browser Actions.
BookmarkButton is then anchored to the floating LocationBar.

LocationBar max width is 1080px which is calculated from Brave 0.23's 900px width and increased with appropriate space for the Brave Actions.

app/brave_generated_resources.grd

@@ -146,6 +146,9 @@ By installing this extension, you are agreeing to the Google Widevine Terms of U
       <message name="IDS_SETTINGS_APPEARANCE_SETTINGS_BRAVE_THEMES" desc="The label for brave theme change setting options">
         Brave colors
       </message>
+      <message name="IDS_SETTINGS_APPEARANCE_SETTINGS_LOCATION_BAR_IS_WIDE" desc="The label for whether the location bar should display wide or not">
+        Use wide location bar
+      </message>
       <!-- Tor -->
       <message name="IDS_NEW_TOR_IDENTITY" desc="The text label of a menu item for requesting new Tor identity">
         New Tor Identity

browser/brave_profile_prefs.cc

@@ -26,7 +26,9 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
 
   RegisterAlternatePrivateSearchEngineProfilePrefs(registry);
 
+  // appearance
   BraveThemeService::RegisterProfilePrefs(registry);
+  registry->RegisterBooleanPref(kLocationBarIsWide, false);
 
   tor::TorProfileService::RegisterProfilePrefs(registry);
 

browser/extensions/BUILD.gn

@@ -6,6 +6,8 @@ source_set("extensions") {
     "api/brave_shields_api.h",
     "api/content_settings/brave_content_settings_store.cc",
     "api/content_settings/brave_content_settings_store.h",
+    "api/settings_private/brave_prefs_util.cc",
+    "api/settings_private/brave_prefs_util.h",
     "brave_component_extension.cc",
     "brave_component_extension.h",
     "brave_component_extension_resource_manager.cc",

browser/extensions/api/settings_private/brave_prefs_util.cc

@@ -0,0 +1,39 @@
+/* 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 http://mozilla.org/MPL/2.0/. */
+
+#include "brave/browser/extensions/api/settings_private/brave_prefs_util.h"
+
+#include "brave/common/pref_names.h"
+#include "chrome/browser/extensions/api/settings_private/prefs_util.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/common/extensions/api/settings_private.h"
+
+namespace extensions{
+
+namespace settings_api = api::settings_private;
+
+const PrefsUtil::TypedPrefMap& BravePrefsUtil::GetWhitelistedKeys() {
+  // Static cache, similar to parent class
+  static PrefsUtil::TypedPrefMap* s_brave_whitelist = nullptr;
+  if (s_brave_whitelist)
+    return *s_brave_whitelist;
+  s_brave_whitelist = new PrefsUtil::TypedPrefMap();
+  // Start with parent class whitelist
+  const auto chromium_prefs = PrefsUtil::GetWhitelistedKeys();
+  s_brave_whitelist->insert(chromium_prefs.begin(), chromium_prefs.end());
+  // Add Brave values to the whitelist
+#if !defined(OS_CHROMEOS)
+  // import data
+  (*s_brave_whitelist)[::prefs::kImportDialogCookies] =
+      settings_api::PrefType::PREF_TYPE_BOOLEAN;
+  (*s_brave_whitelist)[::prefs::kImportDialogStats] =
+      settings_api::PrefType::PREF_TYPE_BOOLEAN;
+#endif
+  // appearance prefs
+  (*s_brave_whitelist)[kLocationBarIsWide] =
+    settings_api::PrefType::PREF_TYPE_BOOLEAN;
+  return *s_brave_whitelist;
+}
+
+}

browser/extensions/api/settings_private/brave_prefs_util.h

@@ -0,0 +1,22 @@
+/* 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 http://mozilla.org/MPL/2.0/. */
+
+#ifndef BRAVE_BROWSER_EXTENSIONS_API_SETTINGS_PRIVATE_BRAVE_PREFS_UTIL_H
+#define BRAVE_BROWSER_EXTENSIONS_API_SETTINGS_PRIVATE_BRAVE_PREFS_UTIL_H
+
+#include "chrome/browser/extensions/api/settings_private/prefs_util.h"
+namespace extensions{
+
+class BravePrefsUtil : public PrefsUtil {
+  public:
+    using PrefsUtil::PrefsUtil;
+    // Gets the list of whitelisted pref keys -- that is, those which correspond
+    // to prefs that clients of the settingsPrivate API may retrieve and
+    // manipulate.
+    const PrefsUtil::TypedPrefMap& GetWhitelistedKeys() override;
+};
+
+}
+
+#endif

browser/resources/settings/brave_appearance_page/brave_appearance_page.html

@@ -6,7 +6,7 @@
 <link rel="import" href="../settings_shared_css.html">
 <link rel="import" href="../settings_vars_css.html">
 
-<dom-module id="settings-brave-appearance-page">
+<dom-module id="settings-brave-appearance-theme">
   <template>
     <style include="settings-shared md-select iron-flex">
     </style>
@@ -25,3 +25,15 @@
   </template>
   <script src="brave_appearance_page.js"></script>
 </dom-module>
+
+<dom-module id="settings-brave-appearance-toolbar">
+  <template>
+    <style include="settings-shared md-select iron-flex">
+    </style>
+    <settings-toggle-button
+      pref="{{prefs.brave.location_bar_is_wide}}"
+      label="$i18n{appearanceSettingsLocationBarIsWide}"
+    >
+    </settings-toggle-button>
+  </template>
+</dom-module>

browser/resources/settings/brave_appearance_page/brave_appearance_page.js

@@ -5,12 +5,8 @@
 (function() {
 'use strict';
 
-/**
- * 'settings-brave-appearance-page' is the settings page containing brave's
- * appearance settings.
- */
 Polymer({
-  is: 'settings-brave-appearance-page',
+  is: 'settings-brave-appearance-theme',
 
   properties: {
     braveThemeTypes_: {
@@ -54,4 +50,12 @@ Polymer({
     this.browserProxy_.setBraveThemeType(this.$.braveThemeType.value);
   },
 });
-})();
+
+/**
+ * 'settings-brave-appearance-toolbar' is the settings page area containing
+ * brave's appearance settings related to the toolbar.
+ */
+Polymer({
+  is: 'settings-brave-appearance-toolbar',
+});
+})();

browser/ui/views/toolbar/brave_toolbar_view.cc

@@ -5,6 +5,7 @@
 #include "brave/browser/ui/views/toolbar/brave_toolbar_view.h"
 
 #include "brave/browser/ui/views/toolbar/bookmark_button.h"
+#include "brave/common/pref_names.h"
 #include "chrome/browser/defaults.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/browser/extensions/extension_util.h"
@@ -43,7 +44,11 @@ void BraveToolbarView::Init() {
       bookmarks::prefs::kEditBookmarksEnabled, browser()->profile()->GetPrefs(),
       base::Bind(&BraveToolbarView::OnEditBookmarksEnabledChanged,
                  base::Unretained(this)));
-
+  // track changes in wide locationbar setting
+  location_bar_is_wide_.Init(
+      kLocationBarIsWide, browser()->profile()->GetPrefs(),
+      base::Bind(&BraveToolbarView::OnLocationBarIsWideChanged,
+                 base::Unretained(this)));
   // Only location bar in non-normal mode
   if (!is_display_mode_normal()) {
     return;
@@ -59,6 +64,11 @@ void BraveToolbarView::OnEditBookmarksEnabledChanged() {
   Update(nullptr);
 }
 
+void BraveToolbarView::OnLocationBarIsWideChanged() {
+  Layout();
+  SchedulePaint();
+}
+
 void BraveToolbarView::Update(content::WebContents* tab) {
   ToolbarView::Update(tab);
   // Decide whether to show the bookmark button
@@ -89,6 +99,88 @@ void BraveToolbarView::ShowBookmarkBubble(
     star_view->OnBubbleWidgetCreated(bubble_widget);
 }
 
+// Returns right-margin
+int BraveToolbarView::SetLocationBarBounds(const int available_width,
+                                          const int location_bar_min_width,
+                                          const int next_element_x,
+                                          const int element_padding) {
+  DCHECK(initialized_);
+  DCHECK(is_display_mode_normal());
+  // Allow the option of the LocationBar having horizontal margin.
+  // With this option, we support a dynamic percentage of margin, with
+  // a maximum width.
+  const bool restrict_location_bar_width = !location_bar_is_wide_.GetValue();
+  const int location_bar_max_width = restrict_location_bar_width
+                                      ? 1080
+                                      : available_width;
+  const auto toolbar_width = width();
+  int location_bar_width = available_width;
+  int location_bar_margin_h = 0;
+  int location_bar_center_offset = 0;
+  if (restrict_location_bar_width) {
+    double location_bar_margin_h_pc = 0.07;
+    if (toolbar_width < 700)
+      location_bar_margin_h_pc = 0;
+    else if (toolbar_width < 850)
+      location_bar_margin_h_pc = 0.03;
+    else if (toolbar_width < 1000)
+      location_bar_margin_h_pc = 0.05;
+    // Apply the target margin, adjusting for min and max width of LocationBar
+    // Make sure any margin doesn't shrink the LocationBar beyond minimum width
+    if (available_width > location_bar_min_width) {
+      int location_bar_max_margin_h = (
+                                        available_width -
+                                        location_bar_min_width
+                                      ) /
+                                      2;
+      location_bar_margin_h = std::min(
+                                      (int)(
+                                        toolbar_width * location_bar_margin_h_pc
+                                      ),
+                                      location_bar_max_margin_h
+                                    );
+      location_bar_width = available_width - (location_bar_margin_h * 2);
+      // Allow the margin to expand so LocationBar is restrained to max width
+      if (location_bar_width > location_bar_max_width) {
+        location_bar_margin_h += (location_bar_width - location_bar_max_width) /
+                                  2;
+        location_bar_width = location_bar_max_width;
+      }
+    }
+    // Center LocationBar as much as possible within Toolbar
+    const int location_bar_toolbar_center_point =
+                                        next_element_x +
+                                        location_bar_margin_h +
+                                        (location_bar_width / 2);
+    // Calculate offset - positive for move left and negative for move right
+    location_bar_center_offset = location_bar_toolbar_center_point -
+                                                            (toolbar_width / 2);
+    // Can't shim more than we have space for, so restrict to margin size
+    // or in the case of moving-right, 25% of the space since we want to avoid
+    // touching browser actions where possible
+    location_bar_center_offset = (location_bar_center_offset > 0)
+                                ? std::min(location_bar_margin_h,
+                                          location_bar_center_offset)
+                                : std::max((int)(-location_bar_margin_h * .25),
+                                          location_bar_center_offset);
+  }
+  // Apply offset to margin
+  const int location_bar_margin_l = location_bar_margin_h -
+                                    location_bar_center_offset;
+  const int location_bar_margin_r = location_bar_margin_h +
+                                    location_bar_center_offset;
+
+  const int location_x = next_element_x + location_bar_margin_l;
+  const int location_height = location_bar_->GetPreferredSize().height();
+  const int location_y = (height() - location_height) / 2;
+
+  location_bar_->SetBounds(location_x, location_y,
+                           location_bar_width,
+                           location_height);
+
+  return location_bar_margin_r;
+}
+
 void BraveToolbarView::Layout() {
   // If we have not been initialized yet just do nothing.
   if (!initialized_)
@@ -144,15 +236,15 @@ void BraveToolbarView::Layout() {
   } else {
     home_->SetVisible(false);
   }
-
-  // position Brave's BookmarkButton
-  if (bookmark_ && bookmark_->visible()) {
-    next_element_x += element_padding;
-    bookmark_->SetBounds(next_element_x, toolbar_button_y, bookmark_->GetPreferredSize().width(), toolbar_button_height);
-    next_element_x = bookmark_->bounds().right();
-  }
-  next_element_x += GetLayoutConstant(TOOLBAR_STANDARD_SPACING);
-
+  next_element_x += element_padding;
+
+  // Position Brave's BookmarkButton
+  // Only reserve space since the positioning will happen after LocationBar
+  // position is calculated so we can anchor BookmarkButton inside LocationBar's
+  // margin.
+  const bool bookmark_show = (bookmark_ && bookmark_->visible());
+  if (bookmark_show)
+    next_element_x += bookmark_->GetPreferredSize().width() + element_padding;
   int app_menu_width = app_menu_button_->GetPreferredSize().width();
   const int right_padding = GetLayoutConstant(TOOLBAR_STANDARD_SPACING);
 
@@ -169,19 +261,31 @@ void BraveToolbarView::Layout() {
     available_width -= avatar_->GetPreferredSize().width();
     available_width -= element_padding;
   }
-  // Don't allow the omnibox to shrink to the point of non-existence, so
-  // subtract its minimum width from the available width to reserve it.
+
+  // Allow the extension actions to take up a share of the available width,
+  // but consider the minimum for the location bar
+  const int location_bar_min_width = location_bar_->GetMinimumSize().width();
   const int browser_actions_width = browser_actions_->GetWidthForMaxWidth(
-      available_width - location_bar_->GetMinimumSize().width());
+      available_width - location_bar_min_width);
   available_width -= browser_actions_width;
-  const int location_bar_width = available_width;
 
-  const int location_height = location_bar_->GetPreferredSize().height();
-  const int location_y = (height() - location_height) / 2;
-  location_bar_->SetBounds(next_element_x, location_y,
-                           location_bar_width, location_height);
-
-  next_element_x = location_bar_->bounds().right();
+  const int location_bar_margin_r = SetLocationBarBounds(available_width,
+                                                        location_bar_min_width,
+                                                        next_element_x,
+                                                        element_padding);
+  // Position the bookmark button so it is anchored to the LocationBar
+  auto location_bar_bounds = location_bar_->bounds();
+  if (bookmark_show) {
+    const int bookmark_width = bookmark_->GetPreferredSize().width();
+    const int bookmark_x = location_bar_bounds.x() -
+                            bookmark_width -
+                            element_padding;
+    bookmark_->SetBounds(bookmark_x,
+                        toolbar_button_y,
+                        bookmark_width,
+                        toolbar_button_height);
+  }
+  next_element_x = location_bar_bounds.right() + location_bar_margin_r;
 
   // Note height() may be zero in fullscreen.
   const int browser_actions_height =

browser/ui/views/toolbar/brave_toolbar_view.h

@@ -20,6 +20,7 @@ class BraveToolbarView : public ToolbarView {
     void Layout() override;
     void Update(content::WebContents* tab) override;
     void OnEditBookmarksEnabledChanged();
+    void OnLocationBarIsWideChanged();
     void ShowBookmarkBubble(const GURL& url,
                           bool already_bookmarked,
                           bookmarks::BookmarkBubbleObserver* observer) override;
@@ -29,9 +30,14 @@ class BraveToolbarView : public ToolbarView {
     // These two functions call through to GetSizeInternal(), passing themselves
     // as the function pointer |View::*get_size|.
     gfx::Size GetSizeInternal(gfx::Size (View::*get_size)() const) const override;
+    int SetLocationBarBounds(const int available_width,
+                            const int location_bar_min_width,
+                            const int next_element_x,
+                            const int element_padding);
     BookmarkButton* bookmark_ = nullptr;
     // Tracks the preference to determine whether bookmark editing is allowed.
     BooleanPrefMember edit_bookmarks_enabled_;
+    BooleanPrefMember location_bar_is_wide_;
 };
 
 #endif

chromium_src/chrome/browser/extensions/api/settings_private/settings_private_delegate.cc

@@ -0,0 +1,5 @@
+#include "brave/browser/extensions/api/settings_private/brave_prefs_util.h"
+
+#define PrefsUtil BravePrefsUtil
+#include "../../../../../../chrome/browser/extensions/api/settings_private/settings_private_delegate.cc"
+#undef PrefsUtil

chromium_src/chrome/browser/extensions/api/settings_private/settings_private_event_router.cc

@@ -0,0 +1,5 @@
+#include "brave/browser/extensions/api/settings_private/brave_prefs_util.h"
+
+#define PrefsUtil BravePrefsUtil
+#include "../../../../../../chrome/browser/extensions/api/settings_private/settings_private_event_router.cc"
+#undef PrefsUtil

chromium_src/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc

@@ -29,6 +29,8 @@ void BraveAddCommonStrings(content::WebUIDataSource* html_source, Profile* profi
       IDS_SETTINGS_SITE_SETTINGS_AUTOPLAY_ASK_RECOMMENDED},
     {"appearanceSettingsBraveTheme",
       IDS_SETTINGS_APPEARANCE_SETTINGS_BRAVE_THEMES},
+    {"appearanceSettingsLocationBarIsWide",
+      IDS_SETTINGS_APPEARANCE_SETTINGS_LOCATION_BAR_IS_WIDE},
   };
   AddLocalizedStringsBulk(html_source, localized_strings,
                           arraysize(localized_strings));

common/pref_names.cc

@@ -19,3 +19,4 @@ const char kWidevineOptedIn[] = "brave.widevine_opted_in";
 const char kUseAlternatePrivateSearchEngine[] =
     "brave.use_alternate_private_search_engine";
 const char kBraveThemeType[] = "brave.theme.type";
+const char kLocationBarIsWide[] = "brave.location_bar_is_wide";

common/pref_names.h

@@ -19,5 +19,6 @@ extern const char kAdBlockCurrentRegion[];
 extern const char kWidevineOptedIn[];
 extern const char kUseAlternatePrivateSearchEngine[];
 extern const char kBraveThemeType[];
+extern const char kLocationBarIsWide[];
 
 #endif  // BRAVE_COMMON_PREF_NAMES_H_