From 2375d4cf7ae5edfa1a6b2511b88b832855ded762 Mon Sep 17 00:00:00 2001 From: Pete Miller Date: Mon, 5 Aug 2019 22:27:32 -0700 Subject: [PATCH] Toolbar: hide the profile button if there is only 1 user profile Fix https://github.com/brave/brave-browser/issues/5091 --- .../ui/views/toolbar/brave_toolbar_view.cc | 51 ++++++- browser/ui/views/toolbar/brave_toolbar_view.h | 13 +- .../toolbar/brave_toolbar_view_browsertest.cc | 129 ++++++++++++++++++ test/BUILD.gn | 1 + 4 files changed, 189 insertions(+), 5 deletions(-) create mode 100644 browser/ui/views/toolbar/brave_toolbar_view_browsertest.cc diff --git a/browser/ui/views/toolbar/brave_toolbar_view.cc b/browser/ui/views/toolbar/brave_toolbar_view.cc index 21ee54d43e13..a22591a1e0d3 100644 --- a/browser/ui/views/toolbar/brave_toolbar_view.cc +++ b/browser/ui/views/toolbar/brave_toolbar_view.cc @@ -10,9 +10,12 @@ #include #include "base/bind.h" +#include "brave/browser/profiles/profile_util.h" #include "brave/browser/ui/views/toolbar/bookmark_button.h" #include "brave/common/pref_names.h" +#include "chrome/browser/browser_process.h" #include "chrome/browser/defaults.h" +#include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/ui/bookmarks/bookmark_bubble_sign_in_delegate.h" #include "chrome/browser/ui/layout_constants.h" #include "chrome/browser/ui/views/bookmarks/bookmark_bubble_view.h" @@ -75,11 +78,24 @@ gfx::Insets CalcLocationBarMargin(int toolbar_width, return {0, location_bar_margin_l, 0, location_bar_margin_r}; } +bool HasMultipleUserProfiles() { + ProfileAttributesStorage* profile_storage = + &g_browser_process->profile_manager()->GetProfileAttributesStorage(); + size_t profile_count = profile_storage->GetNumberOfProfiles(); + return (profile_count != 1); +} + +bool IsAvatarButtonHideable(Profile* profile) { + return !brave::IsTorProfile(profile) && + !profile->IsIncognitoProfile() && + !profile->IsGuestSession(); +} + } // namespace BraveToolbarView::BraveToolbarView(Browser* browser, BrowserView* browser_view) - : ToolbarView(browser, browser_view) { -} + : ToolbarView(browser, browser_view), + profile_observer_(this) { } BraveToolbarView::~BraveToolbarView() {} @@ -92,14 +108,21 @@ void BraveToolbarView::Init() { return; } + Profile* profile = browser()->profile(); + + // Track changes in profile count + if (IsAvatarButtonHideable(profile)) { + profile_observer_.Add( + &g_browser_process->profile_manager()->GetProfileAttributesStorage()); + } // track changes in bookmarks enabled setting edit_bookmarks_enabled_.Init( - bookmarks::prefs::kEditBookmarksEnabled, browser()->profile()->GetPrefs(), + bookmarks::prefs::kEditBookmarksEnabled, profile->GetPrefs(), base::Bind(&BraveToolbarView::OnEditBookmarksEnabledChanged, base::Unretained(this))); // track changes in wide locationbar setting location_bar_is_wide_.Init( - kLocationBarIsWide, browser()->profile()->GetPrefs(), + kLocationBarIsWide, profile->GetPrefs(), base::Bind(&BraveToolbarView::OnLocationBarIsWideChanged, base::Unretained(this))); @@ -136,6 +159,16 @@ void BraveToolbarView::OnThemeChanged() { bookmark_->UpdateImage(); } +void BraveToolbarView::OnProfileAdded(const base::FilePath& profile_path) { + Update(nullptr); +} + +void BraveToolbarView::OnProfileWasRemoved( + const base::FilePath& profile_path, + const base::string16& profile_name) { + Update(nullptr); +} + void BraveToolbarView::LoadImages() { ToolbarView::LoadImages(); if (bookmark_) @@ -149,6 +182,16 @@ void BraveToolbarView::Update(content::WebContents* tab) { bookmark_->SetVisible(browser_defaults::bookmarks_enabled && edit_bookmarks_enabled_.GetValue()); } + // Remove avatar menu if only a single user profile exists. + // Always show if private / tor / guest window, as an indicator. + auto* avatar_button = GetAvatarToolbarButton(); + if (avatar_button) { + auto* profile = browser_->profile(); + const bool should_show_profile = + !IsAvatarButtonHideable(profile) || + HasMultipleUserProfiles(); + avatar_button->SetVisible(should_show_profile); + } } void BraveToolbarView::ShowBookmarkBubble( diff --git a/browser/ui/views/toolbar/brave_toolbar_view.h b/browser/ui/views/toolbar/brave_toolbar_view.h index 94a0ab78dd1d..9da25a8680be 100644 --- a/browser/ui/views/toolbar/brave_toolbar_view.h +++ b/browser/ui/views/toolbar/brave_toolbar_view.h @@ -6,12 +6,15 @@ #ifndef BRAVE_BROWSER_UI_VIEWS_TOOLBAR_BRAVE_TOOLBAR_VIEW_H_ #define BRAVE_BROWSER_UI_VIEWS_TOOLBAR_BRAVE_TOOLBAR_VIEW_H_ +#include "base/scoped_observer.h" +#include "chrome/browser/profiles/profile_attributes_storage.h" #include "chrome/browser/ui/views/toolbar/toolbar_view.h" #include "components/prefs/pref_member.h" class BookmarkButton; -class BraveToolbarView : public ToolbarView { +class BraveToolbarView : public ToolbarView, + public ProfileAttributesStorage::Observer { public: explicit BraveToolbarView(Browser* browser, BrowserView* browser_view); ~BraveToolbarView() override; @@ -32,12 +35,20 @@ class BraveToolbarView : public ToolbarView { void ResetLocationBarBounds(); void ResetBookmarkButtonBounds(); + // ProfileAttributesStorage::Observer: + void OnProfileAdded(const base::FilePath& profile_path) override; + void OnProfileWasRemoved(const base::FilePath& profile_path, + const base::string16& profile_name) override; + BookmarkButton* bookmark_ = nullptr; // Tracks the preference to determine whether bookmark editing is allowed. BooleanPrefMember edit_bookmarks_enabled_; BooleanPrefMember location_bar_is_wide_; // Whether this toolbar has been initialized. bool brave_initialized_ = false; + // Tracks profile count to determine whether profile switcher should be shown. + ScopedObserver + profile_observer_; }; #endif // BRAVE_BROWSER_UI_VIEWS_TOOLBAR_BRAVE_TOOLBAR_VIEW_H_ diff --git a/browser/ui/views/toolbar/brave_toolbar_view_browsertest.cc b/browser/ui/views/toolbar/brave_toolbar_view_browsertest.cc new file mode 100644 index 000000000000..e9564260de9f --- /dev/null +++ b/browser/ui/views/toolbar/brave_toolbar_view_browsertest.cc @@ -0,0 +1,129 @@ +// Copyright 2019 The Brave Authors. All rights reserved. +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// you can obtain one at http://mozilla.org/MPL/2.0/. + +#include "chrome/browser/browser_process.h" +#include "chrome/browser/chrome_notification_types.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/profiles/profile_manager.h" +#include "chrome/browser/profiles/profile_window.h" +#include "chrome/browser/search_engines/template_url_service_factory.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/browser/ui/browser_finder.h" +#include "chrome/browser/ui/browser_list.h" +#include "chrome/browser/ui/views/frame/browser_view.h" +#include "chrome/browser/ui/views/toolbar/toolbar_view.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "chrome/test/base/search_test_utils.h" +#include "content/public/browser/notification_service.h" +#include "content/public/test/test_utils.h" +#include "ui/views/view.h" + +// An observer that returns back to test code after a new profile is +// initialized. +void OnUnblockOnProfileCreation(base::RunLoop* run_loop, + Profile* profile, + Profile::CreateStatus status) { + if (status == Profile::CREATE_STATUS_INITIALIZED) + run_loop->Quit(); +} + +class BraveToolbarViewTest : public InProcessBrowserTest { + public: + BraveToolbarViewTest() = default; + ~BraveToolbarViewTest() override = default; + + void SetUpOnMainThread() override { + Init(browser()); + } + + void Init(Browser* browser) { + BrowserView* browser_view = + BrowserView::GetBrowserViewForBrowser(browser); + ASSERT_NE(browser_view, nullptr); + toolbar_view_ = browser_view->toolbar(); + ASSERT_NE(toolbar_view_, nullptr); + } + + protected: + bool is_avatar_button_shown() { + views::View* button = toolbar_view_->GetAvatarToolbarButton(); + DCHECK(button); + return button->GetVisible(); + } + + private: + ToolbarView* toolbar_view_; + DISALLOW_COPY_AND_ASSIGN(BraveToolbarViewTest); +}; + +IN_PROC_BROWSER_TEST_F(BraveToolbarViewTest, + AvatarButtonNotShownSingleProfile) { + EXPECT_EQ(false, is_avatar_button_shown()); +} + +IN_PROC_BROWSER_TEST_F(BraveToolbarViewTest, AvatarButtonIsShownGuestProfile) { + // Open a Guest window. + EXPECT_EQ(1U, BrowserList::GetInstance()->size()); + content::WindowedNotificationObserver browser_creation_observer( + chrome::NOTIFICATION_BROWSER_OPENED, + content::NotificationService::AllSources()); + profiles::SwitchToGuestProfile(ProfileManager::CreateCallback()); + base::RunLoop().RunUntilIdle(); + browser_creation_observer.Wait(); + EXPECT_EQ(2U, BrowserList::GetInstance()->size()); + + // Retrieve the new Guest profile. + Profile* guest = g_browser_process->profile_manager()->GetProfileByPath( + ProfileManager::GetGuestProfilePath()); + + // Access the browser with the Guest profile and re-init test for it. + Browser* browser = chrome::FindAnyBrowser(guest, true); + EXPECT_TRUE(browser); + Init(browser); + EXPECT_EQ(true, is_avatar_button_shown()); +} + +IN_PROC_BROWSER_TEST_F(BraveToolbarViewTest, + AvatarButtonIsShownMultipleProfiles) { + // Should not be shown in first profile, at first + EXPECT_EQ(false, is_avatar_button_shown()); + + // Create an additional profile. + ProfileManager* profile_manager = g_browser_process->profile_manager(); + ProfileAttributesStorage& storage = + profile_manager->GetProfileAttributesStorage(); + base::FilePath current_profile_path = browser()->profile()->GetPath(); + base::FilePath new_path = profile_manager->GenerateNextProfileDirectoryPath(); + base::RunLoop run_loop; + profile_manager->CreateProfileAsync( + new_path, base::Bind(&OnUnblockOnProfileCreation, &run_loop), + base::string16(), std::string()); + run_loop.Run(); + ASSERT_EQ(2u, storage.GetNumberOfProfiles()); + Profile* new_profile = profile_manager->GetProfileByPath(new_path); + + // check it's now shown in first profile + EXPECT_EQ(true, is_avatar_button_shown()); + + // Open the new profile + EXPECT_EQ(1U, BrowserList::GetInstance()->size()); + content::WindowedNotificationObserver browser_creation_observer( + chrome::NOTIFICATION_BROWSER_OPENED, + content::NotificationService::AllSources()); + profiles::OpenBrowserWindowForProfile( + ProfileManager::CreateCallback(), + false, true, true, + new_profile, + Profile::CREATE_STATUS_INITIALIZED); + base::RunLoop().RunUntilIdle(); + browser_creation_observer.Wait(); + EXPECT_EQ(2U, BrowserList::GetInstance()->size()); + + // Check it's shown in second profile + Browser* browser = chrome::FindAnyBrowser(new_profile, true); + EXPECT_TRUE(browser); + Init(browser); + EXPECT_EQ(true, is_avatar_button_shown()); +} diff --git a/test/BUILD.gn b/test/BUILD.gn index 9d4afb0aaeac..4165ac4b5875 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -386,6 +386,7 @@ test("brave_browser_tests") { "//brave/browser/ui/content_settings/brave_autoplay_blocked_image_model_browsertest.cc", "//brave/browser/ui/views/brave_actions/brave_actions_container_browsertest.cc", "//brave/browser/ui/views/profiles/brave_profile_chooser_view_browsertest.cc", + "//brave/browser/ui/views/toolbar/brave_toolbar_view_browsertest.cc", "//brave/browser/ui/webui/brave_new_tab_ui_browsertest.cc", "//brave/browser/ui/webui/brave_welcome_ui_browsertest.cc", "//brave/browser/ui/toolbar/brave_app_menu_model_browsertest.cc",