Add always show bookmark bar on NTP option

Some user might want to show bookmark on NTP when show bookmark bar option is false.
brave · Jul 5, 2019 · cfc0497 · cfc0497
1 parent 547dc51
commit cfc0497
Show file tree

Hide file tree

Showing 12 changed files with 106 additions and 17 deletions.
diff --git a/app/brave_generated_resources.grd b/app/brave_generated_resources.grd
@@ -222,6 +222,9 @@ By installing this extension, you are agreeing to the Google Widevine Terms of U
       <message name="IDS_SETTINGS_HIDE_BRAVE_REWARDS_BUTTON_DESC" desc="The description for settings switch controlling the visibility of Brave Rewards button">
         Hides the Brave Rewards button in the location bar when Brave Rewards is not enabled
       </message>
+      <message name="IDS_SETTINGS_ALWAYS_SHOW_BOOKMARK_BAR_ON_NTP" desc="The label for settings switch controlling the visibility of bookmarks bar on NTP">
+        Always show bookmarks bar on New Tab page
+      </message>
       <!-- Tor -->
       <message name="IDS_NEW_TOR_IDENTITY" desc="The text label of a menu item for requesting new Tor identity">
         New Tor Identity

diff --git a/browser/brave_profile_prefs.cc b/browser/brave_profile_prefs.cc
@@ -129,6 +129,8 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
   registry->RegisterBooleanPref(kNewTabPageShowClock, true);
   registry->RegisterBooleanPref(kNewTabPageShowTopSites, true);
   registry->RegisterBooleanPref(kNewTabPageShowStats, true);
+
+  registry->RegisterBooleanPref(kAlwaysShowBookmarkBarOnNTP, false);
 }
 
 }  // namespace brave
