Commit 3ab32772 authored by mlerman's avatar mlerman Committed by Commit bot

Reland of: Change default code flag to NewAvatarMenu.

Originally landed here: https://codereview.chromium.org/845373002/
Reverted here: https://codereview.chromium.org/1020863007/
because it caused issues on memory bots.

BUG=287883
TBR=rogerta@chromium.org (already reviewed in original CL, there are no changes here)

Review URL: https://codereview.chromium.org/1038423004

Cr-Commit-Position: refs/heads/master@{#324227}
parent 917c61c1
...@@ -152,7 +152,8 @@ void DeleteBitmap(const base::FilePath& image_path) { ...@@ -152,7 +152,8 @@ void DeleteBitmap(const base::FilePath& image_path) {
ProfileInfoCache::ProfileInfoCache(PrefService* prefs, ProfileInfoCache::ProfileInfoCache(PrefService* prefs,
const base::FilePath& user_data_dir) const base::FilePath& user_data_dir)
: prefs_(prefs), : prefs_(prefs),
user_data_dir_(user_data_dir) { user_data_dir_(user_data_dir),
disable_avatar_download_for_testing_(false) {
// Populate the cache // Populate the cache
DictionaryPrefUpdate update(prefs_, prefs::kProfileInfoCache); DictionaryPrefUpdate update(prefs_, prefs::kProfileInfoCache);
base::DictionaryValue* cache = update.Get(); base::DictionaryValue* cache = update.Get();
...@@ -181,7 +182,7 @@ ProfileInfoCache::ProfileInfoCache(PrefService* prefs, ...@@ -181,7 +182,7 @@ ProfileInfoCache::ProfileInfoCache(PrefService* prefs,
// If needed, start downloading the high-res avatars and migrate any legacy // If needed, start downloading the high-res avatars and migrate any legacy
// profile names. // profile names.
if (switches::IsNewAvatarMenu()) if (switches::IsNewAvatarMenu() && !disable_avatar_download_for_testing_)
MigrateLegacyProfileNamesAndDownloadAvatars(); MigrateLegacyProfileNamesAndDownloadAvatars();
} }
...@@ -220,7 +221,7 @@ void ProfileInfoCache::AddProfileToCache( ...@@ -220,7 +221,7 @@ void ProfileInfoCache::AddProfileToCache(
sorted_keys_.insert(FindPositionForProfile(key, name), key); sorted_keys_.insert(FindPositionForProfile(key, name), key);
if (switches::IsNewAvatarMenu()) if (switches::IsNewAvatarMenu() && !disable_avatar_download_for_testing_)
DownloadHighResAvatarIfNeeded(icon_index, profile_path); DownloadHighResAvatarIfNeeded(icon_index, profile_path);
FOR_EACH_OBSERVER(ProfileInfoCacheObserver, FOR_EACH_OBSERVER(ProfileInfoCacheObserver,
...@@ -318,7 +319,7 @@ base::string16 ProfileInfoCache::GetUserNameOfProfileAtIndex( ...@@ -318,7 +319,7 @@ base::string16 ProfileInfoCache::GetUserNameOfProfileAtIndex(
} }
const gfx::Image& ProfileInfoCache::GetAvatarIconOfProfileAtIndex( const gfx::Image& ProfileInfoCache::GetAvatarIconOfProfileAtIndex(
size_t index) const { size_t index) {
if (IsUsingGAIAPictureOfProfileAtIndex(index)) { if (IsUsingGAIAPictureOfProfileAtIndex(index)) {
const gfx::Image* image = GetGAIAPictureOfProfileAtIndex(index); const gfx::Image* image = GetGAIAPictureOfProfileAtIndex(index);
if (image) if (image)
...@@ -544,7 +545,7 @@ void ProfileInfoCache::SetAvatarIconOfProfileAtIndex(size_t index, ...@@ -544,7 +545,7 @@ void ProfileInfoCache::SetAvatarIconOfProfileAtIndex(size_t index,
base::FilePath profile_path = GetPathOfProfileAtIndex(index); base::FilePath profile_path = GetPathOfProfileAtIndex(index);
if (switches::IsNewAvatarMenu()) if (switches::IsNewAvatarMenu() && !disable_avatar_download_for_testing_)
DownloadHighResAvatarIfNeeded(icon_index, profile_path); DownloadHighResAvatarIfNeeded(icon_index, profile_path);
FOR_EACH_OBSERVER(ProfileInfoCacheObserver, FOR_EACH_OBSERVER(ProfileInfoCacheObserver,
...@@ -866,6 +867,7 @@ void ProfileInfoCache::DownloadHighResAvatarIfNeeded( ...@@ -866,6 +867,7 @@ void ProfileInfoCache::DownloadHighResAvatarIfNeeded(
#if defined(OS_ANDROID) || defined(OS_IOS) || defined(OS_CHROMEOS) #if defined(OS_ANDROID) || defined(OS_IOS) || defined(OS_CHROMEOS)
return; return;
#endif #endif
DCHECK(!disable_avatar_download_for_testing_);
const base::FilePath& file_path = const base::FilePath& file_path =
profiles::GetPathOfHighResAvatarAtIndex(icon_index); profiles::GetPathOfHighResAvatarAtIndex(icon_index);
...@@ -1065,6 +1067,10 @@ const gfx::Image* ProfileInfoCache::LoadAvatarPictureFromPath( ...@@ -1065,6 +1067,10 @@ const gfx::Image* ProfileInfoCache::LoadAvatarPictureFromPath(
return cached_avatar_images_[key]; return cached_avatar_images_[key];
} }
// Don't download the image if downloading is disabled for tests.
if (disable_avatar_download_for_testing_)
return NULL;
// If the picture is already being loaded then don't try loading it again. // If the picture is already being loaded then don't try loading it again.
if (cached_avatar_images_loading_[key]) if (cached_avatar_images_loading_[key])
return NULL; return NULL;
......
...@@ -62,7 +62,7 @@ class ProfileInfoCache : public ProfileInfoInterface, ...@@ -62,7 +62,7 @@ class ProfileInfoCache : public ProfileInfoInterface,
base::FilePath GetPathOfProfileAtIndex(size_t index) const override; base::FilePath GetPathOfProfileAtIndex(size_t index) const override;
base::Time GetProfileActiveTimeAtIndex(size_t index) const override; base::Time GetProfileActiveTimeAtIndex(size_t index) const override;
base::string16 GetUserNameOfProfileAtIndex(size_t index) const override; base::string16 GetUserNameOfProfileAtIndex(size_t index) const override;
const gfx::Image& GetAvatarIconOfProfileAtIndex(size_t index) const override; const gfx::Image& GetAvatarIconOfProfileAtIndex(size_t index) override;
std::string GetLocalAuthCredentialsOfProfileAtIndex( std::string GetLocalAuthCredentialsOfProfileAtIndex(
size_t index) const override; size_t index) const override;
// Note that a return value of false could mean an error in collection or // Note that a return value of false could mean an error in collection or
...@@ -148,6 +148,11 @@ class ProfileInfoCache : public ProfileInfoInterface, ...@@ -148,6 +148,11 @@ class ProfileInfoCache : public ProfileInfoInterface,
void AddObserver(ProfileInfoCacheObserver* obs); void AddObserver(ProfileInfoCacheObserver* obs);
void RemoveObserver(ProfileInfoCacheObserver* obs); void RemoveObserver(ProfileInfoCacheObserver* obs);
void set_disable_avatar_download_for_testing(
bool disable_avatar_download_for_testing) {
disable_avatar_download_for_testing_ = disable_avatar_download_for_testing;
}
private: private:
FRIEND_TEST_ALL_PREFIXES(ProfileInfoCacheTest, DownloadHighResAvatarTest); FRIEND_TEST_ALL_PREFIXES(ProfileInfoCacheTest, DownloadHighResAvatarTest);
...@@ -224,6 +229,10 @@ class ProfileInfoCache : public ProfileInfoInterface, ...@@ -224,6 +229,10 @@ class ProfileInfoCache : public ProfileInfoInterface,
std::map<std::string, ProfileAvatarDownloader*> std::map<std::string, ProfileAvatarDownloader*>
avatar_images_downloads_in_progress_; avatar_images_downloads_in_progress_;
// Determines of the ProfileAvatarDownloader should be created and executed
// or not. Only set to true for tests.
bool disable_avatar_download_for_testing_;
DISALLOW_COPY_AND_ASSIGN(ProfileInfoCache); DISALLOW_COPY_AND_ASSIGN(ProfileInfoCache);
}; };
......
...@@ -546,27 +546,37 @@ TEST_F(ProfileInfoCacheTest, DownloadHighResAvatarTest) { ...@@ -546,27 +546,37 @@ TEST_F(ProfileInfoCacheTest, DownloadHighResAvatarTest) {
switches::EnableNewAvatarMenuForTesting( switches::EnableNewAvatarMenuForTesting(
base::CommandLine::ForCurrentProcess()); base::CommandLine::ForCurrentProcess());
EXPECT_EQ(0U, GetCache()->GetNumberOfProfiles()); // The TestingProfileManager's ProfileInfoCache doesn't download avatars.
ProfileInfoCache profile_info_cache(g_browser_process->local_state(),
testing_profile_manager_.profile_manager()->user_data_dir());
// // Make sure there are no avatars already on disk.
const size_t kIconIndex = 0;
base::FilePath icon_path =
profiles::GetPathOfHighResAvatarAtIndex(kIconIndex);
EXPECT_FALSE(base::PathExists(icon_path));
EXPECT_EQ(0U, profile_info_cache.GetNumberOfProfiles());
base::FilePath path_1 = GetProfilePath("path_1"); base::FilePath path_1 = GetProfilePath("path_1");
GetCache()->AddProfileToCache(path_1, ASCIIToUTF16("name_1"), profile_info_cache.AddProfileToCache(path_1, ASCIIToUTF16("name_1"),
base::string16(), 0, std::string()); base::string16(), kIconIndex, std::string());
EXPECT_EQ(1U, GetCache()->GetNumberOfProfiles()); EXPECT_EQ(1U, profile_info_cache.GetNumberOfProfiles());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
// We haven't downloaded any high-res avatars yet. // We haven't downloaded any high-res avatars yet.
EXPECT_EQ(0U, GetCache()->cached_avatar_images_.size()); EXPECT_EQ(0U, profile_info_cache.cached_avatar_images_.size());
// After adding a new profile, the download of high-res avatar will be // After adding a new profile, the download of high-res avatar will be
// triggered if the flag kNewAvatarMenu has been set. But the downloader // triggered if the flag kNewAvatarMenu has been set. But the downloader
// won't ever call OnFetchComplete in the test. // won't ever call OnFetchComplete in the test.
EXPECT_EQ(1U, GetCache()->avatar_images_downloads_in_progress_.size()); EXPECT_EQ(1U, profile_info_cache.avatar_images_downloads_in_progress_.size());
EXPECT_FALSE(GetCache()->GetHighResAvatarOfProfileAtIndex(0)); EXPECT_FALSE(profile_info_cache.GetHighResAvatarOfProfileAtIndex(0));
// Simulate downloading a high-res avatar. // Simulate downloading a high-res avatar.
const size_t kIconIndex = 0;
ProfileAvatarDownloader avatar_downloader( ProfileAvatarDownloader avatar_downloader(
kIconIndex, GetCache()->GetPathOfProfileAtIndex(0), GetCache()); kIconIndex, profile_info_cache.GetPathOfProfileAtIndex(0),
&profile_info_cache);
// Put a real bitmap into "bitmap". 2x2 bitmap of green 32 bit pixels. // Put a real bitmap into "bitmap". 2x2 bitmap of green 32 bit pixels.
SkBitmap bitmap; SkBitmap bitmap;
...@@ -577,23 +587,21 @@ TEST_F(ProfileInfoCacheTest, DownloadHighResAvatarTest) { ...@@ -577,23 +587,21 @@ TEST_F(ProfileInfoCacheTest, DownloadHighResAvatarTest) {
GURL("http://www.google.com/avatar.png"), &bitmap); GURL("http://www.google.com/avatar.png"), &bitmap);
// Now the download should not be in progress anymore. // Now the download should not be in progress anymore.
EXPECT_EQ(0U, GetCache()->avatar_images_downloads_in_progress_.size()); EXPECT_EQ(0U, profile_info_cache.avatar_images_downloads_in_progress_.size());
std::string file_name = std::string file_name =
profiles::GetDefaultAvatarIconFileNameAtIndex(kIconIndex); profiles::GetDefaultAvatarIconFileNameAtIndex(kIconIndex);
// The file should have been cached and saved. // The file should have been cached and saved.
EXPECT_EQ(1U, GetCache()->cached_avatar_images_.size()); EXPECT_EQ(1U, profile_info_cache.cached_avatar_images_.size());
EXPECT_TRUE(GetCache()->GetHighResAvatarOfProfileAtIndex(0)); EXPECT_TRUE(profile_info_cache.GetHighResAvatarOfProfileAtIndex(0));
EXPECT_EQ(GetCache()->cached_avatar_images_[file_name], EXPECT_EQ(profile_info_cache.cached_avatar_images_[file_name],
GetCache()->GetHighResAvatarOfProfileAtIndex(0)); profile_info_cache.GetHighResAvatarOfProfileAtIndex(0));
// Make sure everything has completed, and the file has been written to disk. // Make sure everything has completed, and the file has been written to disk.
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
// Clean up. // Clean up.
base::FilePath icon_path =
profiles::GetPathOfHighResAvatarAtIndex(kIconIndex);
EXPECT_NE(std::string::npos, icon_path.MaybeAsASCII().find(file_name)); EXPECT_NE(std::string::npos, icon_path.MaybeAsASCII().find(file_name));
EXPECT_TRUE(base::PathExists(icon_path)); EXPECT_TRUE(base::PathExists(icon_path));
EXPECT_TRUE(base::DeleteFile(icon_path, true)); EXPECT_TRUE(base::DeleteFile(icon_path, true));
......
...@@ -37,8 +37,7 @@ class ProfileInfoInterface { ...@@ -37,8 +37,7 @@ class ProfileInfoInterface {
virtual base::string16 GetUserNameOfProfileAtIndex(size_t index) const = 0; virtual base::string16 GetUserNameOfProfileAtIndex(size_t index) const = 0;
virtual const gfx::Image& GetAvatarIconOfProfileAtIndex( virtual const gfx::Image& GetAvatarIconOfProfileAtIndex(size_t index) = 0;
size_t index) const = 0;
virtual std::string GetLocalAuthCredentialsOfProfileAtIndex( virtual std::string GetLocalAuthCredentialsOfProfileAtIndex(
size_t index) const = 0; size_t index) const = 0;
......
...@@ -51,6 +51,7 @@ cr.define('options.browser_options', function() { ...@@ -51,6 +51,7 @@ cr.define('options.browser_options', function() {
var iconEl = this.ownerDocument.createElement('img'); var iconEl = this.ownerDocument.createElement('img');
iconEl.className = 'profile-img'; iconEl.className = 'profile-img';
iconEl.style.content = getProfileAvatarIcon(profileInfo.iconURL); iconEl.style.content = getProfileAvatarIcon(profileInfo.iconURL);
iconEl.alt = '';
containerEl.appendChild(iconEl); containerEl.appendChild(iconEl);
var nameEl = this.ownerDocument.createElement('div'); var nameEl = this.ownerDocument.createElement('div');
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/test/base/browser_with_test_window_test.h" #include "chrome/test/base/browser_with_test_window_test.h"
#include "components/signin/core/common/profile_management_switches.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
namespace { namespace {
...@@ -21,6 +22,8 @@ namespace { ...@@ -21,6 +22,8 @@ namespace {
class BookmarkSyncPromoControllerTest : public BrowserWithTestWindowTest { class BookmarkSyncPromoControllerTest : public BrowserWithTestWindowTest {
public: public:
void SetUp() override { void SetUp() override {
switches::DisableNewAvatarMenuForTesting(
base::CommandLine::ForCurrentProcess());
BrowserWithTestWindowTest::SetUp(); BrowserWithTestWindowTest::SetUp();
ASSERT_TRUE(profile()); ASSERT_TRUE(profile());
// Adds TestExtensionSystem, since signin uses the gaia auth extension. // Adds TestExtensionSystem, since signin uses the gaia auth extension.
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#import "chrome/browser/ui/cocoa/profiles/avatar_icon_controller.h" #import "chrome/browser/ui/cocoa/profiles/avatar_icon_controller.h"
#include "base/command_line.h"
#include "base/mac/scoped_nsobject.h" #include "base/mac/scoped_nsobject.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "chrome/browser/autocomplete/autocomplete_classifier_factory.h" #include "chrome/browser/autocomplete/autocomplete_classifier_factory.h"
...@@ -22,10 +23,13 @@ ...@@ -22,10 +23,13 @@
#include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
#include "components/bookmarks/test/bookmark_test_helpers.h" #include "components/bookmarks/test/bookmark_test_helpers.h"
#include "components/signin/core/common/profile_management_switches.h"
class AvatarIconControllerTest : public CocoaProfileTest { class AvatarIconControllerTest : public CocoaProfileTest {
public: public:
void SetUp() override { void SetUp() override {
switches::DisableNewAvatarMenuForTesting(
base::CommandLine::ForCurrentProcess());
CocoaProfileTest::SetUp(); CocoaProfileTest::SetUp();
ASSERT_TRUE(browser()); ASSERT_TRUE(browser());
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#import "chrome/browser/ui/cocoa/profiles/avatar_menu_bubble_controller.h" #import "chrome/browser/ui/cocoa/profiles/avatar_menu_bubble_controller.h"
#include "base/command_line.h"
#include "base/mac/scoped_nsobject.h" #include "base/mac/scoped_nsobject.h"
#include "base/memory/scoped_ptr.h" #include "base/memory/scoped_ptr.h"
#include "base/message_loop/message_pump_mac.h" #include "base/message_loop/message_pump_mac.h"
...@@ -15,6 +16,7 @@ ...@@ -15,6 +16,7 @@
#import "chrome/browser/ui/cocoa/cocoa_test_helper.h" #import "chrome/browser/ui/cocoa/cocoa_test_helper.h"
#include "chrome/test/base/testing_browser_process.h" #include "chrome/test/base/testing_browser_process.h"
#include "chrome/test/base/testing_profile_manager.h" #include "chrome/test/base/testing_profile_manager.h"
#include "components/signin/core/common/profile_management_switches.h"
#include "testing/gtest_mac.h" #include "testing/gtest_mac.h"
#import "ui/base/cocoa/controls/hyperlink_button_cell.h" #import "ui/base/cocoa/controls/hyperlink_button_cell.h"
#include "ui/events/test/cocoa_test_event_utils.h" #include "ui/events/test/cocoa_test_event_utils.h"
...@@ -26,6 +28,9 @@ class AvatarMenuBubbleControllerTest : public CocoaTest { ...@@ -26,6 +28,9 @@ class AvatarMenuBubbleControllerTest : public CocoaTest {
} }
void SetUp() override { void SetUp() override {
switches::DisableNewAvatarMenuForTesting(
base::CommandLine::ForCurrentProcess());
CocoaTest::SetUp(); CocoaTest::SetUp();
ASSERT_TRUE(manager_.SetUp()); ASSERT_TRUE(manager_.SetUp());
...@@ -78,7 +83,7 @@ TEST_F(AvatarMenuBubbleControllerTest, InitialLayout) { ...@@ -78,7 +83,7 @@ TEST_F(AvatarMenuBubbleControllerTest, InitialLayout) {
NSView* contents = [[controller() window] contentView]; NSView* contents = [[controller() window] contentView];
EXPECT_EQ(4U, [[contents subviews] count]); EXPECT_EQ(4U, [[contents subviews] count]);
// Loop over the itmes and match the viewController views to subviews. // Loop over the items and match the viewController views to subviews.
NSMutableArray* subviews = NSMutableArray* subviews =
[NSMutableArray arrayWithArray:[contents subviews]]; [NSMutableArray arrayWithArray:[contents subviews]];
for (AvatarMenuItemController* viewController in [controller() items]) { for (AvatarMenuItemController* viewController in [controller() items]) {
......
...@@ -80,12 +80,12 @@ class ProfileMenuControllerTest : public CocoaProfileTest { ...@@ -80,12 +80,12 @@ class ProfileMenuControllerTest : public CocoaProfileTest {
TEST_F(ProfileMenuControllerTest, InitializeMenu) { TEST_F(ProfileMenuControllerTest, InitializeMenu) {
NSMenu* menu = [controller() menu]; NSMenu* menu = [controller() menu];
// <sep>, Edit, <sep>, New. // Profile, <sep>, Edit, <sep>, New.
ASSERT_EQ(4, [menu numberOfItems]); ASSERT_EQ(5, [menu numberOfItems]);
TestBottomItems(); TestBottomItems();
EXPECT_TRUE([menu_item() isHidden]); EXPECT_FALSE([menu_item() isHidden]);
} }
TEST_F(ProfileMenuControllerTest, CreateItemWithTitle) { TEST_F(ProfileMenuControllerTest, CreateItemWithTitle) {
...@@ -100,9 +100,9 @@ TEST_F(ProfileMenuControllerTest, CreateItemWithTitle) { ...@@ -100,9 +100,9 @@ TEST_F(ProfileMenuControllerTest, CreateItemWithTitle) {
TEST_F(ProfileMenuControllerTest, RebuildMenu) { TEST_F(ProfileMenuControllerTest, RebuildMenu) {
NSMenu* menu = [controller() menu]; NSMenu* menu = [controller() menu];
EXPECT_EQ(4, [menu numberOfItems]); EXPECT_EQ(5, [menu numberOfItems]);
EXPECT_TRUE([menu_item() isHidden]); EXPECT_FALSE([menu_item() isHidden]);
// Create some more profiles on the manager. // Create some more profiles on the manager.
TestingProfileManager* manager = testing_profile_manager(); TestingProfileManager* manager = testing_profile_manager();
...@@ -130,12 +130,12 @@ TEST_F(ProfileMenuControllerTest, InsertItems) { ...@@ -130,12 +130,12 @@ TEST_F(ProfileMenuControllerTest, InsertItems) {
base::scoped_nsobject<NSMenu> menu([[NSMenu alloc] initWithTitle:@""]); base::scoped_nsobject<NSMenu> menu([[NSMenu alloc] initWithTitle:@""]);
ASSERT_EQ(0, [menu numberOfItems]); ASSERT_EQ(0, [menu numberOfItems]);
// With only one profile, insertItems should be a no-op. // Even with one profile items can still be inserted.
BOOL result = [controller() insertItemsIntoMenu:menu BOOL result = [controller() insertItemsIntoMenu:menu
atOffset:0 atOffset:0
fromDock:NO]; fromDock:NO];
EXPECT_FALSE(result); EXPECT_TRUE(result);
EXPECT_EQ(0, [menu numberOfItems]); EXPECT_EQ(1, [menu numberOfItems]);
[menu removeAllItems]; [menu removeAllItems];
// Same for use in building the dock menu. // Same for use in building the dock menu.
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "chrome/browser/ui/browser_list.h" #include "chrome/browser/ui/browser_list.h"
#include "chrome/browser/ui/singleton_tabs.h" #include "chrome/browser/ui/singleton_tabs.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/browser/ui/views/profiles/profile_chooser_view.h"
#include "chrome/common/chrome_switches.h" #include "chrome/common/chrome_switches.h"
#include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
...@@ -23,6 +24,12 @@ ...@@ -23,6 +24,12 @@
#include "ui/events/event_constants.h" #include "ui/events/event_constants.h"
#include "ui/gfx/range/range.h" #include "ui/gfx/range/range.h"
#if defined(OS_CHROMEOS)
const bool kHasProfileChooser = false;
#else
const bool kHasProfileChooser = true;
#endif
class BookmarkBubbleSignInDelegateTest : public InProcessBrowserTest { class BookmarkBubbleSignInDelegateTest : public InProcessBrowserTest {
public: public:
BookmarkBubbleSignInDelegateTest() {} BookmarkBubbleSignInDelegateTest() {}
...@@ -55,8 +62,12 @@ IN_PROC_BROWSER_TEST_F(BookmarkBubbleSignInDelegateTest, OnSignInLinkClicked) { ...@@ -55,8 +62,12 @@ IN_PROC_BROWSER_TEST_F(BookmarkBubbleSignInDelegateTest, OnSignInLinkClicked) {
delegate->OnSignInLinkClicked(); delegate->OnSignInLinkClicked();
// A new tab should have been opened and the browser should be visible. if (kHasProfileChooser) {
EXPECT_TRUE(ProfileChooserView::IsShowing());
EXPECT_EQ(starting_tab_count, browser()->tab_strip_model()->count());
} else {
EXPECT_EQ(starting_tab_count + 1, browser()->tab_strip_model()->count()); EXPECT_EQ(starting_tab_count + 1, browser()->tab_strip_model()->count());
}
} }
IN_PROC_BROWSER_TEST_F(BookmarkBubbleSignInDelegateTest, IN_PROC_BROWSER_TEST_F(BookmarkBubbleSignInDelegateTest,
...@@ -68,8 +79,12 @@ IN_PROC_BROWSER_TEST_F(BookmarkBubbleSignInDelegateTest, ...@@ -68,8 +79,12 @@ IN_PROC_BROWSER_TEST_F(BookmarkBubbleSignInDelegateTest,
delegate->OnSignInLinkClicked(); delegate->OnSignInLinkClicked();
// A new tab should have been opened and the browser should be visible. if (kHasProfileChooser) {
EXPECT_TRUE(ProfileChooserView::IsShowing());
EXPECT_EQ(starting_tab_count, browser()->tab_strip_model()->count()); EXPECT_EQ(starting_tab_count, browser()->tab_strip_model()->count());
} else {
EXPECT_EQ(starting_tab_count, browser()->tab_strip_model()->count());
}
} }
IN_PROC_BROWSER_TEST_F(BookmarkBubbleSignInDelegateTest, IN_PROC_BROWSER_TEST_F(BookmarkBubbleSignInDelegateTest,
...@@ -86,11 +101,15 @@ IN_PROC_BROWSER_TEST_F(BookmarkBubbleSignInDelegateTest, ...@@ -86,11 +101,15 @@ IN_PROC_BROWSER_TEST_F(BookmarkBubbleSignInDelegateTest,
delegate->OnSignInLinkClicked(); delegate->OnSignInLinkClicked();
if (kHasProfileChooser) {
// ProfileChooser doesn't show in an incognito window.
EXPECT_FALSE(ProfileChooserView::IsShowing());
} else {
// A new tab should have been opened in the normal browser, which should be // A new tab should have been opened in the normal browser, which should be
// visible. // visible.
int tab_count_normal = browser()->tab_strip_model()->count(); int tab_count_normal = browser()->tab_strip_model()->count();
EXPECT_EQ(starting_tab_count_normal + 1, tab_count_normal); EXPECT_EQ(starting_tab_count_normal + 1, tab_count_normal);
}
// No effect is expected on the incognito browser. // No effect is expected on the incognito browser.
int tab_count_incognito = incognito_browser->tab_strip_model()->count(); int tab_count_incognito = incognito_browser->tab_strip_model()->count();
EXPECT_EQ(starting_tab_count_incognito, tab_count_incognito); EXPECT_EQ(starting_tab_count_incognito, tab_count_incognito);
...@@ -117,8 +136,12 @@ IN_PROC_BROWSER_TEST_F(BookmarkBubbleSignInDelegateTest, BrowserRemoved) { ...@@ -117,8 +136,12 @@ IN_PROC_BROWSER_TEST_F(BookmarkBubbleSignInDelegateTest, BrowserRemoved) {
delegate->OnSignInLinkClicked(); delegate->OnSignInLinkClicked();
if (kHasProfileChooser) {
EXPECT_TRUE(ProfileChooserView::IsShowing());
} else {
// A new tab should have been opened in the extra browser, which should be // A new tab should have been opened in the extra browser, which should be
// visible. // visible.
int tab_count = extra_browser->tab_strip_model()->count(); int tab_count = extra_browser->tab_strip_model()->count();
EXPECT_EQ(starting_tab_count + 1, tab_count); EXPECT_EQ(starting_tab_count + 1, tab_count);
}
} }
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/test_switches.h" #include "chrome/test/base/test_switches.h"
#include "chrome/test/base/testing_browser_process.h" #include "chrome/test/base/testing_browser_process.h"
#include "components/signin/core/common/profile_management_switches.h"
class AvatarMenuButtonTest : public InProcessBrowserTest { class AvatarMenuButtonTest : public InProcessBrowserTest {
public: public:
...@@ -24,6 +25,7 @@ class AvatarMenuButtonTest : public InProcessBrowserTest { ...@@ -24,6 +25,7 @@ class AvatarMenuButtonTest : public InProcessBrowserTest {
~AvatarMenuButtonTest() override; ~AvatarMenuButtonTest() override;
protected: protected:
void SetUpCommandLine(base::CommandLine* command_line) override;
void CreateTestingProfile(); void CreateTestingProfile();
AvatarMenuButton* GetAvatarMenuButton(); AvatarMenuButton* GetAvatarMenuButton();
void StartAvatarMenu(); void StartAvatarMenu();
...@@ -38,6 +40,10 @@ AvatarMenuButtonTest::AvatarMenuButtonTest() { ...@@ -38,6 +40,10 @@ AvatarMenuButtonTest::AvatarMenuButtonTest() {
AvatarMenuButtonTest::~AvatarMenuButtonTest() { AvatarMenuButtonTest::~AvatarMenuButtonTest() {
} }
void AvatarMenuButtonTest::SetUpCommandLine(base::CommandLine* command_line) {
switches::DisableNewAvatarMenuForTesting(command_line);
}
void AvatarMenuButtonTest::CreateTestingProfile() { void AvatarMenuButtonTest::CreateTestingProfile() {
ProfileManager* profile_manager = g_browser_process->profile_manager(); ProfileManager* profile_manager = g_browser_process->profile_manager();
EXPECT_EQ(1u, profile_manager->GetNumberOfProfiles()); EXPECT_EQ(1u, profile_manager->GetNumberOfProfiles());
......
...@@ -100,15 +100,15 @@ void OpenNewWindowForProfile( ...@@ -100,15 +100,15 @@ void OpenNewWindowForProfile(
} }
std::string GetAvatarImageAtIndex( std::string GetAvatarImageAtIndex(
size_t index, const ProfileInfoCache& info_cache) { size_t index, ProfileInfoCache* info_cache) {
bool is_gaia_picture = bool is_gaia_picture =
info_cache.IsUsingGAIAPictureOfProfileAtIndex(index) && info_cache->IsUsingGAIAPictureOfProfileAtIndex(index) &&
info_cache.GetGAIAPictureOfProfileAtIndex(index); info_cache->GetGAIAPictureOfProfileAtIndex(index);
// If the avatar is too small (i.e. the old-style low resolution avatar), // If the avatar is too small (i.e. the old-style low resolution avatar),
// it will be pixelated when displayed in the User Manager, so we should // it will be pixelated when displayed in the User Manager, so we should
// return the placeholder avatar instead. // return the placeholder avatar instead.
gfx::Image avatar_image = info_cache.GetAvatarIconOfProfileAtIndex(index); gfx::Image avatar_image = info_cache->GetAvatarIconOfProfileAtIndex(index);
if (avatar_image.Width() <= profiles::kAvatarIconWidth || if (avatar_image.Width() <= profiles::kAvatarIconWidth ||
avatar_image.Height() <= profiles::kAvatarIconHeight ) { avatar_image.Height() <= profiles::kAvatarIconHeight ) {
avatar_image = ui::ResourceBundle::GetSharedInstance().GetImageNamed( avatar_image = ui::ResourceBundle::GetSharedInstance().GetImageNamed(
...@@ -725,8 +725,8 @@ void UserManagerScreenHandler::GetLocalizedValues( ...@@ -725,8 +725,8 @@ void UserManagerScreenHandler::GetLocalizedValues(
void UserManagerScreenHandler::SendUserList() { void UserManagerScreenHandler::SendUserList() {
base::ListValue users_list; base::ListValue users_list;
const ProfileInfoCache& info_cache = ProfileInfoCache* info_cache =
g_browser_process->profile_manager()->GetProfileInfoCache(); &g_browser_process->profile_manager()->GetProfileInfoCache();
user_auth_type_map_.clear(); user_auth_type_map_.clear();
// Profile deletion is not allowed in Metro mode. // Profile deletion is not allowed in Metro mode.
...@@ -735,14 +735,14 @@ void UserManagerScreenHandler::SendUserList() { ...@@ -735,14 +735,14 @@ void UserManagerScreenHandler::SendUserList() {
can_remove = !ash::Shell::HasInstance(); can_remove = !ash::Shell::HasInstance();
#endif #endif
for (size_t i = 0; i < info_cache.GetNumberOfProfiles(); ++i) { for (size_t i = 0; i < info_cache->GetNumberOfProfiles(); ++i) {
base::DictionaryValue* profile_value = new base::DictionaryValue(); base::DictionaryValue* profile_value = new base::DictionaryValue();
base::FilePath profile_path = info_cache.GetPathOfProfileAtIndex(i); base::FilePath profile_path = info_cache->GetPathOfProfileAtIndex(i);
profile_value->SetString( profile_value->SetString(
kKeyUsername, info_cache.GetUserNameOfProfileAtIndex(i)); kKeyUsername, info_cache->GetUserNameOfProfileAtIndex(i));
profile_value->SetString( profile_value->SetString(
kKeyEmailAddress, info_cache.GetUserNameOfProfileAtIndex(i)); kKeyEmailAddress, info_cache->GetUserNameOfProfileAtIndex(i));
profile_value->SetString( profile_value->SetString(
kKeyDisplayName, kKeyDisplayName,
profiles::GetAvatarNameForProfile(profile_path)); profiles::GetAvatarNameForProfile(profile_path));
...@@ -750,11 +750,11 @@ void UserManagerScreenHandler::SendUserList() { ...@@ -750,11 +750,11 @@ void UserManagerScreenHandler::SendUserList() {
kKeyProfilePath, base::CreateFilePathValue(profile_path)); kKeyProfilePath, base::CreateFilePathValue(profile_path));
profile_value->SetBoolean(kKeyPublicAccount, false); profile_value->SetBoolean(kKeyPublicAccount, false);
profile_value->SetBoolean(kKeyLegacySupervisedUser, profile_value->SetBoolean(kKeyLegacySupervisedUser,
info_cache.ProfileIsLegacySupervisedAtIndex(i)); info_cache->ProfileIsLegacySupervisedAtIndex(i));
profile_value->SetBoolean( profile_value->SetBoolean(
kKeyChildUser, info_cache.ProfileIsChildAtIndex(i)); kKeyChildUser, info_cache->ProfileIsChildAtIndex(i));
profile_value->SetBoolean( profile_value->SetBoolean(
kKeyNeedsSignin, info_cache.ProfileIsSigninRequiredAtIndex(i)); kKeyNeedsSignin, info_cache->ProfileIsSigninRequiredAtIndex(i));
profile_value->SetBoolean(kKeyIsOwner, false); profile_value->SetBoolean(kKeyIsOwner, false);
profile_value->SetBoolean(kKeyCanRemove, can_remove); profile_value->SetBoolean(kKeyCanRemove, can_remove);
profile_value->SetBoolean(kKeyIsDesktop, true); profile_value->SetBoolean(kKeyIsDesktop, true);
......
...@@ -440,7 +440,6 @@ ...@@ -440,7 +440,6 @@
'browser/ui/autofill/test_generated_credit_card_bubble_view.h', 'browser/ui/autofill/test_generated_credit_card_bubble_view.h',
'browser/ui/blocked_content/popup_blocker_browsertest.cc', 'browser/ui/blocked_content/popup_blocker_browsertest.cc',
'browser/ui/bookmarks/bookmark_browsertest.cc', 'browser/ui/bookmarks/bookmark_browsertest.cc',
'browser/ui/bookmarks/bookmark_bubble_sign_in_delegate_browsertest.cc',
'browser/ui/browser_browsertest.cc', 'browser/ui/browser_browsertest.cc',
'browser/ui/browser_close_browsertest.cc', 'browser/ui/browser_close_browsertest.cc',
'browser/ui/browser_command_controller_browsertest.cc', 'browser/ui/browser_command_controller_browsertest.cc',
...@@ -604,6 +603,7 @@ ...@@ -604,6 +603,7 @@
'browser/ui/views/autofill/card_unmask_prompt_view_tester_views.h', 'browser/ui/views/autofill/card_unmask_prompt_view_tester_views.h',
'browser/ui/views/autofill/password_generation_popup_view_tester_views.cc', 'browser/ui/views/autofill/password_generation_popup_view_tester_views.cc',
'browser/ui/views/autofill/password_generation_popup_view_tester_views.h', 'browser/ui/views/autofill/password_generation_popup_view_tester_views.h',
'browser/ui/views/bookmarks/bookmark_bubble_sign_in_delegate_browsertest.cc',
'browser/ui/views/collected_cookies_views_browsertest.cc', 'browser/ui/views/collected_cookies_views_browsertest.cc',
'browser/ui/views/extensions/bookmark_override_browsertest.cc', 'browser/ui/views/extensions/bookmark_override_browsertest.cc',
'browser/ui/views/extensions/extension_install_dialog_view_browsertest.cc', 'browser/ui/views/extensions/extension_install_dialog_view_browsertest.cc',
......
...@@ -240,5 +240,7 @@ void TestingProfileManager::SetUpInternal() { ...@@ -240,5 +240,7 @@ void TestingProfileManager::SetUpInternal() {
profile_manager_ = new testing::ProfileManager(profiles_dir_.path()); profile_manager_ = new testing::ProfileManager(profiles_dir_.path());
browser_process_->SetProfileManager(profile_manager_); // Takes ownership. browser_process_->SetProfileManager(profile_manager_); // Takes ownership.
profile_manager_->GetProfileInfoCache().
set_disable_avatar_download_for_testing(true);
called_set_up_ = true; called_set_up_ = true;
} }
...@@ -79,16 +79,16 @@ State GetProcessState() { ...@@ -79,16 +79,16 @@ State GetProcessState() {
} else if (is_new_avatar_menu) { } else if (is_new_avatar_menu) {
return STATE_NEW_AVATAR_MENU; return STATE_NEW_AVATAR_MENU;
} else if (not_new_profile_management) { } else if (not_new_profile_management) {
return STATE_OLD_AVATAR_MENU; return STATE_NEW_AVATAR_MENU;
} else if (not_consistent_identity) { } else if (not_consistent_identity) {
return STATE_OLD_AVATAR_MENU; return STATE_NEW_AVATAR_MENU;
} }
// Set the default state // Set the default state
#if defined(OS_ANDROID) || defined(OS_IOS) #if defined(OS_ANDROID) || defined(OS_IOS)
State state = STATE_ACCOUNT_CONSISTENCY; State state = STATE_ACCOUNT_CONSISTENCY;
#else #else
State state = STATE_OLD_AVATAR_MENU; State state = STATE_NEW_AVATAR_MENU;
#endif #endif
if (!trial_type.empty()) { if (!trial_type.empty()) {
...@@ -99,7 +99,7 @@ State GetProcessState() { ...@@ -99,7 +99,7 @@ State GetProcessState() {
} else if (trial_type == "NewAvatarMenu") { } else if (trial_type == "NewAvatarMenu") {
state = STATE_NEW_AVATAR_MENU; state = STATE_NEW_AVATAR_MENU;
} else { } else {
state = STATE_OLD_AVATAR_MENU; state = STATE_NEW_AVATAR_MENU;
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment