Extract kArcEnabled preference from ArcSessionManager part 1.

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

Index: chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc
diff --git a/chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc b/chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc
index 017ecaa23967ecdede358b9d5744db36615be9a9..7ecb2df98345aa76f9043de299f26d8c717398f7 100644
--- a/chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc
+++ b/chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc
@@ -13,6 +13,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/task_scheduler/post_task.h"
+#include "chrome/browser/chromeos/arc/arc_session_manager.h"
 #include "chrome/browser/chromeos/arc/arc_util.h"
 #include "chrome/browser/chromeos/arc/policy/arc_policy_util.h"
 #include "chrome/browser/profiles/profile.h"
@@ -141,9 +142,14 @@ void DeleteAppFolderFromFileThread(const base::FilePath& path) {
   DCHECK(deleted);
 }
 
-bool IsArcEnabled() {
+bool IsArcEnabled(const Profile* profile) {

[Yusuke Sato, 2017/02/17 22:27:09]

Shouldn't we rename this function?

[hidehiko, 2017/02/20 18:18:44]

Changed. Also, moved IsArcPlayStoreEnabledForProfile() to caller, in order to
let this function focus on checking ARC aliveness.

+  // TODO(crbug.com/672829): Due to shutdown procedure dependency,
+  // ArcAppListPrefs may try to touch ArcSessionManager related stuff.
+  // It is necessary to check it here, in addition to whether Google Play Store

[Yusuke Sato, 2017/02/17 22:27:09]

optional: Probably it's better to directly explain when IsAllowed() returns
false too.

[hidehiko, 2017/02/20 18:18:44]

Done.

+  // is enabled.
   const auto* arc_session_manager = arc::ArcSessionManager::Get();
-  return arc_session_manager && arc_session_manager->IsArcPlayStoreEnabled();
+  return arc_session_manager && arc_session_manager->IsAllowed() &&
+         arc::IsArcPlayStoreEnabledForProfile(profile);
 }
 
 bool GetInt64FromPref(const base::DictionaryValue* dict,
@@ -263,8 +269,11 @@ void ArcAppListPrefs::StartPrefs() {
   arc::ArcSessionManager* arc_session_manager = arc::ArcSessionManager::Get();
   CHECK(arc_session_manager);
 
-  if (arc_session_manager->profile())
-    OnArcPlayStoreEnabledChanged(arc_session_manager->IsArcPlayStoreEnabled());
+  if (arc_session_manager->profile()) {
+    DCHECK_EQ(profile_, arc_session_manager->profile());

[hidehiko, 2017/02/17 15:07:19]

Note: If ArcSessionManager has profile, it should be as same as one this
instance has, because ArcAppListPrefsFactory creates an instance only if the
given Profile meets ARC's requirement. Anyway, just in case, check it here.

[Yusuke Sato, 2017/02/17 22:27:09]

Copy the comment to the code?

[hidehiko, 2017/02/20 18:18:44]

Done, but in another CL.

[Yusuke Sato, 2017/02/21 20:59:43]

Acknowledged.

+    OnArcPlayStoreEnabledChanged(
+        arc::IsArcPlayStoreEnabledForProfile(profile_));
+  }
   arc_session_manager->AddObserver(this);
 
   app_instance_holder_->AddObserver(this);
@@ -385,7 +394,7 @@ bool ArcAppListPrefs::HasObserver(Observer* observer) {
 
 std::unique_ptr<ArcAppListPrefs::PackageInfo> ArcAppListPrefs::GetPackage(
     const std::string& package_name) const {
-  if (!IsArcEnabled())
+  if (!IsArcEnabled(profile_))
     return nullptr;
 
   const base::DictionaryValue* package = nullptr;
@@ -417,7 +426,7 @@ std::unique_ptr<ArcAppListPrefs::PackageInfo> ArcAppListPrefs::GetPackage(
 }
 
 std::vector<std::string> ArcAppListPrefs::GetAppIds() const {
-  if (!IsArcEnabled()) {
+  if (!IsArcEnabled(profile_)) {
     // Default ARC apps available before OptIn.
     std::vector<std::string> ids;
     for (const auto& default_app : default_apps_.app_map()) {
@@ -450,7 +459,7 @@ std::vector<std::string> ArcAppListPrefs::GetAppIdsNoArcEnabledCheck() const {
 std::unique_ptr<ArcAppListPrefs::AppInfo> ArcAppListPrefs::GetApp(
     const std::string& app_id) const {
   // Information for default app is available before ARC enabled.
-  if (!IsArcEnabled() && !default_apps_.HasApp(app_id))
+  if (!IsArcEnabled(profile_) && !default_apps_.HasApp(app_id))
     return std::unique_ptr<AppInfo>();
 
   const base::DictionaryValue* app = nullptr;
@@ -505,7 +514,7 @@ std::unique_ptr<ArcAppListPrefs::AppInfo> ArcAppListPrefs::GetApp(
 }
 
 bool ArcAppListPrefs::IsRegistered(const std::string& app_id) const {
-  if (!IsArcEnabled() && !default_apps_.HasApp(app_id))
+  if (!IsArcEnabled(profile_) && !default_apps_.HasApp(app_id))
     return false;
 
   const base::DictionaryValue* app = nullptr;
@@ -704,7 +713,7 @@ void ArcAppListPrefs::MaybeAddNonLaunchableApp(
     const base::Optional<std::string>& name,
     const std::string& package_name,
     const std::string& activity) {
-  DCHECK(IsArcEnabled());
+  DCHECK(arc::IsArcPlayStoreEnabledForProfile(profile_));
   if (IsRegistered(GetAppId(package_name, activity)))
     return;
 
@@ -841,7 +850,7 @@ void ArcAppListPrefs::RemoveApp(const std::string& app_id) {
 
 void ArcAppListPrefs::AddOrUpdatePackagePrefs(
     PrefService* prefs, const arc::mojom::ArcPackageInfo& package) {
-  DCHECK(IsArcEnabled());
+  DCHECK(arc::IsArcPlayStoreEnabledForProfile(profile_));
   const std::string& package_name = package.package_name;
   default_apps_.MaybeMarkPackageUninstalled(package_name, false);
   if (package_name.empty()) {
@@ -864,7 +873,7 @@ void ArcAppListPrefs::AddOrUpdatePackagePrefs(
 
 void ArcAppListPrefs::RemovePackageFromPrefs(PrefService* prefs,
                                              const std::string& package_name) {
-  DCHECK(IsArcEnabled());
+  DCHECK(arc::IsArcPlayStoreEnabledForProfile(profile_));
   default_apps_.MaybeMarkPackageUninstalled(package_name, true);
   if (!default_apps_.HasPackage(package_name)) {
     DictionaryPrefUpdate update(prefs, prefs::kArcPackages);
@@ -881,7 +890,7 @@ void ArcAppListPrefs::RemovePackageFromPrefs(PrefService* prefs,
 
 void ArcAppListPrefs::OnAppListRefreshed(
     std::vector<arc::mojom::AppInfoPtr> apps) {
-  DCHECK(IsArcEnabled());
+  DCHECK(arc::IsArcPlayStoreEnabledForProfile(profile_));
   std::vector<std::string> old_apps = GetAppIds();
 
   ready_apps_.clear();
@@ -1170,7 +1179,7 @@ bool ArcAppListPrefs::IsUnknownPackage(const std::string& package_name) const {
 
 void ArcAppListPrefs::OnPackageAdded(
     arc::mojom::ArcPackageInfoPtr package_info) {
-  DCHECK(IsArcEnabled());
+  DCHECK(arc::IsArcPlayStoreEnabledForProfile(profile_));
 
   // Ignore packages installed by internal sync.
   DCHECK(sync_service_);
@@ -1188,7 +1197,7 @@ void ArcAppListPrefs::OnPackageAdded(
 
 void ArcAppListPrefs::OnPackageModified(
     arc::mojom::ArcPackageInfoPtr package_info) {
-  DCHECK(IsArcEnabled());
+  DCHECK(arc::IsArcPlayStoreEnabledForProfile(profile_));
   AddOrUpdatePackagePrefs(prefs_, *package_info);
   for (auto& observer : observer_list_)
     observer.OnPackageModified(*package_info);
@@ -1196,7 +1205,7 @@ void ArcAppListPrefs::OnPackageModified(
 
 void ArcAppListPrefs::OnPackageListRefreshed(
     std::vector<arc::mojom::ArcPackageInfoPtr> packages) {
-  DCHECK(IsArcEnabled());
+  DCHECK(arc::IsArcPlayStoreEnabledForProfile(profile_));
 
   const std::vector<std::string> old_packages(GetPackagesFromPrefs());
   std::unordered_set<std::string> current_packages;
@@ -1228,7 +1237,7 @@ std::vector<std::string> ArcAppListPrefs::GetPackagesFromPrefs() const {
 std::vector<std::string> ArcAppListPrefs::GetPackagesFromPrefs(
     bool installed) const {
   std::vector<std::string> packages;
-  if (!IsArcEnabled() && installed)
+  if (!IsArcEnabled(profile_) && installed)
     return packages;
 
   const base::DictionaryValue* package_prefs =