diff --git a/browser/extensions/api/settings_private/brave_prefs_util.cc b/browser/extensions/api/settings_private/brave_prefs_util.cc
@@ -74,6 +74,8 @@ const PrefsUtil::TypedPrefMap& BravePrefsUtil::GetWhitelistedKeys() {
     settings_api::PrefType::PREF_TYPE_BOOLEAN;
   (*s_brave_whitelist)[browsing_data::prefs::kDeleteHostedAppsDataOnExit] =
     settings_api::PrefType::PREF_TYPE_BOOLEAN;
+  (*s_brave_whitelist)[kAlwaysShowBookmarkBarOnNTP] =
+      settings_api::PrefType::PREF_TYPE_BOOLEAN;
   // WebTorrent pref
   (*s_brave_whitelist)[kWebTorrentEnabled] =
       settings_api::PrefType::PREF_TYPE_BOOLEAN;

diff --git a/browser/resources/settings/brave_appearance_page/brave_appearance_page.html b/browser/resources/settings/brave_appearance_page/brave_appearance_page.html
@@ -37,5 +37,10 @@
       sub-label="$i18n{appearanceSettingsHideBraveRewardsButtonDesc}"
     >
     </settings-toggle-button>
+    <settings-toggle-button
+      pref="{{prefs.brave.always_show_bookmark_bar_on_ntp}}"
+      label="$i18n{appearanceSettingsAlwaysShowBookmarkBarOnNTP}"
+    >
+    </settings-toggle-button>
   </template>
 </dom-module>
diff --git a/browser/ui/bookmark/bookmark_tab_helper_browsertest.cc b/browser/ui/bookmark/bookmark_tab_helper_browsertest.cc
@@ -5,6 +5,7 @@
 
 #include <vector>
 
+#include "brave/common/pref_names.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/search.h"
@@ -16,6 +17,7 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/bookmarks/browser/bookmark_utils.h"
+#include "components/prefs/pref_service.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test_utils.h"
@@ -36,6 +38,20 @@ bool IsNTP(content::WebContents* web_contents) {
          search::NavEntryIsInstantNTP(web_contents, entry);
 }
 
+void AddBookmarkNode(Profile* profile) {
+  const GURL url = GURL("https://www.brave.com");
+  bookmarks::BookmarkModel* bookmark_model =
+      BookmarkModelFactory::GetForBrowserContext(profile);
+
+  std::vector<const bookmarks::BookmarkNode*> nodes;
+  bookmark_model->GetNodesByURL(url, &nodes);
+  EXPECT_EQ(0UL, nodes.size());
+
+  bookmarks::AddIfNotBookmarked(bookmark_model, url, base::string16());
+  bookmark_model->GetNodesByURL(url, &nodes);
+  EXPECT_EQ(1UL, nodes.size());
+}
+
 }  // namespace
 
 IN_PROC_BROWSER_TEST_F(BookmarkTabHelperBrowserTest,
@@ -47,18 +63,33 @@ IN_PROC_BROWSER_TEST_F(BookmarkTabHelperBrowserTest,
   chrome::ToggleBookmarkBar(browser());
   EXPECT_EQ(BookmarkBar::SHOW, browser()->bookmark_bar_state());
 
-  const GURL url = GURL("https://www.brave.com");
-  bookmarks::BookmarkModel* bookmark_model =
-      BookmarkModelFactory::GetForBrowserContext(browser()->profile());
+  AddBookmarkNode(browser()->profile());
 
-  std::vector<const bookmarks::BookmarkNode*> nodes;
-  bookmark_model->GetNodesByURL(url, &nodes);
-  EXPECT_EQ(0UL, nodes.size());
+  chrome::ToggleBookmarkBar(browser());
 
-  bookmarks::AddIfNotBookmarked(bookmark_model, url, base::string16());
-  bookmark_model->GetNodesByURL(url, &nodes);
-  EXPECT_EQ(1UL, nodes.size());
+  // Check bookmark is still hidden on NTP.
+  EXPECT_EQ(BookmarkBar::HIDDEN, browser()->bookmark_bar_state());
+}
 
+IN_PROC_BROWSER_TEST_F(BookmarkTabHelperBrowserTest,
+                       AlwaysShowBookmarkBarOnNTPTest) {
+  auto* profile = browser()->profile();
+  // Check default is false.
+  EXPECT_FALSE(profile->GetPrefs()->GetBoolean(kAlwaysShowBookmarkBarOnNTP));
+  auto* contents = browser()->tab_strip_model()->GetActiveWebContents();
+  EXPECT_TRUE(content::NavigateToURL(contents,
+                                     GURL(chrome::kChromeUINewTabURL)));
+  EXPECT_TRUE(IsNTP(contents));
+  AddBookmarkNode(profile);
+  profile->GetPrefs()->SetBoolean(kAlwaysShowBookmarkBarOnNTP, true);
+
+  // Check bookmark is visible on NTP.
+  EXPECT_EQ(BookmarkBar::SHOW, browser()->bookmark_bar_state());
+
+  // Check bookmark is still visible on NTP regardless of kBookmarkBar pref
+  // change.
   chrome::ToggleBookmarkBar(browser());
-  EXPECT_EQ(BookmarkBar::HIDDEN, browser()->bookmark_bar_state());
+  EXPECT_EQ(BookmarkBar::SHOW, browser()->bookmark_bar_state());
+  chrome::ToggleBookmarkBar(browser());
+  EXPECT_EQ(BookmarkBar::SHOW, browser()->bookmark_bar_state());
 }
diff --git a/chromium_src/chrome/browser/ui/bookmarks/bookmark_tab_helper.cc b/chromium_src/chrome/browser/ui/bookmarks/bookmark_tab_helper.cc
@@ -0,0 +1,21 @@
+/* Copyright (c) 2019 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 http://mozilla.org/MPL/2.0/. */
+
+#include "brave/common/pref_names.h"
+#include "chrome/browser/profiles/profile.h"
+#include "components/prefs/pref_service.h"
+
+namespace {
+bool ShouldShowBookmarkBarOnNTP(Profile* profile) {
+  PrefService* prefs = profile->GetPrefs();
+    return prefs->GetBoolean(kAlwaysShowBookmarkBarOnNTP);
+}
+}  // namespace
+
+#define ReturnFalseIfBookmarkBarShouldHide(profile) \
+    if (!ShouldShowBookmarkBarOnNTP(profile))  \
+      return false;
+
+#include "../../../../../../chrome/browser/ui/bookmarks/bookmark_tab_helper.cc"  // NOLINT
diff --git a/chromium_src/chrome/browser/ui/browser.cc b/chromium_src/chrome/browser/ui/browser.cc
@@ -3,11 +3,19 @@
  * 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 "chrome/browser/ui/browser_command_controller.h"
-#include "chrome/browser/ui/browser_content_setting_bubble_model_delegate.h"
-#include "brave/browser/ui/brave_browser_content_setting_bubble_model_delegate.h"
 #include "brave/browser/ui/brave_browser_command_controller.h"
+#include "brave/browser/ui/brave_browser_content_setting_bubble_model_delegate.h"
 #include "brave/browser/ui/toolbar/brave_location_bar_model_delegate.h"
+#include "brave/common/pref_names.h"
+#include "chrome/browser/ui/browser_command_controller.h"
+#include "chrome/browser/ui/browser_content_setting_bubble_model_delegate.h"
+
+#define RegisterAlwaysShowBookmarkBarOnNTPChange \
+    profile_pref_registrar_.Add( \
+        kAlwaysShowBookmarkBarOnNTP, \
+        base::BindRepeating(&Browser::UpdateBookmarkBarState, \
+                            base::Unretained(this), \
+                            BOOKMARK_BAR_STATE_CHANGE_PREF_CHANGE));
 
 #define BrowserContentSettingBubbleModelDelegate \
   BraveBrowserContentSettingBubbleModelDelegate

diff --git a/chromium_src/chrome/browser/ui/webui/settings/settings_localized_strings_provider.cc b/chromium_src/chrome/browser/ui/webui/settings/settings_localized_strings_provider.cc
@@ -54,6 +54,8 @@ void BraveAddCommonStrings(content::WebUIDataSource* html_source,
        IDS_SETTINGS_HIDE_BRAVE_REWARDS_BUTTON_LABEL},
     {"appearanceSettingsHideBraveRewardsButtonDesc",
        IDS_SETTINGS_HIDE_BRAVE_REWARDS_BUTTON_DESC},
+    {"appearanceSettingsAlwaysShowBookmarkBarOnNTP",
+       IDS_SETTINGS_ALWAYS_SHOW_BOOKMARK_BAR_ON_NTP},
     {"braveShieldsTitle",
       IDS_SETTINGS_BRAVE_SHIELDS_TITLE},
     {"braveShieldsDefaultsSectionTitle",

diff --git a/common/pref_names.cc b/common/pref_names.cc
@@ -57,3 +57,5 @@ const char kNewTabPageShowBackgroundImage[] =
 const char kNewTabPageShowClock[] = "brave.new_tab_page.show_clock";
 const char kNewTabPageShowTopSites[] = "brave.new_tab_page.show_top_sites";
 const char kNewTabPageShowStats[] = "brave.new_tab_page.show_stats";
+const char kAlwaysShowBookmarkBarOnNTP[] =
+    "brave.always_show_bookmark_bar_on_ntp";
diff --git a/common/pref_names.h b/common/pref_names.h
@@ -50,5 +50,6 @@ extern const char kNewTabPageShowBackgroundImage[];
 extern const char kNewTabPageShowClock[];
 extern const char kNewTabPageShowTopSites[];
 extern const char kNewTabPageShowStats[];
+extern const char kAlwaysShowBookmarkBarOnNTP[];
 
 #endif  // BRAVE_COMMON_PREF_NAMES_H_
diff --git a/patches/chrome-browser-ui-bookmarks-bookmark_tab_helper.cc.patch b/patches/chrome-browser-ui-bookmarks-bookmark_tab_helper.cc.patch
@@ -1,12 +1,12 @@
 diff --git a/chrome/browser/ui/bookmarks/bookmark_tab_helper.cc b/chrome/browser/ui/bookmarks/bookmark_tab_helper.cc
-index f6ef61a0ac86ae55e8f9e80ece0758253af48520..888d656cf7080ebc32e90fc492e70a9b394d7026 100644
+index f6ef61a0ac86ae55e8f9e80ece0758253af48520..203e02628dab1ae085c5ebbb5d8906398dc103f2 100644
 --- a/chrome/browser/ui/bookmarks/bookmark_tab_helper.cc
 +++ b/chrome/browser/ui/bookmarks/bookmark_tab_helper.cc
-@@ -67,6 +67,7 @@ bool BookmarkTabHelper::ShouldShowBookmarkBar() const {
+@@ -66,6 +66,7 @@ bool BookmarkTabHelper::ShouldShowBookmarkBar() const {
+   if (prefs->IsManagedPreference(bookmarks::prefs::kShowBookmarkBar) &&
        !prefs->GetBoolean(bookmarks::prefs::kShowBookmarkBar))
      return false;
++  ReturnFalseIfBookmarkBarShouldHide(profile);
 
-+  return false;
    // The bookmark bar is only shown on the NTP if the user
    // has added something to it.
-   return IsNTP(web_contents()) && bookmark_model_ &&
diff --git a/patches/chrome-browser-ui-browser.cc.patch b/patches/chrome-browser-ui-browser.cc.patch
@@ -0,0 +1,12 @@
+diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
+index d4e1c21bc21f1f975c52372f375adaecc402af0b..d5fbd70d5f330a222af531991a1ecd936a114709 100644
+--- a/chrome/browser/ui/browser.cc
++++ b/chrome/browser/ui/browser.cc
+@@ -465,6 +465,7 @@ Browser::Browser(const CreateParams& params)
+       base::BindRepeating(&Browser::UpdateBookmarkBarState,
+                           base::Unretained(this),
+                           BOOKMARK_BAR_STATE_CHANGE_PREF_CHANGE));
++  RegisterAlwaysShowBookmarkBarOnNTPChange
+
+   if (search::IsInstantExtendedAPIEnabled() && is_type_tabbed())
+     instant_controller_.reset(new BrowserInstantController(this));