Skip to content

Commit

Permalink
Add always show bookmark bar on NTP option
Browse files Browse the repository at this point in the history
Some user might want to show bookmark on NTP
when they choose to hide bookmark bar.
  • Loading branch information
simonhong committed Aug 20, 2019
1 parent 05c7b61 commit 0b9171b
Show file tree
Hide file tree
Showing 19 changed files with 336 additions and 25 deletions.
3 changes: 3 additions & 0 deletions app/brave_generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,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
Expand Down
2 changes: 2 additions & 0 deletions browser/brave_tab_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "brave/browser/brave_drm_tab_helper.h"
#include "brave/browser/greaselion/greaselion_tab_helper.h"
#include "brave/browser/ui/bookmark/brave_bookmark_tab_helper.h"
#include "brave/components/brave_ads/browser/ads_tab_helper.h"
#include "brave/components/brave_rewards/browser/buildflags/buildflags.h"
#include "brave/components/brave_shields/browser/buildflags/buildflags.h" // For STP
Expand Down Expand Up @@ -36,6 +37,7 @@ void AttachTabHelpers(content::WebContents* web_contents) {
#endif
// Add tab helpers here unless they are intended for android too
BraveDrmTabHelper::CreateForWebContents(web_contents);
BraveBookmarkTabHelper::CreateForWebContents(web_contents);
#endif

#if BUILDFLAG(BRAVE_STP_ENABLED)
Expand Down
8 changes: 8 additions & 0 deletions browser/browser_context_keyed_service_factories.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@
#include "brave/components/brave_ads/browser/ads_service_factory.h"
#include "brave/components/brave_rewards/browser/rewards_service_factory.h"

#if !defined(OS_ANDROID)
#include "brave/browser/ui/bookmark/bookmark_prefs_service_factory.h"
#endif

namespace brave {

void EnsureBrowserContextKeyedServiceFactoriesBuilt() {
Expand All @@ -23,6 +27,10 @@ void EnsureBrowserContextKeyedServiceFactoriesBuilt() {
greaselion::GreaselionServiceFactory::GetInstance();
TorProfileServiceFactory::GetInstance();
SearchEngineProviderServiceFactory::GetInstance();

#if !defined(OS_ANDROID)
BookmarkPrefsServiceFactory::GetInstance();
#endif
}

} // namespace brave
2 changes: 2 additions & 0 deletions browser/extensions/api/settings_private/brave_prefs_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
6 changes: 6 additions & 0 deletions browser/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ source_set("ui") {

if (!is_android) {
sources += [
"bookmark/brave_bookmark_tab_helper.cc",
"bookmark/brave_bookmark_tab_helper.h",
"bookmark/bookmark_prefs_service.cc",
"bookmark/bookmark_prefs_service.h",
"bookmark/bookmark_prefs_service_factory.cc",
"bookmark/bookmark_prefs_service_factory.h",
"brave_browser_command_controller.cc",
"brave_browser_command_controller.h",
"brave_browser_content_setting_bubble_model_delegate.cc",
Expand Down
32 changes: 32 additions & 0 deletions browser/ui/bookmark/bookmark_prefs_service.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/* 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/browser/ui/bookmark/bookmark_prefs_service.h"

#include "brave/common/pref_names.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_list.h"

BookmarkPrefsService::BookmarkPrefsService(Profile* profile)
: profile_(profile),
prefs_(profile->GetPrefs()) {
pref_change_registrar_.Init(prefs_);
pref_change_registrar_.Add(
kAlwaysShowBookmarkBarOnNTP,
base::BindRepeating(&BookmarkPrefsService::OnPreferenceChanged,
base::Unretained(this)));
}

BookmarkPrefsService::~BookmarkPrefsService() = default;

void BookmarkPrefsService::OnPreferenceChanged() {
for (Browser* browser : *BrowserList::GetInstance()) {
if (profile_->IsSameProfile(browser->profile())) {
browser->UpdateBookmarkBarState(
Browser::BOOKMARK_BAR_STATE_CHANGE_PREF_CHANGE);
}
}
}
31 changes: 31 additions & 0 deletions browser/ui/bookmark/bookmark_prefs_service.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/* 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/. */

#ifndef BRAVE_BROWSER_UI_BOOKMARK_BOOKMARK_PREFS_SERVICE_H_
#define BRAVE_BROWSER_UI_BOOKMARK_BOOKMARK_PREFS_SERVICE_H_

#include "components/keyed_service/core/keyed_service.h"
#include "components/prefs/pref_change_registrar.h"
#include "content/public/browser/browser_thread.h"

class PrefService;
class Profile;

class BookmarkPrefsService : public KeyedService {
public:
explicit BookmarkPrefsService(Profile* profile);
~BookmarkPrefsService() override;

private:
void OnPreferenceChanged();

Profile* profile_;
PrefService* prefs_;
PrefChangeRegistrar pref_change_registrar_;

DISALLOW_COPY_AND_ASSIGN(BookmarkPrefsService);
};

#endif // BRAVE_BROWSER_UI_BOOKMARK_BOOKMARK_PREFS_SERVICE_H_
51 changes: 51 additions & 0 deletions browser/ui/bookmark/bookmark_prefs_service_factory.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/* 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/browser/ui/bookmark/bookmark_prefs_service_factory.h"

#include "brave/browser/ui/bookmark/bookmark_prefs_service.h"
#include "brave/common/pref_names.h"
#include "chrome/browser/profiles/incognito_helpers.h"
#include "chrome/browser/profiles/profile.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/pref_registry/pref_registry_syncable.h"

// static
BookmarkPrefsService* BookmarkPrefsServiceFactory::GetForBrowserContext(
content::BrowserContext* context) {
return static_cast<BookmarkPrefsService*>(
GetInstance()->GetServiceForBrowserContext(context, true));
}

// static
BookmarkPrefsServiceFactory* BookmarkPrefsServiceFactory::GetInstance() {
return base::Singleton<BookmarkPrefsServiceFactory>::get();
}

BookmarkPrefsServiceFactory::BookmarkPrefsServiceFactory()
: BrowserContextKeyedServiceFactory(
"BookmarkPrefsService",
BrowserContextDependencyManager::GetInstance()) {}

BookmarkPrefsServiceFactory::~BookmarkPrefsServiceFactory() {}

KeyedService* BookmarkPrefsServiceFactory::BuildServiceInstanceFor(
content::BrowserContext* context) const {
return new BookmarkPrefsService(Profile::FromBrowserContext(context));
}

content::BrowserContext* BookmarkPrefsServiceFactory::GetBrowserContextToUse(
content::BrowserContext* context) const {
return chrome::GetBrowserContextRedirectedInIncognito(context);
}

bool BookmarkPrefsServiceFactory::ServiceIsCreatedWithBrowserContext() const {
return true;
}

void BookmarkPrefsServiceFactory::RegisterProfilePrefs(
user_prefs::PrefRegistrySyncable* registry) {
registry->RegisterBooleanPref(kAlwaysShowBookmarkBarOnNTP, false);
}
39 changes: 39 additions & 0 deletions browser/ui/bookmark/bookmark_prefs_service_factory.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/* 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/. */

#ifndef BRAVE_BROWSER_UI_BOOKMARK_BOOKMARK_PREFS_SERVICE_FACTORY_H_
#define BRAVE_BROWSER_UI_BOOKMARK_BOOKMARK_PREFS_SERVICE_FACTORY_H_

#include "base/memory/singleton.h"
#include "components/keyed_service/content/browser_context_keyed_service_factory.h"

class BookmarkPrefsService;

class BookmarkPrefsServiceFactory : public BrowserContextKeyedServiceFactory {
public:
static BookmarkPrefsService* GetForBrowserContext(
content::BrowserContext* context);

static BookmarkPrefsServiceFactory* GetInstance();

private:
friend struct base::DefaultSingletonTraits<BookmarkPrefsServiceFactory>;

BookmarkPrefsServiceFactory();
~BookmarkPrefsServiceFactory() override;

// BrowserContextKeyedServiceFactory:
KeyedService* BuildServiceInstanceFor(
content::BrowserContext* profile) const override;
content::BrowserContext* GetBrowserContextToUse(
content::BrowserContext* context) const override;
bool ServiceIsCreatedWithBrowserContext() const override;
void RegisterProfilePrefs(
user_prefs::PrefRegistrySyncable* registry) override;

DISALLOW_COPY_AND_ASSIGN(BookmarkPrefsServiceFactory);
};

#endif // BRAVE_BROWSER_UI_BOOKMARK_BOOKMARK_PREFS_SERVICE_FACTORY_H_
51 changes: 41 additions & 10 deletions browser/ui/bookmark/bookmark_tab_helper_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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,
Expand All @@ -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());
}
48 changes: 48 additions & 0 deletions browser/ui/bookmark/brave_bookmark_tab_helper.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/* 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/browser/ui/bookmark/brave_bookmark_tab_helper.h"

#include "brave/common/pref_names.h"
#include "chrome/browser/profiles/profile.h"
#include "components/prefs/pref_service.h"

BraveBookmarkTabHelper::BraveBookmarkTabHelper(
content::WebContents* web_contents)
: web_contents_(web_contents) {
}

BraveBookmarkTabHelper::~BraveBookmarkTabHelper() {
}

void BraveBookmarkTabHelper::AddObserver(BookmarkTabHelperObserver* observer) {
BookmarkTabHelper::FromWebContents(web_contents_)->AddObserver(observer);
}

void BraveBookmarkTabHelper::RemoveObserver(
BookmarkTabHelperObserver* observer) {
BookmarkTabHelper::FromWebContents(web_contents_)->RemoveObserver(observer);
}

bool BraveBookmarkTabHelper::ShouldShowBookmarkBar() {
BookmarkTabHelper* helper =
BookmarkTabHelper::FromWebContents(web_contents_);
if (!helper)
return false;

// Originally, bookmark is visible for NTP when bookmarks are non empty.
// In that case, we want to hide bookmark bar on NTP if user chooses to hide.
// When bookmark should be hidden, we do not change it.
bool should_show = helper->ShouldShowBookmarkBar();
if (should_show) {
Profile* profile =
Profile::FromBrowserContext(web_contents_->GetBrowserContext());
should_show = profile->GetPrefs()->GetBoolean(kAlwaysShowBookmarkBarOnNTP);
}

return should_show;
}

WEB_CONTENTS_USER_DATA_KEY_IMPL(BraveBookmarkTabHelper)
Loading

0 comments on commit 0b9171b

Please sign in to comment.