Adds AppListServiceImplBrowserTest.ShowLoadedProfiles

Currently there is only AppListServiceInteractiveTest which is excessive
for tests that do not need to be interactive. It also leaves gaps in the
testing of the AppListServiceImpl, since it only uses the parent,
AppListService interface.

This CL adds scaffolding for better app list test coverage, leading up
to better testing for Chrome's AppListViewDelegate, which is currently
hard to get at in a test.

The test is a non-interactive version of showing the app list, which
verifies expectations about the actual Profile* that is missing from
current tests (e.g. TestingAppListServiceImpl has it mocked out to
return NULL, since unit_tests don't have a ProfileManager).

BUG=169114

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

Cr-Commit-Position: refs/heads/master@{#294138}

chrome/browser/ui/app_list/app_list_service_impl.h

@@ -23,6 +23,10 @@ namespace base {
 class FilePath;
 }
 
+namespace test {
+class AppListServiceImplTestApi;
+}
+
 // Parts of the AppListService implementation shared between platforms.
 class AppListServiceImpl : public AppListService,
                            public ProfileInfoCacheObserver {
@@ -67,6 +71,7 @@ class AppListServiceImpl : public AppListService,
   void PerformStartupChecks(Profile* initial_profile);
 
  private:
+  friend class test::AppListServiceImplTestApi;
   static void SendAppListStats();
 
   // Loads a profile asynchronously and calls OnProfileLoaded() when done.

chrome/browser/ui/app_list/app_list_service_impl_browsertest.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ui/app_list/app_list_service_impl.h"
+
+#include "base/prefs/pref_service.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/ui/app_list/test/chrome_app_list_test_support.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/test/base/in_process_browser_test.h"
+
+namespace test {
+
+// Test API to access private members of AppListServiceImpl.
+class AppListServiceImplTestApi {
+ public:
+  explicit AppListServiceImplTestApi(AppListServiceImpl* impl) : impl_(impl) {}
+
+  ProfileLoader* profile_loader() { return impl_->profile_loader_.get(); }
+
+ private:
+  AppListServiceImpl* impl_;
+
+  DISALLOW_COPY_AND_ASSIGN(AppListServiceImplTestApi);
+};
+
+}  // namespace test
+
+// Browser Test for AppListServiceImpl that runs on all platforms supporting
+// app_list.
+class AppListServiceImplBrowserTest : public InProcessBrowserTest {
+ public:
+  AppListServiceImplBrowserTest() {}
+
+  // Overridden from InProcessBrowserTest:
+  virtual void SetUpOnMainThread() OVERRIDE {
+    service_ = test::GetAppListServiceImpl();
+    test_api_.reset(new test::AppListServiceImplTestApi(service_));
+  }
+
+ protected:
+  AppListServiceImpl* service_;
+  scoped_ptr<test::AppListServiceImplTestApi> test_api_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AppListServiceImplBrowserTest);
+};
+
+// Test that showing a loaded profile for the first time is lazy and
+// synchronous. Then tests that showing a second loaded profile without
+// dismissing correctly switches profiles.
+IN_PROC_BROWSER_TEST_F(AppListServiceImplBrowserTest, ShowLoadedProfiles) {
+  PrefService* local_state = g_browser_process->local_state();
+  EXPECT_FALSE(local_state->HasPrefPath(prefs::kAppListProfile));
+
+  // When never shown, profile path should match the last used profile.
+  base::FilePath user_data_dir =
+      g_browser_process->profile_manager()->user_data_dir();
+  EXPECT_EQ(service_->GetProfilePath(user_data_dir),
+            browser()->profile()->GetPath());
+
+  // Just requesting the profile path shouldn't set it.
+  EXPECT_FALSE(local_state->HasPrefPath(prefs::kAppListProfile));
+
+  // Loading the Profile* should be lazy, except on ChromeOS where it is bound
+  // to ChromeLauncherController, which always has a profile.
+#if defined(OS_CHROMEOS)
+  EXPECT_TRUE(service_->GetCurrentAppListProfile());
+#else
+  EXPECT_FALSE(service_->GetCurrentAppListProfile());
+#endif
+
+  // Showing the app list for an unspecified profile, uses the loaded profile.
+  service_->Show();
+
+  // Load should be synchronous.
+  EXPECT_FALSE(test_api_->profile_loader()->IsAnyProfileLoading());
+  EXPECT_EQ(service_->GetCurrentAppListProfile(), browser()->profile());
+
+#if defined(OS_CHROMEOS)
+  // ChromeOS doesn't record the app list profile pref, and doesn't do profile
+  // switching.
+  EXPECT_FALSE(local_state->HasPrefPath(prefs::kAppListProfile));
+
+#else
+  // Preference should be updated automatically.
+  EXPECT_TRUE(local_state->HasPrefPath(prefs::kAppListProfile));
+  EXPECT_EQ(local_state->GetString(prefs::kAppListProfile),
+            browser()->profile()->GetPath().BaseName().MaybeAsASCII());
+
+  // Show for a second, pre-loaded profile without dismissing. Don't try this on
+  // ChromeOS because it does not support profile switching the app list.
+  Profile* profile2 = test::CreateSecondProfileAsync();
+  service_->ShowForProfile(profile2);
+
+  // Current profile and saved path should update synchronously.
+  EXPECT_FALSE(test_api_->profile_loader()->IsAnyProfileLoading());
+  EXPECT_EQ(profile2->GetPath(), service_->GetProfilePath(user_data_dir));
+  EXPECT_EQ(profile2, service_->GetCurrentAppListProfile());
+#endif
+}

chrome/browser/ui/app_list/app_list_service_interactive_uitest.cc

@@ -35,24 +35,7 @@ class AppListServiceInteractiveTest : public InProcessBrowserTest {
   AppListServiceInteractiveTest()
     : profile2_(NULL) {}
 
-  void InitSecondProfile() {
-    ProfileManager* profile_manager = g_browser_process->profile_manager();
-    base::FilePath temp_profile_dir =
-        profile_manager->user_data_dir().AppendASCII("Profile 1");
-    profile_manager->CreateProfileAsync(
-        temp_profile_dir,
-        base::Bind(&AppListServiceInteractiveTest::OnProfileCreated,
-                   this),
-        base::string16(), base::string16(), std::string());
-    content::RunMessageLoop();  // Will stop in OnProfileCreated().
-  }
-
-  void OnProfileCreated(Profile* profile, Profile::CreateStatus status) {
-    if (status == Profile::CREATE_STATUS_INITIALIZED) {
-      profile2_ = profile;
-      base::MessageLoop::current()->Quit();
-    }
-  }
+  void InitSecondProfile() { profile2_ = test::CreateSecondProfileAsync(); }
 
  protected:
   Profile* profile2_;

chrome/browser/ui/app_list/test/chrome_app_list_test_support.cc

@@ -4,12 +4,53 @@
 
 #include "chrome/browser/ui/app_list/test/chrome_app_list_test_support.h"
 
+#include "base/run_loop.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/app_list/app_list_service.h"
+#include "chrome/browser/ui/app_list/app_list_service_impl.h"
 #include "chrome/browser/ui/app_list/app_list_syncable_service.h"
 #include "chrome/browser/ui/app_list/app_list_syncable_service_factory.h"
 
 namespace test {
 
+namespace {
+
+class CreateProfileHelper {
+ public:
+  CreateProfileHelper() : profile_(NULL) {}
+
+  Profile* CreateAsync() {
+    ProfileManager* profile_manager = g_browser_process->profile_manager();
+    base::FilePath temp_profile_dir =
+        profile_manager->user_data_dir().AppendASCII("Profile 1");
+    profile_manager->CreateProfileAsync(
+        temp_profile_dir,
+        base::Bind(&CreateProfileHelper::OnProfileCreated,
+                   base::Unretained(this)),
+        base::string16(),
+        base::string16(),
+        std::string());
+    run_loop_.Run();
+    return profile_;
+  }
+
+ private:
+  void OnProfileCreated(Profile* profile, Profile::CreateStatus status) {
+    if (status == Profile::CREATE_STATUS_INITIALIZED) {
+      profile_ = profile;
+      run_loop_.Quit();
+    }
+  }
+
+  base::RunLoop run_loop_;
+  Profile* profile_;
+
+  DISALLOW_COPY_AND_ASSIGN(CreateProfileHelper);
+};
+
+}  // namespace
+
 app_list::AppListModel* GetAppListModel(AppListService* service) {
   return app_list::AppListSyncableServiceFactory::GetForProfile(
       service->GetCurrentAppListProfile())->model();
@@ -20,4 +61,15 @@ AppListService* GetAppListService() {
   return AppListService::Get(chrome::GetActiveDesktop());
 }
 
+AppListServiceImpl* GetAppListServiceImpl() {
+  // AppListServiceImpl is the only subclass of AppListService, which has pure
+  // virtuals. So this must either be NULL, or an AppListServiceImpl.
+  return static_cast<AppListServiceImpl*>(GetAppListService());
+}
+
+Profile* CreateSecondProfileAsync() {
+  CreateProfileHelper helper;
+  return helper.CreateAsync();
+}
+
 }  // namespace test

chrome/browser/ui/app_list/test/chrome_app_list_test_support.h

@@ -10,14 +10,22 @@ class AppListModel;
 }
 
 class AppListService;
+class AppListServiceImpl;
+class Profile;
 
 namespace test {
 
 // Gets the model keyed to the profile currently associated with |service|.
 app_list::AppListModel* GetAppListModel(AppListService* service);
 
-// Gets the app list service for the desktop type currently being tested.
+// Gets the app list service for the desktop type currently being tested. These
+// are the same, but split so that files don't need to know that the impl is a
+// subclass.
 AppListService* GetAppListService();
+AppListServiceImpl* GetAppListServiceImpl();
+
+// Creates a second profile in a nested message loop for testing the app list.
+Profile* CreateSecondProfileAsync();
 
 }  // namespace test
 

chrome/chrome_tests.gypi

@@ -1396,6 +1396,7 @@
         'browser/translate/cld_data_harness.h',
         'browser/translate/translate_manager_browsertest.cc',
         'browser/ui/app_list/app_list_controller_browsertest.cc',
+        'browser/ui/app_list/app_list_service_impl_browsertest.cc',
         'browser/ui/app_list/app_list_service_views_browsertest.cc',
         'browser/ui/app_list/search/people/people_provider_browsertest.cc',
         'browser/ui/app_list/search/webstore/webstore_provider_browsertest.cc',