Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(308)

Side by Side Diff: chrome/browser/ui/app_list/app_list_service_unittest.cc

Issue 492163002: Fix Profile* lifetime issues in Chrome's AppListViewDelegate (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Add constructor comment Created 6 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
OLDNEW
1 // Copyright 2013 The Chromium Authors. All rights reserved. 1 // Copyright 2013 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "base/command_line.h" 5 #include "base/command_line.h"
6 #include "base/files/file_path.h" 6 #include "base/files/file_path.h"
7 #include "base/memory/scoped_ptr.h" 7 #include "base/memory/scoped_ptr.h"
8 #include "base/prefs/pref_registry_simple.h" 8 #include "base/prefs/pref_registry_simple.h"
9 #include "base/prefs/pref_service.h" 9 #include "base/prefs/pref_service.h"
10 #include "base/prefs/pref_service_factory.h" 10 #include "base/prefs/pref_service_factory.h"
11 #include "base/prefs/testing_pref_store.h" 11 #include "base/prefs/testing_pref_store.h"
12 #include "chrome/browser/profiles/profile.h" 12 #include "chrome/browser/profiles/profile.h"
13 #include "chrome/browser/profiles/profiles_state.h" 13 #include "chrome/browser/profiles/profiles_state.h"
14 #include "chrome/browser/ui/app_list/app_list_service.h" 14 #include "chrome/browser/ui/app_list/app_list_service.h"
15 #include "chrome/browser/ui/app_list/app_list_service_impl.h" 15 #include "chrome/browser/ui/app_list/app_list_service_impl.h"
16 #include "chrome/browser/ui/app_list/test/fake_profile.h" 16 #include "chrome/browser/ui/app_list/test/fake_profile.h"
17 #include "chrome/browser/ui/app_list/test/fake_profile_store.h" 17 #include "chrome/browser/ui/app_list/test/fake_profile_store.h"
18 #include "chrome/common/chrome_constants.h" 18 #include "chrome/common/chrome_constants.h"
19 #include "chrome/common/chrome_switches.h" 19 #include "chrome/common/chrome_switches.h"
20 #include "chrome/common/pref_names.h" 20 #include "chrome/common/pref_names.h"
21 #include "testing/gtest/include/gtest/gtest.h" 21 #include "testing/gtest/include/gtest/gtest.h"
22 22
23 class TestingAppListServiceImpl : public AppListServiceImpl { 23 class TestingAppListServiceImpl : public AppListServiceImpl {
24 public: 24 public:
25 TestingAppListServiceImpl(const CommandLine& command_line, 25 TestingAppListServiceImpl(const CommandLine& command_line,
26 PrefService* local_state, 26 PrefService* local_state,
27 scoped_ptr<ProfileStore> profile_store) 27 scoped_ptr<ProfileStore> profile_store)
28 : AppListServiceImpl(command_line, local_state, profile_store.Pass()), 28 : AppListServiceImpl(command_line, local_state, profile_store.Pass()),
29 showing_for_profile_(NULL) {} 29 showing_for_profile_(NULL),
30 destroy_app_list_call_count_(0) {}
30 31
31 Profile* showing_for_profile() const { 32 Profile* showing_for_profile() const {
32 return showing_for_profile_; 33 return showing_for_profile_;
33 } 34 }
34 35
36 int destroy_app_list_call_count() const {
37 return destroy_app_list_call_count_;
38 }
39
35 void PerformStartupChecks(Profile* profile) { 40 void PerformStartupChecks(Profile* profile) {
36 AppListServiceImpl::PerformStartupChecks(profile); 41 AppListServiceImpl::PerformStartupChecks(profile);
37 } 42 }
38 43
44 // AppListService overrides:
39 virtual Profile* GetCurrentAppListProfile() OVERRIDE { 45 virtual Profile* GetCurrentAppListProfile() OVERRIDE {
40 // We don't return showing_for_profile_ here because that is only defined if 46 // We don't return showing_for_profile_ here because that is only defined if
41 // the app list is visible. 47 // the app list is visible.
42 return NULL; 48 return NULL;
43 } 49 }
44 50
45 virtual void CreateForProfile(Profile* requested_profile) OVERRIDE { 51 virtual void CreateForProfile(Profile* requested_profile) OVERRIDE {
46 } 52 }
47 53
48 virtual void ShowForProfile(Profile* requested_profile) OVERRIDE { 54 virtual void ShowForProfile(Profile* requested_profile) OVERRIDE {
(...skipping 10 matching lines...) Expand all
59 } 65 }
60 66
61 virtual gfx::NativeWindow GetAppListWindow() OVERRIDE { 67 virtual gfx::NativeWindow GetAppListWindow() OVERRIDE {
62 return NULL; 68 return NULL;
63 } 69 }
64 70
65 virtual AppListControllerDelegate* GetControllerDelegate() OVERRIDE { 71 virtual AppListControllerDelegate* GetControllerDelegate() OVERRIDE {
66 return NULL; 72 return NULL;
67 } 73 }
68 74
75 // AppListServiceImpl overrides:
76 virtual void DestroyAppList() OVERRIDE { ++destroy_app_list_call_count_; }
77
69 private: 78 private:
70 Profile* showing_for_profile_; 79 Profile* showing_for_profile_;
80 int destroy_app_list_call_count_;
81
82 DISALLOW_COPY_AND_ASSIGN(TestingAppListServiceImpl);
71 }; 83 };
72 84
73 class AppListServiceUnitTest : public testing::Test { 85 class AppListServiceUnitTest : public testing::Test {
74 public: 86 public:
75 AppListServiceUnitTest() {} 87 AppListServiceUnitTest() {}
76 88
77 virtual void SetUp() OVERRIDE { 89 virtual void SetUp() OVERRIDE {
78 SetupWithCommandLine(CommandLine(CommandLine::NO_PROGRAM)); 90 SetupWithCommandLine(CommandLine(CommandLine::NO_PROGRAM));
79 } 91 }
80 92
(...skipping 57 matching lines...) Expand 10 before | Expand all | Expand 10 after
138 base::FilePath initial_profile_path = 150 base::FilePath initial_profile_path =
139 user_data_dir_.AppendASCII(chrome::kInitialProfile); 151 user_data_dir_.AppendASCII(chrome::kInitialProfile);
140 EXPECT_EQ(initial_profile_path, 152 EXPECT_EQ(initial_profile_path,
141 service_->GetProfilePath(profile_store_->GetUserDataDir())); 153 service_->GetProfilePath(profile_store_->GetUserDataDir()));
142 } 154 }
143 155
144 TEST_F(AppListServiceUnitTest, 156 TEST_F(AppListServiceUnitTest,
145 RemovedProfileResetsToLastUsedProfileIfExists) { 157 RemovedProfileResetsToLastUsedProfileIfExists) {
146 local_state_->SetString(prefs::kProfileLastUsed, "last-used"); 158 local_state_->SetString(prefs::kProfileLastUsed, "last-used");
147 EnableAppList(); 159 EnableAppList();
160 EXPECT_EQ(0, service_->destroy_app_list_call_count());
148 profile_store_->RemoveProfile(profile1_.get()); 161 profile_store_->RemoveProfile(profile1_.get());
162
149 base::FilePath last_used_profile_path = 163 base::FilePath last_used_profile_path =
150 user_data_dir_.AppendASCII("last-used"); 164 user_data_dir_.AppendASCII("last-used");
151 EXPECT_EQ(last_used_profile_path, 165 EXPECT_EQ(last_used_profile_path,
152 service_->GetProfilePath(profile_store_->GetUserDataDir())); 166 service_->GetProfilePath(profile_store_->GetUserDataDir()));
167
168 // Ensure a tear-down was triggered, since there would be references to the
169 // destroyed Profile, and the last-used profile could be getting loaded
170 // asynchronously.
171 EXPECT_EQ(1, service_->destroy_app_list_call_count());
153 } 172 }
154 173
155 TEST_F(AppListServiceUnitTest, SwitchingProfilesPersists) { 174 TEST_F(AppListServiceUnitTest, SwitchingProfilesPersists) {
156 profile_store_->LoadProfile(profile1_.get()); 175 profile_store_->LoadProfile(profile1_.get());
157 profile_store_->LoadProfile(profile2_.get()); 176 profile_store_->LoadProfile(profile2_.get());
158 EnableAppList(); 177 EnableAppList();
159 service_->SetProfilePath(profile2_->GetPath()); 178 service_->SetProfilePath(profile2_->GetPath());
160 service_->Show(); 179 service_->Show();
161 EXPECT_EQ(profile2_.get(), service_->showing_for_profile()); 180 EXPECT_EQ(profile2_.get(), service_->showing_for_profile());
162 EXPECT_EQ(profile2_->GetPath(), 181 EXPECT_EQ(profile2_->GetPath(),
(...skipping 61 matching lines...) Expand 10 before | Expand all | Expand 10 after
224 EXPECT_EQ(AppListService::ENABLE_FOR_APP_INSTALL, 243 EXPECT_EQ(AppListService::ENABLE_FOR_APP_INSTALL,
225 local_state_->GetInteger(prefs::kAppListEnableMethod)); 244 local_state_->GetInteger(prefs::kAppListEnableMethod));
226 EXPECT_NE(0, local_state_->GetInt64(prefs::kAppListEnableTime)); 245 EXPECT_NE(0, local_state_->GetInt64(prefs::kAppListEnableTime));
227 246
228 // An auto-show here should update the enable method to prevent recording it 247 // An auto-show here should update the enable method to prevent recording it
229 // as ENABLE_FOR_APP_INSTALL. 248 // as ENABLE_FOR_APP_INSTALL.
230 service_->AutoShowForProfile(profile1_.get()); 249 service_->AutoShowForProfile(profile1_.get());
231 EXPECT_EQ(AppListService::ENABLE_SHOWN_UNDISCOVERED, 250 EXPECT_EQ(AppListService::ENABLE_SHOWN_UNDISCOVERED,
232 local_state_->GetInteger(prefs::kAppListEnableMethod)); 251 local_state_->GetInteger(prefs::kAppListEnableMethod));
233 } 252 }
OLDNEW
« no previous file with comments | « chrome/browser/ui/app_list/app_list_service_mac.mm ('k') | chrome/browser/ui/app_list/app_list_service_views.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698