arc: Fix race conditon when Play Store is started too early.

chrome/browser/ui/app_list/arc/arc_app_test.cc

 // Copyright 2016 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/arc/arc_app_test.h"
 
 #include "base/command_line.h"
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
@@ skipping 77 matching lines @@
           base::Bind(arc::FakeArcSession::Create)));
   DCHECK(arc::ArcSessionManager::Get());
   arc::ArcSessionManager::DisableUIForTesting();
   arc::ArcAuthNotification::DisableForTesting();
   arc_session_manager_->SetProfile(profile_);
   arc_play_store_enabled_preference_handler_ =
       base::MakeUnique<arc::ArcPlayStoreEnabledPreferenceHandler>(
           profile_, arc_session_manager_.get());
   arc_play_store_enabled_preference_handler_->Start();
 
-  arc_app_list_pref_ = ArcAppListPrefs::Get(profile_);
-  DCHECK(arc_app_list_pref_);
-  base::RunLoop run_loop;
-  arc_app_list_pref_->SetDefaltAppsReadyCallback(run_loop.QuitClosure());
-  run_loop.Run();
+  if (wait_default_apps_)
+    WaitForDefaultApps();
 
   // Check initial conditions.
   if (arc::ShouldArcAlwaysStart()) {
     // When ARC first starts, it runs in opt-out mode of Play Store.
     EXPECT_TRUE(arc_session_manager_->IsSessionRunning());
   } else {
     arc::SetArcPlayStoreEnabledForProfile(profile_, true);
     EXPECT_FALSE(arc_session_manager_->IsSessionRunning());
   }
 
+  arc_app_list_pref_ = ArcAppListPrefs::Get(profile_);

[hidehiko, 2017/03/16 16:23:19]

Initializing twice looks not straightforward.

Could you:

move L110-L111 to L97 (just before "if (wait_default_apps_)" block),
and remove L118?
Note: Keeping DCHECK at L118 should make sense, meaning arc_app_list_pref_ needs
to be set beforehand, IMO.

[khmel, 2017/03/16 16:38:47]

Done.

+  DCHECK(arc_app_list_pref_);
   app_instance_.reset(new arc::FakeAppInstance(arc_app_list_pref_));
   arc_service_manager_->arc_bridge_service()->app()->SetInstance(
       app_instance_.get());
 }
 
+void ArcAppTest::WaitForDefaultApps() {
+  arc_app_list_pref_ = ArcAppListPrefs::Get(profile_);
+  DCHECK(arc_app_list_pref_);
+  base::RunLoop run_loop;
+  arc_app_list_pref_->SetDefaltAppsReadyCallback(run_loop.QuitClosure());
+  run_loop.Run();
+}
+
 void ArcAppTest::CreateFakeAppsAndPackages() {
   arc::mojom::AppInfo app;
   // Make sure we have enough data for test.
   for (int i = 0; i < 3; ++i) {
     app.name = base::StringPrintf("Fake App %d", i);
     app.package_name = base::StringPrintf("fake.app.%d", i);
     app.activity = base::StringPrintf("fake.app.%d.activity", i);
     app.sticky = false;
     fake_apps_.push_back(app);
   }
@@ skipping 96 matching lines @@
   }
 }
 
 bool ArcAppTest::FindPackage(const arc::mojom::ArcPackageInfo& package) {
   for (auto fake_package : fake_packages_) {
     if (package.package_name == fake_package.package_name)
       return true;
   }
   return false;
 }