From 13ebde93d8fc1a92d47d423d17754243cf0ec472 Mon Sep 17 00:00:00 2001 From: petemill Date: Thu, 27 Jun 2019 23:43:53 -0700 Subject: [PATCH] New Tab Page WebUI: Use Profile Preferences to store page options instead of localStorage This allows: - Surfacing these preferences outside of the NTP - Syncing / migrating the preferences - Making the change immediately for all open NTP Use FireWebUIListener to push data to javascript event subscribers. Still uses render_view_host->SetWebUIProperty (like the existing NTP data), which we may port in the future to a DataSource Dictionary like the other WebUI in chromium. Provides any data retreived or updated to redux store via redux action, but does not use redux actions to ask for a data change. Fix https://github.com/brave/brave-browser/issues/5064 --- browser/brave_profile_prefs.cc | 3 ++ .../ui/webui/brave_new_tab_message_handler.cc | 44 +++++++++++++++++++ .../ui/webui/brave_new_tab_message_handler.h | 2 + browser/ui/webui/brave_new_tab_ui.cc | 21 +++++++++ browser/ui/webui/brave_new_tab_ui.h | 1 + common/pref_names.cc | 2 + common/pref_names.h | 1 + .../actions/new_tab_actions.ts | 5 ++- .../brave_new_tab_ui/api/preferences.ts | 41 +++++++++++++++++ components/brave_new_tab_ui/brave_new_tab.tsx | 12 ++++- .../brave_new_tab_ui/components/app.tsx | 16 +++++-- .../components/newTab/index.tsx | 7 ++- .../constants/new_tab_types.ts | 2 +- .../reducers/new_tab_reducer.tsx | 13 ++++-- components/brave_new_tab_ui/storage.ts | 2 +- .../actions/new_tab_actions_test.ts | 5 --- .../reducers/new_tab_reducer_test.ts | 11 ----- 17 files changed, 158 insertions(+), 30 deletions(-) create mode 100644 components/brave_new_tab_ui/api/preferences.ts diff --git a/browser/brave_profile_prefs.cc b/browser/brave_profile_prefs.cc index 59cf80afdda2..39855e8b3260 100644 --- a/browser/brave_profile_prefs.cc +++ b/browser/brave_profile_prefs.cc @@ -120,6 +120,9 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) { // IPFS companion extension registry->RegisterBooleanPref(kIPFSCompanionEnabled, false); + + // New Tab Page + registry->RegisterBooleanPref(kNewTabPageShowBackgroundImage, true); } } // namespace brave diff --git a/browser/ui/webui/brave_new_tab_message_handler.cc b/browser/ui/webui/brave_new_tab_message_handler.cc index f87cd5318294..ed78f15e8748 100644 --- a/browser/ui/webui/brave_new_tab_message_handler.cc +++ b/browser/ui/webui/brave_new_tab_message_handler.cc @@ -5,6 +5,8 @@ #include "brave/browser/ui/webui/brave_new_tab_message_handler.h" +#include + #include "base/bind.h" #include "base/values.h" #include "brave/browser/ui/webui/brave_new_tab_ui.h" @@ -40,6 +42,10 @@ void BraveNewTabMessageHandler::OnJavascriptAllowed() { pref_change_registrar_.Add(kAlternativeSearchEngineProviderInTor, base::Bind(&BraveNewTabMessageHandler::OnPrivatePropertiesChanged, base::Unretained(this))); + // New Tab Page preferences + pref_change_registrar_.Add(kNewTabPageShowBackgroundImage, + base::Bind(&BraveNewTabMessageHandler::OnPreferencesChanged, + base::Unretained(this))); } void BraveNewTabMessageHandler::OnJavascriptDisallowed() { @@ -57,6 +63,11 @@ void BraveNewTabMessageHandler::RegisterMessages() { base::BindRepeating( &BraveNewTabMessageHandler::HandleToggleAlternativeSearchEngineProvider, base::Unretained(this))); + web_ui()->RegisterMessageCallback( + "saveNewTabPagePref", + base::BindRepeating( + &BraveNewTabMessageHandler::HandleSaveNewTabPagePref, + base::Unretained(this))); } void BraveNewTabMessageHandler::HandleInitialized(const base::ListValue* args) { @@ -69,6 +80,34 @@ void BraveNewTabMessageHandler::HandleToggleAlternativeSearchEngineProvider( Profile::FromWebUI(web_ui())); } +void BraveNewTabMessageHandler::HandleSaveNewTabPagePref( + const base::ListValue* args) { + if (args->GetSize() != 2) { + LOG(ERROR) << "Invalid input"; + return; + } + PrefService* prefs = Profile::FromWebUI(web_ui())->GetPrefs(); + // Collect args + std::string settingsKeyInput = args->GetList()[0].GetString(); + auto settingsValue = args->GetList()[1].Clone(); + // Validate args + // Note: if we introduce any non-bool settings values + // then perform this type check within the appropriate key conditionals. + if (!settingsValue.is_bool()) { + LOG(ERROR) << "Invalid value type"; + return; + } + const auto settingsValueBool = settingsValue.GetBool(); + std::string settingsKey; + if (settingsKeyInput == "showBackgroundImage") { + settingsKey = kNewTabPageShowBackgroundImage; + } else { + LOG(ERROR) << "Invalid setting key"; + return; + } + prefs->SetBoolean(settingsKey, settingsValueBool); +} + void BraveNewTabMessageHandler::OnPrivatePropertiesChanged() { new_tab_web_ui_->OnPrivatePropertiesChanged(); } @@ -77,3 +116,8 @@ void BraveNewTabMessageHandler::OnStatsChanged() { new_tab_web_ui_->OnStatsChanged(); FireWebUIListener("stats-updated"); } + +void BraveNewTabMessageHandler::OnPreferencesChanged() { + new_tab_web_ui_->OnPreferencesChanged(); + FireWebUIListener("preferences-changed"); +} diff --git a/browser/ui/webui/brave_new_tab_message_handler.h b/browser/ui/webui/brave_new_tab_message_handler.h index cd392e25a897..b47f8a023c3a 100644 --- a/browser/ui/webui/brave_new_tab_message_handler.h +++ b/browser/ui/webui/brave_new_tab_message_handler.h @@ -25,10 +25,12 @@ class BraveNewTabMessageHandler : public content::WebUIMessageHandler { void OnJavascriptDisallowed() override; void HandleInitialized(const base::ListValue* args); + void HandleSaveNewTabPagePref(const base::ListValue* args); void HandleToggleAlternativeSearchEngineProvider( const base::ListValue* args); void OnStatsChanged(); + void OnPreferencesChanged(); void OnPrivatePropertiesChanged(); PrefChangeRegistrar pref_change_registrar_; diff --git a/browser/ui/webui/brave_new_tab_ui.cc b/browser/ui/webui/brave_new_tab_ui.cc index 304016d4de94..f16b6d1a8d1b 100644 --- a/browser/ui/webui/brave_new_tab_ui.cc +++ b/browser/ui/webui/brave_new_tab_ui.cc @@ -35,6 +35,7 @@ void BraveNewTabUI::UpdateWebUIProperties() { auto* render_view_host = GetRenderViewHost(); SetStatsWebUIProperties(render_view_host); SetPrivateWebUIProperties(render_view_host); + SetPreferencesWebUIProperties(render_view_host); } void BraveNewTabUI::SetStatsWebUIProperties( @@ -86,6 +87,26 @@ void BraveNewTabUI::SetPrivateWebUIProperties( } } +void BraveNewTabUI::SetPreferencesWebUIProperties( + content::RenderViewHost* render_view_host) { + DCHECK(IsSafeToSetWebUIProperties()); + Profile* profile = Profile::FromWebUI(web_ui()); + PrefService* prefs = profile->GetPrefs(); + if (render_view_host) { + render_view_host->SetWebUIProperty( + "showBackgroundImage", + prefs->GetBoolean(kNewTabPageShowBackgroundImage) ? "true" + : "false"); + } +} + + +void BraveNewTabUI::OnPreferencesChanged() { + if (IsSafeToSetWebUIProperties()) { + SetPreferencesWebUIProperties(GetRenderViewHost()); + } +} + void BraveNewTabUI::OnPrivatePropertiesChanged() { if (IsSafeToSetWebUIProperties()) { SetPrivateWebUIProperties(GetRenderViewHost()); diff --git a/browser/ui/webui/brave_new_tab_ui.h b/browser/ui/webui/brave_new_tab_ui.h index 22a60288df7b..37e849d02008 100644 --- a/browser/ui/webui/brave_new_tab_ui.h +++ b/browser/ui/webui/brave_new_tab_ui.h @@ -26,6 +26,7 @@ class BraveNewTabUI : public BasicUI { private: // BasicUI overrides void UpdateWebUIProperties() override; + void SetPreferencesWebUIProperties(content::RenderViewHost* render_view_host); void SetStatsWebUIProperties(content::RenderViewHost* render_view_host); void SetPrivateWebUIProperties(content::RenderViewHost* render_view_host); diff --git a/common/pref_names.cc b/common/pref_names.cc index 09ddf949cb68..3a12d7b7d011 100644 --- a/common/pref_names.cc +++ b/common/pref_names.cc @@ -52,3 +52,5 @@ const char kWebTorrentEnabled[] = "brave.webtorrent_enabled"; const char kHangoutsEnabled[] = "brave.hangouts_enabled"; const char kHideBraveRewardsButton[] = "brave.hide_brave_rewards_button"; const char kIPFSCompanionEnabled[] = "brave.ipfs_companion_enabled"; +const char kNewTabPageShowBackgroundImage[] = + "brave.new_tab_page.show_background_image"; diff --git a/common/pref_names.h b/common/pref_names.h index b42d737430ba..a7513eb4abb9 100644 --- a/common/pref_names.h +++ b/common/pref_names.h @@ -46,5 +46,6 @@ extern const char kWebTorrentEnabled[]; extern const char kHangoutsEnabled[]; extern const char kHideBraveRewardsButton[]; extern const char kIPFSCompanionEnabled[]; +extern const char kNewTabPageShowBackgroundImage[]; #endif // BRAVE_COMMON_PREF_NAMES_H_ diff --git a/components/brave_new_tab_ui/actions/new_tab_actions.ts b/components/brave_new_tab_ui/actions/new_tab_actions.ts index 22720b0fd209..a2a84a3224b4 100644 --- a/components/brave_new_tab_ui/actions/new_tab_actions.ts +++ b/components/brave_new_tab_ui/actions/new_tab_actions.ts @@ -6,6 +6,7 @@ import { action } from 'typesafe-actions' // Constants import { types } from '../constants/new_tab_types' +import { Preferences } from '../api/preferences' export const topSitesDataUpdated = (topSites: NewTab.Site[]) => action(types.NEW_TAB_TOP_SITES_DATA_UPDATED, { topSites @@ -71,4 +72,6 @@ export const showSettingsMenu = () => action(types.NEW_TAB_SHOW_SETTINGS_MENU) export const closeSettingsMenu = () => action(types.NEW_TAB_CLOSE_SETTINGS_MENU) -export const toggleShowBackgroundImage = () => action(types.NEW_TAB_TOGGLE_SHOW_BACKGROUND_IMAGE) +export const preferencesUpdated = (preferences: Preferences) => action(types.NEW_TAB_PREFERENCES_UPDATED, { + preferences +}) diff --git a/components/brave_new_tab_ui/api/preferences.ts b/components/brave_new_tab_ui/api/preferences.ts new file mode 100644 index 000000000000..f6eab6abcf17 --- /dev/null +++ b/components/brave_new_tab_ui/api/preferences.ts @@ -0,0 +1,41 @@ +// 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/. + +// +// Manages get and set of NTP preference data +// Ensures everything to do with communication +// with the WebUI backend is all in 1 place, +// especially string keys. +// + +export type Preferences = { + showBackgroundImage: boolean +} + +function getWebUIBooleanVal (key: string): boolean { + return (chrome.getVariableValue(key).toLowerCase() === 'true') +} + +export function getPreferences (): Promise { + // Note(petemill): Returning as promise allows this + // to be async even though it isn't right now. + // Enforces practice of not setting directly + // in a redux reducer. + return Promise.resolve({ + showBackgroundImage: getWebUIBooleanVal('showBackgroundImage') + }) +} + +function sendSavePref (key: string, value: any) { + chrome.send('saveNewTabPagePref', [key, value]) +} + +export function saveShowBackgroundImage (value: boolean): void { + sendSavePref('showBackgroundImage', value) +} + +export function addChangeListener (listener: () => void): void { + window.cr.addWebUIListener('preferences-changed', listener) +} diff --git a/components/brave_new_tab_ui/brave_new_tab.tsx b/components/brave_new_tab_ui/brave_new_tab.tsx index 5877b9121ee8..708d5fdf7b4b 100644 --- a/components/brave_new_tab_ui/brave_new_tab.tsx +++ b/components/brave_new_tab_ui/brave_new_tab.tsx @@ -9,6 +9,8 @@ import { Provider } from 'react-redux' import Theme from 'brave-ui/theme/brave-default' import { ThemeProvider } from 'brave-ui/theme' import * as dataFetchAPI from './api/dataFetch' +import * as preferencesAPI from './api/preferences' + // Components import App from './components/app' @@ -20,7 +22,7 @@ import 'emptykit.css' import '../fonts/poppins.css' import '../fonts/muli.css' -function initialize () { +async function initialize () { render( @@ -31,6 +33,13 @@ function initialize () { ) window.i18nTemplate.process(window.document, window.loadTimeData) handleAPIEvents() + await updatePreferences() +} + +async function updatePreferences () { + const preferences = await preferencesAPI.getPreferences() + const actions = dataFetchAPI.getActions() + actions.preferencesUpdated(preferences) } function updateStats () { @@ -41,6 +50,7 @@ function updateStats () { function handleAPIEvents () { chrome.send('newTabPageInitialized', []) window.cr.addWebUIListener('stats-updated', updateStats) + preferencesAPI.addChangeListener(updatePreferences) } document.addEventListener('DOMContentLoaded', initialize) diff --git a/components/brave_new_tab_ui/components/app.tsx b/components/brave_new_tab_ui/components/app.tsx index 0f8cac0a82c6..22ad13fed00b 100644 --- a/components/brave_new_tab_ui/components/app.tsx +++ b/components/brave_new_tab_ui/components/app.tsx @@ -1,6 +1,7 @@ -/* 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/. */ +// 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/. import * as React from 'react' import { bindActionCreators, Dispatch } from 'redux' @@ -12,6 +13,7 @@ import NewTabPage from './newTab' // Utils import * as newTabActions from '../actions/new_tab_actions' +import * as PreferencesAPI from '../api/preferences' interface Props { actions: any @@ -29,7 +31,13 @@ class DefaultPage extends React.Component { return this.props.newTabData.isIncognito ? - : + : ( + + ) } } diff --git a/components/brave_new_tab_ui/components/newTab/index.tsx b/components/brave_new_tab_ui/components/newTab/index.tsx index 245d583470dd..a504a0444dfd 100644 --- a/components/brave_new_tab_ui/components/newTab/index.tsx +++ b/components/brave_new_tab_ui/components/newTab/index.tsx @@ -24,8 +24,9 @@ import FooterInfo from './footerInfo' import SiteRemovalNotification from './notification' interface Props { - actions: any newTabData: NewTab.State + actions: any + saveShowBackgroundImage: (value: boolean) => void } class NewTabPage extends React.Component { @@ -63,7 +64,9 @@ class NewTabPage extends React.Component { } toggleShowBackgroundImage = () => { - this.props.actions.toggleShowBackgroundImage() + this.props.saveShowBackgroundImage( + !this.props.newTabData.showBackgroundImage + ) } showSettings = () => { diff --git a/components/brave_new_tab_ui/constants/new_tab_types.ts b/components/brave_new_tab_ui/constants/new_tab_types.ts index 4bc9788ed2c5..62958751a90f 100644 --- a/components/brave_new_tab_ui/constants/new_tab_types.ts +++ b/components/brave_new_tab_ui/constants/new_tab_types.ts @@ -20,5 +20,5 @@ export const enum types { NEW_TAB_USE_ALTERNATIVE_PRIVATE_SEARCH_ENGINE = '@@newtab/NEW_TAB_USE_ALTERNATIVE_PRIVATE_SEARCH_ENGINE', NEW_TAB_SHOW_SETTINGS_MENU = '@@newtab/NEW_TAB_SHOW_SETTINGS_MENU', NEW_TAB_CLOSE_SETTINGS_MENU = '@@newtab/NEW_TAB_CLOSE_SETTINGS_MENU', - NEW_TAB_TOGGLE_SHOW_BACKGROUND_IMAGE = '@@newtab/NEW_TAB_TOGGLE_SHOW_BACKGROUND_IMAGE' + NEW_TAB_PREFERENCES_UPDATED = '@@newtab/NEW_TAB_PREFERENCES_UPDATED' } diff --git a/components/brave_new_tab_ui/reducers/new_tab_reducer.tsx b/components/brave_new_tab_ui/reducers/new_tab_reducer.tsx index 1e2ebd015479..7a50ed211f4f 100644 --- a/components/brave_new_tab_ui/reducers/new_tab_reducer.tsx +++ b/components/brave_new_tab_ui/reducers/new_tab_reducer.tsx @@ -6,6 +6,7 @@ import { Reducer } from 'redux' // Constants import { types } from '../constants/new_tab_types' +import { Preferences } from '../api/preferences' // API import * as gridAPI from '../api/topSites/grid' @@ -34,10 +35,6 @@ export const newTabReducer: Reducer = (state: NewTab.S state = { ...state, showSettings: false } break - case types.NEW_TAB_TOGGLE_SHOW_BACKGROUND_IMAGE: - state = { ...state, showBackgroundImage: !state.showBackgroundImage } - break - case types.BOOKMARK_ADDED: const topSite: NewTab.Site | undefined = state.topSites.find((site) => site.url === payload.url) if (topSite) { @@ -157,6 +154,14 @@ export const newTabReducer: Reducer = (state: NewTab.S state = { ...state, useAlternativePrivateSearchEngine: payload.shouldUse } break + case types.NEW_TAB_PREFERENCES_UPDATED: + const preferences: Preferences = payload.preferences + state = { + ...state, + ...preferences + } + break + default: break } diff --git a/components/brave_new_tab_ui/storage.ts b/components/brave_new_tab_ui/storage.ts index e2c0b3792471..49c9c7d05977 100644 --- a/components/brave_new_tab_ui/storage.ts +++ b/components/brave_new_tab_ui/storage.ts @@ -10,7 +10,7 @@ import { debounce } from '../common/debounce' const keyName = 'new-tab-data' const defaultState: NewTab.State = { - showBackgroundImage: true, + showBackgroundImage: false, showSettings: false, topSites: [], ignoredTopSites: [], diff --git a/components/test/brave_new_tab_ui/actions/new_tab_actions_test.ts b/components/test/brave_new_tab_ui/actions/new_tab_actions_test.ts index 3321ea5337f0..f7dd302569ab 100644 --- a/components/test/brave_new_tab_ui/actions/new_tab_actions_test.ts +++ b/components/test/brave_new_tab_ui/actions/new_tab_actions_test.ts @@ -154,9 +154,4 @@ describe('newTabActions', () => { type: types.NEW_TAB_SHOW_SETTINGS_MENU }) }) - it('toggleShowBackgroundImage', () => { - expect(actions.toggleShowBackgroundImage()).toEqual({ - type: types.NEW_TAB_TOGGLE_SHOW_BACKGROUND_IMAGE - }) - }) }) diff --git a/components/test/brave_new_tab_ui/reducers/new_tab_reducer_test.ts b/components/test/brave_new_tab_ui/reducers/new_tab_reducer_test.ts index 3619603ef248..36c4323651dc 100644 --- a/components/test/brave_new_tab_ui/reducers/new_tab_reducer_test.ts +++ b/components/test/brave_new_tab_ui/reducers/new_tab_reducer_test.ts @@ -352,15 +352,4 @@ describe('newTabReducer', () => { expect(assertion).toEqual(expected) }) }) - - describe('NEW_TAB_TOGGLE_SHOW_BACKGROUND_IMAGE', () => { - it('should toggle showBackgroundimage status to be true', () => { - const mockState = { ...fakeState, showBackgroundImage: false } - const expected = { ...mockState, showBackgroundImage: true } - const assertion = newTabReducer(mockState, { - type: types.NEW_TAB_TOGGLE_SHOW_BACKGROUND_IMAGE - }) - expect(assertion).toEqual(expected) - }) - }) })