Fix Launcher crash in ChromeOS guest mode.

The CHECK-fail was introduced in r315272 (64121b7), which switched the app list to run with the "original profile" rather than the incognito profile. Partially reverted that CL so that the app list runs with the incognito profile, and fixed StartPageService to correctly use the incognito profile (rather than being null when the app list profile is incognito), which addresses the original issue (http://crbug.com/440484). Had to work around DCHECK on shutdown in StartPageService due to crbug.com/463419. Added a DCHECK to catch use of non-incognito profiles in guest mode early (without having to select a search result to trigger the crash). Added AppListControllerGuestModeBrowserTest to test opening the app list in guest mode. BUG=460437,440484 Review URL: https://codereview.chromium.org/976553003 Cr-Commit-Position: refs/heads/master@{#319036} (cherry picked from commit 3bbe0ce) [email protected] Review URL: https://codereview.chromium.org/987623002 Cr-Commit-Position: refs/branch-heads/2311@{#158} Cr-Branched-From: 09b7de5-refs/heads/master@{#317474}
crosswalk-project · Mar 6, 2015 · 8b2777d · 8b2777d
1 parent 5939e2a
commit 8b2777d
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 3 deletions.
diff --git a/chrome/browser/ui/app_list/app_list_controller_browsertest.cc b/chrome/browser/ui/app_list/app_list_controller_browsertest.cc
@@ -15,14 +15,21 @@
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/host_desktop.h"
 #include "chrome/common/chrome_paths.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/testing_profile.h"
 #include "ui/app_list/app_list_model.h"
 #include "ui/app_list/app_list_switches.h"
 #include "ui/app_list/search_box_model.h"
 #include "ui/app_list/search_result.h"
 #include "ui/app_list/search_result_observer.h"
 #include "ui/base/models/list_model_observer.h"
 
+#if defined(OS_CHROMEOS)
+#include "chromeos/chromeos_switches.h"
+#include "chromeos/login/user_names.h"
+#endif  // defined(OS_CHROMEOS)
+
 // Browser Test for AppListController that runs on all platforms supporting
 // app_list.
 typedef InProcessBrowserTest AppListControllerBrowserTest;
@@ -212,3 +219,37 @@ IN_PROC_BROWSER_TEST_F(AppListControllerSearchResultsBrowserTest,
   StopWatchingResults();
   service->DismissAppList();
 }
+
+#if defined(OS_CHROMEOS)
+
+class AppListControllerGuestModeBrowserTest : public InProcessBrowserTest {
+ public:
+  AppListControllerGuestModeBrowserTest() {}
+
+ protected:
+  void SetUpCommandLine(base::CommandLine* command_line) override;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AppListControllerGuestModeBrowserTest);
+};
+
+void AppListControllerGuestModeBrowserTest::SetUpCommandLine(
+    base::CommandLine* command_line) {
+  command_line->AppendSwitch(chromeos::switches::kGuestSession);
+  command_line->AppendSwitchASCII(chromeos::switches::kLoginUser,
+                                  chromeos::login::kGuestUserName);
+  command_line->AppendSwitchASCII(chromeos::switches::kLoginProfile,
+                                  TestingProfile::kTestUserProfileDir);
+  command_line->AppendSwitch(switches::kIncognito);
+}
+
+// Test creating the initial app list in guest mode.
+IN_PROC_BROWSER_TEST_F(AppListControllerGuestModeBrowserTest, Incognito) {
+  AppListService* service = test::GetAppListService();
+  EXPECT_TRUE(service->GetCurrentAppListProfile());
+
+  service->ShowForProfile(browser()->profile());
+  EXPECT_EQ(browser()->profile(), service->GetCurrentAppListProfile());
+}
+
+#endif  // defined(OS_CHROMEOS)
diff --git a/chrome/browser/ui/app_list/app_list_service_impl.cc b/chrome/browser/ui/app_list/app_list_service_impl.cc
@@ -261,7 +261,7 @@ AppListServiceImpl::~AppListServiceImpl() {}
 AppListViewDelegate* AppListServiceImpl::GetViewDelegate(Profile* profile) {
   if (!view_delegate_)
     view_delegate_.reset(new AppListViewDelegate(GetControllerDelegate()));
-  view_delegate_->SetProfile(profile->GetOriginalProfile());
+  view_delegate_->SetProfile(profile);
   return view_delegate_.get();
 }
 

diff --git a/chrome/browser/ui/app_list/app_list_view_delegate.cc b/chrome/browser/ui/app_list/app_list_view_delegate.cc
@@ -285,6 +285,13 @@ void AppListViewDelegate::SetProfile(Profile* new_profile) {
     return;
   }
 
+  // If we are in guest mode, the new profile should be an incognito profile.
+  // Otherwise, this may later hit a check (same condition as this one) in
+  // Browser::Browser when opening links in a browser window (see
+  // http://crbug.com/460437).
+  DCHECK(!profile_->IsGuestSession() || profile_->IsOffTheRecord())
+      << "Guest mode must use incognito profile";
+
   // TODO(vadimt): Remove ScopedTracker below once crbug.com/431326 is fixed.
   tracked_objects::ScopedTracker tracking_profile3(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(

diff --git a/chrome/browser/ui/app_list/start_page_service.cc b/chrome/browser/ui/app_list/start_page_service.cc
@@ -86,6 +86,15 @@ class StartPageService::ProfileDestroyObserver
  public:
   explicit ProfileDestroyObserver(StartPageService* service)
       : service_(service) {
+    if (service_->profile()->IsOffTheRecord()) {
+      // We need to be notified when the original profile gets destroyed as well
+      // as the OTR profile, because the original profile will be destroyed
+      // first, and a DCHECK at that time ensures that the OTR profile has 0
+      // hosts. See http://crbug.com/463419.
+      registrar_.Add(
+          this, chrome::NOTIFICATION_PROFILE_DESTROYED,
+          content::Source<Profile>(service_->profile()->GetOriginalProfile()));
+    }
     registrar_.Add(this,
                    chrome::NOTIFICATION_PROFILE_DESTROYED,
                    content::Source<Profile>(service_->profile()));
@@ -98,7 +107,9 @@ class StartPageService::ProfileDestroyObserver
                const content::NotificationSource& source,
                const content::NotificationDetails& details) override {
     DCHECK_EQ(chrome::NOTIFICATION_PROFILE_DESTROYED, type);
-    DCHECK_EQ(service_->profile(), content::Source<Profile>(source).ptr());
+    DCHECK(service_->profile()->IsSameProfile(
+        content::Source<Profile>(source).ptr()));
+    registrar_.RemoveAll();
     service_->Shutdown();
   }
 

diff --git a/chrome/browser/ui/app_list/start_page_service_factory.cc b/chrome/browser/ui/app_list/start_page_service_factory.cc
@@ -55,7 +55,7 @@ KeyedService* StartPageServiceFactory::BuildServiceInstanceFor(
 content::BrowserContext* StartPageServiceFactory::GetBrowserContextToUse(
     content::BrowserContext* context) const {
   // The start page service needs an instance in ChromeOS guest mode.
-  return chrome::GetBrowserContextRedirectedInIncognito(context);
+  return chrome::GetBrowserContextOwnInstanceInIncognito(context);
 }
 
 }  // namespace app_list