Skip to content

Commit

Permalink
Disabled not working sidebar items in guest window
Browse files Browse the repository at this point in the history
  • Loading branch information
simonhong committed Nov 10, 2023
1 parent 233fc8b commit ba72cbe
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 5 deletions.
19 changes: 19 additions & 0 deletions browser/ui/sidebar/sidebar_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,25 @@ IN_PROC_BROWSER_TEST_F(SidebarBrowserTest, UnManagedPanelEntryTest) {
EXPECT_EQ(SidePanelEntryId::kBookmarks, panel_ui->GetCurrentEntryId());
}

IN_PROC_BROWSER_TEST_F(SidebarBrowserTest, DisabledItemsTestWithGuestWindow) {
auto* guest_browser = static_cast<BraveBrowser*>(CreateGuestBrowser());
auto* controller = guest_browser->sidebar_controller();
auto* model = controller->model();
auto sidebar_items_contents_view = GetSidebarItemsContentsView(controller);
for (const auto& item : model->GetAllSidebarItems()) {
auto index = model->GetIndexOf(item);
ASSERT_TRUE(index.has_value());
auto* item_view = sidebar_items_contents_view->children()[*index];
ASSERT_TRUE(item_view);
if (IsBuiltInType(item) &&
SidebarService::IsDisabledItemForGuest(item.built_in_item_type)) {
EXPECT_FALSE(item_view->GetEnabled());
} else {
EXPECT_TRUE(item_view->GetEnabled());
}
}
}

class SidebarBrowserTestWithPlaylist : public SidebarBrowserTest {
public:
SidebarBrowserTestWithPlaylist() {
Expand Down
4 changes: 2 additions & 2 deletions browser/ui/sidebar/sidebar_service_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ SidebarServiceFactory::~SidebarServiceFactory() = default;

KeyedService* SidebarServiceFactory::BuildServiceInstanceFor(
content::BrowserContext* context) const {
auto* prefs = Profile::FromBrowserContext(context)->GetPrefs();
return new SidebarService(prefs);
auto* profile = Profile::FromBrowserContext(context);
return new SidebarService(profile->GetPrefs(), profile->IsGuestSession());
}

content::BrowserContext* SidebarServiceFactory::GetBrowserContextToUse(
Expand Down
4 changes: 4 additions & 0 deletions browser/ui/views/sidebar/sidebar_items_contents_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,10 @@ void SidebarItemsContentsView::UpdateItemViewStateAt(size_t index,
item_view->SetEnabled(false);
}
}

if (item.disabled && item_view->GetEnabled()) {
item_view->SetEnabled(false);
}
}
}

Expand Down
1 change: 1 addition & 0 deletions components/sidebar/sidebar_item.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ struct SidebarItem {
std::u16string title;
// Set false to open this item in new tab.
bool open_in_panel = false;
bool disabled = false;
};

bool IsBuiltInType(const SidebarItem& item);
Expand Down
19 changes: 17 additions & 2 deletions components/sidebar/sidebar_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,20 @@ bool SidebarItemUpdate::operator==(const SidebarItemUpdate& update) const {
url_updated == update.url_updated;
}

// static
bool SidebarService::IsDisabledItemForGuest(SidebarItem::BuiltInItemType type) {
switch (type) {
case SidebarItem::BuiltInItemType::kBookmarks:
case SidebarItem::BuiltInItemType::kReadingList:
case SidebarItem::BuiltInItemType::kChatUI:
case SidebarItem::BuiltInItemType::kPlaylist:
return true;
default:
return false;
}
NOTREACHED_NORETURN();
}

// static
void SidebarService::RegisterProfilePrefs(PrefRegistrySimple* registry,
version_info::Channel channel) {
Expand All @@ -138,8 +152,8 @@ void SidebarService::RegisterProfilePrefs(PrefRegistrySimple* registry,
registry->RegisterIntegerPref(kSidePanelWidth, kDefaultSidePanelWidth);
}

SidebarService::SidebarService(PrefService* prefs)
: prefs_(prefs), sidebar_p3a_(prefs) {
SidebarService::SidebarService(PrefService* prefs, bool is_guest)
: prefs_(prefs), sidebar_p3a_(prefs), is_guest_(is_guest) {
DCHECK(prefs_);
MigratePrefSidebarBuiltInItemsToHidden();

Expand Down Expand Up @@ -613,6 +627,7 @@ std::vector<SidebarItem> SidebarService::GetDefaultSidebarItems() const {
for (const auto& item_type : SidebarService::kDefaultBuiltInItemTypes) {
if (auto item = GetBuiltInItemForType(item_type);
item.built_in_item_type != SidebarItem::BuiltInItemType::kNone) {
item.disabled = (is_guest_ && IsDisabledItemForGuest(item_type));
items.push_back(std::move(item));
}
}
Expand Down
7 changes: 6 additions & 1 deletion components/sidebar/sidebar_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class SidebarService : public KeyedService {
static void RegisterProfilePrefs(PrefRegistrySimple* registry,
version_info::Channel channel);

explicit SidebarService(PrefService* prefs);
explicit SidebarService(PrefService* prefs, bool is_guest = false);
~SidebarService() override;

const std::vector<SidebarItem>& items() const { return items_; }
Expand Down Expand Up @@ -110,6 +110,10 @@ class SidebarService : public KeyedService {
FRIEND_TEST_ALL_PREFIXES(SidebarServiceTest, AddRemoveItems);
FRIEND_TEST_ALL_PREFIXES(SidebarBrowserTest, ItemAddedBubbleAnchorViewTest);
FRIEND_TEST_ALL_PREFIXES(SidebarBrowserTest, ItemAddedScrollTest);
FRIEND_TEST_ALL_PREFIXES(SidebarBrowserTest,
DisabledItemsTestWithGuestWindow);

static bool IsDisabledItemForGuest(SidebarItem::BuiltInItemType type);

void LoadSidebarItems();
void UpdateSidebarItemsToPrefStore();
Expand All @@ -129,6 +133,7 @@ class SidebarService : public KeyedService {

p3a::SidebarP3A sidebar_p3a_;

bool is_guest_ = false;
base::ObserverList<Observer> observers_;
PrefChangeRegistrar pref_change_registrar_;
};
Expand Down

0 comments on commit ba72cbe

Please sign in to comment.