Skip to content

Commit

Permalink
Merge pull request #2869 from brave/add_always_show_bookmark_on_ntp_o…
Browse files Browse the repository at this point in the history
…ption

Add always show bookmark bar on NTP option
  • Loading branch information
simonhong committed Aug 23, 2019
2 parents 411d76e + 1033a4b commit f5e6114
Show file tree
Hide file tree
Showing 20 changed files with 353 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 f5e6114

Please sign in to comment.