Skip to content

Commit

Permalink
Uplift of #25679 (squashed) to beta
Browse files Browse the repository at this point in the history
  • Loading branch information
brave-builds committed Oct 11, 2024
1 parent 26a07e5 commit 8a69005
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 9 deletions.
39 changes: 31 additions & 8 deletions browser/ui/sidebar/sidebar_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -314,14 +314,14 @@ IN_PROC_BROWSER_TEST_F(SidebarBrowserTest, BasicTest) {
EXPECT_EQ(expected_count, model()->GetAllSidebarItems().size());
// Activate item that opens in panel.
const size_t first_panel_item_index = GetFirstPanelItemIndex();
controller()->ActivateItemAt(first_panel_item_index);
const auto& first_panel_item =
controller()->model()->GetAllSidebarItems()[first_panel_item_index];
controller()->ActivatePanelItem(first_panel_item.built_in_item_type);
WaitUntil(
base::BindLambdaForTesting([&]() { return !!model()->active_index(); }));
EXPECT_THAT(model()->active_index(), Optional(first_panel_item_index));
EXPECT_TRUE(controller()->IsActiveIndex(first_panel_item_index));

// Try to activate item at index 1.
// Default item at index 1 opens in new tab. So, sidebar active index is not
// changed. Still active index is 2.

// Get first index of item that opens in panel.
const size_t first_web_item_index = GetFirstWebItemIndex();
const auto item = model()->GetAllSidebarItems()[first_web_item_index];
Expand All @@ -331,10 +331,15 @@ IN_PROC_BROWSER_TEST_F(SidebarBrowserTest, BasicTest) {
EXPECT_THAT(model()->active_index(), Optional(active_item_index));

// Setting std::nullopt means deactivate current active tab.
controller()->ActivateItemAt(std::nullopt);
controller()->DeactivateCurrentPanel();
WaitUntil(
base::BindLambdaForTesting([&]() { return !model()->active_index(); }));
EXPECT_THAT(model()->active_index(), Eq(std::nullopt));

controller()->ActivateItemAt(active_item_index);
controller()->ActivatePanelItem(first_panel_item.built_in_item_type);
WaitUntil(
base::BindLambdaForTesting([&]() { return !!model()->active_index(); }));
EXPECT_THAT(model()->active_index(), Optional(active_item_index));

auto* sidebar_service =
SidebarServiceFactory::GetForProfile(browser()->profile());
Expand Down Expand Up @@ -1109,7 +1114,24 @@ IN_PROC_BROWSER_TEST_F(SidebarBrowserTestWithAIChat, TabSpecificPanel) {
ASSERT_TRUE(global_item_index.has_value());
auto tab_specific_item_index = model()->GetIndexOf(kTabSpecificItemType);
ASSERT_TRUE(tab_specific_item_index.has_value());
// Open 2 more tabs

ASSERT_TRUE(ui_test_utils::NavigateToURLWithDisposition(
browser(), GURL("brave://newtab/"),
WindowOpenDisposition::NEW_FOREGROUND_TAB,
ui_test_utils::BROWSER_TEST_WAIT_FOR_LOAD_STOP));
ASSERT_EQ(tab_model()->GetTabCount(), 2);

// Open contextual panel from Tab 0.
tab_model()->ActivateTabAt(0);
SimulateSidebarItemClickAt(tab_specific_item_index.value());
EXPECT_EQ(model()->active_index(), tab_specific_item_index);

// Delete Tab 0 and check model doesn't have active index.
tab_model()->DetachAndDeleteWebContentsAt(0);
EXPECT_FALSE(!!model()->active_index());
ASSERT_EQ(tab_model()->GetTabCount(), 1);

// Create two more tab for test below.
ASSERT_TRUE(ui_test_utils::NavigateToURLWithDisposition(
browser(), GURL("brave://newtab/"),
WindowOpenDisposition::NEW_FOREGROUND_TAB,
Expand All @@ -1119,6 +1141,7 @@ IN_PROC_BROWSER_TEST_F(SidebarBrowserTestWithAIChat, TabSpecificPanel) {
WindowOpenDisposition::NEW_FOREGROUND_TAB,
ui_test_utils::BROWSER_TEST_WAIT_FOR_LOAD_STOP));
ASSERT_EQ(tab_model()->GetTabCount(), 3);

// Open a "global" panel from Tab 0
tab_model()->ActivateTabAt(0);
// Tab changed flag should be cleared after ActivateTabAt() executed.
Expand Down
12 changes: 12 additions & 0 deletions browser/ui/sidebar/sidebar_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,18 @@ void SidebarController::AddItemWithCurrentTab() {
SidebarItem::BuiltInItemType::kNone, false));
}

void SidebarController::UpdateActiveItemState(
std::optional<SidebarItem::BuiltInItemType> active_panel_item) {
if (!active_panel_item) {
ActivateItemAt(std::nullopt);
return;
}

if (auto index = sidebar_model_->GetIndexOf(*active_panel_item)) {
ActivateItemAt(*index);
}
}

void SidebarController::SetSidebar(Sidebar* sidebar) {
DCHECK(!sidebar_);
// |sidebar| can be null in unit test.
Expand Down
2 changes: 2 additions & 0 deletions browser/ui/sidebar/sidebar_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ class SidebarController : public SidebarService::Observer {
std::optional<size_t> index,
WindowOpenDisposition disposition = WindowOpenDisposition::CURRENT_TAB);
void AddItemWithCurrentTab();
void UpdateActiveItemState(std::optional<SidebarItem::BuiltInItemType>
active_panel_item = std::nullopt);

// Ask panel item activation state change to SidePanelUI.
void ActivatePanelItem(SidebarItem::BuiltInItemType panel_item);
Expand Down
18 changes: 18 additions & 0 deletions browser/ui/sidebar/sidebar_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,24 @@ SidePanelEntryId SidePanelIdFromSideBarItemType(BuiltInItemType type) {
"not have a panel Id.";
}

std::optional<BuiltInItemType> BuiltInItemTypeFromSidePanelId(
SidePanelEntryId id) {
switch (id) {
case SidePanelEntryId::kReadingList:
return BuiltInItemType::kReadingList;
case SidePanelEntryId::kBookmarks:
return BuiltInItemType::kBookmarks;
case SidePanelEntryId::kPlaylist:
return BuiltInItemType::kPlaylist;
case SidePanelEntryId::kChatUI:
return BuiltInItemType::kChatUI;
default:
break;
}

return std::nullopt;
}

SidePanelEntryId SidePanelIdFromSideBarItem(const SidebarItem& item) {
CHECK(item.open_in_panel) << static_cast<int>(item.built_in_item_type);
return SidePanelIdFromSideBarItemType(item.built_in_item_type);
Expand Down
2 changes: 2 additions & 0 deletions browser/ui/sidebar/sidebar_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ GURL ConvertURLToBuiltInItemURL(const GURL& url);
SidePanelEntryId SidePanelIdFromSideBarItemType(
SidebarItem::BuiltInItemType type);
SidePanelEntryId SidePanelIdFromSideBarItem(const SidebarItem& item);
std::optional<SidebarItem::BuiltInItemType> BuiltInItemTypeFromSidePanelId(
SidePanelEntryId id);
void SetLastUsedSidePanel(PrefService* prefs,
std::optional<SidePanelEntryId> id);
std::optional<SidePanelEntryId> GetLastUsedSidePanel(Browser* browser);
Expand Down
23 changes: 22 additions & 1 deletion browser/ui/views/sidebar/sidebar_container_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ SidebarContainerView::SidebarContainerView(

SetNotifyEnterExitOnChild(true);
side_panel_ = AddChildView(std::move(side_panel));
side_panel_view_state_observation_.Observe(side_panel_coordinator_);
}

SidebarContainerView::~SidebarContainerView() = default;
Expand Down Expand Up @@ -813,6 +814,26 @@ void SidebarContainerView::OnRegistryDestroying(SidePanelRegistry* registry) {
}
}

void SidebarContainerView::UpdateActiveItemState() {
DVLOG(1) << "Update active item state";

auto* controller = GetBraveBrowser()->sidebar_controller();
std::optional<sidebar::SidebarItem::BuiltInItemType> current_type;
if (auto entry_id = side_panel_coordinator_->GetCurrentEntryId()) {
current_type = sidebar::BuiltInItemTypeFromSidePanelId(*entry_id);
}
controller->UpdateActiveItemState(current_type);
}

void SidebarContainerView::OnSidePanelDidClose() {
// As contextual registry is owned by TabFeatures,
// that registry is destroyed before coordinator notifies OnEntryHidden() when
// tab is closed. In this case, we should update sidebar ui(active item state)
// with this notification. If not, sidebar ui's active item state is not
// changed.
UpdateActiveItemState();
}

void SidebarContainerView::OnTabStripModelChanged(
TabStripModel* tab_strip_model,
const TabStripModelChange& change,
Expand Down Expand Up @@ -864,8 +885,8 @@ void SidebarContainerView::OnTabStripModelChanged(
for (const auto& contents : change.GetRemove()->contents) {
StopObservingContextualSidePanelRegistry(contents.contents);
}
return;
}
return;
}
}

Expand Down
8 changes: 8 additions & 0 deletions browser/ui/views/sidebar/sidebar_container_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#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 "chrome/browser/ui/views/side_panel/side_panel_view_state_observer.h"
#include "components/prefs/pref_member.h"
#include "ui/events/event_observer.h"
#include "ui/gfx/animation/slide_animation.h"
Expand Down Expand Up @@ -58,6 +59,7 @@ class SidebarContainerView
public sidebar::SidebarModel::Observer,
public SidePanelEntryObserver,
public SidePanelRegistryObserver,
public SidePanelViewStateObserver,
public TabStripModelObserver {
METADATA_HEADER(SidebarContainerView, views::View)
public:
Expand Down Expand Up @@ -134,6 +136,9 @@ class SidebarContainerView
const TabStripModelChange& change,
const TabStripSelectionChange& selection) override;

// SidePanelViewStateObserver:
void OnSidePanelDidClose() override;

private:
friend class sidebar::SidebarBrowserTest;

Expand Down Expand Up @@ -186,6 +191,7 @@ class SidebarContainerView
content::WebContents* contents);
void StopObservingContextualSidePanelRegistry(content::WebContents* contents);
void StopObservingContextualSidePanelRegistry(SidePanelRegistry* registry);
void UpdateActiveItemState();

raw_ptr<BraveBrowser> browser_ = nullptr;
raw_ptr<SidePanelCoordinator> side_panel_coordinator_ = nullptr;
Expand All @@ -205,6 +211,8 @@ class SidebarContainerView
std::unique_ptr<views::EventMonitor> browser_window_event_monitor_;
std::unique_ptr<SidebarShowOptionsEventDetectWidget> show_options_widget_;
BooleanPrefMember show_side_panel_button_;
base::ScopedObservation<SidePanelCoordinator, SidePanelViewStateObserver>
side_panel_view_state_observation_{this};
base::ScopedObservation<sidebar::SidebarModel,
sidebar::SidebarModel::Observer>
sidebar_model_observation_{this};
Expand Down

0 comments on commit 8a69005

Please sign in to comment.