From 1bb9e9c82a8c018ead70dc5b57035a6d636713ab Mon Sep 17 00:00:00 2001 From: Pete Miller Date: Mon, 13 Jun 2022 17:03:50 -0700 Subject: [PATCH 1/3] Use chromium's side panel inside our side bar container, and insert Reading List Chromium's side panel is controlled by Brave's side bar buttons, via chromium's side panel coordinator --- app/brave_main_delegate_browsertest.cc | 1 - app/theme/brave_theme_resources.grd | 1 + .../brave/sidebar_reading_list_focused.png | Bin 0 -> 671 bytes .../brave/sidebar_reading_list_focused.png | Bin 0 -> 1453 bytes app/vector_icons/BUILD.gn | 1 + app/vector_icons/sidebar_reading_list.icon | 77 +++++ brave_paks.gni | 8 - browser/brave_content_browser_client.cc | 10 - .../net/brave_network_audit_browsertest.cc | 2 +- browser/resources/sidebar/BUILD.gn | 30 -- browser/resources/sidebar/bookmarks/BUILD.gn | 85 ----- .../sidebar/bookmarks/bookmark_folder.html | 239 ------------- .../sidebar/bookmarks/bookmark_folder.ts | 232 ------------- .../sidebar/bookmarks/bookmarks.html | 34 -- .../sidebar/bookmarks/bookmarks_api_proxy.ts | 75 ----- .../bookmarks/bookmarks_drag_manager.ts | 266 --------------- .../sidebar/bookmarks/bookmarks_list.html | 4 - .../sidebar/bookmarks/bookmarks_list.ts | 314 ------------------ .../sidebar/bookmarks/tsconfig_base.json | 9 - browser/ui/BUILD.gn | 18 +- browser/ui/sidebar/BUILD.gn | 4 - browser/ui/sidebar/sidebar.h | 7 - browser/ui/sidebar/sidebar_browsertest.cc | 50 +-- browser/ui/sidebar/sidebar_controller.cc | 5 +- browser/ui/sidebar/sidebar_model.cc | 103 ++---- browser/ui/sidebar/sidebar_model.h | 13 +- browser/ui/sidebar/sidebar_model_data.cc | 74 ----- browser/ui/sidebar/sidebar_model_data.h | 48 --- browser/ui/sidebar/sidebar_unittest.cc | 24 +- browser/ui/sidebar/sidebar_utils.cc | 2 +- .../sidebar/sidebar_web_contents_delegate.cc | 40 --- .../sidebar/sidebar_web_contents_delegate.h | 31 -- browser/ui/tabs/brave_tab_strip_model.cc | 5 - browser/ui/tabs/brave_tab_strip_model.h | 2 - browser/ui/views/frame/brave_browser_view.cc | 12 +- .../views/frame/brave_browser_view_layout.cc | 15 + .../views/frame/brave_browser_view_layout.h | 20 ++ .../ui/views/side_panel/brave_side_panel.cc | 67 ++++ .../ui/views/side_panel/brave_side_panel.h | 39 +++ .../views/sidebar/sidebar_container_view.cc | 229 ++++++++----- .../ui/views/sidebar/sidebar_container_view.h | 55 ++- .../ui/views/sidebar/sidebar_control_view.cc | 9 +- .../ui/views/sidebar/sidebar_control_view.h | 11 +- .../sidebar/sidebar_items_contents_view.cc | 4 + .../ui/views/sidebar/sidebar_panel_webview.cc | 52 --- .../ui/views/sidebar/sidebar_panel_webview.h | 43 --- .../views/sidebar/sidebar_side_panel_utils.cc | 31 ++ .../views/sidebar/sidebar_side_panel_utils.h | 19 ++ .../webui/brave_web_ui_controller_factory.cc | 13 - browser/ui/webui/sidebar/BUILD.gn | 17 - browser/ui/webui/sidebar/sidebar.mojom | 30 -- .../sidebar/sidebar_bookmarks_page_handler.cc | 153 --------- .../sidebar/sidebar_bookmarks_page_handler.h | 37 --- .../ui/webui/sidebar/sidebar_bookmarks_ui.cc | 71 ---- .../ui/webui/sidebar/sidebar_bookmarks_ui.h | 44 --- .../chrome/browser/ui/browser_finder.cc | 44 --- chromium_src/chrome/browser/ui/ui_features.cc | 12 +- .../browser/ui/views/frame/browser_view.cc | 11 + .../browser/ui/views/frame/browser_view.h | 8 + .../ui/views/frame/browser_view_layout.h | 14 + .../side_panel/side_panel_coordinator.cc | 10 + .../side_search_browser_controller.cc | 16 + .../side_search_browser_controller.h | 16 + .../browser/ui/views/toolbar/toolbar_view.cc | 3 - .../features/reading_list_switches.cc | 19 -- common/extensions/api/_api_features.json | 17 +- components/constants/webui_url_constants.cc | 1 - components/constants/webui_url_constants.h | 1 - .../resources/brave_components_strings.grd | 3 + components/sidebar/constants.h | 5 - components/sidebar/sidebar_item.cc | 15 +- components/sidebar/sidebar_item.h | 6 + components/sidebar/sidebar_service.cc | 15 +- components/sidebar/sidebar_service.h | 1 + ...side_panel-side_panel_coordinator.cc.patch | 12 + ...ser-ui-views-toolbar-toolbar_view.cc.patch | 10 +- .../chrome/app/vector_icons/side_panel.icon | 23 ++ 77 files changed, 703 insertions(+), 2344 deletions(-) create mode 100644 app/theme/default_100_percent/brave/sidebar_reading_list_focused.png create mode 100644 app/theme/default_200_percent/brave/sidebar_reading_list_focused.png create mode 100644 app/vector_icons/sidebar_reading_list.icon delete mode 100644 browser/resources/sidebar/BUILD.gn delete mode 100644 browser/resources/sidebar/bookmarks/BUILD.gn delete mode 100644 browser/resources/sidebar/bookmarks/bookmark_folder.html delete mode 100644 browser/resources/sidebar/bookmarks/bookmark_folder.ts delete mode 100644 browser/resources/sidebar/bookmarks/bookmarks.html delete mode 100644 browser/resources/sidebar/bookmarks/bookmarks_api_proxy.ts delete mode 100644 browser/resources/sidebar/bookmarks/bookmarks_drag_manager.ts delete mode 100644 browser/resources/sidebar/bookmarks/bookmarks_list.html delete mode 100644 browser/resources/sidebar/bookmarks/bookmarks_list.ts delete mode 100644 browser/resources/sidebar/bookmarks/tsconfig_base.json delete mode 100644 browser/ui/sidebar/sidebar_model_data.cc delete mode 100644 browser/ui/sidebar/sidebar_model_data.h delete mode 100644 browser/ui/sidebar/sidebar_web_contents_delegate.cc delete mode 100644 browser/ui/sidebar/sidebar_web_contents_delegate.h create mode 100644 browser/ui/views/frame/brave_browser_view_layout.cc create mode 100644 browser/ui/views/frame/brave_browser_view_layout.h create mode 100644 browser/ui/views/side_panel/brave_side_panel.cc create mode 100644 browser/ui/views/side_panel/brave_side_panel.h delete mode 100644 browser/ui/views/sidebar/sidebar_panel_webview.cc delete mode 100644 browser/ui/views/sidebar/sidebar_panel_webview.h create mode 100644 browser/ui/views/sidebar/sidebar_side_panel_utils.cc create mode 100644 browser/ui/views/sidebar/sidebar_side_panel_utils.h delete mode 100644 browser/ui/webui/sidebar/BUILD.gn delete mode 100644 browser/ui/webui/sidebar/sidebar.mojom delete mode 100644 browser/ui/webui/sidebar/sidebar_bookmarks_page_handler.cc delete mode 100644 browser/ui/webui/sidebar/sidebar_bookmarks_page_handler.h delete mode 100644 browser/ui/webui/sidebar/sidebar_bookmarks_ui.cc delete mode 100644 browser/ui/webui/sidebar/sidebar_bookmarks_ui.h delete mode 100644 chromium_src/chrome/browser/ui/browser_finder.cc create mode 100644 chromium_src/chrome/browser/ui/views/frame/browser_view_layout.h create mode 100644 chromium_src/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc create mode 100644 chromium_src/chrome/browser/ui/views/side_search/side_search_browser_controller.cc create mode 100644 chromium_src/chrome/browser/ui/views/side_search/side_search_browser_controller.h delete mode 100644 chromium_src/components/reading_list/features/reading_list_switches.cc create mode 100644 patches/chrome-browser-ui-views-side_panel-side_panel_coordinator.cc.patch create mode 100644 vector_icons/chrome/app/vector_icons/side_panel.icon diff --git a/app/brave_main_delegate_browsertest.cc b/app/brave_main_delegate_browsertest.cc index 55d1ca93d0c8..7e436d13c957 100644 --- a/app/brave_main_delegate_browsertest.cc +++ b/app/brave_main_delegate_browsertest.cc @@ -150,7 +150,6 @@ IN_PROC_BROWSER_TEST_F(BraveMainDelegateBrowserTest, DisabledFeatures) { &permissions::features::kPermissionOnDeviceNotificationPredictions, &privacy_sandbox::kOverridePrivacySandboxSettingsLocalTesting, &privacy_sandbox::kPrivacySandboxSettings3, - &reading_list::switches::kReadLater, &shared_highlighting::kSharedHighlightingAmp, #if !BUILDFLAG(IS_ANDROID) &translate::kTFLiteLanguageDetectionEnabled, diff --git a/app/theme/brave_theme_resources.grd b/app/theme/brave_theme_resources.grd index 9512f0fd1202..2a219d52acfd 100644 --- a/app/theme/brave_theme_resources.grd +++ b/app/theme/brave_theme_resources.grd @@ -42,6 +42,7 @@ + diff --git a/app/theme/default_100_percent/brave/sidebar_reading_list_focused.png b/app/theme/default_100_percent/brave/sidebar_reading_list_focused.png new file mode 100644 index 0000000000000000000000000000000000000000..1996ed38860b093b22d5f6710bf01f1217e48dc7 GIT binary patch literal 671 zcmV;Q0$}}#P)D3Wd!?d&-G@(+`gdc zl@dE55f&3o>=CM|jjAC-z0cP5lm5U3CFu(BTNpCjxcJU?=umC?O2>7pfAh)7NjEOPwi^FLs= z%a_+FBo^O49&q^^#aDTwtLi}trfhUy&zv8X12?J*hz|+XDM5Z%y58d<_evExkz#7g z#P=yOVudJ4srqeop(7xCN0rCe76fkO0bT|ehn7XbAn9q#ST+qR0{(Db#hBWt z-kp5?^wg|~yGu7HT-4={kGfoTe~mpmalh*fK^3t=KB5@~yAwyYG_}!gqFs{F@)ei^ zl_NbaA+=6av zhX^7jTLVCZL{ZoL9;kUgH>@dapBWBz_z&D!zPJuhTqX!z+cU`%c}D<|3A|1aAnyUf zExh~6c>-3PSn;%?X(2>tzy3X$fjbEb`L?dYvtU;;Rw~Nu|KverU65>sfXq}OgaKgy zR0c5+b)5)QDDpQ-Sn5et2`gq2VZ3w1dj>3Ml-O91Hz-O)28s@T#ZOYhyS%p)qu6{@ zR}1swKmE1=Gtgh1Zpr(9X1;k-4pofB>e*qxnuULF@q-;!E(ZY)I(wc^Xjacg9Q#Ug zfc#{2ikuI#DK`pWVL}Ok#a`Xhf{=BvFSbZaeI}xk{{sR5kCxNC!)AcX@f8t(M4ordx$o}xslz+wQ4&nQA=KzXYpBlceK$N)?oE6%0og`v$+4|r!WJdbO80NA|ZN7 zw514^OlWPh1xa@$H@+AHj)maw7cPFxjizI{5xqL1Dl$*Sifdj^+T;eB2Y$Wq;Z@Fy zQn5o2R%I5#qj*x+u>11iXMJzw-U}QM@)2slEtv^yfDk1pset3E2ypP+rQM^6II?lL z_w+|=iD*-EBEx%AVW3aI$UR2Fi;#^7IU|#!AzQtz%9Do%12z_$zzWi-k@BUn_wLmN z4XW&jq!qI<5#W&VG;?UnR)=u-TYokrFUE$_ZQ!T2KU)xE7g{?|xAmLF!fp)~9B+aoT_28J?0MX18r41uZb<$Z$XG^eN;293o7SH1Z;cmsu$#1(azRpV@+7?LZRs&hqPIGkmPLDhDYynB*4|El0$n+MVI%Lf-`uy{04~vYRbxDI_`pgDQPY!Vd8?K z-Xm#7`CHN(kGw;jlBl_m(map); -#endif - // Brave News #if !BUILDFLAG(IS_ANDROID) if (base::FeatureList::IsEnabled(brave_today::features::kBraveNewsFeature)) { diff --git a/browser/net/brave_network_audit_browsertest.cc b/browser/net/brave_network_audit_browsertest.cc index 2dbf5d26bfba..eeb741440c10 100644 --- a/browser/net/brave_network_audit_browsertest.cc +++ b/browser/net/brave_network_audit_browsertest.cc @@ -309,7 +309,7 @@ IN_PROC_BROWSER_TEST_F(BraveNetworkAuditTest, BasicTests) { auto* sidebar_model = sidebar_controller->model(); auto all_items = sidebar_model->GetAllSidebarItems(); const int item_num = all_items.size(); - const int builtin_panel_item_total = 1; + const int builtin_panel_item_total = 2; int builtin_panel_item_count = 0; for (int i = 0; i < item_num; ++i) { auto item = all_items[i]; diff --git a/browser/resources/sidebar/BUILD.gn b/browser/resources/sidebar/BUILD.gn deleted file mode 100644 index e0a2b7165c75..000000000000 --- a/browser/resources/sidebar/BUILD.gn +++ /dev/null @@ -1,30 +0,0 @@ -# Copyright (c) 2022 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("//brave/resources/brave_grit.gni") -import("//chrome/common/features.gni") -import("//ui/webui/resources/tools/generate_grd.gni") - -brave_grit("resources") { - defines = chrome_grit_defines - - enable_input_discovery_for_gn_analyze = false - source = "$target_gen_dir/sidebar_resources.grd" - deps = [ ":build_grd" ] - - outputs = [ - "grit/sidebar_resources.h", - "grit/sidebar_resources_map.cc", - "grit/sidebar_resources_map.h", - "sidebar_resources.pak", - ] -} - -generate_grd("build_grd") { - grd_prefix = "sidebar" - out_grd = "$target_gen_dir/${grd_prefix}_resources.grd" - grdp_files = [ "$target_gen_dir/bookmarks/resources.grdp" ] - deps = [ "bookmarks:build_grdp" ] -} diff --git a/browser/resources/sidebar/bookmarks/BUILD.gn b/browser/resources/sidebar/bookmarks/BUILD.gn deleted file mode 100644 index b227f7ebcec3..000000000000 --- a/browser/resources/sidebar/bookmarks/BUILD.gn +++ /dev/null @@ -1,85 +0,0 @@ -# Copyright (c) 2022 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("//tools/grit/preprocess_if_expr.gni") -import("//tools/polymer/html_to_js.gni") -import("//tools/typescript/ts_library.gni") -import("//ui/webui/resources/tools/generate_grd.gni") - -preprocess_folder = - "$root_gen_dir/brave/browser/resources/sidebar/bookmarks/preprocessed" - -generate_grd("build_grdp") { - grd_prefix = "sidebar_bookmarks" - out_grd = "$target_gen_dir/resources.grdp" - deps = [ ":build_ts" ] - manifest_files = [ - "$root_gen_dir/brave/browser/resources/sidebar/bookmarks/tsconfig.manifest", - ] - input_files = [ "bookmarks.html" ] - input_files_base_dir = rebase_path(".", "//") -} - -preprocess_if_expr("preprocess") { - in_folder = "./" - out_folder = preprocess_folder - in_files = [ - "bookmarks_api_proxy.ts", - "bookmarks_drag_manager.ts", - ] -} - -preprocess_if_expr("preprocess_generated") { - deps = [ ":web_components" ] - in_folder = target_gen_dir - out_folder = preprocess_folder - in_files = [ - "bookmark_folder.ts", - "bookmarks_list.ts", - ] -} - -preprocess_if_expr("preprocess_mojo") { - deps = [ "//brave/browser/ui/webui/sidebar:mojo_bindings_webui_js" ] - in_folder = "$root_gen_dir/mojom-webui/brave/browser/ui/webui/sidebar/" - out_folder = preprocess_folder - out_manifest = "$target_gen_dir/preprocessed_mojo_manifest.json" - in_files = [ "sidebar.mojom-webui.js" ] -} - -html_to_js("web_components") { - js_files = [ - "bookmark_folder.ts", - "bookmarks_list.ts", - ] -} - -ts_library("build_ts") { - tsconfig_base = "tsconfig_base.json" - root_dir = "$target_gen_dir/preprocessed" - out_dir = "$target_gen_dir/tsc" - in_files = [ - "bookmark_folder.ts", - "bookmarks_list.ts", - "bookmarks_api_proxy.ts", - "bookmarks_drag_manager.ts", - "sidebar.mojom-webui.js", - ] - definitions = [ - "//tools/typescript/definitions/bookmark_manager_private.d.ts", - "//tools/typescript/definitions/bookmarks.d.ts", - "//tools/typescript/definitions/chrome_event.d.ts", - ] - deps = [ - "//third_party/polymer/v3_0:library", - "//ui/webui/resources:library", - "//ui/webui/resources/mojo:library", - ] - extra_deps = [ - ":preprocess", - ":preprocess_generated", - ":preprocess_mojo", - ] -} diff --git a/browser/resources/sidebar/bookmarks/bookmark_folder.html b/browser/resources/sidebar/bookmarks/bookmark_folder.html deleted file mode 100644 index 43d9ddc7d08a..000000000000 --- a/browser/resources/sidebar/bookmarks/bookmark_folder.html +++ /dev/null @@ -1,239 +0,0 @@ - -
- - - -
\ No newline at end of file diff --git a/browser/resources/sidebar/bookmarks/bookmark_folder.ts b/browser/resources/sidebar/bookmarks/bookmark_folder.ts deleted file mode 100644 index 6bf101bfd645..000000000000 --- a/browser/resources/sidebar/bookmarks/bookmark_folder.ts +++ /dev/null @@ -1,232 +0,0 @@ -// Copyright 2021 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'chrome://resources/cr_elements/cr_icon_button/cr_icon_button.m.js'; -import 'chrome://resources/cr_elements/shared_vars_css.m.js'; -import 'chrome://resources/cr_elements/mwb_element_shared_style.css.js'; - -import {getFaviconForPageURL} from 'chrome://resources/js/icon.js'; -import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js'; - -import {BookmarksApiProxy} from './bookmarks_api_proxy.js'; - -/** Event interface for dom-repeat. */ -interface RepeaterMouseEvent extends MouseEvent { - clientX: number; - clientY: number; - model: { - item: chrome.bookmarks.BookmarkTreeNode, - }; -} - -export interface BookmarkFolderElement { - $: { - children: HTMLElement, - }; -} - -// Event name for open state of a folder being changed. -export const FOLDER_OPEN_CHANGED_EVENT = 'bookmark-folder-open-changed'; - -export class BookmarkFolderElement extends PolymerElement { - static get is() { - return 'bookmark-folder'; - } - - static get template() { - return html`{__html_template__}`; - } - - static get properties() { - return { - childDepth_: { - type: Number, - value: 1, - }, - - depth: { - type: Number, - observer: 'onDepthChanged_', - value: 0, - }, - - folder: Object, - - open_: { - type: Boolean, - value: false, - }, - - openFolders: { - type: Array, - observer: 'onOpenFoldersChanged_', - }, - }; - } - - private childDepth_: number; - depth: number; - folder: chrome.bookmarks.BookmarkTreeNode; - private open_: boolean; - openFolders: string[]; - private bookmarksApi_: BookmarksApiProxy = BookmarksApiProxy.getInstance(); - - static get observers() { - return [ - 'onChildrenLengthChanged_(folder.children.length)', - ]; - } - - private getAriaExpanded_(): string|undefined { - if (!this.folder.children || this.folder.children.length === 0) { - // Remove the attribute for empty folders that cannot be expanded. - return undefined; - } - - return this.open_ ? 'true' : 'false'; - } - - private onBookmarkAuxClick_(event: RepeaterMouseEvent) { - if (event.button !== 1) { - // Not a middle click. - return; - } - - event.preventDefault(); - event.stopPropagation(); - this.bookmarksApi_.openBookmark(event.model.item.url!, this.depth, { - middleButton: true, - altKey: event.altKey, - ctrlKey: event.ctrlKey, - metaKey: event.metaKey, - shiftKey: event.shiftKey, - }); - } - - private onBookmarkClick_(event: RepeaterMouseEvent) { - event.preventDefault(); - event.stopPropagation(); - this.bookmarksApi_.openBookmark(event.model.item.url!, this.depth, { - middleButton: false, - altKey: event.altKey, - ctrlKey: event.ctrlKey, - metaKey: event.metaKey, - shiftKey: event.shiftKey, - }); - } - - private onBookmarkContextMenu_(event: RepeaterMouseEvent) { - event.preventDefault(); - event.stopPropagation(); - this.bookmarksApi_.showContextMenu( - event.model.item.id, event.clientX, event.clientY); - } - - private onFolderContextMenu_(event: MouseEvent) { - event.preventDefault(); - event.stopPropagation(); - this.bookmarksApi_.showContextMenu( - this.folder.id, event.clientX, event.clientY); - } - - private getBookmarkIcon_(url: string): string { - return getFaviconForPageURL(url, false); - } - - private onChildrenLengthChanged_() { - if (this.folder.children) { - this.style.setProperty( - '--child-count', this.folder.children!.length.toString()); - } else { - this.style.setProperty('--child-count', '0'); - } - } - - private onDepthChanged_() { - this.childDepth_ = this.depth + 1; - this.style.setProperty('--node-depth', `${this.depth}`); - this.style.setProperty('--child-depth', `${this.childDepth_}`); - } - - private onFolderClick_(event: Event) { - event.preventDefault(); - event.stopPropagation(); - - if (!this.folder.children || this.folder.children.length === 0) { - // No reason to open if there are no children to show. - return; - } - - this.open_ = !this.open_; - this.dispatchEvent(new CustomEvent(FOLDER_OPEN_CHANGED_EVENT, { - bubbles: true, - composed: true, - detail: { - id: this.folder.id, - open: this.open_, - } - })); - } - - private onOpenFoldersChanged_() { - this.open_ = - Boolean(this.openFolders) && this.openFolders.includes(this.folder.id); - } - - private getFocusableRows_(): HTMLElement[] { - return Array.from( - this.shadowRoot!.querySelectorAll('.row, bookmark-folder')); - } - - moveFocus(delta: -1|1): boolean { - const currentFocus = this.shadowRoot!.activeElement; - if (currentFocus instanceof BookmarkFolderElement && - currentFocus.moveFocus(delta)) { - // If focus is already inside a nested folder, delegate the focus to the - // nested folder and return early if successful. - return true; - } - - let moveFocusTo = null; - const focusableRows = this.getFocusableRows_(); - if (currentFocus) { - // If focus is in this folder, move focus to the next or previous - // focusable row. - const currentFocusIndex = - focusableRows.indexOf(currentFocus as HTMLElement); - moveFocusTo = focusableRows[currentFocusIndex + delta]; - } else { - // If focus is not in this folder yet, move focus to either end. - moveFocusTo = delta === 1 ? focusableRows[0] : - focusableRows[focusableRows.length - 1]; - } - - if (moveFocusTo instanceof BookmarkFolderElement) { - return moveFocusTo.moveFocus(delta); - } else if (moveFocusTo) { - moveFocusTo.focus(); - return true; - } else { - return false; - } - } -} - -customElements.define(BookmarkFolderElement.is, BookmarkFolderElement); - -interface DraggableElement extends HTMLElement { - dataBookmark: chrome.bookmarks.BookmarkTreeNode; -} - -export function getBookmarkFromElement(element: HTMLElement) { - return (element as DraggableElement).dataBookmark; -} - -export function isValidDropTarget(element: HTMLElement) { - return element.id === 'folder' || element.classList.contains('bookmark'); -} - -export function isBookmarkFolderElement(element: HTMLElement): boolean { - return element.id === 'folder'; -} \ No newline at end of file diff --git a/browser/resources/sidebar/bookmarks/bookmarks.html b/browser/resources/sidebar/bookmarks/bookmarks.html deleted file mode 100644 index f720ed28842a..000000000000 --- a/browser/resources/sidebar/bookmarks/bookmarks.html +++ /dev/null @@ -1,34 +0,0 @@ - - - - - $i18n{bookmarksTitle} - - - - - - - - - - diff --git a/browser/resources/sidebar/bookmarks/bookmarks_api_proxy.ts b/browser/resources/sidebar/bookmarks/bookmarks_api_proxy.ts deleted file mode 100644 index 322ea97b9ca8..000000000000 --- a/browser/resources/sidebar/bookmarks/bookmarks_api_proxy.ts +++ /dev/null @@ -1,75 +0,0 @@ -// Copyright 2021 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import {ChromeEvent} from '/tools/typescript/definitions/chrome_event.js'; -import {ClickModifiers} from 'chrome://resources/mojo/ui/base/mojom/window_open_disposition.mojom-webui.js'; - -import {BookmarksPageHandlerFactory, BookmarksPageHandlerRemote} from './sidebar.mojom-webui.js'; - -let instance: BookmarksApiProxy|null = null; - -export class BookmarksApiProxy { - callbackRouter: {[key: string]: ChromeEvent}; - handler: BookmarksPageHandlerRemote; - - constructor() { - this.callbackRouter = { - onChanged: chrome.bookmarks.onChanged, - onChildrenReordered: chrome.bookmarks.onChildrenReordered, - onCreated: chrome.bookmarks.onCreated, - onMoved: chrome.bookmarks.onMoved, - onRemoved: chrome.bookmarks.onRemoved, - }; - - this.handler = new BookmarksPageHandlerRemote(); - - const factory = BookmarksPageHandlerFactory.getRemote(); - factory.createBookmarksPageHandler( - this.handler.$.bindNewPipeAndPassReceiver()); - } - - cutBookmark(id: string): Promise { - chrome.bookmarkManagerPrivate.cut([id]); - return Promise.resolve(); - } - - copyBookmark(id: string): Promise { - return new Promise(resolve => { - chrome.bookmarkManagerPrivate.copy([id], resolve); - }); - } - - getFolders(): Promise { - return new Promise(resolve => chrome.bookmarks.getTree(results => { - if (results[0] && results[0].children) { - resolve(results[0].children); - return; - } - resolve([]); - })); - } - - openBookmark(url: string, depth: number, clickModifiers: ClickModifiers) { - this.handler.openBookmark({url}, depth, clickModifiers); - } - - pasteToBookmark(parentId: string, destinationId?: string): Promise { - const destination = destinationId ? [destinationId] : []; - return new Promise(resolve => { - chrome.bookmarkManagerPrivate.paste(parentId, destination, resolve); - }); - } - - showContextMenu(id: string, x: number, y: number) { - this.handler.showContextMenu(id, {x, y}); - } - - static getInstance() { - return instance || (instance = new BookmarksApiProxy()); - } - - static setInstance(obj: BookmarksApiProxy) { - instance = obj; - } -} \ No newline at end of file diff --git a/browser/resources/sidebar/bookmarks/bookmarks_drag_manager.ts b/browser/resources/sidebar/bookmarks/bookmarks_drag_manager.ts deleted file mode 100644 index 04cabff4f163..000000000000 --- a/browser/resources/sidebar/bookmarks/bookmarks_drag_manager.ts +++ /dev/null @@ -1,266 +0,0 @@ -// Copyright 2021 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import '../strings.m.js'; - -import {EventTracker} from 'chrome://resources/js/event_tracker.m.js'; -import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js'; - -import {getBookmarkFromElement, isBookmarkFolderElement, isValidDropTarget} from './bookmark_folder.js'; - -export const DROP_POSITION_ATTR = 'drop-position'; - -const ROOT_FOLDER_ID = '0'; - -// Ms to wait during a dragover to open closed folder. -let folderOpenerTimeoutDelay = 1000; -export function overrideFolderOpenerTimeoutDelay(ms: number) { - folderOpenerTimeoutDelay = ms; -} - -export enum DropPosition { - ABOVE = 'above', - INTO = 'into', - BELOW = 'below', -} - -interface BookmarksDragDelegate extends HTMLElement { - getAscendants(bookmarkId: string): string[]; - getIndex(bookmark: chrome.bookmarks.BookmarkTreeNode): number; - isFolderOpen(bookmark: chrome.bookmarks.BookmarkTreeNode): boolean; - openFolder(folderId: string): void; -} - -class DragSession { - private delegate_: BookmarksDragDelegate; - private dragData_: chrome.bookmarkManagerPrivate.DragData; - private lastDragOverElement_: HTMLElement|null = null; - private lastPointerWasTouch_ = false; - private folderOpenerTimeout_: number|null = null; - - constructor( - delegate: BookmarksDragDelegate, - dragData: chrome.bookmarkManagerPrivate.DragData) { - this.delegate_ = delegate; - this.dragData_ = dragData; - } - - start(e: DragEvent) { - chrome.bookmarkManagerPrivate.startDrag( - this.dragData_.elements!.map(bookmark => bookmark.id), 0, - this.lastPointerWasTouch_, e.clientX, e.clientY); - } - - update(e: DragEvent) { - const dragOverElement = e.composedPath().find(target => { - return target instanceof HTMLElement && isValidDropTarget(target); - }) as HTMLElement; - if (!dragOverElement) { - return; - } - - if (dragOverElement !== this.lastDragOverElement_) { - this.resetState_(); - } - - const dragOverBookmark = getBookmarkFromElement(dragOverElement); - const ascendants = this.delegate_.getAscendants(dragOverBookmark.id); - const isInvalidDragOverTarget = dragOverBookmark.unmodifiable || - this.dragData_.elements && - this.dragData_.elements.some( - element => ascendants.indexOf(element.id) !== -1); - if (isInvalidDragOverTarget) { - this.lastDragOverElement_ = null; - return; - } - - const isDraggingOverFolder = isBookmarkFolderElement(dragOverElement); - const dragOverElRect = dragOverElement.getBoundingClientRect(); - const dragOverYRatio = - (e.clientY - dragOverElRect.top) / dragOverElRect.height; - - let dropPosition; - if (isDraggingOverFolder) { - const folderIsOpen = this.delegate_.isFolderOpen(dragOverBookmark); - if (dragOverBookmark.parentId === ROOT_FOLDER_ID) { - // Cannot drag above or below children of root folder. - dropPosition = DropPosition.INTO; - } else if (dragOverYRatio <= .25) { - dropPosition = DropPosition.ABOVE; - } else if (dragOverYRatio <= .75) { - dropPosition = DropPosition.INTO; - } else if (folderIsOpen) { - // If a folder is open, its child bookmarks appear immediately below it - // so it should not be possible to drop a bookmark right below an open - // folder. - dropPosition = DropPosition.INTO; - } else { - dropPosition = DropPosition.BELOW; - } - } else { - dropPosition = - dragOverYRatio <= .5 ? DropPosition.ABOVE : DropPosition.BELOW; - } - dragOverElement.setAttribute(DROP_POSITION_ATTR, dropPosition); - - if (dropPosition === DropPosition.INTO && - !this.delegate_.isFolderOpen(dragOverBookmark) && - !this.folderOpenerTimeout_) { - // Queue a timeout to auto-open the dragged over folder. - this.folderOpenerTimeout_ = setTimeout(() => { - this.delegate_.openFolder(dragOverBookmark.id); - this.folderOpenerTimeout_ = null; - }, folderOpenerTimeoutDelay); - } - - this.lastDragOverElement_ = dragOverElement; - } - - cancel() { - this.resetState_(); - this.lastDragOverElement_ = null; - } - - finish() { - if (!this.lastDragOverElement_) { - return; - } - - const dropTargetBookmark = - getBookmarkFromElement(this.lastDragOverElement_); - const dropPosition = this.lastDragOverElement_.getAttribute( - DROP_POSITION_ATTR) as DropPosition; - this.resetState_(); - - if (isBookmarkFolderElement(this.lastDragOverElement_) && - dropPosition === DropPosition.INTO) { - chrome.bookmarkManagerPrivate.drop( - dropTargetBookmark.id, /* index */ undefined, - /* callback */ undefined); - return; - } - - let toIndex = this.delegate_.getIndex(dropTargetBookmark); - toIndex += dropPosition === DropPosition.BELOW ? 1 : 0; - chrome.bookmarkManagerPrivate.drop( - dropTargetBookmark.parentId!, toIndex, /* callback */ undefined); - } - - private resetState_() { - if (this.lastDragOverElement_) { - this.lastDragOverElement_.removeAttribute(DROP_POSITION_ATTR); - } - - if (this.folderOpenerTimeout_ !== null) { - clearTimeout(this.folderOpenerTimeout_); - this.folderOpenerTimeout_ = null; - } - } - - static createFromBookmark( - delegate: BookmarksDragDelegate, - bookmark: chrome.bookmarks.BookmarkTreeNode) { - return new DragSession(delegate, { - elements: [bookmark], - sameProfile: true, - }); - } -} - -export class BookmarksDragManager { - private delegate_: BookmarksDragDelegate; - private dragSession_: DragSession|null; - private eventTracker_: EventTracker = new EventTracker(); - - constructor(delegate: BookmarksDragDelegate) { - this.delegate_ = delegate; - } - - startObserving() { - this.eventTracker_.add( - this.delegate_, 'dragstart', e => this.onDragStart_(e as DragEvent)); - this.eventTracker_.add( - this.delegate_, 'dragover', e => this.onDragOver_(e as DragEvent)); - this.eventTracker_.add( - this.delegate_, 'dragleave', () => this.onDragLeave_()); - this.eventTracker_.add(this.delegate_, 'dragend', () => this.cancelDrag_()); - this.eventTracker_.add( - this.delegate_, 'drop', e => this.onDrop_(e as DragEvent)); - - if (loadTimeData.getBoolean('bookmarksDragAndDropEnabled')) { - chrome.bookmarkManagerPrivate.onDragEnter.addListener( - (dragData: chrome.bookmarkManagerPrivate.DragData) => - this.onChromeDragEnter_(dragData)); - chrome.bookmarkManagerPrivate.onDragLeave.addListener( - () => this.cancelDrag_()); - } - } - - stopObserving() { - this.eventTracker_.removeAll(); - } - - private cancelDrag_() { - if (!this.dragSession_) { - return; - } - this.dragSession_.cancel(); - this.dragSession_ = null; - } - - private onChromeDragEnter_(dragData: chrome.bookmarkManagerPrivate.DragData) { - if (this.dragSession_) { - // A drag session is already in flight. - return; - } - - this.dragSession_ = new DragSession(this.delegate_, dragData); - } - - private onDragStart_(e: DragEvent) { - e.preventDefault(); - if (!loadTimeData.getBoolean('bookmarksDragAndDropEnabled')) { - return; - } - - const bookmark = getBookmarkFromElement( - e.composedPath().find(target => (target as HTMLElement).draggable) as - HTMLElement); - if (!bookmark || - /* Cannot drag root's children. */ bookmark.parentId === - ROOT_FOLDER_ID || - bookmark.unmodifiable) { - return; - } - - this.dragSession_ = - DragSession.createFromBookmark(this.delegate_, bookmark); - this.dragSession_.start(e); - } - - private onDragOver_(e: DragEvent) { - e.preventDefault(); - if (!this.dragSession_) { - return; - } - this.dragSession_.update(e); - } - - private onDragLeave_() { - if (!this.dragSession_) { - return; - } - - this.dragSession_.cancel(); - } - - private onDrop_(e: DragEvent) { - if (!this.dragSession_) { - return; - } - - e.preventDefault(); - this.dragSession_.finish(); - } -} \ No newline at end of file diff --git a/browser/resources/sidebar/bookmarks/bookmarks_list.html b/browser/resources/sidebar/bookmarks/bookmarks_list.html deleted file mode 100644 index e24c1622e104..000000000000 --- a/browser/resources/sidebar/bookmarks/bookmarks_list.html +++ /dev/null @@ -1,4 +0,0 @@ - \ No newline at end of file diff --git a/browser/resources/sidebar/bookmarks/bookmarks_list.ts b/browser/resources/sidebar/bookmarks/bookmarks_list.ts deleted file mode 100644 index efb0123bc58d..000000000000 --- a/browser/resources/sidebar/bookmarks/bookmarks_list.ts +++ /dev/null @@ -1,314 +0,0 @@ -// Copyright 2021 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js'; - -import {BookmarkFolderElement, FOLDER_OPEN_CHANGED_EVENT, getBookmarkFromElement, isBookmarkFolderElement} from './bookmark_folder.js'; -import {BookmarksApiProxy} from './bookmarks_api_proxy.js'; -import {BookmarksDragManager} from './bookmarks_drag_manager.js'; - -// Key for localStorage object that refers to all the open folders. -export const LOCAL_STORAGE_OPEN_FOLDERS_KEY = 'openFolders'; - -export class BookmarksListElement extends PolymerElement { - static get is() { - return 'bookmarks-list'; - } - - static get template() { - return html`{__html_template__}`; - } - - static get properties() { - return { - folders_: { - type: Array, - value: () => [], - }, - - openFolders_: { - type: Array, - value: () => [], - }, - }; - } - - private bookmarksApi_: BookmarksApiProxy = BookmarksApiProxy.getInstance(); - private bookmarksDragManager_: BookmarksDragManager = - new BookmarksDragManager(this); - private listeners_ = new Map(); - private folders_: chrome.bookmarks.BookmarkTreeNode[]; - private openFolders_: string[]; - - override ready() { - super.ready(); - this.addEventListener( - FOLDER_OPEN_CHANGED_EVENT, - e => this.onFolderOpenChanged_( - e as CustomEvent<{id: string, open: boolean}>)); - this.addEventListener('keydown', e => this.onKeydown_(e)); - } - - override connectedCallback() { - super.connectedCallback(); - this.setAttribute('role', 'tree'); - this.bookmarksApi_.getFolders().then(folders => { - this.folders_ = folders; - - this.addListener_( - 'onChildrenReordered', - (id: string, reorderedInfo: chrome.bookmarks.ReorderInfo) => - this.onChildrenReordered_(id, reorderedInfo)); - this.addListener_( - 'onChanged', - (id: string, changedInfo: chrome.bookmarks.ChangeInfo) => - this.onChanged_(id, changedInfo)); - this.addListener_( - 'onCreated', - (_id: string, node: chrome.bookmarks.BookmarkTreeNode) => - this.onCreated_(node)); - this.addListener_( - 'onMoved', - (_id: string, movedInfo: chrome.bookmarks.MoveInfo) => - this.onMoved_(movedInfo)); - this.addListener_('onRemoved', (id: string) => this.onRemoved_(id)); - - try { - const openFolders = window.localStorage[LOCAL_STORAGE_OPEN_FOLDERS_KEY]; - this.openFolders_ = JSON.parse(openFolders); - } catch (error) { - this.openFolders_ = [this.folders_[0]!.id]; - window.localStorage[LOCAL_STORAGE_OPEN_FOLDERS_KEY] = - JSON.stringify(this.openFolders_); - } - - this.bookmarksDragManager_.startObserving(); - }); - } - - override disconnectedCallback() { - for (const [eventName, callback] of this.listeners_.entries()) { - this.bookmarksApi_.callbackRouter[eventName]!.removeListener(callback); - } - this.bookmarksDragManager_.stopObserving(); - } - - /** BookmarksDragDelegate */ - getAscendants(bookmarkId: string): string[] { - const path = this.findPathToId_(bookmarkId); - return path.map(bookmark => bookmark.id); - } - - /** BookmarksDragDelegate */ - getIndex(bookmark: chrome.bookmarks.BookmarkTreeNode): number { - const path = this.findPathToId_(bookmark.id); - const parent = path[path.length - 2]; - if (!parent || !parent.children) { - return -1; - } - return parent.children.findIndex((child) => child.id === bookmark.id); - } - /** BookmarksDragDelegate */ - isFolderOpen(bookmark: chrome.bookmarks.BookmarkTreeNode): boolean { - return this.openFolders_.some(id => bookmark.id === id); - } - - /** BookmarksDragDelegate */ - openFolder(folderId: string) { - this.changeFolderOpenStatus_(folderId, true); - } - - private addListener_(eventName: string, callback: Function): void { - this.bookmarksApi_.callbackRouter[eventName]!.addListener(callback); - this.listeners_.set(eventName, callback); - } - - /** - * Finds the node within the nested array of folders and returns the path to - * the node in the tree. - */ - private findPathToId_(id: string): chrome.bookmarks.BookmarkTreeNode[] { - const path: chrome.bookmarks.BookmarkTreeNode[] = []; - - function findPathByIdInternal_( - id: string, node: chrome.bookmarks.BookmarkTreeNode) { - if (node.id === id) { - path.push(node); - return true; - } - - if (!node.children) { - return false; - } - - path.push(node); - const foundInChildren = - node.children.some(child => findPathByIdInternal_(id, child)); - if (!foundInChildren) { - path.pop(); - } - - return foundInChildren; - } - - this.folders_.some(folder => findPathByIdInternal_(id, folder)); - return path; - } - - /** - * Reduces an array of nodes to a string to notify Polymer of changes to the - * nested array. - */ - private getPathString_(path: chrome.bookmarks.BookmarkTreeNode[]): string { - return path.reduce((reducedString, pathItem, index) => { - if (index === 0) { - return `folders_.${this.folders_.indexOf(pathItem)}`; - } - - const parent = path[index - 1]; - return `${reducedString}.children.${parent!.children!.indexOf(pathItem)}`; - }, ''); - } - - private onChanged_(id: string, changedInfo: chrome.bookmarks.ChangeInfo) { - const path = this.findPathToId_(id); - Object.assign(path[path.length - 1], changedInfo); - - const pathString = this.getPathString_(path); - Object.keys(changedInfo) - .forEach(key => this.notifyPath(`${pathString}.${key}`)); - } - - private onChildrenReordered_( - id: string, reorderedInfo: chrome.bookmarks.ReorderInfo) { - const path = this.findPathToId_(id); - const parent = path[path.length - 1]; - const childById = parent!.children!.reduce((map, node) => { - map.set(node.id, node); - return map; - }, new Map()); - parent!.children = reorderedInfo.childIds.map(id => childById.get(id)); - const pathString = this.getPathString_(path); - this.notifyPath(`${pathString}.children`); - } - - private onCreated_(node: chrome.bookmarks.BookmarkTreeNode) { - const pathToParent = this.findPathToId_(node.parentId as string); - const pathToParentString = this.getPathString_(pathToParent); - const parent = pathToParent[pathToParent.length - 1]; - if (parent && !parent.children) { - // Newly created folders in this session may not have an array of - // children yet, so create an empty one. - parent.children = []; - } - this.splice(`${pathToParentString}.children`, node.index!, 0, node); - } - - private changeFolderOpenStatus_(id: string, open: boolean) { - const alreadyOpenIndex = this.openFolders_.indexOf(id); - if (open && alreadyOpenIndex === -1) { - this.openFolders_.push(id); - } else if (!open) { - this.openFolders_.splice(alreadyOpenIndex, 1); - } - - // Assign to a new array so that listeners are triggered. - this.openFolders_ = [...this.openFolders_]; - window.localStorage[LOCAL_STORAGE_OPEN_FOLDERS_KEY] = - JSON.stringify(this.openFolders_); - } - - private onFolderOpenChanged_(event: CustomEvent) { - const {id, open} = event.detail; - this.changeFolderOpenStatus_(id, open); - } - - private onKeydown_(event: KeyboardEvent) { - if (['ArrowDown', 'ArrowUp'].includes(event.key)) { - this.handleArrowKeyNavigation_(event); - return; - } - - if (!event.ctrlKey && !event.metaKey) { - return; - } - - event.preventDefault(); - const eventTarget = event.composedPath()[0] as HTMLElement; - const bookmarkData = getBookmarkFromElement(eventTarget); - if (!bookmarkData) { - return; - } - - if (event.key === 'x') { - this.bookmarksApi_.cutBookmark(bookmarkData.id); - } else if (event.key === 'c') { - this.bookmarksApi_.copyBookmark(bookmarkData.id); - } else if (event.key === 'v') { - if (isBookmarkFolderElement(eventTarget)) { - this.bookmarksApi_.pasteToBookmark(bookmarkData.id); - } else { - this.bookmarksApi_.pasteToBookmark( - bookmarkData.parentId!, bookmarkData.id); - } - } - } - - private handleArrowKeyNavigation_(event: KeyboardEvent) { - if (!(this.shadowRoot!.activeElement instanceof BookmarkFolderElement)) { - // If the key event did not happen within a BookmarkFolderElement, do - // not do anything. - return; - } - - // Prevent arrow keys from causing scroll. - event.preventDefault(); - - const allFolderElements: BookmarkFolderElement[] = - Array.from(this.shadowRoot!.querySelectorAll('bookmark-folder')); - - const delta = event.key === 'ArrowUp' ? -1 : 1; - let currentIndex = - allFolderElements.indexOf(this.shadowRoot!.activeElement); - let focusHasMoved = false; - while (!focusHasMoved) { - focusHasMoved = allFolderElements[currentIndex]!.moveFocus(delta); - currentIndex = (currentIndex + delta + allFolderElements.length) % - allFolderElements.length; - } - } - - private onMoved_(movedInfo: chrome.bookmarks.MoveInfo) { - // Get old path and remove node from oldParent at oldIndex. - const oldParentPath = this.findPathToId_(movedInfo.oldParentId); - const oldParentPathString = this.getPathString_(oldParentPath); - const oldParent = oldParentPath[oldParentPath.length - 1]; - const movedNode = oldParent!.children![movedInfo.oldIndex]; - Object.assign( - movedNode, {index: movedInfo.index, parentId: movedInfo.parentId}); - this.splice(`${oldParentPathString}.children`, movedInfo.oldIndex, 1); - - // Get new parent's path and add the node to the new parent at index. - const newParentPath = this.findPathToId_(movedInfo.parentId); - const newParentPathString = this.getPathString_(newParentPath); - const newParent = newParentPath[newParentPath.length - 1]; - if (newParent && !newParent.children) { - newParent.children = []; - } - this.splice( - `${newParentPathString}.children`, movedInfo.index, 0, movedNode); - } - - private onRemoved_(id: string) { - const oldPath = this.findPathToId_(id); - const removedNode = oldPath.pop()!; - const oldParent = oldPath[oldPath.length - 1]!; - const oldParentPathString = this.getPathString_(oldPath); - this.splice( - `${oldParentPathString}.children`, - oldParent.children!.indexOf(removedNode), 1); - } -} - -customElements.define(BookmarksListElement.is, BookmarksListElement); diff --git a/browser/resources/sidebar/bookmarks/tsconfig_base.json b/browser/resources/sidebar/bookmarks/tsconfig_base.json deleted file mode 100644 index 7ceb2c0b2e52..000000000000 --- a/browser/resources/sidebar/bookmarks/tsconfig_base.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "extends": "../../../../../tools/typescript/tsconfig_base.json", - "compilerOptions": { - "allowJs": true, - "noPropertyAccessFromIndexSignature": false, - "noUnusedLocals": false, - "strictPropertyInitialization": false - } -} diff --git a/browser/ui/BUILD.gn b/browser/ui/BUILD.gn index a4a78f48fe44..0d2e8557c637 100644 --- a/browser/ui/BUILD.gn +++ b/browser/ui/BUILD.gn @@ -202,6 +202,8 @@ source_set("ui") { "views/frame/brave_browser_frame.h", "views/frame/brave_browser_view.cc", "views/frame/brave_browser_view.h", + "views/frame/brave_browser_view_layout.cc", + "views/frame/brave_browser_view_layout.h", "views/frame/brave_opaque_browser_frame_view.cc", "views/frame/brave_opaque_browser_frame_view.h", "views/frame/brave_window_frame_graphic.cc", @@ -212,6 +214,8 @@ source_set("ui") { "views/omnibox/brave_search_conversion_promotion_view.h", "views/rounded_separator.cc", "views/rounded_separator.h", + "views/side_panel/brave_side_panel.cc", + "views/side_panel/brave_side_panel.h", "views/tabs/brave_browser_tab_strip_controller.cc", "views/tabs/brave_browser_tab_strip_controller.h", "views/tabs/brave_tab.cc", @@ -416,16 +420,10 @@ source_set("ui") { if (enable_sidebar) { deps += [ - "//brave/browser/resources/sidebar:resources", "//brave/browser/ui/sidebar", - "//brave/browser/ui/webui/sidebar:mojo_bindings", "//brave/components/sidebar", "//chrome/app:generated_resources", - "//components/bookmarks/browser", "//components/favicon_base", - "//mojo/public/cpp/bindings", - "//ui/base/mojom", - "//ui/webui", ] sources += [ @@ -455,14 +453,10 @@ source_set("ui") { "views/sidebar/sidebar_items_contents_view.h", "views/sidebar/sidebar_items_scroll_view.cc", "views/sidebar/sidebar_items_scroll_view.h", - "views/sidebar/sidebar_panel_webview.cc", - "views/sidebar/sidebar_panel_webview.h", "views/sidebar/sidebar_show_options_event_detect_widget.cc", "views/sidebar/sidebar_show_options_event_detect_widget.h", - "webui/sidebar/sidebar_bookmarks_page_handler.cc", - "webui/sidebar/sidebar_bookmarks_page_handler.h", - "webui/sidebar/sidebar_bookmarks_ui.cc", - "webui/sidebar/sidebar_bookmarks_ui.h", + "views/sidebar/sidebar_side_panel_utils.cc", + "views/sidebar/sidebar_side_panel_utils.h", ] } diff --git a/browser/ui/sidebar/BUILD.gn b/browser/ui/sidebar/BUILD.gn index 476f894773b4..baa7183ebea4 100644 --- a/browser/ui/sidebar/BUILD.gn +++ b/browser/ui/sidebar/BUILD.gn @@ -16,14 +16,10 @@ source_set("sidebar") { "sidebar_controller.h", "sidebar_model.cc", "sidebar_model.h", - "sidebar_model_data.cc", - "sidebar_model_data.h", "sidebar_service_factory.cc", "sidebar_service_factory.h", "sidebar_utils.cc", "sidebar_utils.h", - "sidebar_web_contents_delegate.cc", - "sidebar_web_contents_delegate.h", ] deps = [ diff --git a/browser/ui/sidebar/sidebar.h b/browser/ui/sidebar/sidebar.h index 209d3367305a..65f789d67354 100644 --- a/browser/ui/sidebar/sidebar.h +++ b/browser/ui/sidebar/sidebar.h @@ -32,13 +32,6 @@ class Sidebar { SidebarService::ShowSidebarOption show_option) = 0; // Update current sidebar UI. virtual void UpdateSidebar() = 0; - virtual void ShowCustomContextMenu( - const gfx::Point& point, - std::unique_ptr menu_model) = 0; - virtual void HideCustomContextMenu() = 0; - virtual bool HandleKeyboardEvent( - content::WebContents* source, - const content::NativeWebKeyboardEvent& event) = 0; protected: virtual ~Sidebar() {} diff --git a/browser/ui/sidebar/sidebar_browsertest.cc b/browser/ui/sidebar/sidebar_browsertest.cc index 490d25b5809f..8bd61a3445b5 100644 --- a/browser/ui/sidebar/sidebar_browsertest.cc +++ b/browser/ui/sidebar/sidebar_browsertest.cc @@ -78,8 +78,8 @@ IN_PROC_BROWSER_TEST_F(SidebarBrowserTest, BasicTest) { // Check sidebar UI is initalized properly. EXPECT_TRUE(!!controller()->sidebar()); - // Currently we have 3 default items. - EXPECT_EQ(3UL, model()->GetAllSidebarItems().size()); + // Currently we have 4 default items. + EXPECT_EQ(4UL, model()->GetAllSidebarItems().size()); // Activate item that opens in panel. controller()->ActivateItemAt(2); EXPECT_EQ(2, model()->active_index()); @@ -99,19 +99,9 @@ IN_PROC_BROWSER_TEST_F(SidebarBrowserTest, BasicTest) { controller()->ActivateItemAt(2); - // Sidebar items at 2, 3 are opened in panel. - // Check their webcontents are sidebar webcontents. - EXPECT_TRUE(model()->IsSidebarWebContents(model()->GetWebContentsAt(2))); - EXPECT_FALSE( - model()->IsSidebarWebContents(tab_model()->GetActiveWebContents())); - EXPECT_EQ(browser(), - chrome::FindBrowserWithWebContents(model()->GetWebContentsAt(2))); - EXPECT_EQ(browser(), chrome::FindBrowserWithWebContents( - tab_model()->GetActiveWebContents())); - // Remove Item at index 0 change active index from 3 to 2. SidebarServiceFactory::GetForProfile(browser()->profile())->RemoveItemAt(0); - EXPECT_EQ(2UL, model()->GetAllSidebarItems().size()); + EXPECT_EQ(3UL, model()->GetAllSidebarItems().size()); EXPECT_EQ(1, model()->active_index()); // If current active tab is not NTP, we can add current url to sidebar. @@ -130,31 +120,31 @@ IN_PROC_BROWSER_TEST_F(SidebarBrowserTest, BasicTest) { } IN_PROC_BROWSER_TEST_F(SidebarBrowserTest, WebTypePanelTest) { - // By default, sidebar has 3 items. - EXPECT_EQ(3UL, model()->GetAllSidebarItems().size()); - ASSERT_TRUE( - ui_test_utils::NavigateToURL(browser(), GURL("brave://settings/"))); - - EXPECT_TRUE(CanAddCurrentActiveTabToSidebar(browser())); - controller()->AddItemWithCurrentTab(); - // have 4 items. + // By default, sidebar has 4 items. EXPECT_EQ(4UL, model()->GetAllSidebarItems().size()); + // Add an item + ASSERT_TRUE( + ui_test_utils::NavigateToURL(browser(), GURL("brave://settings/"))); int current_tab_index = tab_model()->active_index(); EXPECT_EQ(0, current_tab_index); + EXPECT_TRUE(CanAddCurrentActiveTabToSidebar(browser())); + controller()->AddItemWithCurrentTab(); + // Verify new size + EXPECT_EQ(5UL, model()->GetAllSidebarItems().size()); - // Load NTP in newtab and activate it. (tab index 1) + // Load NTP in a new tab and activate it. (tab index 1) ASSERT_TRUE(ui_test_utils::NavigateToURLWithDisposition( browser(), GURL("brave://newtab/"), WindowOpenDisposition::NEW_FOREGROUND_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_LOAD_STOP)); current_tab_index = tab_model()->active_index(); - EXPECT_EQ(1, tab_model()->active_index()); + EXPECT_EQ(1, current_tab_index); // Activate sidebar item(brave://settings) and check existing first tab is // activated. - auto item = model()->GetAllSidebarItems()[3]; - controller()->ActivateItemAt(3); + auto item = model()->GetAllSidebarItems()[4]; + controller()->ActivateItemAt(4); EXPECT_EQ(0, tab_model()->active_index()); EXPECT_EQ(tab_model()->GetWebContentsAt(0)->GetVisibleURL(), item.url); @@ -167,16 +157,6 @@ IN_PROC_BROWSER_TEST_F(SidebarBrowserTest, WebTypePanelTest) { EXPECT_EQ(2, tab_model()->count()); } -IN_PROC_BROWSER_TEST_F(SidebarBrowserTest, - FindBrowserWorksWithoutSidebarController) { - NavigateParams navigate_params(browser(), GURL("brave://newtab/"), - ui::PAGE_TRANSITION_TYPED); - navigate_params.disposition = WindowOpenDisposition::NEW_POPUP; - ui_test_utils::NavigateToURL(&navigate_params); - EXPECT_TRUE(chrome::FindBrowserWithWebContents( - navigate_params.navigated_or_inserted_contents)); -} - IN_PROC_BROWSER_TEST_F(SidebarBrowserTest, IterateBuiltInWebTypeTest) { // Click builtin wallet item and it's loaded at current active tab. auto item = model()->GetAllSidebarItems()[1]; diff --git a/browser/ui/sidebar/sidebar_controller.cc b/browser/ui/sidebar/sidebar_controller.cc index 52a9c6630768..b8ffb73dc798 100644 --- a/browser/ui/sidebar/sidebar_controller.cc +++ b/browser/ui/sidebar/sidebar_controller.cc @@ -10,7 +10,6 @@ #include "brave/browser/ui/brave_browser.h" #include "brave/browser/ui/sidebar/sidebar.h" #include "brave/browser/ui/sidebar/sidebar_model.h" -#include "brave/browser/ui/sidebar/sidebar_model_data.h" #include "brave/browser/ui/sidebar/sidebar_service_factory.h" #include "brave/browser/ui/sidebar/sidebar_utils.h" #include "brave/components/sidebar/sidebar_service.h" @@ -67,6 +66,7 @@ void SidebarController::ActivateItemAt(int index, DCHECK_GE(index, -1); if (index == -1) { sidebar_model_->SetActiveIndex(index); + UpdateSidebarVisibility(); return; } @@ -74,6 +74,7 @@ void SidebarController::ActivateItemAt(int index, // Only an item for panel can get activated. if (item.open_in_panel) { sidebar_model_->SetActiveIndex(index); + UpdateSidebarVisibility(); return; } @@ -134,8 +135,6 @@ void SidebarController::LoadAtTab(const GURL& url) { void SidebarController::OnShowSidebarOptionChanged( SidebarService::ShowSidebarOption option) { - // Clear active state whenever sidebar enabled state is changed. - ActivateItemAt(-1); UpdateSidebarVisibility(); } diff --git a/browser/ui/sidebar/sidebar_model.cc b/browser/ui/sidebar/sidebar_model.cc index 3e12fdb6904b..79388b9996db 100644 --- a/browser/ui/sidebar/sidebar_model.cc +++ b/browser/ui/sidebar/sidebar_model.cc @@ -9,8 +9,8 @@ #include #include "base/logging.h" +#include "base/ranges/algorithm.h" #include "base/time/time.h" -#include "brave/browser/ui/sidebar/sidebar_model_data.h" #include "brave/browser/ui/sidebar/sidebar_service_factory.h" #include "brave/components/sidebar/sidebar_item.h" #include "chrome/browser/favicon/favicon_service_factory.h" @@ -63,9 +63,11 @@ SidebarModel::~SidebarModel() = default; void SidebarModel::Init(history::HistoryService* history_service) { // Start with saved item list. - for (const auto& item : GetAllSidebarItems()) - AddItem(item, -1, false); - + int index = -1; + for (const auto& item : GetAllSidebarItems()) { + index++; + AddItem(item, index, false); + } sidebar_observed_.Observe(GetSidebarService(profile_)); // Can be null in test. if (history_service) @@ -83,18 +85,14 @@ void SidebarModel::RemoveObserver(Observer* observer) { void SidebarModel::AddItem(const SidebarItem& item, int index, bool user_gesture) { - if (index == -1) { - data_.push_back(std::make_unique(profile_)); - } else { - data_.insert(data_.begin() + index, - std::make_unique(profile_)); - } + // Sidebar service should always call with a valid index equal to the + // index of the SidebarItem. + DCHECK_GE(index, 0); for (Observer& obs : observers_) { - // Index starts at zero. If |index| is -1, add as a last item. - obs.OnItemAdded(item, index == -1 ? data_.size() - 1 : index, user_gesture); + obs.OnItemAdded(item, index, user_gesture); } - // If active_index_ is not -1, check this addition affetcs active index. + // If active_index_ is not -1, check this addition affects active index. if (active_index_ != -1 && active_index_ >= index) UpdateActiveIndexAndNotify(index); @@ -108,32 +106,27 @@ void SidebarModel::OnItemAdded(const SidebarItem& item, int index) { } void SidebarModel::OnItemMoved(const SidebarItem& item, int from, int to) { - // Cache active model data to find its new index after moving. - SidebarModelData* active_data = nullptr; - if (active_index_ != -1) { - active_data = data_[active_index_].get(); - } - - std::unique_ptr data = std::move(data_[from]); - data_.erase(data_.begin() + from); - data_.insert(data_.begin() + to, std::move(data)); - for (Observer& obs : observers_) obs.OnItemMoved(item, from, to); - if (!active_data) + if (active_index_ == -1) return; // Find new active items index. - const int data_size = data_.size(); - for (int i = 0; i < data_size; ++i) { - if (data_[i].get() == active_data) { - UpdateActiveIndexAndNotify(i); - return; - } + const bool active_index_is_unaffected = + ((active_index_ > from && active_index_ > to) || + (active_index_ < from && active_index_ < to)); + if (active_index_is_unaffected) { + return; } - - NOTREACHED(); + int new_active_index = -1; + if (active_index_ == from) { + new_active_index = to; + } else { + new_active_index = (to < from) ? active_index_ + 1 : active_index_ - 1; + } + DCHECK_GE(new_active_index, 0); + UpdateActiveIndexAndNotify(new_active_index); } void SidebarModel::OnWillRemoveItem(const SidebarItem& item, int index) { @@ -170,7 +163,6 @@ void SidebarModel::OnURLVisited(history::HistoryService* history_service, } void SidebarModel::RemoveItemAt(int index) { - data_.erase(data_.begin() + index); for (Observer& obs : observers_) obs.OnItemRemoved(index); @@ -184,64 +176,29 @@ void SidebarModel::SetActiveIndex(int index, bool load) { if (index == active_index_) return; - // Don't load url if it's already loaded. If not, new loading is started - // whenever item is activated. - // TODO(simonhong): Maybe we should have reload option? - if (load && index != -1 && !IsLoadedAt(index)) - LoadURLAt(GetAllSidebarItems()[index].url, index); - UpdateActiveIndexAndNotify(index); } -content::WebContents* SidebarModel::GetWebContentsAt(int index) { - // Only webcontents is requested for items that opens in panel. - // Opens in new tab doesn't need to get webcontents here. - DCHECK(GetAllSidebarItems()[index].open_in_panel); - - return data_[index]->GetWebContents(); -} - -bool SidebarModel::IsSidebarWebContents( - const content::WebContents* web_contents) const { - for (const auto& data : data_) { - if (data->web_contents() && data->web_contents() == web_contents) - return true; - } - - return false; -} - -const std::vector SidebarModel::GetAllSidebarItems() const { +const std::vector& SidebarModel::GetAllSidebarItems() const { return GetSidebarService(profile_)->items(); } -bool SidebarModel::IsLoadedAt(int index) const { - DCHECK(GetAllSidebarItems()[index].open_in_panel); - - return data_[index]->IsLoaded(); -} - bool SidebarModel::IsSidebarHasAllBuiltiInItems() const { return GetSidebarService(profile_)->GetHiddenDefaultSidebarItems().empty(); } int SidebarModel::GetIndexOf(const SidebarItem& item) const { const auto items = GetAllSidebarItems(); - const auto iter = - std::find_if(items.begin(), items.end(), - [item](const auto& i) { return item.url == i.url; }); + const auto iter = base::ranges::find_if(items, [&item](const auto& i) { + return (item.built_in_item_type == i.built_in_item_type && + item.url == i.url); + }); if (iter == items.end()) return -1; return std::distance(items.begin(), iter); } -void SidebarModel::LoadURLAt(const GURL& url, int index) { - DCHECK(GetAllSidebarItems()[index].open_in_panel); - - data_[index]->LoadURL(url); -} - void SidebarModel::UpdateActiveIndexAndNotify(int new_active_index) { if (new_active_index == active_index_) return; diff --git a/browser/ui/sidebar/sidebar_model.h b/browser/ui/sidebar/sidebar_model.h index cc245662cb74..73df5d6d55f9 100644 --- a/browser/ui/sidebar/sidebar_model.h +++ b/browser/ui/sidebar/sidebar_model.h @@ -13,7 +13,6 @@ #include "base/observer_list.h" #include "base/observer_list_types.h" #include "base/scoped_observation.h" -#include "brave/browser/ui/sidebar/sidebar_model_data.h" #include "brave/components/sidebar/sidebar_service.h" #include "components/history/core/browser/history_service.h" #include "components/history/core/browser/history_service_observer.h" @@ -44,9 +43,7 @@ class Profile; namespace sidebar { -class SidebarModelData; - -// Manage sidebar's runtime state. +// Manage sidebar's runtime state for active index and icons. // Each browser window has different runtime state. // Observe SidebarService to get item add/deletion notification. class SidebarModel : public SidebarService::Observer, @@ -81,15 +78,9 @@ class SidebarModel : public SidebarService::Observer, // |false| is used in unit test. void SetActiveIndex(int index, bool load = true); // Returns true if webcontents of item at |index| already loaded url. - bool IsLoadedAt(int index) const; bool IsSidebarHasAllBuiltiInItems() const; int GetIndexOf(const SidebarItem& item) const; - // Don't cache web_contents. It can be deleted during the runtime. - content::WebContents* GetWebContentsAt(int index); - // Returns true when |web_contents| is used by sidebar panel. - bool IsSidebarWebContents(const content::WebContents* web_contents) const; - // Don't cache item list. list can be changed during the runtime. const std::vector GetAllSidebarItems() const; @@ -115,7 +106,6 @@ class SidebarModel : public SidebarService::Observer, void AddItem(const SidebarItem& item, int index, bool user_gesture); void RemoveItemAt(int index); void UpdateActiveIndexAndNotify(int new_active_index); - void LoadURLAt(const GURL& url, int index); // TODO(simonhong): Use separated class for fetching favicon from this model // class. @@ -135,7 +125,6 @@ class SidebarModel : public SidebarService::Observer, Profile* profile_ = nullptr; std::unique_ptr task_tracker_; base::ObserverList observers_; - std::vector> data_; base::ScopedObservation sidebar_observed_{this}; base::ScopedObservation diff --git a/browser/ui/sidebar/sidebar_model_data.cc b/browser/ui/sidebar/sidebar_model_data.cc deleted file mode 100644 index 5aae474a2a5f..000000000000 --- a/browser/ui/sidebar/sidebar_model_data.cc +++ /dev/null @@ -1,74 +0,0 @@ -/* Copyright (c) 2021 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/sidebar/sidebar_model_data.h" - -#include - -#include "brave/browser/ui/sidebar/sidebar_web_contents_delegate.h" -#include "chrome/browser/profiles/profile.h" -#include "content/public/browser/navigation_controller.h" -#include "content/public/browser/web_contents.h" -#include "content/public/common/url_constants.h" -#include "extensions/buildflags/buildflags.h" -#include "url/gurl.h" - -#if BUILDFLAG(ENABLE_EXTENSIONS) -#include "chrome/browser/extensions/tab_helper.h" -#include "extensions/browser/view_type_utils.h" -#include "extensions/common/mojom/view_type.mojom.h" -#endif - -namespace sidebar { - -namespace { - -void AttachTabHelpersForSidebar(content::WebContents* contents) { -#if BUILDFLAG(ENABLE_EXTENSIONS) - extensions::SetViewType(contents, extensions::mojom::ViewType::kTabContents); - extensions::TabHelper::CreateForWebContents(contents); -#endif -} - -} // namespace - -SidebarModelData::SidebarModelData(Profile* profile) : profile_(profile) {} - -SidebarModelData::~SidebarModelData() { - // Make sure to destroy contents first because contents refers delegate. - contents_.reset(); - contents_delegate_.reset(); -} - -content::WebContents* SidebarModelData::GetWebContents() { - if (!contents_) { - content::WebContents::CreateParams params(profile_); - contents_ = content::WebContents::Create(params); - contents_delegate_.reset(new SidebarWebContentsDelegate); - contents_->SetDelegate(contents_delegate_.get()); - - AttachTabHelpersForSidebar(contents_.get()); - } - - return contents_.get(); -} - -void SidebarModelData::LoadURL(const GURL& url) { - // Only internal webui can be loaded in sidebar panel now. - // Revisit when we want to load any url in panel. - DCHECK(url.SchemeIs(content::kChromeUIScheme)); - GetWebContents()->GetController().LoadURL(url, content::Referrer(), - ui::PAGE_TRANSITION_AUTO_TOPLEVEL, - std::string()); -} - -bool SidebarModelData::IsLoaded() const { - if (!contents_) - return false; - - return !contents_->GetVisibleURL().is_empty(); -} - -} // namespace sidebar diff --git a/browser/ui/sidebar/sidebar_model_data.h b/browser/ui/sidebar/sidebar_model_data.h deleted file mode 100644 index c36bf2df4c04..000000000000 --- a/browser/ui/sidebar/sidebar_model_data.h +++ /dev/null @@ -1,48 +0,0 @@ -/* Copyright (c) 2021 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_SIDEBAR_SIDEBAR_MODEL_DATA_H_ -#define BRAVE_BROWSER_UI_SIDEBAR_SIDEBAR_MODEL_DATA_H_ - -#include - -class GURL; -class Profile; - -namespace content { -class WebContents; -} // namespace content - -namespace sidebar { - -class SidebarWebContentsDelegate; - -// SidebarModelData represents sidebar each item's runtime state. -// Each sidebar item of built in type owns WebContents. -class SidebarModelData { - public: - explicit SidebarModelData(Profile* profile); - virtual ~SidebarModelData(); - - SidebarModelData(const SidebarModelData&) = delete; - SidebarModelData& operator=(const SidebarModelData&) = delete; - - content::WebContents* GetWebContents(); - - // nullptr if GetWebContents() is not called yet. - content::WebContents* web_contents() const { return contents_.get(); } - - void LoadURL(const GURL& url); - bool IsLoaded() const; - - private: - Profile* profile_ = nullptr; - std::unique_ptr contents_delegate_; - std::unique_ptr contents_; -}; - -} // namespace sidebar - -#endif // BRAVE_BROWSER_UI_SIDEBAR_SIDEBAR_MODEL_DATA_H_ diff --git a/browser/ui/sidebar/sidebar_unittest.cc b/browser/ui/sidebar/sidebar_unittest.cc index 86c47e941357..6f089c2050c0 100644 --- a/browser/ui/sidebar/sidebar_unittest.cc +++ b/browser/ui/sidebar/sidebar_unittest.cc @@ -64,26 +64,25 @@ class SidebarModelTest : public testing::Test, public SidebarModel::Observer { }; TEST_F(SidebarModelTest, ItemsChangedTest) { - EXPECT_EQ(0UL, model()->data_.size()); + auto built_in_items_size = + SidebarService::GetDefaultBuiltInItemTypes_ForTesting().size(); + EXPECT_EQ(built_in_items_size, service()->items().size()); model()->Init(nullptr); - EXPECT_EQ(service()->items().size(), model()->data_.size()); EXPECT_EQ(-1, model()->active_index()); - // Add one more item to test with 4 items. + // Add one more item to test with 5 items. SidebarItem new_item; new_item.url = GURL("https://brave.com"); service()->AddItem(new_item); - EXPECT_EQ(4UL, service()->items().size()); - EXPECT_EQ(4UL, model()->data_.size()); - EXPECT_EQ(service()->items().size(), model()->data_.size()); + EXPECT_EQ(built_in_items_size + 1, service()->items().size()); // Move item at 1 to at index 2. // Total size and active index is not changed when currently active index is // -1. - const size_t model_size = model()->data_.size(); + const size_t items_size = service()->items().size(); // Cache data at index 1. - auto* model_data = model()->data_[1].get(); + const auto item_data = service()->items()[1]; EXPECT_FALSE(on_item_moved_called_); EXPECT_FALSE(on_active_index_changed_called_); @@ -91,9 +90,12 @@ TEST_F(SidebarModelTest, ItemsChangedTest) { EXPECT_TRUE(on_item_moved_called_); EXPECT_FALSE(on_active_index_changed_called_); - EXPECT_EQ(model_data, model()->data_[2].get()); + EXPECT_EQ(item_data.built_in_item_type, + service()->items()[2].built_in_item_type); + EXPECT_EQ(item_data.url, service()->items()[2].url); + EXPECT_EQ(item_data.title, service()->items()[2].title); EXPECT_EQ(-1, model()->active_index()); - EXPECT_EQ(model_size, model()->data_.size()); + EXPECT_EQ(items_size, service()->items().size()); model()->SetActiveIndex(1, false); EXPECT_EQ(1, model()->active_index()); @@ -140,8 +142,6 @@ TEST(SidebarUtilTest, ConvertURLToBuiltInItemURLTest) { EXPECT_EQ(GURL(kBraveTalkURL), ConvertURLToBuiltInItemURL( GURL("https://talk.brave.com/1Ar1vHfLBWX2sAdi"))); - EXPECT_EQ(GURL(kSidebarBookmarksURL), - ConvertURLToBuiltInItemURL(GURL(chrome::kChromeUIBookmarksURL))); EXPECT_EQ( GURL(kBraveUIWalletPageURL), ConvertURLToBuiltInItemURL(GURL("chrome://wallet/crypto/onboarding"))); diff --git a/browser/ui/sidebar/sidebar_utils.cc b/browser/ui/sidebar/sidebar_utils.cc index b414e6785958..b006de984fe1 100644 --- a/browser/ui/sidebar/sidebar_utils.cc +++ b/browser/ui/sidebar/sidebar_utils.cc @@ -75,7 +75,7 @@ bool CanUseSidebar(Browser* browser) { // if sidebar panel already has bookmarks item. GURL ConvertURLToBuiltInItemURL(const GURL& url) { if (url == GURL(chrome::kChromeUIBookmarksURL)) - return GURL(kSidebarBookmarksURL); + return GURL(chrome::kChromeUIBookmarksSidePanelURL); if (url.host() == kBraveTalkHost) return GURL(kBraveTalkURL); diff --git a/browser/ui/sidebar/sidebar_web_contents_delegate.cc b/browser/ui/sidebar/sidebar_web_contents_delegate.cc deleted file mode 100644 index e8b88c6dcedd..000000000000 --- a/browser/ui/sidebar/sidebar_web_contents_delegate.cc +++ /dev/null @@ -1,40 +0,0 @@ -/* Copyright (c) 2021 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/sidebar/sidebar_web_contents_delegate.h" - -#include "brave/browser/ui/brave_browser.h" -#include "brave/browser/ui/sidebar/sidebar.h" -#include "brave/browser/ui/sidebar/sidebar_controller.h" -#include "chrome/browser/ui/browser_finder.h" - -namespace sidebar { - -namespace { - -SidebarController* GetSidebarController(content::WebContents* source) { - auto* brave_browser = - static_cast(chrome::FindBrowserWithWebContents(source)); - if (!brave_browser) - return nullptr; - - return brave_browser->sidebar_controller(); -} - -} // namespace - -SidebarWebContentsDelegate::SidebarWebContentsDelegate() = default; -SidebarWebContentsDelegate::~SidebarWebContentsDelegate() = default; - -bool SidebarWebContentsDelegate::HandleKeyboardEvent( - content::WebContents* source, - const content::NativeWebKeyboardEvent& event) { - if (auto* sidebar_controller = GetSidebarController(source)) - return sidebar_controller->sidebar()->HandleKeyboardEvent(source, event); - - return false; -} - -} // namespace sidebar diff --git a/browser/ui/sidebar/sidebar_web_contents_delegate.h b/browser/ui/sidebar/sidebar_web_contents_delegate.h deleted file mode 100644 index 80fa8ecd7d04..000000000000 --- a/browser/ui/sidebar/sidebar_web_contents_delegate.h +++ /dev/null @@ -1,31 +0,0 @@ -/* Copyright (c) 2021 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_SIDEBAR_SIDEBAR_WEB_CONTENTS_DELEGATE_H_ -#define BRAVE_BROWSER_UI_SIDEBAR_SIDEBAR_WEB_CONTENTS_DELEGATE_H_ - -#include "content/public/browser/web_contents_delegate.h" - -namespace sidebar { - -class SidebarWebContentsDelegate : public content::WebContentsDelegate { - public: - SidebarWebContentsDelegate(); - ~SidebarWebContentsDelegate() override; - - SidebarWebContentsDelegate(const SidebarWebContentsDelegate&) = delete; - SidebarWebContentsDelegate& operator=(const SidebarWebContentsDelegate&) = - delete; - - private: - // content::WebContentsDelegate overrides: - bool HandleKeyboardEvent( - content::WebContents* source, - const content::NativeWebKeyboardEvent& event) override; -}; - -} // namespace sidebar - -#endif // BRAVE_BROWSER_UI_SIDEBAR_SIDEBAR_WEB_CONTENTS_DELEGATE_H_ diff --git a/browser/ui/tabs/brave_tab_strip_model.cc b/browser/ui/tabs/brave_tab_strip_model.cc index 4624a8314e2e..de47b85f361d 100644 --- a/browser/ui/tabs/brave_tab_strip_model.cc +++ b/browser/ui/tabs/brave_tab_strip_model.cc @@ -76,8 +76,3 @@ void BraveTabStripModel::SelectMRUTab(TabRelativeDirection direction, void BraveTabStripModel::StopMRUCycling() { mru_cycle_list_.clear(); } - -bool BraveTabStripModel::IsReadLaterSupportedForAny( - const std::vector& indices) { - return false; -} diff --git a/browser/ui/tabs/brave_tab_strip_model.h b/browser/ui/tabs/brave_tab_strip_model.h index cea573c9ba25..905433605700 100644 --- a/browser/ui/tabs/brave_tab_strip_model.h +++ b/browser/ui/tabs/brave_tab_strip_model.h @@ -24,8 +24,6 @@ class BraveTabStripModel : public TabStripModel { void SelectRelativeTab(TabRelativeDirection direction, UserGestureDetails detail) override; - bool IsReadLaterSupportedForAny(const std::vector& indices) override; - // Set the next tab when doing a MRU cycling with Ctrl-tab void SelectMRUTab( TabRelativeDirection direction, diff --git a/browser/ui/views/frame/brave_browser_view.cc b/browser/ui/views/frame/brave_browser_view.cc index 68ffeb143a32..55f68fe4b890 100644 --- a/browser/ui/views/frame/brave_browser_view.cc +++ b/browser/ui/views/frame/brave_browser_view.cc @@ -180,11 +180,15 @@ BraveBrowserView::BraveBrowserView(std::unique_ptr browser) auto brave_contents_container = std::make_unique(); - // Wrap |contents_container_| within our new |brave_contents_container_|. - // |brave_contents_container_| also contains sidebar. - auto orignal_contents_container = RemoveChildViewT(contents_container_.get()); + // Wrap chromium side panel with our sidebar container + auto original_side_panel = RemoveChildViewT(right_aligned_side_panel_.get()); sidebar_container_view_ = brave_contents_container->AddChildView( - std::make_unique(GetBraveBrowser())); + std::make_unique(GetBraveBrowser(), + side_panel_coordinator(), + std::move(original_side_panel))); + right_aligned_side_panel_ = sidebar_container_view_->side_panel(); + // Put sidebar into chromium contents area + auto orignal_contents_container = RemoveChildViewT(contents_container_.get()); original_contents_container_ = brave_contents_container->AddChildView( std::move(orignal_contents_container)); brave_contents_container->SetLayoutManager( diff --git a/browser/ui/views/frame/brave_browser_view_layout.cc b/browser/ui/views/frame/brave_browser_view_layout.cc new file mode 100644 index 000000000000..8b77dceb52dd --- /dev/null +++ b/browser/ui/views/frame/brave_browser_view_layout.cc @@ -0,0 +1,15 @@ +// Copyright (c) 2022 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/views/frame/brave_browser_view_layout.h" + +void BraveBrowserViewLayout::LayoutSidePanelView( + views::View* side_panel, + gfx::Rect& contents_container_bounds) { + // We don't want to do any special layout for brave's sidebar (which + // is the parent of chromium's side panel). We simply + // use flex layout to put it to the side of the content view. + return; +} diff --git a/browser/ui/views/frame/brave_browser_view_layout.h b/browser/ui/views/frame/brave_browser_view_layout.h new file mode 100644 index 000000000000..92de4b564632 --- /dev/null +++ b/browser/ui/views/frame/brave_browser_view_layout.h @@ -0,0 +1,20 @@ +// Copyright (c) 2022 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_VIEWS_FRAME_BRAVE_BROWSER_VIEW_LAYOUT_H_ +#define BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_BROWSER_VIEW_LAYOUT_H_ + +#include "chrome/browser/ui/views/frame/browser_view_layout.h" + +class BraveBrowserViewLayout : public BrowserViewLayout { + public: + using BrowserViewLayout::BrowserViewLayout; + + // BrowserViewLayout overrides: + void LayoutSidePanelView(views::View* side_panel, + gfx::Rect& contents_container_bounds) override; +}; + +#endif // BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_BROWSER_VIEW_LAYOUT_H_ diff --git a/browser/ui/views/side_panel/brave_side_panel.cc b/browser/ui/views/side_panel/brave_side_panel.cc new file mode 100644 index 000000000000..15a0f4aaa5db --- /dev/null +++ b/browser/ui/views/side_panel/brave_side_panel.cc @@ -0,0 +1,67 @@ +// Copyright (c) 2022 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/views/side_panel/brave_side_panel.h" + +#include + +#include "base/ranges/algorithm.h" +#include "chrome/browser/themes/theme_properties.h" +#include "ui/base/metadata/metadata_impl_macros.h" +#include "ui/base/theme_provider.h" +#include "ui/views/border.h" +#include "ui/views/layout/fill_layout.h" + +BraveSidePanel::BraveSidePanel(BrowserView* browser_view) { + SetVisible(false); + SetLayoutManager(std::make_unique()); + + // TODO(pbos): Reconsider if SetPanelWidth() should add borders, if so move + // accounting for the border into SetPanelWidth(), otherwise remove this TODO. + constexpr int kDefaultWidth = 320; + SetPreferredSize(gfx::Size(kDefaultWidth, 1)); + AddObserver(this); +} + +BraveSidePanel::~BraveSidePanel() { + RemoveObserver(this); +} + +void BraveSidePanel::UpdateVisibility() { + const bool any_child_visible = base::ranges::any_of( + children(), [](const auto* view) { return view->GetVisible(); }); + SetVisible(any_child_visible); +} + +void BraveSidePanel::UpdateBorder() { + if (const ui::ThemeProvider* theme_provider = GetThemeProvider()) { + constexpr int kBorderThickness = 1; + // Negative top border so panel is flush with main tab content + SetBorder(views::CreateSolidSidedBorder( + gfx::Insets::TLBR(-1, 0, 0, kBorderThickness), + theme_provider->GetColor( + ThemeProperties::COLOR_TOOLBAR_CONTENT_AREA_SEPARATOR))); + } +} + +void BraveSidePanel::ChildVisibilityChanged(View* child) { + UpdateVisibility(); +} + +void BraveSidePanel::OnThemeChanged() { + View::OnThemeChanged(); + UpdateBorder(); +} + +void BraveSidePanel::OnChildViewAdded(View* observed_view, View* child) { + UpdateVisibility(); +} + +void BraveSidePanel::OnChildViewRemoved(View* observed_view, View* child) { + UpdateVisibility(); +} + +BEGIN_METADATA(BraveSidePanel, views::View) +END_METADATA diff --git a/browser/ui/views/side_panel/brave_side_panel.h b/browser/ui/views/side_panel/brave_side_panel.h new file mode 100644 index 000000000000..ed4a1659066e --- /dev/null +++ b/browser/ui/views/side_panel/brave_side_panel.h @@ -0,0 +1,39 @@ +// Copyright (c) 2022 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_VIEWS_SIDE_PANEL_BRAVE_SIDE_PANEL_H_ +#define BRAVE_BROWSER_UI_VIEWS_SIDE_PANEL_BRAVE_SIDE_PANEL_H_ + +#include "ui/base/metadata/metadata_header_macros.h" +#include "ui/views/view.h" +#include "ui/views/view_observer.h" + +class BrowserView; + +// Replacement for chromium's SidePanel which defines a +// unique inset and border style compared to Brave +class BraveSidePanel : public views::View, public views::ViewObserver { + public: + METADATA_HEADER(BraveSidePanel); + // Same signature as chromium SidePanel + explicit BraveSidePanel(BrowserView* browser_view); + BraveSidePanel(const BraveSidePanel&) = delete; + BraveSidePanel& operator=(const BraveSidePanel&) = delete; + ~BraveSidePanel() override; + + private: + void UpdateVisibility(); + void UpdateBorder(); + + // views::View: + void ChildVisibilityChanged(View* child) override; + void OnThemeChanged() override; + + // views::ViewObserver: + void OnChildViewAdded(View* observed_view, View* child) override; + void OnChildViewRemoved(View* observed_view, View* child) override; +}; + +#endif // BRAVE_BROWSER_UI_VIEWS_SIDE_PANEL_BRAVE_SIDE_PANEL_H_ diff --git a/browser/ui/views/sidebar/sidebar_container_view.cc b/browser/ui/views/sidebar/sidebar_container_view.cc index d61f1433b7ba..1258f8a5e4b8 100644 --- a/browser/ui/views/sidebar/sidebar_container_view.cc +++ b/browser/ui/views/sidebar/sidebar_container_view.cc @@ -7,20 +7,23 @@ #include +#include "base/auto_reset.h" #include "base/bind.h" #include "brave/browser/themes/theme_properties.h" #include "brave/browser/ui/brave_browser.h" #include "brave/browser/ui/sidebar/sidebar_controller.h" #include "brave/browser/ui/sidebar/sidebar_model.h" -#include "brave/browser/ui/sidebar/sidebar_model_data.h" #include "brave/browser/ui/sidebar/sidebar_service_factory.h" #include "brave/browser/ui/views/frame/brave_browser_view.h" +#include "brave/browser/ui/views/side_panel/brave_side_panel.h" #include "brave/browser/ui/views/sidebar/sidebar_control_view.h" -#include "brave/browser/ui/views/sidebar/sidebar_panel_webview.h" +#include "brave/browser/ui/views/sidebar/sidebar_side_panel_utils.h" +#include "brave/components/sidebar/sidebar_item.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/themes/theme_properties.h" #include "chrome/browser/ui/browser_window.h" #include "chrome/browser/ui/views/frame/browser_view.h" +#include "chrome/browser/ui/views/side_panel/side_panel_entry.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/native_web_keyboard_event.h" #include "content/public/browser/web_contents.h" @@ -42,19 +45,6 @@ sidebar::SidebarService* GetSidebarService(BraveBrowser* browser) { return sidebar::SidebarServiceFactory::GetForProfile(browser->profile()); } -size_t GetPreferredPanelWidthForCurrentItem(BraveBrowser* browser) { - auto* controller = browser->sidebar_controller(); - int active_index = controller->model()->active_index(); - const auto item = GetSidebarService(browser)->items()[active_index]; - // Shortcut type doesn't use panel. - if (!item.open_in_panel) - return 0; - - constexpr size_t kPanelWidthForBuiltIn = 260; - constexpr size_t kPanelWidthForNonBuiltIn = 360; - return IsBuiltInType(item) ? kPanelWidthForBuiltIn : kPanelWidthForNonBuiltIn; -} - } // namespace class SidebarContainerView::BrowserWindowEventObserver @@ -80,7 +70,7 @@ class SidebarContainerView::BrowserWindowEventObserver &window_event_position); if (!host_->sidebar_control_view_->GetLocalBounds().Contains( window_event_position) && - !host_->ShouldShowSidebar()) { + !host_->ShouldForceShowSidebar()) { host_->StopBrowserWindowEventMonitoring(); host_->ShowSidebar(false, true); } @@ -90,11 +80,16 @@ class SidebarContainerView::BrowserWindowEventObserver SidebarContainerView* host_ = nullptr; }; -SidebarContainerView::SidebarContainerView(BraveBrowser* browser) +SidebarContainerView::SidebarContainerView( + BraveBrowser* browser, + SidePanelCoordinator* side_panel_coordinator, + std::unique_ptr side_panel) : browser_(browser), + side_panel_coordinator_(side_panel_coordinator), browser_window_event_observer_( std::make_unique(this)) { SetNotifyEnterExitOnChild(true); + side_panel_ = AddChildView(std::move(side_panel)); } SidebarContainerView::~SidebarContainerView() = default; @@ -103,7 +98,19 @@ void SidebarContainerView::Init() { initialized_ = true; sidebar_model_ = browser_->sidebar_controller()->model(); - observed_.Observe(sidebar_model_); + sidebar_model_observation_.Observe(sidebar_model_); + + auto* browser_view = BrowserView::GetBrowserViewForBrowser(browser_); + DCHECK(browser_view); + + auto* side_panel_registry = + browser_view->side_panel_coordinator()->GetGlobalSidePanelRegistry(); + panel_registry_observation_.Observe(side_panel_registry); + + for (const auto& entry : side_panel_registry->entries()) { + DVLOG(1) << "Observing panel entry in ctor: " << entry->name(); + panel_entry_observations_.AddObservation(entry.get()); + } AddChildViews(); // Hide by default. Visibility will be controlled by show options later. @@ -112,75 +119,52 @@ void SidebarContainerView::Init() { void SidebarContainerView::SetSidebarShowOption( sidebar::SidebarService::ShowSidebarOption show_option) { - if (show_option == ShowSidebarOption::kShowAlways) { - ShowSidebar(true, false); - return; - } - - if (show_option == ShowSidebarOption::kShowNever) { - ShowSidebar(false, false); - return; - } - - ShowSidebar(false, true); + UpdateSidebarVisibility(show_option); } void SidebarContainerView::UpdateSidebar() { sidebar_control_view_->Update(); } -void SidebarContainerView::ShowCustomContextMenu( - const gfx::Point& point, - std::unique_ptr menu_model) { - if (!sidebar_panel_webview_->GetVisible()) { - LOG(ERROR) << __func__ - << " sidebar panel UI is loaded at non sidebar panel!"; - return; - } - - sidebar_panel_webview_->ShowCustomContextMenu(point, std::move(menu_model)); +void SidebarContainerView::MenuClosed() { + UpdateSidebarVisibility(); } -void SidebarContainerView::HideCustomContextMenu() { - if (!sidebar_panel_webview_->GetVisible()) { - LOG(ERROR) << __func__ - << " sidebar panel UI is loaded at non sidebar panel!"; - return; +void SidebarContainerView::UpdateBackground() { + if (const ui::ThemeProvider* theme_provider = GetThemeProvider()) { + // Fill background because panel's color uses alpha value. + SetBackground(views::CreateSolidBackground( + theme_provider->GetColor(ThemeProperties::COLOR_TOOLBAR))); } - - sidebar_panel_webview_->HideCustomContextMenu(); } -bool SidebarContainerView::HandleKeyboardEvent( - content::WebContents* source, - const content::NativeWebKeyboardEvent& event) { - DCHECK(sidebar_panel_webview_->GetVisible()); - return sidebar_panel_webview_->TreatUnHandledKeyboardEvent(source, event); +void SidebarContainerView::UpdateSidebarVisibility() { + const auto show_option = GetSidebarService(browser_)->GetSidebarShowOption(); + UpdateSidebarVisibility(show_option); } -void SidebarContainerView::UpdateBackgroundAndBorder() { - if (const ui::ThemeProvider* theme_provider = GetThemeProvider()) { - constexpr int kBorderThickness = 1; - // Fill background because panel's color uses alpha value. - SetBackground(views::CreateSolidBackground( - theme_provider->GetColor(ThemeProperties::COLOR_TOOLBAR))); - if (sidebar_panel_webview_ && sidebar_panel_webview_->GetVisible()) { - SetBorder(views::CreateSolidSidedBorder( - gfx::Insets::TLBR(0, 0, 0, kBorderThickness), - theme_provider->GetColor( - ThemeProperties::COLOR_TOOLBAR_CONTENT_AREA_SEPARATOR))); - } else { - // Don't need right side border when panel is closed. - SetBorder(nullptr); - } +void SidebarContainerView::UpdateSidebarVisibility( + sidebar::SidebarService::ShowSidebarOption show_option) { + // Always show, don't need to use mouse event detection. + if (show_option == ShowSidebarOption::kShowAlways) { + ShowSidebar(true, false); + return; + } + // Never show, except if there's a UI operation in progress. Still + // don't need to use mouse event detection. + if (show_option == ShowSidebarOption::kShowNever) { + ShowSidebar(ShouldForceShowSidebar(), false); + return; } + // Only show if mouse is hovered or there's a UI operation in progress. + ShowSidebar(IsMouseHovered() || ShouldForceShowSidebar(), true); } void SidebarContainerView::AddChildViews() { + // Insert to index 0 because |side_panel_| will already be at 0 but + // we want the controls first. sidebar_control_view_ = - AddChildView(std::make_unique(browser_)); - sidebar_panel_webview_ = - AddChildView(std::make_unique(browser_->profile())); + AddChildViewAt(std::make_unique(this, browser_), 0); } void SidebarContainerView::Layout() { @@ -191,10 +175,9 @@ void SidebarContainerView::Layout() { sidebar_control_view_->GetPreferredSize().width(); sidebar_control_view_->SetBounds(0, 0, control_view_preferred_width, height()); - if (sidebar_panel_webview_->GetVisible()) { - sidebar_panel_webview_->SetBounds( - control_view_preferred_width, 0, - GetPreferredPanelWidthForCurrentItem(browser_), height()); + if (side_panel_->GetVisible()) { + side_panel_->SetBounds(control_view_preferred_width, 0, + side_panel_->GetPreferredSize().width(), height()); } } @@ -205,8 +188,8 @@ gfx::Size SidebarContainerView::CalculatePreferredSize() const { int preferred_width = sidebar_control_view_->GetPreferredSize().width() + GetInsets().width(); - if (sidebar_panel_webview_->GetVisible()) - preferred_width += GetPreferredPanelWidthForCurrentItem(browser_); + if (side_panel_->GetVisible()) + preferred_width += side_panel_->GetPreferredSize().width(); // height is determined by parent. return {preferred_width, 0}; } @@ -214,11 +197,21 @@ gfx::Size SidebarContainerView::CalculatePreferredSize() const { void SidebarContainerView::OnThemeChanged() { View::OnThemeChanged(); - UpdateBackgroundAndBorder(); + UpdateBackground(); } -bool SidebarContainerView::ShouldShowSidebar() const { - return sidebar_panel_webview_->GetVisible() || +bool SidebarContainerView::ShouldForceShowSidebar() const { + // Always show if panel should be visible. It is more reliable to check + // whether the active index is a panel item rather than checking if + // side_panel_ is visible. + bool panel_is_active = false; + if (auto active_index = sidebar_model_->active_index(); active_index != -1) { + const auto& items = sidebar_model_->GetAllSidebarItems(); + const auto& active_item = items[active_index]; + panel_is_active = active_item.open_in_panel; + } + // Always show if user is reordering or a menu is visible + return panel_is_active || sidebar_control_view_->IsItemReorderingInProgress() || sidebar_control_view_->IsBubbleWidgetVisible(); } @@ -251,7 +244,7 @@ void SidebarContainerView::OnMouseExited(const ui::MouseEvent& event) { if (!autohide_sidebar) return; - if (ShouldShowSidebar()) { + if (ShouldForceShowSidebar()) { StartBrowserWindowEventMonitoring(); return; } @@ -260,23 +253,28 @@ void SidebarContainerView::OnMouseExited(const ui::MouseEvent& event) { } void SidebarContainerView::OnActiveIndexChanged(int old_index, int new_index) { + DVLOG(1) << "OnActiveIndexChanged: " << old_index << " to " << new_index; if (new_index == -1) { - sidebar_panel_webview_->SetVisible(false); + // `is_side_panel_event_` is used because `SidePanelCoordinator::Close` + // unfortunately calls both the event handler for `OnEntryHidden` as well + // as removing the View. Without it, we end up calling + // `SidePanelCoordinator::Close` recursively when the event originates from + // the SidePanelCoordinator itself (as opposed to out Sidebar buttons). This + // would then attempt to remove the entry's panel View twice. + // TODO(petemill): Consider reorganising the control between sidebar and + // sidepanel so that this is clearer. + if (!is_side_panel_event_) + side_panel_coordinator_->Close(); GetFocusManager()->ClearFocus(); - } else { - const auto item = sidebar_model_->GetAllSidebarItems()[new_index]; + } else if (!is_side_panel_event_) { + const auto& item = sidebar_model_->GetAllSidebarItems()[new_index]; if (item.open_in_panel) { - sidebar_panel_webview_->SetWebContents( - sidebar_model_->GetWebContentsAt(new_index)); - sidebar_panel_webview_->SetVisible(true); - // When panel is opened, it will get focused. - sidebar_panel_webview_->RequestFocus(); + // Get side panel entry information + side_panel_coordinator_->Show(SidePanelIdFromSideBarItem(item)); } else { - sidebar_panel_webview_->SetVisible(false); - GetFocusManager()->ClearFocus(); + side_panel_coordinator_->Close(); } } - UpdateBackgroundAndBorder(); InvalidateLayout(); } @@ -337,5 +335,56 @@ void SidebarContainerView::StopBrowserWindowEventMonitoring() { browser_window_event_monitor_.reset(); } +void SidebarContainerView::OnEntryShown(SidePanelEntry* entry) { + base::AutoReset auto_reset(&is_side_panel_event_, true); + + // Make sure item is selected. We need to observe the SidePanel system + // as well as Sidebar as there are other ways than Sidebar for SidePanel + // items to be shown and hidden, e.g. toolbar button. + DVLOG(1) << "Panel shown: " << entry->name(); + for (const auto& item : sidebar_model_->GetAllSidebarItems()) { + if (!item.open_in_panel) { + continue; + } + if (entry->id() == sidebar::SidePanelIdFromSideBarItem(item)) { + auto side_bar_index = sidebar_model_->GetIndexOf(item); + auto* controller = browser_->sidebar_controller(); + controller->ActivateItemAt(side_bar_index); + break; + } + } +} + +void SidebarContainerView::OnEntryHidden(SidePanelEntry* entry) { + base::AutoReset auto_reset(&is_side_panel_event_, true); + // Make sure item is deselected + DVLOG(1) << "Panel hidden: " << entry->name(); + for (const auto& item : sidebar_model_->GetAllSidebarItems()) { + if (!item.open_in_panel) { + continue; + } + if (entry->id() == sidebar::SidePanelIdFromSideBarItem(item)) { + auto side_bar_index = sidebar_model_->GetIndexOf(item); + auto* controller = browser_->sidebar_controller(); + if (controller->IsActiveIndex(side_bar_index)) { + controller->ActivateItemAt(-1); + } + break; + } + } +} + +void SidebarContainerView::OnEntryRegistered(SidePanelEntry* entry) { + // Observe when it's shown or hidden + DVLOG(1) << "Observing panel entry in registry observer: " << entry->name(); + panel_entry_observations_.AddObservation(entry); +} + +void SidebarContainerView::OnEntryWillDeregister(SidePanelEntry* entry) { + // Stop observing + DVLOG(1) << "Unobserving panel entry in registry observer: " << entry->name(); + panel_entry_observations_.RemoveObservation(entry); +} + BEGIN_METADATA(SidebarContainerView, views::View) END_METADATA diff --git a/browser/ui/views/sidebar/sidebar_container_view.h b/browser/ui/views/sidebar/sidebar_container_view.h index 99174bcdf765..8544eed05d6c 100644 --- a/browser/ui/views/sidebar/sidebar_container_view.h +++ b/browser/ui/views/sidebar/sidebar_container_view.h @@ -9,12 +9,18 @@ #include #include "base/memory/raw_ptr.h" +#include "base/scoped_multi_source_observation.h" #include "base/scoped_observation.h" #include "base/timer/timer.h" #include "brave/browser/ui/sidebar/sidebar.h" #include "brave/browser/ui/sidebar/sidebar_model.h" +#include "brave/browser/ui/views/side_panel/brave_side_panel.h" +#include "brave/browser/ui/views/sidebar/sidebar_control_view.h" #include "brave/browser/ui/views/sidebar/sidebar_show_options_event_detect_widget.h" #include "brave/components/sidebar/sidebar_service.h" +#include "chrome/browser/ui/views/side_panel/side_panel_coordinator.h" +#include "chrome/browser/ui/views/side_panel/side_panel_entry_observer.h" +#include "chrome/browser/ui/views/side_panel/side_panel_registry_observer.h" #include "ui/events/event_observer.h" #include "ui/views/view.h" @@ -27,20 +33,24 @@ class SidebarBrowserTest; } // namespace sidebar class BraveBrowser; -class SidebarControlView; -class SidebarPanelWebView; +class SidePanelEntry; // This view is the parent view of all sidebar ui. // Thi will include sidebar items button, add button, settings button and panel // view. class SidebarContainerView : public sidebar::Sidebar, + public SidebarControlView::Delegate, public views::View, public SidebarShowOptionsEventDetectWidget::Delegate, - public sidebar::SidebarModel::Observer { + public sidebar::SidebarModel::Observer, + public SidePanelEntryObserver, + public SidePanelRegistryObserver { public: METADATA_HEADER(SidebarContainerView); - explicit SidebarContainerView(BraveBrowser* browser); + SidebarContainerView(BraveBrowser* browser, + SidePanelCoordinator* side_panel_coordinator, + std::unique_ptr side_panel); ~SidebarContainerView() override; SidebarContainerView(const SidebarContainerView&) = delete; @@ -48,17 +58,15 @@ class SidebarContainerView void Init(); + BraveSidePanel* side_panel() { return side_panel_; } + // Sidebar overrides: void SetSidebarShowOption( sidebar::SidebarService::ShowSidebarOption show_option) override; void UpdateSidebar() override; - void ShowCustomContextMenu( - const gfx::Point& point, - std::unique_ptr menu_model) override; - void HideCustomContextMenu() override; - bool HandleKeyboardEvent( - content::WebContents* source, - const content::NativeWebKeyboardEvent& event) override; + + // SidebarControlView::Delegate overrides: + void MenuClosed() override; // views::View overrides: void Layout() override; @@ -73,17 +81,28 @@ class SidebarContainerView // sidebar::SidebarModel::Observer overrides: void OnActiveIndexChanged(int old_index, int new_index) override; + // SidePanelEntryObserver: + void OnEntryShown(SidePanelEntry* entry) override; + void OnEntryHidden(SidePanelEntry* entry) override; + + // SidePanelRegistryObserver: + void OnEntryRegistered(SidePanelEntry* entry) override; + void OnEntryWillDeregister(SidePanelEntry* entry) override; + private: friend class sidebar::SidebarBrowserTest; class BrowserWindowEventObserver; void AddChildViews(); - void UpdateBackgroundAndBorder(); + void UpdateBackground(); + void UpdateSidebarVisibility(); + void UpdateSidebarVisibility( + sidebar::SidebarService::ShowSidebarOption show_option); void ShowOptionsEventDetectWidget(bool show); void ShowSidebar(bool show_sidebar, bool show_event_detect_widget); SidebarShowOptionsEventDetectWidget* GetEventDetectWidget(); - bool ShouldShowSidebar() const; + bool ShouldForceShowSidebar() const; // On some condition(ex, add item bubble is visible), // sidebar should not be hidden even if mouse goes out from sidebar ui. @@ -102,17 +121,23 @@ class SidebarContainerView void DoHideSidebar(bool show_event_detect_widget); raw_ptr browser_ = nullptr; + raw_ptr side_panel_coordinator_ = nullptr; + raw_ptr side_panel_ = nullptr; raw_ptr sidebar_model_ = nullptr; - raw_ptr sidebar_panel_webview_ = nullptr; raw_ptr sidebar_control_view_ = nullptr; bool initialized_ = false; + bool is_side_panel_event_ = false; base::OneShotTimer sidebar_hide_timer_; std::unique_ptr browser_window_event_observer_; std::unique_ptr browser_window_event_monitor_; std::unique_ptr show_options_widget_; base::ScopedObservation - observed_{this}; + sidebar_model_observation_{this}; + base::ScopedMultiSourceObservation + panel_entry_observations_{this}; + base::ScopedObservation + panel_registry_observation_{this}; }; #endif // BRAVE_BROWSER_UI_VIEWS_SIDEBAR_SIDEBAR_CONTAINER_VIEW_H_ diff --git a/browser/ui/views/sidebar/sidebar_control_view.cc b/browser/ui/views/sidebar/sidebar_control_view.cc index 0499eabbc63b..b3bb0fda1d11 100644 --- a/browser/ui/views/sidebar/sidebar_control_view.cc +++ b/browser/ui/views/sidebar/sidebar_control_view.cc @@ -51,8 +51,9 @@ class ControlViewMenuModel : public ui::SimpleMenuModel { } // namespace -SidebarControlView::SidebarControlView(BraveBrowser* browser) - : browser_(browser) { +SidebarControlView::SidebarControlView(Delegate* delegate, + BraveBrowser* browser) + : delegate_(delegate), browser_(browser) { set_context_menu_controller(this); AddChildViews(); @@ -170,6 +171,10 @@ bool SidebarControlView::IsCommandIdChecked(int command_id) const { service->GetSidebarShowOption(); } +void SidebarControlView::MenuClosed(ui::SimpleMenuModel* source) { + delegate_->MenuClosed(); +} + std::u16string SidebarControlView::GetTooltipTextFor( const views::View* view) const { if (view == sidebar_settings_view_) diff --git a/browser/ui/views/sidebar/sidebar_control_view.h b/browser/ui/views/sidebar/sidebar_control_view.h index 843494837ce3..a97e4f08fc8f 100644 --- a/browser/ui/views/sidebar/sidebar_control_view.h +++ b/browser/ui/views/sidebar/sidebar_control_view.h @@ -39,7 +39,14 @@ class SidebarControlView : public views::View, public sidebar::SidebarModel::Observer { public: METADATA_HEADER(SidebarControlView); - explicit SidebarControlView(BraveBrowser* browser); + class Delegate { + public: + virtual void MenuClosed() {} + + protected: + ~Delegate() = default; + }; + SidebarControlView(Delegate* delegate, BraveBrowser* browser); ~SidebarControlView() override; SidebarControlView(const SidebarControlView&) = delete; @@ -58,6 +65,7 @@ class SidebarControlView : public views::View, // ui::SimpleMenuModel::Delegate overrides: void ExecuteCommand(int command_id, int event_flags) override; bool IsCommandIdChecked(int command_id) const override; + void MenuClosed(ui::SimpleMenuModel* source) override; // SidebarButtonView::Delegate overrides: std::u16string GetTooltipTextFor(const views::View* view) const override; @@ -85,6 +93,7 @@ class SidebarControlView : public views::View, void UpdateSettingsButtonState(); void UpdateBackgroundAndBorder(); + raw_ptr delegate_ = nullptr; raw_ptr browser_ = nullptr; raw_ptr sidebar_items_view_ = nullptr; raw_ptr sidebar_item_add_view_ = nullptr; diff --git a/browser/ui/views/sidebar/sidebar_items_contents_view.cc b/browser/ui/views/sidebar/sidebar_items_contents_view.cc index 4aab171d4c67..916a71ca42f9 100644 --- a/browser/ui/views/sidebar/sidebar_items_contents_view.cc +++ b/browser/ui/views/sidebar/sidebar_items_contents_view.cc @@ -433,6 +433,10 @@ gfx::ImageSkia SidebarItemsContentsView::GetImageForBuiltInItems( focused_image_resource = IDR_SIDEBAR_BOOKMARKS_FOCUSED; normal_image_icon = &kSidebarBookmarksIcon; break; + case sidebar::SidebarItem::BuiltInItemType::kReadingList: + focused_image_resource = IDR_SIDEBAR_READING_LIST_FOCUSED; + normal_image_icon = &kSidebarReadingListIcon; + break; case sidebar::SidebarItem::BuiltInItemType::kHistory: focused_image_resource = IDR_SIDEBAR_HISTORY_FOCUSED; normal_image_icon = &kSidebarHistoryIcon; diff --git a/browser/ui/views/sidebar/sidebar_panel_webview.cc b/browser/ui/views/sidebar/sidebar_panel_webview.cc deleted file mode 100644 index e476af654c39..000000000000 --- a/browser/ui/views/sidebar/sidebar_panel_webview.cc +++ /dev/null @@ -1,52 +0,0 @@ -/* Copyright (c) 2022 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/views/sidebar/sidebar_panel_webview.h" - -#include - -#include "ui/base/metadata/metadata_impl_macros.h" -#include "ui/base/models/menu_model.h" -#include "ui/views/controls/menu/menu_runner.h" - -SidebarPanelWebView::SidebarPanelWebView( - content::BrowserContext* browser_context) - : WebView(browser_context) { - SetVisible(false); - set_allow_accelerators(true); -} - -SidebarPanelWebView::~SidebarPanelWebView() = default; - -void SidebarPanelWebView::ShowCustomContextMenu( - const gfx::Point& point, - std::unique_ptr menu_model) { - // Show context menu at in screen coordinates. - gfx::Point screen_point = point; - ConvertPointToScreen(this, &screen_point); - context_menu_model_ = std::move(menu_model); - context_menu_runner_ = std::make_unique( - context_menu_model_.get(), - views::MenuRunner::HAS_MNEMONICS | views::MenuRunner::CONTEXT_MENU); - context_menu_runner_->RunMenuAt( - GetWidget(), nullptr, gfx::Rect(screen_point, gfx::Size()), - views::MenuAnchorPosition::kTopLeft, ui::MENU_SOURCE_MOUSE, - web_contents()->GetContentNativeView()); -} - -void SidebarPanelWebView::HideCustomContextMenu() { - if (context_menu_runner_) - context_menu_runner_->Cancel(); -} - -bool SidebarPanelWebView::TreatUnHandledKeyboardEvent( - content::WebContents* source, - const content::NativeWebKeyboardEvent& event) { - return unhandled_keyboard_event_handler_.HandleKeyboardEvent( - event, GetFocusManager()); -} - -BEGIN_METADATA(SidebarPanelWebView, views::WebView) -END_METADATA diff --git a/browser/ui/views/sidebar/sidebar_panel_webview.h b/browser/ui/views/sidebar/sidebar_panel_webview.h deleted file mode 100644 index 8a9e403378bd..000000000000 --- a/browser/ui/views/sidebar/sidebar_panel_webview.h +++ /dev/null @@ -1,43 +0,0 @@ -/* Copyright (c) 2022 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_VIEWS_SIDEBAR_SIDEBAR_PANEL_WEBVIEW_H_ -#define BRAVE_BROWSER_UI_VIEWS_SIDEBAR_SIDEBAR_PANEL_WEBVIEW_H_ - -#include - -#include "ui/views/controls/webview/unhandled_keyboard_event_handler.h" -#include "ui/views/controls/webview/webview.h" - -namespace ui { -class MenuModel; -} // namespace ui - -namespace views { -class MenuRunner; -} // namespace views - -class SidebarPanelWebView : public views::WebView { - public: - METADATA_HEADER(SidebarPanelWebView); - explicit SidebarPanelWebView(content::BrowserContext* browser_context); - ~SidebarPanelWebView() override; - SidebarPanelWebView(const SidebarPanelWebView&) = delete; - SidebarPanelWebView& operator=(const SidebarPanelWebView&) = delete; - - void ShowCustomContextMenu(const gfx::Point& point, - std::unique_ptr menu_model); - void HideCustomContextMenu(); - bool TreatUnHandledKeyboardEvent( - content::WebContents* source, - const content::NativeWebKeyboardEvent& event); - - private: - std::unique_ptr context_menu_runner_; - std::unique_ptr context_menu_model_; - views::UnhandledKeyboardEventHandler unhandled_keyboard_event_handler_; -}; - -#endif // BRAVE_BROWSER_UI_VIEWS_SIDEBAR_SIDEBAR_PANEL_WEBVIEW_H_ diff --git a/browser/ui/views/sidebar/sidebar_side_panel_utils.cc b/browser/ui/views/sidebar/sidebar_side_panel_utils.cc new file mode 100644 index 000000000000..e7b81c78790a --- /dev/null +++ b/browser/ui/views/sidebar/sidebar_side_panel_utils.cc @@ -0,0 +1,31 @@ +// Copyright (c) 2022 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/views/sidebar/sidebar_side_panel_utils.h" + +#include "base/logging.h" +#include "brave/components/sidebar/sidebar_item.h" +#include "chrome/browser/ui/views/side_panel/side_panel_entry.h" + +namespace sidebar { + +using BuiltInItemType = SidebarItem::BuiltInItemType; + +SidePanelEntry::Id SidePanelIdFromSideBarItem(const SidebarItem& item) { + DCHECK(item.open_in_panel); + switch (item.built_in_item_type) { + case BuiltInItemType::kReadingList: + return SidePanelEntry::Id::kReadingList; + case BuiltInItemType::kBookmarks: + return SidePanelEntry::Id::kBookmarks; + default: + // Add a new case for any new types which we want to support. + NOTREACHED() << "Asked for a panel Id from a sidebar item which should " + "not have a panel Id, sending Reading List instead."; + return SidePanelEntry::Id::kReadingList; + } +} + +} // namespace sidebar diff --git a/browser/ui/views/sidebar/sidebar_side_panel_utils.h b/browser/ui/views/sidebar/sidebar_side_panel_utils.h new file mode 100644 index 000000000000..893e17641dbc --- /dev/null +++ b/browser/ui/views/sidebar/sidebar_side_panel_utils.h @@ -0,0 +1,19 @@ +// Copyright (c) 2022 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_VIEWS_SIDEBAR_SIDEBAR_SIDE_PANEL_UTILS_H_ +#define BRAVE_BROWSER_UI_VIEWS_SIDEBAR_SIDEBAR_SIDE_PANEL_UTILS_H_ + +#include "chrome/browser/ui/views/side_panel/side_panel_entry.h" + +namespace sidebar { + +struct SidebarItem; + +SidePanelEntry::Id SidePanelIdFromSideBarItem(const SidebarItem& item); + +} // namespace sidebar + +#endif // BRAVE_BROWSER_UI_VIEWS_SIDEBAR_SIDEBAR_SIDE_PANEL_UTILS_H_ diff --git a/browser/ui/webui/brave_web_ui_controller_factory.cc b/browser/ui/webui/brave_web_ui_controller_factory.cc index 82eccea3b3b1..a3d42612ba10 100644 --- a/browser/ui/webui/brave_web_ui_controller_factory.cc +++ b/browser/ui/webui/brave_web_ui_controller_factory.cc @@ -23,7 +23,6 @@ #include "brave/components/constants/webui_url_constants.h" #include "brave/components/ipfs/buildflags/buildflags.h" #include "brave/components/playlist/buildflags/buildflags.h" -#include "brave/components/sidebar/buildflags/buildflags.h" #include "brave/components/tor/buildflags/buildflags.h" #include "build/build_config.h" #include "chrome/browser/profiles/profile.h" @@ -66,11 +65,6 @@ #include "brave/browser/ui/webui/tor_internals_ui.h" #endif -#if BUILDFLAG(ENABLE_SIDEBAR) -#include "brave/browser/ui/webui/sidebar/sidebar_bookmarks_ui.h" -#include "brave/components/sidebar/constants.h" -#endif - using content::WebUI; using content::WebUIController; @@ -145,10 +139,6 @@ WebUIController* NewWebUI(WebUI* web_ui, const GURL& url) { #if BUILDFLAG(ENABLE_TOR) } else if (host == kTorInternalsHost) { return new TorInternalsUI(web_ui, url.host()); -#endif -#if BUILDFLAG(ENABLE_SIDEBAR) - } else if (host == kSidebarBookmarksHost) { - return new SidebarBookmarksUI(web_ui); #endif } else if (host == kFederatedInternalsHost) { if (base::FeatureList::IsEnabled( @@ -169,9 +159,6 @@ WebUIFactoryFunction GetWebUIFactoryFunction(WebUI* web_ui, const GURL& url) { (url.host_piece() == kIPFSWebUIHost && base::FeatureList::IsEnabled(ipfs::features::kIpfsFeature)) || #endif // BUILDFLAG(ENABLE_IPFS) -#if BUILDFLAG(ENABLE_SIDEBAR) - url.host_piece() == kSidebarBookmarksHost || -#endif #if !BUILDFLAG(IS_ANDROID) url.host_piece() == kWalletPanelHost || url.host_piece() == kWalletPageHost || diff --git a/browser/ui/webui/sidebar/BUILD.gn b/browser/ui/webui/sidebar/BUILD.gn deleted file mode 100644 index 117cebd22a03..000000000000 --- a/browser/ui/webui/sidebar/BUILD.gn +++ /dev/null @@ -1,17 +0,0 @@ -# Copyright (c) 2022 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("//mojo/public/tools/bindings/mojom.gni") - -mojom("mojo_bindings") { - sources = [ "sidebar.mojom" ] - webui_module_path = "/" - public_deps = [ - "//mojo/public/mojom/base", - "//ui/base/mojom", - "//ui/gfx/geometry/mojom", - "//url/mojom:url_mojom_gurl", - ] -} diff --git a/browser/ui/webui/sidebar/sidebar.mojom b/browser/ui/webui/sidebar/sidebar.mojom deleted file mode 100644 index cff4cd6423ff..000000000000 --- a/browser/ui/webui/sidebar/sidebar.mojom +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright (c) 2022 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/. - -module sidebar.mojom; - -import "ui/base/mojom/window_open_disposition.mojom"; -import "ui/gfx/geometry/mojom/geometry.mojom"; -import "url/mojom/url.mojom"; - -// Used by the sidebar's bookmarks WebUI page (for the side panel) to bootstrap -// bidirectional communication. -interface BookmarksPageHandlerFactory { - // The WebUI calls this method when the page is first initialized. - CreateBookmarksPageHandler(pending_receiver handler); -}; - -// Browser-side handler for requests from WebUI page. -interface BookmarksPageHandler { - // Opens a bookmark by URL and passes the parent folder depth for metrics - // collection. - OpenBookmark(url.mojom.Url url, int32 parent_folder_depth, - ui.mojom.ClickModifiers click_modifiers); - - // Opens a context menu for a bookmark node. The id parameter is internally - // an int64 but gets passed as a string from the chrome.bookmarks Extension - // API. - ShowContextMenu(string id, gfx.mojom.Point point); -}; diff --git a/browser/ui/webui/sidebar/sidebar_bookmarks_page_handler.cc b/browser/ui/webui/sidebar/sidebar_bookmarks_page_handler.cc deleted file mode 100644 index 5b3d2babf01a..000000000000 --- a/browser/ui/webui/sidebar/sidebar_bookmarks_page_handler.cc +++ /dev/null @@ -1,153 +0,0 @@ -/* Copyright (c) 2022 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/webui/sidebar/sidebar_bookmarks_page_handler.h" - -#include -#include - -#include "base/memory/weak_ptr.h" -#include "base/strings/string_number_conversions.h" -#include "brave/browser/ui/brave_browser.h" -#include "brave/browser/ui/sidebar/sidebar.h" -#include "brave/browser/ui/sidebar/sidebar_controller.h" -#include "chrome/app/chrome_command_ids.h" -#include "chrome/browser/bookmarks/bookmark_model_factory.h" -#include "chrome/browser/profiles/profile.h" -#include "chrome/browser/ui/bookmarks/bookmark_context_menu_controller.h" -#include "chrome/browser/ui/bookmarks/bookmark_stats.h" -#include "chrome/browser/ui/browser.h" -#include "chrome/browser/ui/browser_finder.h" -#include "chrome/browser/ui/browser_window.h" -#include "components/bookmarks/browser/bookmark_model.h" -#include "components/bookmarks/browser/bookmark_node.h" -#include "components/bookmarks/browser/bookmark_utils.h" -#include "ui/base/models/simple_menu_model.h" -#include "ui/base/mojom/window_open_disposition.mojom.h" -#include "ui/base/window_open_disposition.h" -#include "url/gurl.h" - -namespace { - -class BookmarkContextMenu : public ui::SimpleMenuModel, - public ui::SimpleMenuModel::Delegate, - public BookmarkContextMenuControllerDelegate { - public: - explicit BookmarkContextMenu(Browser* browser, - sidebar::Sidebar* sidebar, - const bookmarks::BookmarkNode* bookmark) - : ui::SimpleMenuModel(this), - controller_(base::WrapUnique(new BookmarkContextMenuController( - browser->window()->GetNativeWindow(), - this, - browser, - browser->profile(), - base::BindRepeating( - [](content::PageNavigator* navigator) { return navigator; }, - browser), - // Do we need our own histogram enum? - BOOKMARK_LAUNCH_LOCATION_SIDE_PANEL_CONTEXT_MENU, - bookmark->parent(), - {bookmark}))), - sidebar_(sidebar) { - AddItem(IDC_BOOKMARK_BAR_OPEN_ALL); - AddItem(IDC_BOOKMARK_BAR_OPEN_ALL_NEW_WINDOW); - AddItem(IDC_BOOKMARK_BAR_OPEN_ALL_INCOGNITO); - AddSeparator(ui::NORMAL_SEPARATOR); - - AddItem(bookmark->is_folder() ? IDC_BOOKMARK_BAR_RENAME_FOLDER - : IDC_BOOKMARK_BAR_EDIT); - AddSeparator(ui::NORMAL_SEPARATOR); - - AddItem(IDC_CUT); - AddItem(IDC_COPY); - AddItem(IDC_PASTE); - AddSeparator(ui::NORMAL_SEPARATOR); - - AddItem(IDC_BOOKMARK_BAR_REMOVE); - AddSeparator(ui::NORMAL_SEPARATOR); - - AddItem(IDC_BOOKMARK_BAR_ADD_NEW_BOOKMARK); - AddItem(IDC_BOOKMARK_BAR_NEW_FOLDER); - AddSeparator(ui::NORMAL_SEPARATOR); - - AddItem(IDC_BOOKMARK_MANAGER); - } - ~BookmarkContextMenu() override = default; - - void ExecuteCommand(int command_id, int event_flags) override { - controller_->ExecuteCommand(command_id, event_flags); - } - - bool IsCommandIdEnabled(int command_id) const override { - return controller_->IsCommandIdEnabled(command_id); - } - - bool IsCommandIdVisible(int command_id) const override { - return controller_->IsCommandIdVisible(command_id); - } - - // BookmarkContextMenuControllerDelegate: - void CloseMenu() override { sidebar_->HideCustomContextMenu(); } - - private: - void AddItem(int command_id) { - ui::SimpleMenuModel::AddItem( - command_id, - controller_->menu_model()->GetLabelAt( - controller_->menu_model()->GetIndexOfCommandId(command_id))); - } - - std::unique_ptr controller_; - sidebar::Sidebar* sidebar_ = nullptr; -}; - -} // namespace - -SidebarBookmarksPageHandler::SidebarBookmarksPageHandler( - mojo::PendingReceiver receiver) - : receiver_(this, std::move(receiver)) {} - -SidebarBookmarksPageHandler::~SidebarBookmarksPageHandler() = default; - -void SidebarBookmarksPageHandler::OpenBookmark( - const GURL& url, - int32_t parent_folder_depth, - ui::mojom::ClickModifiersPtr click_modifiers) { - Browser* browser = chrome::FindLastActive(); - if (!browser) - return; - - WindowOpenDisposition open_location = ui::DispositionFromClick( - click_modifiers->middle_button, click_modifiers->alt_key, - click_modifiers->ctrl_key, click_modifiers->meta_key, - click_modifiers->shift_key); - content::OpenURLParams params(url, content::Referrer(), open_location, - ui::PAGE_TRANSITION_AUTO_BOOKMARK, false); - browser->OpenURL(params); -} - -void SidebarBookmarksPageHandler::ShowContextMenu(const std::string& id_string, - const gfx::Point& point) { - int64_t id; - if (!base::StringToInt64(id_string, &id)) - return; - - Browser* browser = chrome::FindLastActive(); - if (!browser) - return; - - bookmarks::BookmarkModel* bookmark_model = - BookmarkModelFactory::GetForBrowserContext(browser->profile()); - const bookmarks::BookmarkNode* bookmark = - bookmarks::GetBookmarkNodeByID(bookmark_model, id); - if (!bookmark) - return; - - auto* sidebar = - static_cast(browser)->sidebar_controller()->sidebar(); - sidebar->ShowCustomContextMenu( - point, std::make_unique(browser, sidebar, bookmark)); -} diff --git a/browser/ui/webui/sidebar/sidebar_bookmarks_page_handler.h b/browser/ui/webui/sidebar/sidebar_bookmarks_page_handler.h deleted file mode 100644 index 0acd51ef2cab..000000000000 --- a/browser/ui/webui/sidebar/sidebar_bookmarks_page_handler.h +++ /dev/null @@ -1,37 +0,0 @@ -/* Copyright (c) 2022 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_WEBUI_SIDEBAR_SIDEBAR_BOOKMARKS_PAGE_HANDLER_H_ -#define BRAVE_BROWSER_UI_WEBUI_SIDEBAR_SIDEBAR_BOOKMARKS_PAGE_HANDLER_H_ - -#include - -#include "brave/browser/ui/webui/sidebar/sidebar.mojom.h" -#include "mojo/public/cpp/bindings/pending_receiver.h" -#include "mojo/public/cpp/bindings/receiver.h" - -class GURL; - -class SidebarBookmarksPageHandler - : public sidebar::mojom::BookmarksPageHandler { - public: - explicit SidebarBookmarksPageHandler( - mojo::PendingReceiver receiver); - ~SidebarBookmarksPageHandler() override; - SidebarBookmarksPageHandler(const SidebarBookmarksPageHandler&) = delete; - SidebarBookmarksPageHandler& operator=(const SidebarBookmarksPageHandler&) = - delete; - - private: - // sidebar::mojom::BookmarksPageHandler: - void OpenBookmark(const GURL& url, - int32_t parent_folder_depth, - ui::mojom::ClickModifiersPtr click_modifiers) override; - void ShowContextMenu(const std::string& id, const gfx::Point& point) override; - - mojo::Receiver receiver_; -}; - -#endif // BRAVE_BROWSER_UI_WEBUI_SIDEBAR_SIDEBAR_BOOKMARKS_PAGE_HANDLER_H_ diff --git a/browser/ui/webui/sidebar/sidebar_bookmarks_ui.cc b/browser/ui/webui/sidebar/sidebar_bookmarks_ui.cc deleted file mode 100644 index 4d292a52a6a6..000000000000 --- a/browser/ui/webui/sidebar/sidebar_bookmarks_ui.cc +++ /dev/null @@ -1,71 +0,0 @@ -// Copyright (c) 2022 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/webui/sidebar/sidebar_bookmarks_ui.h" - -#include -#include - -#include "brave/browser/resources/sidebar/grit/sidebar_resources.h" -#include "brave/browser/resources/sidebar/grit/sidebar_resources_map.h" -#include "brave/browser/ui/webui/sidebar/sidebar_bookmarks_page_handler.h" -#include "brave/components/constants/webui_url_constants.h" -#include "brave/components/l10n/common/locale_util.h" -#include "chrome/browser/profiles/profile.h" -#include "chrome/browser/ui/webui/favicon_source.h" -#include "chrome/browser/ui/webui/webui_util.h" -#include "chrome/grit/generated_resources.h" -#include "components/bookmarks/common/bookmark_pref_names.h" -#include "components/favicon_base/favicon_url_parser.h" -#include "components/prefs/pref_service.h" -#include "content/public/browser/web_contents.h" -#include "content/public/browser/web_ui.h" -#include "content/public/browser/web_ui_data_source.h" - -SidebarBookmarksUI::SidebarBookmarksUI(content::WebUI* web_ui) - : ui::MojoWebUIController(web_ui) { - content::WebUIDataSource* source = - content::WebUIDataSource::Create(kSidebarBookmarksHost); - static constexpr webui::LocalizedString kLocalizedStrings[] = { - {"bookmarksTitle", IDS_BOOKMARK_MANAGER_TITLE}, - }; - for (const auto& str : kLocalizedStrings) { - std::u16string l10n_str = - brave_l10n::GetLocalizedResourceUTF16String(str.id); - source->AddString(str.name, l10n_str); - } - - Profile* const profile = Profile::FromWebUI(web_ui); - PrefService* prefs = profile->GetPrefs(); - source->AddBoolean( - "bookmarksDragAndDropEnabled", - prefs->GetBoolean(bookmarks::prefs::kEditBookmarksEnabled)); - - content::URLDataSource::Add( - profile, std::make_unique( - profile, chrome::FaviconUrlFormat::kFavicon2)); - webui::SetupWebUIDataSource( - source, base::make_span(kSidebarResources, kSidebarResourcesSize), - IDR_SIDEBAR_BOOKMARKS_BOOKMARKS_HTML); - content::WebUIDataSource::Add(web_ui->GetWebContents()->GetBrowserContext(), - source); -} - -SidebarBookmarksUI::~SidebarBookmarksUI() = default; - -WEB_UI_CONTROLLER_TYPE_IMPL(SidebarBookmarksUI) - -void SidebarBookmarksUI::BindInterface( - mojo::PendingReceiver - receiver) { - bookmarks_page_factory_receiver_.reset(); - bookmarks_page_factory_receiver_.Bind(std::move(receiver)); -} - -void SidebarBookmarksUI::CreateBookmarksPageHandler( - mojo::PendingReceiver receiver) { - bookmarks_page_handler_ = - std::make_unique(std::move(receiver)); -} diff --git a/browser/ui/webui/sidebar/sidebar_bookmarks_ui.h b/browser/ui/webui/sidebar/sidebar_bookmarks_ui.h deleted file mode 100644 index 95b931c3b37e..000000000000 --- a/browser/ui/webui/sidebar/sidebar_bookmarks_ui.h +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright (c) 2022 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_WEBUI_SIDEBAR_SIDEBAR_BOOKMARKS_UI_H_ -#define BRAVE_BROWSER_UI_WEBUI_SIDEBAR_SIDEBAR_BOOKMARKS_UI_H_ - -#include - -#include "brave/browser/ui/webui/sidebar/sidebar.mojom.h" -#include "mojo/public/cpp/bindings/pending_receiver.h" -#include "mojo/public/cpp/bindings/pending_remote.h" -#include "mojo/public/cpp/bindings/receiver.h" -#include "ui/webui/mojo_web_ui_controller.h" - -class SidebarBookmarksPageHandler; - -class SidebarBookmarksUI : public ui::MojoWebUIController, - public sidebar::mojom::BookmarksPageHandlerFactory { - public: - explicit SidebarBookmarksUI(content::WebUI* web_ui); - ~SidebarBookmarksUI() override; - SidebarBookmarksUI(const SidebarBookmarksUI&) = delete; - SidebarBookmarksUI& operator=(const SidebarBookmarksUI&) = delete; - - void BindInterface( - mojo::PendingReceiver - receiver); - - private: - // sidebar::mojom::BookmarksPageHandlerFactory - void CreateBookmarksPageHandler( - mojo::PendingReceiver receiver) - override; - - std::unique_ptr bookmarks_page_handler_; - mojo::Receiver - bookmarks_page_factory_receiver_{this}; - - WEB_UI_CONTROLLER_TYPE_DECL(); -}; - -#endif // BRAVE_BROWSER_UI_WEBUI_SIDEBAR_SIDEBAR_BOOKMARKS_UI_H_ diff --git a/chromium_src/chrome/browser/ui/browser_finder.cc b/chromium_src/chrome/browser/ui/browser_finder.cc deleted file mode 100644 index c451eff89554..000000000000 --- a/chromium_src/chrome/browser/ui/browser_finder.cc +++ /dev/null @@ -1,44 +0,0 @@ -/* Copyright (c) 2021 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/brave_browser.h" -#include "brave/components/sidebar/buildflags/buildflags.h" -#include "chrome/browser/ui/browser_list.h" - -#if BUILDFLAG(ENABLE_SIDEBAR) -#include "brave/browser/ui/sidebar/sidebar_controller.h" -#include "brave/browser/ui/sidebar/sidebar_model.h" -#endif - -#define FindBrowserWithWebContents FindBrowserWithWebContents_ChromiumImpl - -#include "src/chrome/browser/ui/browser_finder.cc" - -#undef FindBrowserWithWebContents - -namespace chrome { - -Browser* FindBrowserWithWebContents(const WebContents* web_contents) { - DCHECK(web_contents); - -#if BUILDFLAG(ENABLE_SIDEBAR) - for (auto* browser : *BrowserList::GetInstance()) { - // Use |sidebar_controller| directly, because even if the feature is - // enabled, SidebarController can be not created for a specific - // BraveBrowser. - const auto* sidebar_controller = - static_cast(browser)->sidebar_controller(); - DCHECK(!sidebar_controller || sidebar_controller->model()); - if (sidebar_controller && - sidebar_controller->model()->IsSidebarWebContents(web_contents)) { - return browser; - } - } -#endif - - return FindBrowserWithWebContents_ChromiumImpl(web_contents); -} - -} // namespace chrome diff --git a/chromium_src/chrome/browser/ui/ui_features.cc b/chromium_src/chrome/browser/ui/ui_features.cc index 95d4b5b6376f..4d1b859fe757 100644 --- a/chromium_src/chrome/browser/ui/ui_features.cc +++ b/chromium_src/chrome/browser/ui/ui_features.cc @@ -5,11 +5,19 @@ #include "src/chrome/browser/ui/ui_features.cc" +#include "base/feature_list.h" #include "base/feature_override.h" namespace features { -OVERRIDE_FEATURE_DEFAULT_STATES({{kTabHoverCardImages, - base::FEATURE_DISABLED_BY_DEFAULT}}); +OVERRIDE_FEATURE_DEFAULT_STATES({{ + {kTabHoverCardImages, base::FEATURE_DISABLED_BY_DEFAULT}, + // Unified SidePanel actually means that each Side Panel item's WebUI is + // a separate page, instead of 1 page that has different functions, e.g. + // (reading list and bookmarks). We want this feature immediately because + // Brave have its own control for showing Side Panels individually via + // Brave's Side Bar. + {kUnifiedSidePanel, base::FEATURE_ENABLED_BY_DEFAULT}, +}}); } // namespace features diff --git a/chromium_src/chrome/browser/ui/views/frame/browser_view.cc b/chromium_src/chrome/browser/ui/views/frame/browser_view.cc index 86c174d47a74..f41e3fc16bb5 100644 --- a/chromium_src/chrome/browser/ui/views/frame/browser_view.cc +++ b/chromium_src/chrome/browser/ui/views/frame/browser_view.cc @@ -3,16 +3,27 @@ * 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 the corresponding header first since it defines the same macros and +// therefore avoid undef before use. +#include "chrome/browser/ui/views/frame/browser_view.h" + +#include "brave/browser/ui/views/frame/brave_browser_view_layout.h" +#include "brave/browser/ui/views/side_panel/brave_side_panel.h" #include "brave/browser/ui/views/tabs/brave_browser_tab_strip_controller.h" #include "brave/browser/ui/views/tabs/brave_tab_strip.h" #include "brave/browser/ui/views/toolbar/brave_toolbar_view.h" #include "chrome/browser/ui/views/frame/tab_strip_region_view.h" +#include "chrome/browser/ui/views/side_panel/side_panel.h" #define ToolbarView BraveToolbarView #define BrowserTabStripController BraveBrowserTabStripController #define TabStrip BraveTabStrip +#define BrowserViewLayout BraveBrowserViewLayout +#define SidePanel BraveSidePanel #include "src/chrome/browser/ui/views/frame/browser_view.cc" #undef ToolbarView #undef BrowserTabStripController #undef TabStrip +#undef BrowserViewLayout +#undef SidePanel diff --git a/chromium_src/chrome/browser/ui/views/frame/browser_view.h b/chromium_src/chrome/browser/ui/views/frame/browser_view.h index a4ece2950ce5..46c438100b77 100644 --- a/chromium_src/chrome/browser/ui/views/frame/browser_view.h +++ b/chromium_src/chrome/browser/ui/views/frame/browser_view.h @@ -7,11 +7,17 @@ #define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_FRAME_BROWSER_VIEW_H_ #include "brave/browser/ui/brave_browser_window.h" +#include "brave/browser/ui/views/frame/brave_browser_view_layout.h" +#include "brave/browser/ui/views/side_panel/brave_side_panel.h" +#include "chrome/browser/ui/views/side_panel/side_panel.h" +#include "chrome/browser/ui/views/side_search/side_search_browser_controller.h" #define BrowserViewLayoutDelegateImpl \ BrowserViewLayoutDelegateImpl; \ friend class BraveBrowserView #define BrowserWindow BraveBrowserWindow +#define BrowserViewLayout BraveBrowserViewLayout +#define SidePanel BraveSidePanel #define GetContentsLayoutManager \ GetContentsLayoutManager_Unused(); \ virtual ContentsLayoutManager* GetContentsLayoutManager @@ -23,6 +29,8 @@ #undef BrowserViewLayoutDelegateImpl #undef BrowserWindow #undef MaybeShowReadingListInSidePanelIPH +#undef BrowserViewLayout +#undef SidePanel #undef GetContentsLayoutManager #endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_FRAME_BROWSER_VIEW_H_ diff --git a/chromium_src/chrome/browser/ui/views/frame/browser_view_layout.h b/chromium_src/chrome/browser/ui/views/frame/browser_view_layout.h new file mode 100644 index 000000000000..01c64b0040e4 --- /dev/null +++ b/chromium_src/chrome/browser/ui/views/frame/browser_view_layout.h @@ -0,0 +1,14 @@ +// Copyright (c) 2022 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_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_FRAME_BROWSER_VIEW_LAYOUT_H_ +#define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_FRAME_BROWSER_VIEW_LAYOUT_H_ + +// Make override-able +#define LayoutSidePanelView virtual LayoutSidePanelView +#include "src/chrome/browser/ui/views/frame/browser_view_layout.h" +#undef LayoutSidePanelView + +#endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_FRAME_BROWSER_VIEW_LAYOUT_H_ diff --git a/chromium_src/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc b/chromium_src/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc new file mode 100644 index 000000000000..961e350cfa83 --- /dev/null +++ b/chromium_src/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc @@ -0,0 +1,10 @@ +// Copyright (c) 2022 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/. + +// Brave has its own side panel navigation in the form of the SideBar, so +// hide the Chromium combobox-style header. +#define BRAVE_SIDE_PANEL_COORDINATOR_CREATE_HEADER header->SetVisible(false); +#include "src/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc" +#undef BRAVE_SIDE_PANEL_COORDINATOR_CREATE_HEADER diff --git a/chromium_src/chrome/browser/ui/views/side_search/side_search_browser_controller.cc b/chromium_src/chrome/browser/ui/views/side_search/side_search_browser_controller.cc new file mode 100644 index 000000000000..8620f2c17b25 --- /dev/null +++ b/chromium_src/chrome/browser/ui/views/side_search/side_search_browser_controller.cc @@ -0,0 +1,16 @@ +// Copyright (c) 2022 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 any corresponding headers first that define the same macros and +// therefore avoid undef before use. +#include "chrome/browser/ui/views/side_search/side_search_browser_controller.h" +#include "chrome/browser/ui/views/frame/browser_view.h" + +#include "brave/browser/ui/views/side_panel/brave_side_panel.h" +#include "chrome/browser/ui/views/side_panel/side_panel.h" + +#define SidePanel BraveSidePanel +#include "src/chrome/browser/ui/views/side_search/side_search_browser_controller.cc" +#undef SidePanel diff --git a/chromium_src/chrome/browser/ui/views/side_search/side_search_browser_controller.h b/chromium_src/chrome/browser/ui/views/side_search/side_search_browser_controller.h new file mode 100644 index 000000000000..ad6b7cc7b0d4 --- /dev/null +++ b/chromium_src/chrome/browser/ui/views/side_search/side_search_browser_controller.h @@ -0,0 +1,16 @@ +// Copyright (c) 2022 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_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_SIDE_SEARCH_SIDE_SEARCH_BROWSER_CONTROLLER_H_ +#define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_SIDE_SEARCH_SIDE_SEARCH_BROWSER_CONTROLLER_H_ + +#include "brave/browser/ui/views/side_panel/brave_side_panel.h" +#include "chrome/browser/ui/views/side_panel/side_panel.h" + +#define SidePanel BraveSidePanel +#include "src/chrome/browser/ui/views/side_search/side_search_browser_controller.h" +#undef SidePanel + +#endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_SIDE_SEARCH_SIDE_SEARCH_BROWSER_CONTROLLER_H_ diff --git a/chromium_src/chrome/browser/ui/views/toolbar/toolbar_view.cc b/chromium_src/chrome/browser/ui/views/toolbar/toolbar_view.cc index f71edc804d2b..29e9a52bf6cc 100644 --- a/chromium_src/chrome/browser/ui/views/toolbar/toolbar_view.cc +++ b/chromium_src/chrome/browser/ui/views/toolbar/toolbar_view.cc @@ -18,11 +18,8 @@ AddChildView(std::make_unique(browser_view_)); \ if (false) -#define BRAVE_TOOLBAR_VIEW_DEACTIVATE_SIDE_PANEL if (false) - #include "src/chrome/browser/ui/views/toolbar/toolbar_view.cc" #undef BRAVE_TOOLBAR_VIEW_INIT -#undef BRAVE_TOOLBAR_VIEW_DEACTIVATE_SIDE_PANEL #if BUILDFLAG(ENABLE_EXTENSIONS) #undef LocationBarView #endif diff --git a/chromium_src/components/reading_list/features/reading_list_switches.cc b/chromium_src/components/reading_list/features/reading_list_switches.cc deleted file mode 100644 index e5433ab0b1f1..000000000000 --- a/chromium_src/components/reading_list/features/reading_list_switches.cc +++ /dev/null @@ -1,19 +0,0 @@ -/* Copyright (c) 2021 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 "src/components/reading_list/features/reading_list_switches.cc" - -#include "base/feature_list.h" -#include "base/feature_override.h" - -namespace reading_list { -namespace switches { - -OVERRIDE_FEATURE_DEFAULT_STATES({{ - {kReadLater, base::FEATURE_DISABLED_BY_DEFAULT}, -}}); - -} // namespace switches -} // namespace reading_list diff --git a/common/extensions/api/_api_features.json b/common/extensions/api/_api_features.json index 6d9515c5539c..c7b8d9b3bcb0 100644 --- a/common/extensions/api/_api_features.json +++ b/common/extensions/api/_api_features.json @@ -26,19 +26,6 @@ "chrome://newtab/*" ] }], - "bookmarkManagerPrivate": [{ - "dependencies": ["permission:bookmarkManagerPrivate"], - "contexts": ["blessed_extension"], - "default_parent": true - }, { - "channel": "stable", - "contexts": ["webui"], - "matches": [ - "chrome://bookmarks/*", - "chrome://read-later.top-chrome/*", - "chrome://sidebar-bookmarks.top-chrome/*" - ] - }], "bookmarks": [{ "dependencies": ["permission:bookmarks"], "contexts": ["blessed_extension"], @@ -48,8 +35,8 @@ "contexts": ["webui"], "matches": [ "chrome://bookmarks/*", - "chrome://read-later.top-chrome/*", - "chrome://sidebar-bookmarks.top-chrome/*" + "chrome://bookmarks-side-panel.top-chrome/*", + "chrome://read-later.top-chrome/*" ], "default_parent": true }, { diff --git a/components/constants/webui_url_constants.cc b/components/constants/webui_url_constants.cc index bf9593aaf747..98cb11f1dd71 100644 --- a/components/constants/webui_url_constants.cc +++ b/components/constants/webui_url_constants.cc @@ -49,7 +49,6 @@ const char kShieldsPanelURL[] = "chrome://brave-shields.top-chrome"; const char kShieldsPanelHost[] = "brave-shields.top-chrome"; const char kCookieListOptInHost[] = "cookie-list-opt-in.top-chrome"; const char kCookieListOptInURL[] = "chrome://cookie-list-opt-in.top-chrome"; -const char kSidebarBookmarksHost[] = "sidebar-bookmarks.top-chrome"; const char kFederatedInternalsURL[] = "brave://federated-internals"; const char kFederatedInternalsHost[] = "federated-internals"; const char kContentFiltersPath[] = "shields/filters"; diff --git a/components/constants/webui_url_constants.h b/components/constants/webui_url_constants.h index 178d211ce2ce..17f261b93fb0 100644 --- a/components/constants/webui_url_constants.h +++ b/components/constants/webui_url_constants.h @@ -50,7 +50,6 @@ extern const char kShieldsPanelURL[]; extern const char kShieldsPanelHost[]; extern const char kCookieListOptInHost[]; extern const char kCookieListOptInURL[]; -extern const char kSidebarBookmarksHost[]; extern const char kFederatedInternalsURL[]; extern const char kFederatedInternalsHost[]; extern const char kContentFiltersPath[]; diff --git a/components/resources/brave_components_strings.grd b/components/resources/brave_components_strings.grd index bd1c042b7dbc..473111969f48 100644 --- a/components/resources/brave_components_strings.grd +++ b/components/resources/brave_components_strings.grd @@ -368,6 +368,9 @@ History + + Reading List + diff --git a/components/sidebar/constants.h b/components/sidebar/constants.h index 87d3400bf2b4..9791862ad0b0 100644 --- a/components/sidebar/constants.h +++ b/components/sidebar/constants.h @@ -14,12 +14,7 @@ constexpr char kSidebarItemBuiltInItemTypeKey[] = "built_in_item_type"; constexpr char kSidebarItemTitleKey[] = "title"; constexpr char kSidebarItemOpenInPanelKey[] = "open_in_panel"; -// TODO(simonhong): Move this to -// //brave/components/constants/webui_url_constants.h when default builtin items // list is provided from chrome layer. -constexpr char kSidebarBookmarksURL[] = - "chrome://sidebar-bookmarks.top-chrome/"; - constexpr char kBraveTalkURL[] = "https://talk.brave.com/widget"; constexpr char kBraveTalkHost[] = "talk.brave.com"; diff --git a/components/sidebar/sidebar_item.cc b/components/sidebar/sidebar_item.cc index 2300f594d372..7cf35b3c255f 100644 --- a/components/sidebar/sidebar_item.cc +++ b/components/sidebar/sidebar_item.cc @@ -8,13 +8,11 @@ namespace sidebar { // static -SidebarItem SidebarItem::Create(const GURL& url, - const std::u16string& title, +SidebarItem SidebarItem::Create(const std::u16string& title, Type type, BuiltInItemType built_in_item_type, bool open_in_panel) { SidebarItem item; - item.url = url; item.title = title; item.type = type; item.built_in_item_type = built_in_item_type; @@ -22,6 +20,17 @@ SidebarItem SidebarItem::Create(const GURL& url, return item; } +// static +SidebarItem SidebarItem::Create(const GURL& url, + const std::u16string& title, + Type type, + BuiltInItemType built_in_item_type, + bool open_in_panel) { + SidebarItem item = Create(title, type, built_in_item_type, open_in_panel); + item.url = url; + return item; +} + SidebarItem::SidebarItem() = default; SidebarItem::~SidebarItem() = default; diff --git a/components/sidebar/sidebar_item.h b/components/sidebar/sidebar_item.h index 0b7040ca0b1e..44bea398068b 100644 --- a/components/sidebar/sidebar_item.h +++ b/components/sidebar/sidebar_item.h @@ -21,9 +21,15 @@ struct SidebarItem { kBraveTalk, kWallet, kBookmarks, + kReadingList, kHistory, }; + static SidebarItem Create(const std::u16string& title, + Type type, + BuiltInItemType built_in_item_type, + bool open_in_panel); + static SidebarItem Create(const GURL& url, const std::u16string& title, Type type, diff --git a/components/sidebar/sidebar_service.cc b/components/sidebar/sidebar_service.cc index e83a91f20290..18c250ecf5f3 100644 --- a/components/sidebar/sidebar_service.cc +++ b/components/sidebar/sidebar_service.cc @@ -45,12 +45,19 @@ SidebarItem GetBuiltInItemForType(SidebarItem::BuiltInItemType type) { SidebarItem::Type::kTypeBuiltIn, SidebarItem::BuiltInItemType::kWallet, false); case SidebarItem::BuiltInItemType::kBookmarks: - return SidebarItem::Create(GURL(kSidebarBookmarksURL), - brave_l10n::GetLocalizedResourceUTF16String( + return SidebarItem::Create(brave_l10n::GetLocalizedResourceUTF16String( IDS_SIDEBAR_BOOKMARKS_ITEM_TITLE), SidebarItem::Type::kTypeBuiltIn, SidebarItem::BuiltInItemType::kBookmarks, true); + case SidebarItem::BuiltInItemType::kReadingList: + return SidebarItem::Create( + // TODO(petemill): Have these items created under brave/browser + // so that we can access common strings, like IDS_READ_LATER_TITLE. + brave_l10n::GetLocalizedResourceUTF16String( + IDS_SIDEBAR_READING_LIST_ITEM_TITLE), + SidebarItem::Type::kTypeBuiltIn, + SidebarItem::BuiltInItemType::kReadingList, true); case SidebarItem::BuiltInItemType::kHistory: return SidebarItem::Create(GURL("chrome://history/"), brave_l10n::GetLocalizedResourceUTF16String( @@ -70,7 +77,7 @@ SidebarItem::BuiltInItemType GetBuiltInItemTypeForURL(const std::string& url) { if (url == "chrome://wallet/") return SidebarItem::BuiltInItemType::kWallet; - if (url == kSidebarBookmarksURL || url == "chrome://bookmarks/") + if (url == "chrome://bookmarks/") return SidebarItem::BuiltInItemType::kBookmarks; if (url == "chrome://history/") @@ -91,6 +98,8 @@ std::vector GetDefaultSidebarItems() { items.push_back(GetBuiltInItemForType(SidebarItem::BuiltInItemType::kWallet)); items.push_back( GetBuiltInItemForType(SidebarItem::BuiltInItemType::kBookmarks)); + items.push_back( + GetBuiltInItemForType(SidebarItem::BuiltInItemType::kReadingList)); return items; } diff --git a/components/sidebar/sidebar_service.h b/components/sidebar/sidebar_service.h index 359e45321359..11e6fb3601f0 100644 --- a/components/sidebar/sidebar_service.h +++ b/components/sidebar/sidebar_service.h @@ -67,6 +67,7 @@ class SidebarService : public KeyedService { SidebarService& operator=(const SidebarService&) = delete; private: + FRIEND_TEST_ALL_PREFIXES(SidebarModelTest, ItemsChangedTest); FRIEND_TEST_ALL_PREFIXES(SidebarServiceTest, AddRemoveItems); void LoadSidebarItems(); diff --git a/patches/chrome-browser-ui-views-side_panel-side_panel_coordinator.cc.patch b/patches/chrome-browser-ui-views-side_panel-side_panel_coordinator.cc.patch new file mode 100644 index 000000000000..51895ceee4d3 --- /dev/null +++ b/patches/chrome-browser-ui-views-side_panel-side_panel_coordinator.cc.patch @@ -0,0 +1,12 @@ +diff --git a/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc b/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc +index 1efe9453a6d271fdb118f39f95630296de13a95e..3353a5aba43a3e8f36766c75e3adc16bbb34f314 100644 +--- a/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc ++++ b/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc +@@ -419,6 +419,7 @@ std::unique_ptr SidePanelCoordinator::CreateHeader() { + ChromeLayoutProvider::Get()->GetDistanceMetric( + ChromeDistanceMetric::DISTANCE_SIDE_PANEL_HEADER_VECTOR_ICON_SIZE))); + ++ BRAVE_SIDE_PANEL_COORDINATOR_CREATE_HEADER + return header; + } + diff --git a/patches/chrome-browser-ui-views-toolbar-toolbar_view.cc.patch b/patches/chrome-browser-ui-views-toolbar-toolbar_view.cc.patch index 990a3faa4081..b69f9b930e77 100644 --- a/patches/chrome-browser-ui-views-toolbar-toolbar_view.cc.patch +++ b/patches/chrome-browser-ui-views-toolbar-toolbar_view.cc.patch @@ -2,15 +2,7 @@ diff --git a/chrome/browser/ui/views/toolbar/toolbar_view.cc b/chrome/browser/ui index 8499344e764b14f52fea1ace029fc4aa6bd5619a..ee822fe7723cb3c17036ef2b7d730b16c8c096fd 100644 --- a/chrome/browser/ui/views/toolbar/toolbar_view.cc +++ b/chrome/browser/ui/views/toolbar/toolbar_view.cc -@@ -298,6 +298,7 @@ void ToolbarView::Init() { - } - - std::unique_ptr side_panel_button; -+ BRAVE_TOOLBAR_VIEW_DEACTIVATE_SIDE_PANEL - if (browser_view_->right_aligned_side_panel()) { - side_panel_button = std::make_unique(browser_); - } -@@ -364,6 +365,7 @@ void ToolbarView::Init() { +@@ -364,6 +364,7 @@ void ToolbarView::Init() { } else { // TODO(crbug.com/932818): Remove this once the // |kAutofillEnableToolbarStatusChip| is fully launched. diff --git a/vector_icons/chrome/app/vector_icons/side_panel.icon b/vector_icons/chrome/app/vector_icons/side_panel.icon new file mode 100644 index 000000000000..1cb8324dabbb --- /dev/null +++ b/vector_icons/chrome/app/vector_icons/side_panel.icon @@ -0,0 +1,23 @@ +// Copyright (c) 2022 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/. + +CANVAS_DIMENSIONS, 18, +FLIPS_IN_RTL, +MOVE_TO, 18, 15.75f, +R_CUBIC_TO, 0, 0.83f, -0.67f, 1.5f, -1.5f, 1.5f, +R_H_LINE_TO, -15, +R_CUBIC_TO, -0.83f, 0, -1.5f, -0.67f, -1.5f, -1.5f, +V_LINE_TO, 2.25f, +CUBIC_TO, 0, 1.42f, 0.67f, 0.75f, 1.5f, 0.75f, +R_H_LINE_TO, 15, +R_CUBIC_TO, 0.83f, 0, 1.5f, 0.67f, 1.5f, 1.5f, +R_V_LINE_TO, 13.5f, +CLOSE, +MOVE_TO, 5.25f, 2.25f, +R_V_LINE_TO, 13.5f, +H_LINE_TO, 16.5f, +V_LINE_TO, 2.25f, +H_LINE_TO, 5.25f, +CLOSE From 54599e7408889a60f5ce2a9a835aedaab7c749a4 Mon Sep 17 00:00:00 2001 From: Pete Miller Date: Mon, 25 Jul 2022 00:45:47 +0100 Subject: [PATCH 2/3] sidebar: migrate pref for which built-in items to show from "inclusive" to "exclusive" This allows new items to be easily added, and only store the item ref by ID instead of all item details --- .../net/brave_network_audit_browsertest.cc | 2 +- browser/ui/sidebar/sidebar_controller.cc | 2 +- browser/ui/sidebar/sidebar_model.h | 2 +- .../sidebar/sidebar_items_contents_view.cc | 6 +- components/sidebar/pref_names.h | 2 + components/sidebar/sidebar_service.cc | 339 +++++++++++---- components/sidebar/sidebar_service.h | 8 +- .../sidebar/sidebar_service_unittest.cc | 389 ++++++++++++++++-- 8 files changed, 611 insertions(+), 139 deletions(-) diff --git a/browser/net/brave_network_audit_browsertest.cc b/browser/net/brave_network_audit_browsertest.cc index eeb741440c10..913dd749420d 100644 --- a/browser/net/brave_network_audit_browsertest.cc +++ b/browser/net/brave_network_audit_browsertest.cc @@ -307,7 +307,7 @@ IN_PROC_BROWSER_TEST_F(BraveNetworkAuditTest, BasicTests) { auto* sidebar_controller = static_cast(browser())->sidebar_controller(); auto* sidebar_model = sidebar_controller->model(); - auto all_items = sidebar_model->GetAllSidebarItems(); + const auto& all_items = sidebar_model->GetAllSidebarItems(); const int item_num = all_items.size(); const int builtin_panel_item_total = 2; int builtin_panel_item_count = 0; diff --git a/browser/ui/sidebar/sidebar_controller.cc b/browser/ui/sidebar/sidebar_controller.cc index b8ffb73dc798..e4ee05b1afcd 100644 --- a/browser/ui/sidebar/sidebar_controller.cc +++ b/browser/ui/sidebar/sidebar_controller.cc @@ -70,7 +70,7 @@ void SidebarController::ActivateItemAt(int index, return; } - const auto item = sidebar_model_->GetAllSidebarItems()[index]; + const auto& item = sidebar_model_->GetAllSidebarItems()[index]; // Only an item for panel can get activated. if (item.open_in_panel) { sidebar_model_->SetActiveIndex(index); diff --git a/browser/ui/sidebar/sidebar_model.h b/browser/ui/sidebar/sidebar_model.h index 73df5d6d55f9..bc79392394dc 100644 --- a/browser/ui/sidebar/sidebar_model.h +++ b/browser/ui/sidebar/sidebar_model.h @@ -82,7 +82,7 @@ class SidebarModel : public SidebarService::Observer, int GetIndexOf(const SidebarItem& item) const; // Don't cache item list. list can be changed during the runtime. - const std::vector GetAllSidebarItems() const; + const std::vector& GetAllSidebarItems() const; // Return -1 if sidebar panel is not opened. int active_index() const { return active_index_; } diff --git a/browser/ui/views/sidebar/sidebar_items_contents_view.cc b/browser/ui/views/sidebar/sidebar_items_contents_view.cc index 916a71ca42f9..d7adcc066826 100644 --- a/browser/ui/views/sidebar/sidebar_items_contents_view.cc +++ b/browser/ui/views/sidebar/sidebar_items_contents_view.cc @@ -103,7 +103,7 @@ void SidebarItemsContentsView::Update() { } void SidebarItemsContentsView::UpdateAllBuiltInItemsViewState() { - const auto items = sidebar_model_->GetAllSidebarItems(); + const auto& items = sidebar_model_->GetAllSidebarItems(); // It's not initialized yet if child view count and items size are different. if (children().size() != items.size()) return; @@ -138,7 +138,7 @@ std::u16string SidebarItemsContentsView::GetTooltipTextFor( if (index == -1) return std::u16string(); - auto item = sidebar_model_->GetAllSidebarItems()[index]; + auto& item = sidebar_model_->GetAllSidebarItems()[index]; if (!item.title.empty()) return item.title; @@ -373,7 +373,7 @@ SidebarItemView* SidebarItemsContentsView::GetItemViewAt(int index) { } void SidebarItemsContentsView::UpdateItemViewStateAt(int index, bool active) { - const auto item = sidebar_model_->GetAllSidebarItems()[index]; + const auto& item = sidebar_model_->GetAllSidebarItems()[index]; SidebarItemView* item_view = GetItemViewAt(index); if (item.open_in_panel) diff --git a/components/sidebar/pref_names.h b/components/sidebar/pref_names.h index de8983151646..08d7355fd2a3 100644 --- a/components/sidebar/pref_names.h +++ b/components/sidebar/pref_names.h @@ -9,6 +9,8 @@ namespace sidebar { constexpr char kSidebarItems[] = "brave.sidebar.sidebar_items"; +constexpr char kSidebarHiddenBuiltInItems[] = + "brave.sidebar.hidden_built_in_items"; constexpr char kSidebarShowOption[] = "brave.sidebar.sidebar_show_option"; constexpr char kSidebarItemAddedFeedbackBubbleShowCount[] = "brave.sidebar.item_added_feedback_bubble_shown_count"; diff --git a/components/sidebar/sidebar_service.cc b/components/sidebar/sidebar_service.cc index 18c250ecf5f3..1a5d7dbb93c5 100644 --- a/components/sidebar/sidebar_service.cc +++ b/components/sidebar/sidebar_service.cc @@ -7,13 +7,20 @@ #include #include +#include #include +#include +#include "base/logging.h" +#include "base/no_destructor.h" +#include "base/notreached.h" +#include "base/ranges/algorithm.h" #include "base/strings/utf_string_conversions.h" #include "base/values.h" #include "brave/components/l10n/common/locale_util.h" #include "brave/components/sidebar/constants.h" #include "brave/components/sidebar/pref_names.h" +#include "brave/components/sidebar/sidebar_item.h" #include "components/grit/brave_components_strings.h" #include "components/prefs/pref_registry_simple.h" #include "components/prefs/pref_service.h" @@ -21,8 +28,6 @@ #include "ui/base/l10n/l10n_util.h" #include "ui/base/resource/resource_bundle.h" -#include "base/logging.h" - using version_info::Channel; namespace sidebar { @@ -70,36 +75,22 @@ SidebarItem GetBuiltInItemForType(SidebarItem::BuiltInItemType type) { return SidebarItem(); } -SidebarItem::BuiltInItemType GetBuiltInItemTypeForURL(const std::string& url) { - if (url == "https://together.brave.com/" || url == "https://talk.brave.com/") - return SidebarItem::BuiltInItemType::kBraveTalk; - - if (url == "chrome://wallet/") - return SidebarItem::BuiltInItemType::kWallet; - - if (url == "chrome://bookmarks/") - return SidebarItem::BuiltInItemType::kBookmarks; - - if (url == "chrome://history/") - return SidebarItem::BuiltInItemType::kHistory; - - NOTREACHED(); - return SidebarItem::BuiltInItemType::kNone; -} - -SidebarItem GetBuiltInItemForURL(const std::string& url) { - return GetBuiltInItemForType(GetBuiltInItemTypeForURL(url)); +const std::vector& GetDefaultBuiltInItemTypes() { + // This is the default display order + static const base::NoDestructor s_built_in_item_types( + std::vector{ + SidebarItem::BuiltInItemType::kBraveTalk, + SidebarItem::BuiltInItemType::kWallet, + SidebarItem::BuiltInItemType::kBookmarks, + SidebarItem::BuiltInItemType::kReadingList}); + return *s_built_in_item_types; } std::vector GetDefaultSidebarItems() { std::vector items; - items.push_back( - GetBuiltInItemForType(SidebarItem::BuiltInItemType::kBraveTalk)); - items.push_back(GetBuiltInItemForType(SidebarItem::BuiltInItemType::kWallet)); - items.push_back( - GetBuiltInItemForType(SidebarItem::BuiltInItemType::kBookmarks)); - items.push_back( - GetBuiltInItemForType(SidebarItem::BuiltInItemType::kReadingList)); + for (const auto& item_type : GetDefaultBuiltInItemTypes()) { + items.push_back(GetBuiltInItemForType(item_type)); + } return items; } @@ -109,6 +100,7 @@ std::vector GetDefaultSidebarItems() { void SidebarService::RegisterProfilePrefs(PrefRegistrySimple* registry, version_info::Channel channel) { registry->RegisterListPref(kSidebarItems); + registry->RegisterListPref(kSidebarHiddenBuiltInItems); registry->RegisterIntegerPref( kSidebarShowOption, channel == Channel::STABLE @@ -117,8 +109,16 @@ void SidebarService::RegisterProfilePrefs(PrefRegistrySimple* registry, registry->RegisterIntegerPref(kSidebarItemAddedFeedbackBubbleShowCount, 0); } +// static +std::vector +SidebarService::GetDefaultBuiltInItemTypes_ForTesting() { + return GetDefaultBuiltInItemTypes(); +} + SidebarService::SidebarService(PrefService* prefs) : prefs_(prefs) { DCHECK(prefs_); + MigratePrefSidebarBuiltInItemsToHidden(); + LoadSidebarItems(); MigrateSidebarShowOptions(); @@ -143,6 +143,97 @@ void SidebarService::MigrateSidebarShowOptions() { } } +void SidebarService::MigratePrefSidebarBuiltInItemsToHidden() { + // kSidebarItems pref used to contain built-in items which should be shown. + // This was changed to store those in a separate pref which contains + // built-in items the user has chosen to hide. However kSidebarItems still + // has entries for built-in items so they can be re-ordered. + // It only stores built-in items that should be hidden so that new + // items will appear, and we can remove old items. + auto* built_in_items_to_hide_preference = + prefs_->FindPreference(kSidebarHiddenBuiltInItems); + if (!built_in_items_to_hide_preference->IsDefaultValue()) { + VLOG(1) << "Not migrating built-in items, migration already complete."; + return; + } + auto* preference = prefs_->FindPreference(kSidebarItems); + if (preference->IsDefaultValue()) { + VLOG(1) << "Not migrating built-in items, pref is still default."; + return; + } + // Only include items that were known prior to this migration + std::vector built_in_items_to_hide; + built_in_items_to_hide.push_back( + GetBuiltInItemForType(SidebarItem::BuiltInItemType::kBraveTalk)); + built_in_items_to_hide.push_back( + GetBuiltInItemForType(SidebarItem::BuiltInItemType::kWallet)); + built_in_items_to_hide.push_back( + GetBuiltInItemForType(SidebarItem::BuiltInItemType::kBookmarks)); + + const auto& items = preference->GetValue()->GetList(); + VLOG(2) << "MigratePrefSidebarBuiltInItemsToHidden: item count is " + << items.size(); + + // Find built-in items in items pref and keep them visible + for (const auto& item : items) { + DVLOG(2) << "Found an item: " << item.DebugString(); + // Verify item is valid + if (!item.is_dict() || item.GetDict().empty()) { + DVLOG(1) << "Item in prefs was not a valid dict: " << item.DebugString(); + continue; + } + // Only care about built-in type + SidebarItem::Type type; + const auto type_value = item.FindIntKey(kSidebarItemTypeKey); + if (!type_value) { + VLOG(1) << "Item has no type item"; + continue; + } + type = static_cast(*type_value); + if (type != SidebarItem::Type::kTypeBuiltIn) { + VLOG(2) << "Item is not built-in type"; + continue; + } + // Found a built-in item to keep + const auto item_id = item.FindIntKey(kSidebarItemBuiltInItemTypeKey); + if (!item_id.has_value()) { + LOG(ERROR) << "MigratePrefSidebarBuiltInItemsToHidden: A built-in item " + "was found in the older pref format without a valid id."; + DVLOG(1) << "Pref list item was: " << item.DebugString(); + } + // Remember not to hide this item + auto iter = base::ranges::find_if( + built_in_items_to_hide, [&item_id](const auto& default_item) { + return default_item.built_in_item_type == + static_cast(*item_id); + }); + // It might be an item which is no longer is offered + if (iter != built_in_items_to_hide.end()) { + built_in_items_to_hide.erase(iter); + } else { + VLOG(1) << "A built-in item was found in the older pref format which is " + "no longer part of the default built-in items, id: " + << *item_id; + } + } + + // Build new pref, if any have been marked for hiding + ListPrefUpdate builtin_items_update(prefs_, kSidebarHiddenBuiltInItems); + if (built_in_items_to_hide.size()) { + for (const auto& item : built_in_items_to_hide) { + DCHECK(item.type == SidebarItem::Type::kTypeBuiltIn); + const auto value = static_cast(item.built_in_item_type); + VLOG(2) << "Marked for hiding built-in item with ID: " << value; + base::Value item_type(value); + builtin_items_update->Append(std::move(item_type)); + } + } else { + // Always store something so that we know migration is done + // when pref isn't default value. + builtin_items_update->ClearList(); + } +} + void SidebarService::AddItem(const SidebarItem& item) { items_.push_back(item); @@ -185,19 +276,45 @@ void SidebarService::MoveItem(int from, int to) { } void SidebarService::UpdateSidebarItemsToPrefStore() { + // Store all items in a list pref. + // Each item gets an entry. Built in items only need their type, and are + // only stored so we preserve their order. Custom items + // need all their detail. + // We also need to explicitly store which built-in items have been hidden + // so that we know which new items the user has been exposed to and which + // they've chosen to hide. ListPrefUpdate update(prefs_, kSidebarItems); update->ClearList(); + DVLOG(2) << "Serializing items (count: " << items_.size() << ")"; + // Serialize each item for (const auto& item : items_) { + DVLOG(2) << "Adding item to pref list: " + << static_cast(item.built_in_item_type); base::Value dict(base::Value::Type::DICTIONARY); - dict.SetStringKey(kSidebarItemURLKey, item.url.spec()); - dict.SetStringKey(kSidebarItemTitleKey, base::UTF16ToUTF8(item.title)); dict.SetIntKey(kSidebarItemTypeKey, static_cast(item.type)); dict.SetIntKey(kSidebarItemBuiltInItemTypeKey, static_cast(item.built_in_item_type)); - dict.SetBoolKey(kSidebarItemOpenInPanelKey, item.open_in_panel); + if (item.type != SidebarItem::Type::kTypeBuiltIn) { + dict.SetStringKey(kSidebarItemURLKey, item.url.spec()); + dict.SetStringKey(kSidebarItemTitleKey, base::UTF16ToUTF8(item.title)); + dict.SetBoolKey(kSidebarItemOpenInPanelKey, item.open_in_panel); + } update->Append(std::move(dict)); } + + // Store which built-in items should be hidden + ListPrefUpdate hide_builtin_update(prefs_, kSidebarHiddenBuiltInItems); + hide_builtin_update->ClearList(); + // TODO(petemill): If we make any hidden-by-default built-in items, + // then this logic needs to change to only consider shown-by-default items, + // and perhaps use a dict for each item to store whether built-in item is + // chosen to be added or removed. + auto hidden_items = GetHiddenDefaultSidebarItems(); + for (const auto& hidden_item : hidden_items) { + base::Value item_type(static_cast(hidden_item.built_in_item_type)); + hide_builtin_update->Append(std::move(item_type)); + } } void SidebarService::AddObserver(Observer* observer) { @@ -212,11 +329,11 @@ std::vector SidebarService::GetHiddenDefaultSidebarItems() const { auto default_items = GetDefaultSidebarItems(); const auto added_default_items = GetDefaultSidebarItemsFromCurrentItems(); for (const auto& added_item : added_default_items) { - auto iter = std::find_if(default_items.begin(), default_items.end(), - [added_item](const auto& default_item) { - return default_item.built_in_item_type == - added_item.built_in_item_type; - }); + auto iter = base::ranges::find_if( + default_items, [&added_item](const auto& default_item) { + return default_item.built_in_item_type == + added_item.built_in_item_type; + }); default_items.erase(iter); } return default_items; @@ -240,69 +357,109 @@ void SidebarService::SetSidebarShowOption(ShowSidebarOption show_options) { } void SidebarService::LoadSidebarItems() { - auto* preference = prefs_->FindPreference(kSidebarItems); - if (preference->IsDefaultValue()) { - items_ = GetDefaultSidebarItems(); - return; - } - - const auto& items = preference->GetValue()->GetList(); - for (const auto& item : items) { - SidebarItem::Type type; - if (const auto value = item.FindIntKey(kSidebarItemTypeKey)) { - type = static_cast(*value); - } else { - continue; - } + auto default_items_to_add = GetDefaultSidebarItems(); - std::string url; - if (const auto* value = item.FindStringKey(kSidebarItemURLKey)) { - url = *value; - } else { - continue; - } - - // Always use latest properties for built-in type item. - if (type == SidebarItem::Type::kTypeBuiltIn) { - SidebarItem built_in_item; - if (const auto value = item.FindIntKey(kSidebarItemBuiltInItemTypeKey)) { - built_in_item = GetBuiltInItemForType( - static_cast(*value)); + // Pref for custom items and custom order + auto* preference = prefs_->FindPreference(kSidebarItems); + if (!preference->IsDefaultValue()) { + const auto& items = preference->GetValue()->GetList(); + for (const auto& item : items) { + DVLOG(2) << "load: " << item.DebugString(); + SidebarItem::Type type; + if (const auto type_value = item.FindIntKey(kSidebarItemTypeKey)) { + type = static_cast(*type_value); } else { - // Fallback when built-in item type key is not existed. - built_in_item = GetBuiltInItemForURL(url); + continue; } - // Remove blocked item from existing users data. - if (!IsBlockedBuiltInItem(built_in_item)) - items_.push_back(built_in_item); - continue; - } - - bool open_in_panel; - if (const auto value = item.FindBoolKey(kSidebarItemOpenInPanelKey)) { - open_in_panel = *value; - } else { - continue; + // Always use latest properties for built-in type item. + if (type == SidebarItem::Type::kTypeBuiltIn) { + if (const auto value = + item.FindIntKey(kSidebarItemBuiltInItemTypeKey)) { + auto id = static_cast(*value); + auto iter = std::find_if( + default_items_to_add.begin(), default_items_to_add.end(), + [id](const auto& default_item) { + return default_item.built_in_item_type == id; + }); + // It might be an item which is no longer is offered as built-in + if (iter == default_items_to_add.end()) { + continue; + } + // Valid built-in item, add it + items_.emplace_back(*std::make_move_iterator(iter)); + default_items_to_add.erase(iter); + continue; + } + } + // Deserialize custom item + std::string url; + if (const auto* value = item.FindStringKey(kSidebarItemURLKey)) { + url = *value; + } else { + continue; + } + // Open in panel for custom items is not yet supported + bool open_in_panel = false; + std::string title; + if (const auto* value = item.FindStringKey(kSidebarItemTitleKey)) { + title = *value; + } + items_.push_back(SidebarItem::Create( + GURL(url), base::UTF8ToUTF16(title), type, + SidebarItem::BuiltInItemType::kNone, open_in_panel)); } + } - // Title can be updated later. - std::string title; - if (const auto* value = item.FindStringKey(kSidebarItemTitleKey)) { - title = *value; + // + // Add built-in items which haven't been shown or hidden. + // + // Don't consider built-in items that the user has already hidden. + auto* hidden_built_in_preference = + prefs_->FindPreference(kSidebarHiddenBuiltInItems); + if (!hidden_built_in_preference->IsDefaultValue()) { + for (const auto& item : hidden_built_in_preference->GetValue()->GetList()) { + // Don't show this built-in item + const auto id = static_cast(item.GetInt()); + DVLOG(2) << "hide built-in item with id: " << item.GetInt(); + auto iter = + std::find_if(default_items_to_add.begin(), default_items_to_add.end(), + [id](const auto& default_item) { + return default_item.built_in_item_type == + static_cast(id); + }); + if (iter != default_items_to_add.end()) { + default_items_to_add.erase(iter); + } else { + DLOG(ERROR) << "Asked to hide an item that was already asked to show. " + "This indicates something is wrong with the " + "serialization process. Id was: " + << item.GetInt(); + } } - - DCHECK(type != SidebarItem::Type::kTypeBuiltIn); - items_.push_back(SidebarItem::Create( - GURL(url), base::UTF8ToUTF16(title), type, - SidebarItem::BuiltInItemType::kNone, open_in_panel)); } -} -// For now, only builtin history item is blocked. -bool SidebarService::IsBlockedBuiltInItem(const SidebarItem& item) const { - if (!IsBuiltInType(item)) - return false; - return item.built_in_item_type == SidebarItem::BuiltInItemType::kHistory; + // Add the items the user has never seen (or never persisted). + // Get the initial order of items so that we can attempt to + // insert at the intended order. + auto default_item_types = GetDefaultBuiltInItemTypes(); + for (const auto& item : default_items_to_add) { + const auto& default_item_iter = + std::find(default_item_types.begin(), default_item_types.end(), + item.built_in_item_type); + auto default_index = default_item_iter - default_item_types.begin(); + // Add at the default index for the first time. For users which haven't + // changed any order, or removed items, this will be at the intentional + // index. For users who have re-ordered, this will be different but still + // acceptable. It will be a minority of cases where it gets inserted in to + // the middle of custom items, but that will still work. + auto index = std::min(static_cast(default_index), + static_cast(items_.size())); + VLOG(2) << "Inserting built-in item (" + << static_cast(item.built_in_item_type) + << " with default index: " << default_index + << " at actual index: " << index; + items_.insert(items_.begin() + index, std::move(item)); + } } void SidebarService::OnPreferenceChanged(const std::string& pref_name) { diff --git a/components/sidebar/sidebar_service.h b/components/sidebar/sidebar_service.h index 11e6fb3601f0..894c8c34b65a 100644 --- a/components/sidebar/sidebar_service.h +++ b/components/sidebar/sidebar_service.h @@ -50,7 +50,7 @@ class SidebarService : public KeyedService { explicit SidebarService(PrefService* prefs); ~SidebarService() override; - const std::vector items() const { return items_; } + const std::vector& items() const { return items_; } void AddItem(const SidebarItem& item); void RemoveItemAt(int index); @@ -69,13 +69,17 @@ class SidebarService : public KeyedService { private: FRIEND_TEST_ALL_PREFIXES(SidebarModelTest, ItemsChangedTest); FRIEND_TEST_ALL_PREFIXES(SidebarServiceTest, AddRemoveItems); + FRIEND_TEST_ALL_PREFIXES(SidebarServiceTest, NewDefaultItemAdded); + + static std::vector + GetDefaultBuiltInItemTypes_ForTesting(); void LoadSidebarItems(); void UpdateSidebarItemsToPrefStore(); std::vector GetDefaultSidebarItemsFromCurrentItems() const; void OnPreferenceChanged(const std::string& pref_name); - bool IsBlockedBuiltInItem(const SidebarItem& item) const; void MigrateSidebarShowOptions(); + void MigratePrefSidebarBuiltInItemsToHidden(); raw_ptr prefs_ = nullptr; std::vector items_; diff --git a/components/sidebar/sidebar_service_unittest.cc b/components/sidebar/sidebar_service_unittest.cc index 7ebb5dc5057b..d65730eec2d5 100644 --- a/components/sidebar/sidebar_service_unittest.cc +++ b/components/sidebar/sidebar_service_unittest.cc @@ -3,11 +3,13 @@ * 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 #include #include #include "brave/components/sidebar/constants.h" #include "brave/components/sidebar/pref_names.h" +#include "brave/components/sidebar/sidebar_item.h" #include "brave/components/sidebar/sidebar_service.h" #include "components/prefs/scoped_user_pref_update.h" #include "components/prefs/testing_pref_service.h" @@ -77,7 +79,7 @@ TEST_F(SidebarServiceTest, AddRemoveItems) { InitService(); // Check the default items count. - EXPECT_EQ(3UL, service_->items().size()); + EXPECT_EQ(4UL, service_->items().size()); EXPECT_EQ(0UL, service_->GetHiddenDefaultSidebarItems().size()); // Cache 1st item to compare after removing. @@ -89,7 +91,7 @@ TEST_F(SidebarServiceTest, AddRemoveItems) { EXPECT_EQ(-1, item_index_on_called_); service_->RemoveItemAt(0); - EXPECT_EQ(2UL, service_->items().size()); + EXPECT_EQ(3UL, service_->items().size()); EXPECT_EQ(1UL, service_->GetHiddenDefaultSidebarItems().size()); EXPECT_EQ(0, item_index_on_called_); EXPECT_TRUE(on_will_remove_item_called_); @@ -100,8 +102,8 @@ TEST_F(SidebarServiceTest, AddRemoveItems) { EXPECT_FALSE(on_item_added_called_); service_->AddItem(item); // New item will be added at last. - EXPECT_EQ(2, item_index_on_called_); - EXPECT_EQ(3UL, service_->items().size()); + EXPECT_EQ(3, item_index_on_called_); + EXPECT_EQ(4UL, service_->items().size()); EXPECT_EQ(0UL, service_->GetHiddenDefaultSidebarItems().size()); EXPECT_TRUE(on_item_added_called_); ClearState(); @@ -111,10 +113,10 @@ TEST_F(SidebarServiceTest, AddRemoveItems) { SidebarItem::Type::kTypeWeb, SidebarItem::BuiltInItemType::kNone, true); EXPECT_TRUE(IsWebType(item2)); service_->AddItem(item2); - EXPECT_EQ(3, item_index_on_called_); - EXPECT_EQ(4UL, service_->items().size()); + EXPECT_EQ(4, item_index_on_called_); + EXPECT_EQ(5UL, service_->items().size()); // Default item count is not changed. - EXPECT_EQ(3UL, service_->GetDefaultSidebarItemsFromCurrentItems().size()); + EXPECT_EQ(4UL, service_->GetDefaultSidebarItemsFromCurrentItems().size()); } TEST_F(SidebarServiceTest, MoveItem) { @@ -125,7 +127,7 @@ TEST_F(SidebarServiceTest, MoveItem) { SidebarItem new_item; new_item.url = GURL("https://brave.com"); service_->AddItem(new_item); - EXPECT_EQ(4UL, service_->items().size()); + EXPECT_EQ(5UL, service_->items().size()); // Move item at 0 to index 2. SidebarItem item = service_->items()[0]; @@ -149,10 +151,343 @@ TEST_F(SidebarServiceTest, MoveItem) { EXPECT_EQ(item.url, service_->items()[1].url); } +TEST_F(SidebarServiceTest, MoveItemSavedToPrefs) { + SidebarService::RegisterProfilePrefs(prefs_.registry(), Channel::DEV); + InitService(); + + // Add one more item to test with 4 items. + SidebarItem new_item; + new_item.url = GURL("https://brave.com"); + service_->AddItem(new_item); + EXPECT_EQ(5UL, service_->items().size()); + + // Move item at 0 to index 2. + SidebarItem item = service_->items()[0]; + GURL url = item.url; + service_->MoveItem(0, 2); + EXPECT_TRUE(on_item_moved_called_); + service_.reset(); + InitService(); + EXPECT_EQ(5UL, service_->items().size()); + EXPECT_EQ(url, service_->items()[2].url); +} + +TEST_F(SidebarServiceTest, HideBuiltInItem) { + SidebarService::RegisterProfilePrefs(prefs_.registry(), Channel::DEV); + // Have prefs which contain a custom item and hides 1 built-in item + { + ListPrefUpdate update(&prefs_, sidebar::kSidebarHiddenBuiltInItems); + update->ClearList(); + update->Append(static_cast(SidebarItem::BuiltInItemType::kBookmarks)); + } + { + ListPrefUpdate update(&prefs_, sidebar::kSidebarItems); + base::Value dict(base::Value::Type::DICTIONARY); + dict.SetStringKey(sidebar::kSidebarItemURLKey, + "https://custom1.brave.com/"); + dict.SetStringKey(sidebar::kSidebarItemTitleKey, "Custom Item 1"); + dict.SetIntKey(sidebar::kSidebarItemTypeKey, + static_cast(SidebarItem::Type::kTypeWeb)); + dict.SetBoolKey(sidebar::kSidebarItemOpenInPanelKey, false); + update->Append(std::move(dict)); + } + + InitService(); + // None of the items should be the hidden one + auto items = service_->items(); + auto bookmark_iter = + std::find_if(items.begin(), items.end(), [](const auto& item) { + return (item.built_in_item_type == + SidebarItem::BuiltInItemType::kBookmarks); + }); + EXPECT_EQ(bookmark_iter, items.end()); + // Check serialization also perists that + service_.reset(); + InitService(); + items = service_->items(); + bookmark_iter = + std::find_if(items.begin(), items.end(), [](const auto& item) { + return (item.built_in_item_type == + SidebarItem::BuiltInItemType::kBookmarks); + }); + EXPECT_EQ(bookmark_iter, items.end()); +} + +TEST_F(SidebarServiceTest, NewDefaultItemAdded) { + SidebarService::RegisterProfilePrefs(prefs_.registry(), Channel::DEV); + // Have prefs which contain a custom item and hides 1 built-in item + { + ListPrefUpdate update(&prefs_, sidebar::kSidebarHiddenBuiltInItems); + update->ClearList(); + update->Append(static_cast(SidebarItem::BuiltInItemType::kBookmarks)); + } + { + ListPrefUpdate update(&prefs_, sidebar::kSidebarItems); + base::Value dict(base::Value::Type::DICTIONARY); + dict.SetStringKey(sidebar::kSidebarItemURLKey, + "https://custom1.brave.com/"); + dict.SetStringKey(sidebar::kSidebarItemTitleKey, "Custom Item 1"); + dict.SetIntKey(sidebar::kSidebarItemTypeKey, + static_cast(SidebarItem::Type::kTypeWeb)); + dict.SetBoolKey(sidebar::kSidebarItemOpenInPanelKey, false); + update->Append(std::move(dict)); + } + + InitService(); + // Make sure other default items are present. They are considered not seen yet + // since kSidebarItems was not default value and did not contain them. + // None of the items should be the hidden one + auto items = service_->items(); + auto bookmark_iter = + std::find_if(items.begin(), items.end(), [](const auto& item) { + return (item.built_in_item_type == + SidebarItem::BuiltInItemType::kBookmarks); + }); + EXPECT_EQ(bookmark_iter, items.end()); + // All other default items should be present even though not present + // in kSidebarItems pref. + std::vector remaining_default_items{ + SidebarItem::BuiltInItemType::kBraveTalk, + SidebarItem::BuiltInItemType::kWallet, + SidebarItem::BuiltInItemType::kReadingList, + }; + // Get expected indexes (the custom item will replace the index of the removed + // built-in). + auto all_default_item_types = + SidebarService::GetDefaultBuiltInItemTypes_ForTesting(); + // There should also be the custom item we added + EXPECT_EQ(remaining_default_items.size() + 1, service_->items().size()); + for (auto built_in_type : remaining_default_items) { + auto iter = std::find_if( + items.begin(), items.end(), [built_in_type](const auto& item) { + return (item.built_in_item_type == built_in_type); + }); + EXPECT_NE(iter, items.end()); + auto expected_index = + std::find(all_default_item_types.begin(), all_default_item_types.end(), + built_in_type) - + all_default_item_types.begin(); + auto index = iter - items.begin(); + EXPECT_EQ(expected_index, index) + << "New item with ID " << static_cast(built_in_type) + << " was inserted at expected index"; + } +} + +TEST_F(SidebarServiceTest, MigratePrefSidebarBuiltInItemsSomeHidden) { + SidebarService::RegisterProfilePrefs(prefs_.registry(), Channel::BETA); + // Make prefs already have old-style builtin items before service + // initialization. + { + ListPrefUpdate update(&prefs_, sidebar::kSidebarItems); + update->ClearList(); + + base::Value dict(base::Value::Type::DICTIONARY); + dict.SetStringKey(sidebar::kSidebarItemURLKey, + "https://anything.brave.com/"); + dict.SetStringKey(sidebar::kSidebarItemTitleKey, "Anything"); + dict.SetIntKey(sidebar::kSidebarItemTypeKey, + static_cast(SidebarItem::Type::kTypeBuiltIn)); + dict.SetIntKey(sidebar::kSidebarItemBuiltInItemTypeKey, + static_cast(SidebarItem::BuiltInItemType::kBraveTalk)); + dict.SetBoolKey(sidebar::kSidebarItemOpenInPanelKey, true); + update->Append(std::move(dict)); + } + + InitService(); + + // Verify service has migrated the "include" of kBraveTalk to be the exclude + // of all other built-in items that were known prior to this migration and are + // still available. + auto* preference = prefs_.FindPreference(kSidebarItems); + auto* hidden_preference = prefs_.FindPreference(kSidebarHiddenBuiltInItems); + EXPECT_EQ(1UL, preference->GetValue()->GetList().size()) + << "Still contains built-in item in original preference, for ordering " + "purposes"; + EXPECT_EQ(2UL, hidden_preference->GetValue()->GetList().size()) + << "Migration resulted in hiding the expected number of items"; + // Verify expected items + EXPECT_EQ(2UL, service_->items().size()); + auto items = service_->items(); + auto talk_iter = + std::find_if(items.begin(), items.end(), [](const auto& item) { + return (item.built_in_item_type == + SidebarItem::BuiltInItemType::kBraveTalk); + }); + auto reading_iter = + std::find_if(items.begin(), items.end(), [](const auto& item) { + return (item.built_in_item_type == + SidebarItem::BuiltInItemType::kReadingList); + }); + EXPECT_NE(talk_iter, items.end()); + EXPECT_NE(reading_iter, items.end()); + // Check service has updated built-in item. Previously url was incorrect. This + // check is to make sure that we don't re-introduce code which stores the URL + // for built-in items. + auto talk_item = *talk_iter; + EXPECT_EQ(talk_item.url, kBraveTalkURL); +} + +TEST_F(SidebarServiceTest, MigratePrefSidebarBuiltInItemsNoneHidden) { + SidebarService::RegisterProfilePrefs(prefs_.registry(), Channel::BETA); + // Make prefs already have ALL old-style builtin items before service + // initialization. This tests that when not adding anything to the new pref + // that re-migration will not break anything. + // Also add a custom item so that the main items pref is not default value. + { + ListPrefUpdate update(&prefs_, sidebar::kSidebarItems); + update->ClearList(); + + std::vector hideable_types{ + SidebarItem::BuiltInItemType::kBraveTalk, + SidebarItem::BuiltInItemType::kWallet, + SidebarItem::BuiltInItemType::kBookmarks, + }; + + for (const auto& built_in_type : hideable_types) { + base::Value dict(base::Value::Type::DICTIONARY); + dict.SetStringKey(sidebar::kSidebarItemURLKey, + "https://anything.brave.com/"); + dict.SetStringKey(sidebar::kSidebarItemTitleKey, "Anything"); + dict.SetIntKey(sidebar::kSidebarItemTypeKey, + static_cast(SidebarItem::Type::kTypeBuiltIn)); + dict.SetIntKey(sidebar::kSidebarItemBuiltInItemTypeKey, + static_cast(built_in_type)); + dict.SetBoolKey(sidebar::kSidebarItemOpenInPanelKey, true); + update->Append(std::move(dict)); + } + + base::Value dict(base::Value::Type::DICTIONARY); + dict.SetStringKey(sidebar::kSidebarItemURLKey, + "https://custom1.brave.com/"); + dict.SetStringKey(sidebar::kSidebarItemTitleKey, "Custom Item 1"); + dict.SetIntKey(sidebar::kSidebarItemTypeKey, + static_cast(SidebarItem::Type::kTypeWeb)); + dict.SetBoolKey(sidebar::kSidebarItemOpenInPanelKey, false); + update->Append(std::move(dict)); + } + + InitService(); + + // Verify service has migrated the "include" of kBraveTalk to be the exclude + // of all other built-in items that were known prior to this migration and are + // still available. + auto* preference = prefs_.FindPreference(kSidebarItems); + auto* hidden_preference = prefs_.FindPreference(kSidebarHiddenBuiltInItems); + EXPECT_EQ(4UL, preference->GetValue()->GetList().size()) + << "Migration still contains built-in items, for ordering purposes"; + EXPECT_EQ(0UL, hidden_preference->GetValue()->GetList().size()) + << "Migration did not result in hiding any built-in items"; + // kSidebarHiddenBuiltInItems being non-default is how we know migration is + // already done. + EXPECT_FALSE(hidden_preference->IsDefaultValue()); + // kSidebarItems should still have custom item + EXPECT_FALSE(preference->IsDefaultValue()); + // Verify expected items = items in pref plus new item (reading list) + EXPECT_EQ(5UL, service_->items().size()); + // Simulate re-launch and check service has still updated items. + // This tests re-migration doesn't occur and hide all the hideable built-in + // items. + service_.reset(); + InitService(); + // Prefs still haven't been re-serialized, so don't contain new items + EXPECT_EQ(4UL, preference->GetValue()->GetList().size()); + EXPECT_EQ(0UL, hidden_preference->GetValue()->GetList().size()); + EXPECT_FALSE(hidden_preference->IsDefaultValue()); + EXPECT_FALSE(preference->IsDefaultValue()); + EXPECT_EQ(5UL, service_->items().size()); + + // Check again after service updates prefs. Force serialization by performing + // a move operation (and move it back). + service_->MoveItem(0, 1); + service_->MoveItem(0, 1); + service_.reset(); + InitService(); + // Pref now includes new default items added after migration (ReadingList), + // so size has increased by 1. + EXPECT_EQ(5UL, preference->GetValue()->GetList().size()); + EXPECT_EQ(0UL, hidden_preference->GetValue()->GetList().size()); + EXPECT_FALSE(hidden_preference->IsDefaultValue()); + EXPECT_FALSE(preference->IsDefaultValue()); + EXPECT_EQ(5UL, service_->items().size()); + // Verify that new a new item not contained in prefs was added at correct + // index. + auto items = service_->items(); + auto iter = std::find_if(items.begin(), items.end(), [](const auto& item) { + return item.built_in_item_type == + SidebarItem::BuiltInItemType::kReadingList; + }); + auto index = iter - items.begin(); + EXPECT_EQ(3, index); +} + +TEST_F(SidebarServiceTest, HidesBuiltInItemsViaPref) { + SidebarService::RegisterProfilePrefs(prefs_.registry(), Channel::BETA); + // Verify default state + InitService(); + auto items = service_->items(); + auto bookmarks_iter = + std::find_if(items.begin(), items.end(), [](const auto& item) { + return (item.built_in_item_type == + SidebarItem::BuiltInItemType::kBookmarks); + }); + EXPECT_NE(bookmarks_iter, items.end()); + + // Update pref to hide bookmarks item + // Make prefs already have old-style builtin items before service + // initialization. + { + ListPrefUpdate update(&prefs_, sidebar::kSidebarHiddenBuiltInItems); + update->ClearList(); + update->Append(static_cast(SidebarItem::BuiltInItemType::kBookmarks)); + } + + // Verify new state doesn't include bookmarks item + InitService(); + items = service_->items(); + auto bookmarks_iter_removed = + std::find_if(items.begin(), items.end(), [](const auto& item) { + return (item.built_in_item_type == + SidebarItem::BuiltInItemType::kBookmarks); + }); + EXPECT_EQ(bookmarks_iter_removed, items.end()); +} + +TEST_F(SidebarServiceTest, HidesBuiltInItemsViaService) { + SidebarService::RegisterProfilePrefs(prefs_.registry(), Channel::BETA); + // Verify default state + InitService(); + auto items = service_->items(); + auto bookmark_item_index = 0u; + bool found = false; + while (bookmark_item_index < items.size()) { + auto item = items[bookmark_item_index]; + if (item.built_in_item_type == SidebarItem::BuiltInItemType::kBookmarks) { + found = true; + break; + } + bookmark_item_index++; + } + EXPECT_TRUE(found); + // Update service to hide bookmarks item + service_->RemoveItemAt(bookmark_item_index); + + // Verify new state doesn't include bookmarks item + service_.reset(); + InitService(); + items = service_->items(); + auto bookmarks_iter_removed = + std::find_if(items.begin(), items.end(), [](const auto& item) { + return (item.built_in_item_type == + SidebarItem::BuiltInItemType::kBookmarks); + }); + EXPECT_EQ(bookmarks_iter_removed, items.end()); +} + TEST_F(SidebarServiceTest, BuiltInItemUpdateTestWithBuiltInItemTypeKey) { SidebarService::RegisterProfilePrefs(prefs_.registry(), Channel::BETA); // Make prefs already have builtin items before service initialization. - // And it has old url. + // And it has old url in old pref format (storing built-in items). { ListPrefUpdate update(&prefs_, sidebar::kSidebarItems); update->ClearList(); @@ -172,7 +507,7 @@ TEST_F(SidebarServiceTest, BuiltInItemUpdateTestWithBuiltInItemTypeKey) { InitService(); // Check service has updated built-in item. Previously url was deprecated.xxx. - EXPECT_EQ(1UL, service_->items().size()); + EXPECT_EQ(2UL, service_->items().size()); EXPECT_EQ(GURL(kBraveTalkURL), service_->items()[0].url); // Simulate re-launch and check service has still updated items. @@ -182,7 +517,7 @@ TEST_F(SidebarServiceTest, BuiltInItemUpdateTestWithBuiltInItemTypeKey) { InitService(); // Check service has updated built-in item. Previously url was deprecated.xxx. - EXPECT_EQ(1UL, service_->items().size()); + EXPECT_EQ(2UL, service_->items().size()); EXPECT_EQ(GURL(kBraveTalkURL), service_->items()[0].url); } @@ -208,42 +543,16 @@ TEST_F(SidebarServiceTest, BuiltInItemDoesntHaveHistoryItem) { InitService(); // Check service doesn't have history item. - EXPECT_EQ(0UL, service_->items().size()); - + for (const auto& item : service_->items()) { + EXPECT_NE(SidebarItem::BuiltInItemType::kHistory, item.built_in_item_type); + } // Make sure history is not included in the not added default items list. auto not_added_default_items = service_->GetHiddenDefaultSidebarItems(); - EXPECT_EQ(3UL, not_added_default_items.size()); for (const auto& item : not_added_default_items) { EXPECT_NE(SidebarItem::BuiltInItemType::kHistory, item.built_in_item_type); } } -TEST_F(SidebarServiceTest, BuiltInItemUpdateTestWithoutBuiltInItemTypeKey) { - SidebarService::RegisterProfilePrefs(prefs_.registry(), Channel::STABLE); - // Prepare built-in item in prefs w/o setting BuiltInItemType. - // If not stored, service uses url to get proper latest properties. - { - ListPrefUpdate update(&prefs_, sidebar::kSidebarItems); - update->ClearList(); - - base::Value dict(base::Value::Type::DICTIONARY); - dict.SetStringKey(sidebar::kSidebarItemURLKey, - "https://together.brave.com/"); - dict.SetStringKey(sidebar::kSidebarItemTitleKey, "Brave together"); - dict.SetIntKey(sidebar::kSidebarItemTypeKey, - static_cast(SidebarItem::Type::kTypeBuiltIn)); - dict.SetBoolKey(sidebar::kSidebarItemOpenInPanelKey, true); - update->Append(std::move(dict)); - } - - InitService(); - - // Check item type is set properly. - EXPECT_EQ(1UL, service_->items().size()); - EXPECT_EQ(SidebarItem::BuiltInItemType::kBraveTalk, - service_->items()[0].built_in_item_type); -} - TEST_F(SidebarServiceTest, SidebarShowOptionsDeprecationTest) { SidebarService::RegisterProfilePrefs(prefs_.registry(), Channel::STABLE); // Show on click is deprecated. From fd2a6b3bb1bd54d1e70593a4bd1194ca16d78654 Mon Sep 17 00:00:00 2001 From: Pete Miller Date: Mon, 25 Jul 2022 21:08:55 +0100 Subject: [PATCH 3/3] Toolbar SidePanel button only opens items added on the sidebar, and hides if none are added --- .../views/sidebar/sidebar_container_view.cc | 23 ++++++++++++++ .../ui/views/sidebar/sidebar_container_view.h | 5 +++ .../side_panel/side_panel_coordinator.cc | 31 +++++++++++++++++++ components/sidebar/sidebar_service.cc | 20 ++++++++++++ components/sidebar/sidebar_service.h | 3 ++ ...side_panel-side_panel_coordinator.cc.patch | 12 +++++-- 6 files changed, 92 insertions(+), 2 deletions(-) diff --git a/browser/ui/views/sidebar/sidebar_container_view.cc b/browser/ui/views/sidebar/sidebar_container_view.cc index 1258f8a5e4b8..16678f1ee021 100644 --- a/browser/ui/views/sidebar/sidebar_container_view.cc +++ b/browser/ui/views/sidebar/sidebar_container_view.cc @@ -24,6 +24,7 @@ #include "chrome/browser/ui/browser_window.h" #include "chrome/browser/ui/views/frame/browser_view.h" #include "chrome/browser/ui/views/side_panel/side_panel_entry.h" +#include "chrome/browser/ui/views/toolbar/toolbar_view.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/native_web_keyboard_event.h" #include "content/public/browser/web_contents.h" @@ -115,6 +116,7 @@ void SidebarContainerView::Init() { AddChildViews(); // Hide by default. Visibility will be controlled by show options later. DoHideSidebar(false); + UpdateToolbarButtonVisibility(); } void SidebarContainerView::SetSidebarShowOption( @@ -278,6 +280,16 @@ void SidebarContainerView::OnActiveIndexChanged(int old_index, int new_index) { InvalidateLayout(); } +void SidebarContainerView::OnItemAdded(const sidebar::SidebarItem& item, + int index, + bool user_gesture) { + UpdateToolbarButtonVisibility(); +} + +void SidebarContainerView::OnItemRemoved(int index) { + UpdateToolbarButtonVisibility(); +} + SidebarShowOptionsEventDetectWidget* SidebarContainerView::GetEventDetectWidget() { if (!show_options_widget_) { @@ -322,6 +334,17 @@ void SidebarContainerView::DoHideSidebar(bool show_event_detect_widget) { InvalidateLayout(); } +void SidebarContainerView::UpdateToolbarButtonVisibility() { + // Coordinate sidebar toolbar button visibility based on + // whether there are any sibar items with a sidepanel. + // This is similar to how chromium's side_panel_coordinator View + // also has some control on the toolbar button. + auto has_panel_item = + GetSidebarService(browser_)->GetDefaultPanelItem().has_value(); + auto* browser_view = BrowserView::GetBrowserViewForBrowser(browser_); + browser_view->toolbar()->side_panel_button()->SetVisible(has_panel_item); +} + void SidebarContainerView::StartBrowserWindowEventMonitoring() { if (browser_window_event_monitor_) return; diff --git a/browser/ui/views/sidebar/sidebar_container_view.h b/browser/ui/views/sidebar/sidebar_container_view.h index 8544eed05d6c..6a32f9b59d7e 100644 --- a/browser/ui/views/sidebar/sidebar_container_view.h +++ b/browser/ui/views/sidebar/sidebar_container_view.h @@ -79,7 +79,11 @@ class SidebarContainerView void ShowSidebar() override; // sidebar::SidebarModel::Observer overrides: + void OnItemAdded(const sidebar::SidebarItem& item, + int index, + bool user_gesture) override; void OnActiveIndexChanged(int old_index, int new_index) override; + void OnItemRemoved(int index) override; // SidePanelEntryObserver: void OnEntryShown(SidePanelEntry* entry) override; @@ -103,6 +107,7 @@ class SidebarContainerView void ShowSidebar(bool show_sidebar, bool show_event_detect_widget); SidebarShowOptionsEventDetectWidget* GetEventDetectWidget(); bool ShouldForceShowSidebar() const; + void UpdateToolbarButtonVisibility(); // On some condition(ex, add item bubble is visible), // sidebar should not be hidden even if mouse goes out from sidebar ui. diff --git a/chromium_src/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc b/chromium_src/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc index 961e350cfa83..28acca3d55d2 100644 --- a/chromium_src/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc +++ b/chromium_src/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc @@ -3,8 +3,39 @@ // 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/sidebar/sidebar_service_factory.h" +#include "brave/browser/ui/views/sidebar/sidebar_side_panel_utils.h" +#include "brave/components/sidebar/sidebar_service.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/ui/views/side_panel/side_panel_entry.h" +#include "third_party/abseil-cpp/absl/types/optional.h" + +namespace { + +absl::optional GetDefaultEntryId(Profile* profile) { + auto* service = sidebar::SidebarServiceFactory::GetForProfile(profile); + auto panel_item = service->GetDefaultPanelItem(); + if (panel_item.has_value()) { + return SidePanelIdFromSideBarItem(panel_item.value()); + } + return absl::nullopt; +} + +} // namespace + // Brave has its own side panel navigation in the form of the SideBar, so // hide the Chromium combobox-style header. #define BRAVE_SIDE_PANEL_COORDINATOR_CREATE_HEADER header->SetVisible(false); + +// Choose Brave's own default, and exclude items that user has removed +// from sidebar. If none are enabled, do nothing. +#define BRAVE_SIDE_PANEL_COORDINATOR_SHOW \ + if (!entry_id.has_value()) { \ + entry_id = GetDefaultEntryId(browser_view_->GetProfile()); \ + if (!entry_id.has_value()) \ + return; \ + } + #include "src/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc" #undef BRAVE_SIDE_PANEL_COORDINATOR_CREATE_HEADER +#undef BRAVE_SIDE_PANEL_COORDINATOR_SHOW diff --git a/components/sidebar/sidebar_service.cc b/components/sidebar/sidebar_service.cc index 1a5d7dbb93c5..1a05a2ce259b 100644 --- a/components/sidebar/sidebar_service.cc +++ b/components/sidebar/sidebar_service.cc @@ -25,6 +25,7 @@ #include "components/prefs/pref_registry_simple.h" #include "components/prefs/pref_service.h" #include "components/prefs/scoped_user_pref_update.h" +#include "third_party/abseil-cpp/absl/types/optional.h" #include "ui/base/l10n/l10n_util.h" #include "ui/base/resource/resource_bundle.h" @@ -351,6 +352,25 @@ SidebarService::ShowSidebarOption SidebarService::GetSidebarShowOption() const { return static_cast(prefs_->GetInteger(kSidebarShowOption)); } +absl::optional SidebarService::GetDefaultPanelItem() const { + static const base::NoDestructor preferred_item_types( + std::vector{ + SidebarItem::BuiltInItemType::kReadingList, + SidebarItem::BuiltInItemType::kBookmarks}); + absl::optional default_item; + for (const auto& type : *preferred_item_types) { + auto found_item_iter = base::ranges::find_if( + items_, + [type](SidebarItem item) { return (item.built_in_item_type == type); }); + if (found_item_iter != items_.end()) { + default_item = *found_item_iter; + DCHECK_EQ(default_item->open_in_panel, true); + break; + } + } + return default_item; +} + void SidebarService::SetSidebarShowOption(ShowSidebarOption show_options) { DCHECK_NE(ShowSidebarOption::kShowOnClick, show_options); prefs_->SetInteger(kSidebarShowOption, static_cast(show_options)); diff --git a/components/sidebar/sidebar_service.h b/components/sidebar/sidebar_service.h index 894c8c34b65a..9a4446c16dc8 100644 --- a/components/sidebar/sidebar_service.h +++ b/components/sidebar/sidebar_service.h @@ -16,6 +16,7 @@ #include "components/keyed_service/core/keyed_service.h" #include "components/prefs/pref_change_registrar.h" #include "components/version_info/channel.h" +#include "third_party/abseil-cpp/absl/types/optional.h" class PrefRegistrySimple; class PrefService; @@ -63,6 +64,8 @@ class SidebarService : public KeyedService { ShowSidebarOption GetSidebarShowOption() const; void SetSidebarShowOption(ShowSidebarOption show_options); + absl::optional GetDefaultPanelItem() const; + SidebarService(const SidebarService&) = delete; SidebarService& operator=(const SidebarService&) = delete; diff --git a/patches/chrome-browser-ui-views-side_panel-side_panel_coordinator.cc.patch b/patches/chrome-browser-ui-views-side_panel-side_panel_coordinator.cc.patch index 51895ceee4d3..cc8b5ff6c04d 100644 --- a/patches/chrome-browser-ui-views-side_panel-side_panel_coordinator.cc.patch +++ b/patches/chrome-browser-ui-views-side_panel-side_panel_coordinator.cc.patch @@ -1,8 +1,16 @@ diff --git a/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc b/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc -index 1efe9453a6d271fdb118f39f95630296de13a95e..3353a5aba43a3e8f36766c75e3adc16bbb34f314 100644 +index 1efe9453a6d271fdb118f39f95630296de13a95e..ef134beaf3bf407580e6fa84a3bbc82f6856984a 100644 --- a/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc +++ b/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc -@@ -419,6 +419,7 @@ std::unique_ptr SidePanelCoordinator::CreateHeader() { +@@ -180,6 +180,7 @@ SidePanelCoordinator::~SidePanelCoordinator() { + } + + void SidePanelCoordinator::Show(absl::optional entry_id) { ++ BRAVE_SIDE_PANEL_COORDINATOR_SHOW + if (!entry_id.has_value()) + entry_id = GetLastActiveEntryId().value_or(kDefaultEntry); + +@@ -419,6 +420,7 @@ std::unique_ptr SidePanelCoordinator::CreateHeader() { ChromeLayoutProvider::Get()->GetDistanceMetric( ChromeDistanceMetric::DISTANCE_SIDE_PANEL_HEADER_VECTOR_ICON_SIZE)));