Commit 6c602d4e authored by WC Leung's avatar WC Leung Committed by Commit Bot

Bug fixes of ProfileAttributesStorageTest.LoadAvatarFromDiskTest

The captioned test was disabled in Linux ASAN because of flakiness. Two
possible causes were identified:

- Possible race between DownloadHighResAvatarTest and
  LoadAvatarFromDiskTest because the path returned by
  GetPathOfHighResAvatarAtIndex is not inside the unique temp directory.

- The directory holding the test icon is not created before running
  base::WriteFile.

This CL fixes the above issues, and changed "const char* bitmap" to
"const char bitmap[]" because of issue 795518.


Bug: 794821, 795518
Change-Id: I0deb2c8acddea86fb94ca27a363afa132f0ac6fc
Reviewed-on: https://chromium-review.googlesource.com/868411Reviewed-by: default avatarLei Zhang <thestig@chromium.org>
Reviewed-by: default avatarBernhard Bauer <bauerb@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: WC Leung <lwchkg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531370}
parent afe928de
...@@ -20,7 +20,6 @@ ...@@ -20,7 +20,6 @@
#include "base/sequenced_task_runner.h" #include "base/sequenced_task_runner.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/test/scoped_path_override.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "base/values.h" #include "base/values.h"
#include "chrome/browser/component_updater/supervised_user_whitelist_installer.h" #include "chrome/browser/component_updater/supervised_user_whitelist_installer.h"
...@@ -198,8 +197,7 @@ class WhitelistLoadObserver { ...@@ -198,8 +197,7 @@ class WhitelistLoadObserver {
class SupervisedUserWhitelistInstallerTest : public testing::Test { class SupervisedUserWhitelistInstallerTest : public testing::Test {
public: public:
SupervisedUserWhitelistInstallerTest() SupervisedUserWhitelistInstallerTest()
: testing_profile_manager_(TestingBrowserProcess::GetGlobal()), : testing_profile_manager_(TestingBrowserProcess::GetGlobal()) {}
user_data_dir_override_(chrome::DIR_USER_DATA) {}
~SupervisedUserWhitelistInstallerTest() override {} ~SupervisedUserWhitelistInstallerTest() override {}
...@@ -297,7 +295,6 @@ class SupervisedUserWhitelistInstallerTest : public testing::Test { ...@@ -297,7 +295,6 @@ class SupervisedUserWhitelistInstallerTest : public testing::Test {
content::TestBrowserThreadBundle thread_bundle_; content::TestBrowserThreadBundle thread_bundle_;
TestingProfileManager testing_profile_manager_; TestingProfileManager testing_profile_manager_;
base::ScopedPathOverride user_data_dir_override_;
data_decoder::TestingJsonParser::ScopedFactoryOverride json_parser_override_; data_decoder::TestingJsonParser::ScopedFactoryOverride json_parser_override_;
TestingPrefServiceSimple local_state_; TestingPrefServiceSimple local_state_;
content::TestServiceManagerContext service_manager_context_; content::TestServiceManagerContext service_manager_context_;
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/command_line.h" #include "base/command_line.h"
#include "base/files/scoped_temp_dir.h"
#include "base/message_loop/message_loop.h" #include "base/message_loop/message_loop.h"
#include "base/process/kill.h" #include "base/process/kill.h"
#include "base/process/process.h" #include "base/process/process.h"
...@@ -43,8 +44,6 @@ ...@@ -43,8 +44,6 @@
#include "chrome/test/base/chrome_unit_test_suite.h" #include "chrome/test/base/chrome_unit_test_suite.h"
#include "chrome/test/base/test_launcher_utils.h" #include "chrome/test/base/test_launcher_utils.h"
#include "chrome/test/base/testing_browser_process.h" #include "chrome/test/base/testing_browser_process.h"
#include "chrome/test/base/testing_profile.h"
#include "chrome/test/base/testing_profile_manager.h"
#include "components/sync_preferences/testing_pref_service_syncable.h" #include "components/sync_preferences/testing_pref_service_syncable.h"
#include "content/public/browser/notification_service.h" #include "content/public/browser/notification_service.h"
#include "content/public/common/content_paths.h" #include "content/public/common/content_paths.h"
...@@ -481,11 +480,6 @@ TEST_F(CloudPrintProxyPolicyStartupTest, StartAndShutdown) { ...@@ -481,11 +480,6 @@ TEST_F(CloudPrintProxyPolicyStartupTest, StartAndShutdown) {
BrowserThread::GetTaskRunnerForThread(BrowserThread::IO), BrowserThread::GetTaskRunnerForThread(BrowserThread::IO),
mojo::edk::ScopedIPCSupport::ShutdownPolicy::FAST); mojo::edk::ScopedIPCSupport::ShutdownPolicy::FAST);
TestingBrowserProcess* browser_process =
TestingBrowserProcess::GetGlobal();
TestingProfileManager profile_manager(browser_process);
ASSERT_TRUE(profile_manager.SetUp());
base::Process process = base::Process process =
Launch("CloudPrintMockService_StartEnabledWaitForQuit"); Launch("CloudPrintMockService_StartEnabledWaitForQuit");
mojo::edk::PeerConnection peer_connection; mojo::edk::PeerConnection peer_connection;
......
...@@ -804,8 +804,11 @@ TEST_F(ProfileAttributesStorageTest, MAYBE_LoadAvatarFromDiskTest) { ...@@ -804,8 +804,11 @@ TEST_F(ProfileAttributesStorageTest, MAYBE_LoadAvatarFromDiskTest) {
profiles::GetPathOfHighResAvatarAtIndex(kIconIndex); profiles::GetPathOfHighResAvatarAtIndex(kIconIndex);
// Create the avatar on the disk, which is a valid 1x1 transparent png. // Create the avatar on the disk, which is a valid 1x1 transparent png.
base::FilePath dir = icon_path.DirName();
ASSERT_FALSE(base::DirectoryExists(dir));
ASSERT_TRUE(base::CreateDirectory(dir));
ASSERT_FALSE(base::PathExists(icon_path)); ASSERT_FALSE(base::PathExists(icon_path));
const char* bitmap = const char bitmap[] =
"\x89\x50\x4E\x47\x0D\x0A\x1A\x0A\x00\x00\x00\x0D\x49\x48\x44\x52" "\x89\x50\x4E\x47\x0D\x0A\x1A\x0A\x00\x00\x00\x0D\x49\x48\x44\x52"
"\x00\x00\x00\x01\x00\x00\x00\x01\x01\x00\x00\x00\x00\x37\x6E\xF9" "\x00\x00\x00\x01\x00\x00\x00\x01\x01\x00\x00\x00\x00\x37\x6E\xF9"
"\x24\x00\x00\x00\x0A\x49\x44\x41\x54\x08\x1D\x63\x60\x00\x00\x00" "\x24\x00\x00\x00\x0A\x49\x44\x41\x54\x08\x1D\x63\x60\x00\x00\x00"
......
...@@ -22,7 +22,6 @@ ...@@ -22,7 +22,6 @@
#include "chrome/browser/profiles/profile_avatar_icon_util.h" #include "chrome/browser/profiles/profile_avatar_icon_util.h"
#include "chrome/browser/profiles/profile_info_cache.h" #include "chrome/browser/profiles/profile_info_cache.h"
#include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/profiles/profile_manager.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_switches.h" #include "chrome/common/chrome_switches.h"
#include "chrome/common/pref_names.h" #include "chrome/common/pref_names.h"
#include "chrome/test/base/testing_browser_process.h" #include "chrome/test/base/testing_browser_process.h"
...@@ -95,9 +94,7 @@ ProfileInfoCache* ProfileNameVerifierObserver::GetCache() { ...@@ -95,9 +94,7 @@ ProfileInfoCache* ProfileNameVerifierObserver::GetCache() {
ProfileInfoCacheTest::ProfileInfoCacheTest() ProfileInfoCacheTest::ProfileInfoCacheTest()
: testing_profile_manager_(TestingBrowserProcess::GetGlobal()), : testing_profile_manager_(TestingBrowserProcess::GetGlobal()),
name_observer_(&testing_profile_manager_), name_observer_(&testing_profile_manager_) {}
user_data_dir_override_(chrome::DIR_USER_DATA) {
}
ProfileInfoCacheTest::~ProfileInfoCacheTest() { ProfileInfoCacheTest::~ProfileInfoCacheTest() {
} }
......
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include <set> #include <set>
#include "base/macros.h" #include "base/macros.h"
#include "base/test/scoped_path_override.h"
#include "chrome/browser/profiles/profile_info_cache_observer.h" #include "chrome/browser/profiles/profile_info_cache_observer.h"
#include "chrome/test/base/testing_profile_manager.h" #include "chrome/test/base/testing_profile_manager.h"
#include "content/public/test/test_browser_thread_bundle.h" #include "content/public/test/test_browser_thread_bundle.h"
...@@ -66,7 +65,6 @@ class ProfileInfoCacheTest : public testing::Test { ...@@ -66,7 +65,6 @@ class ProfileInfoCacheTest : public testing::Test {
private: private:
ProfileNameVerifierObserver name_observer_; ProfileNameVerifierObserver name_observer_;
base::ScopedPathOverride user_data_dir_override_;
}; };
#endif // CHROME_BROWSER_PROFILES_PROFILE_INFO_CACHE_UNITTEST_H_ #endif // CHROME_BROWSER_PROFILES_PROFILE_INFO_CACHE_UNITTEST_H_
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "chrome/browser/profiles/profile_info_cache.h" #include "chrome/browser/profiles/profile_info_cache.h"
#include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/profiles/profile_manager.h"
#include "chrome/common/chrome_constants.h" #include "chrome/common/chrome_constants.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/test/base/testing_browser_process.h" #include "chrome/test/base/testing_browser_process.h"
#include "components/sync_preferences/pref_service_syncable.h" #include "components/sync_preferences/pref_service_syncable.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -45,8 +46,7 @@ TestingProfileManager::TestingProfileManager(TestingBrowserProcess* process) ...@@ -45,8 +46,7 @@ TestingProfileManager::TestingProfileManager(TestingBrowserProcess* process)
: called_set_up_(false), : called_set_up_(false),
browser_process_(process), browser_process_(process),
local_state_(process), local_state_(process),
profile_manager_(NULL) { profile_manager_(nullptr) {}
}
TestingProfileManager::~TestingProfileManager() { TestingProfileManager::~TestingProfileManager() {
// Destroying this class also destroys the LocalState, so make sure the // Destroying this class also destroys the LocalState, so make sure the
...@@ -249,6 +249,8 @@ void TestingProfileManager::SetUpInternal() { ...@@ -249,6 +249,8 @@ void TestingProfileManager::SetUpInternal() {
// Set up the directory for profiles. // Set up the directory for profiles.
ASSERT_TRUE(profiles_dir_.CreateUniqueTempDir()); ASSERT_TRUE(profiles_dir_.CreateUniqueTempDir());
user_data_dir_override_ = std::make_unique<base::ScopedPathOverride>(
chrome::DIR_USER_DATA, profiles_dir_.GetPath());
profile_manager_ = new testing::ProfileManager(profiles_dir_.GetPath()); profile_manager_ = new testing::ProfileManager(profiles_dir_.GetPath());
browser_process_->SetProfileManager(profile_manager_); // Takes ownership. browser_process_->SetProfileManager(profile_manager_); // Takes ownership.
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/files/scoped_temp_dir.h" #include "base/files/scoped_temp_dir.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "base/test/scoped_path_override.h"
#include "chrome/test/base/scoped_testing_local_state.h" #include "chrome/test/base/scoped_testing_local_state.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
...@@ -128,6 +129,12 @@ class TestingProfileManager { ...@@ -128,6 +129,12 @@ class TestingProfileManager {
// The directory in which new profiles are placed. // The directory in which new profiles are placed.
base::ScopedTempDir profiles_dir_; base::ScopedTempDir profiles_dir_;
// The user data directory in the path service is overriden because some
// functions, e.g. GetPathOfHighResAvatarAtIndex, get the user data directory
// by the path service instead of the profile manager. The override is scoped
// with the help of this variable.
std::unique_ptr<base::ScopedPathOverride> user_data_dir_override_;
// Weak reference to the browser process on which the ProfileManager is set. // Weak reference to the browser process on which the ProfileManager is set.
TestingBrowserProcess* browser_process_; TestingBrowserProcess* browser_process_;
......
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