Extract kArcEnabled preference from ArcSessionManager part 1.

chrome/browser/chromeos/arc/arc_util.cc

Index: chrome/browser/chromeos/arc/arc_util.cc
diff --git a/chrome/browser/chromeos/arc/arc_util.cc b/chrome/browser/chromeos/arc/arc_util.cc
index de722a70e63ff37ee2cf24ba237e3a0f4fc0cf71..3711b4949f49a3eda69d398e9f1a9fc411457f97 100644
--- a/chrome/browser/chromeos/arc/arc_util.cc
+++ b/chrome/browser/chromeos/arc/arc_util.cc
@@ -9,7 +9,9 @@
 #include "chrome/browser/chromeos/login/users/chrome_user_manager.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/common/pref_names.h"
 #include "components/arc/arc_util.h"
+#include "components/prefs/pref_service.h"
 #include "components/user_manager/user.h"
 #include "components/user_manager/user_manager.h"
 
@@ -96,4 +98,23 @@ void DisallowArcForTesting() {
   g_disallow_for_testing = true;
 }
 
+bool IsArcPlayStoreEnabledForProfile(const Profile* profile) {
+  return IsArcAllowedForProfile(profile) &&
+         profile->GetPrefs()->GetBoolean(prefs::kArcEnabled);
+}
+
+bool IsArcPlayStoreEnabledPreferenceManagedForProfile(const Profile* profile) {
+  DCHECK(IsArcAllowedForProfile(profile));

[Luis Héctor Chávez, 2017/02/21 16:50:01]

Similar to the comments I left below: this is a probably aggressive behavioral
change because not all places that call this actually ensure one way or the
other that this is true since they rely on the old behavior of being a simple
if-not-allowed-return-false.

[hidehiko, 2017/02/21 17:25:14]

This is not behavior change actually.
In the old code ArcSessionManager::IsArcManaged() has DCHECK(profile_), which is
corresponding to this DCHECK, because if IsArcAllowedForProfile(profile) is
false, ASM::profile_ won't be set.

Technicaly, we can change the behavior of the case where
IsArcAllowedForProfile() returns false, but at the moment, I think it's ok to
keep the DCHECK because it is more strict than returning false.

WDYT?

[Luis Héctor Chávez, 2017/02/21 18:18:08]

Makes sense, but it's still something that can be easy to miss for someone
that's not super careful.

How about we do

if (!IsArcAllowedForProfile(profile)) {
  DCHECK(false) << "ARC is not allowed for profile";
  return false;
}

that way we have the best of both worlds: we don't accidentally regress in
production builds and we get a nicer log.

[hidehiko, 2017/02/23 04:17:36]

Done. BTW, LOG(DFATAL) is the way in this case :-).

+  return profile->GetPrefs()->IsManagedPreference(prefs::kArcEnabled);
+}
+
+void SetArcPlayStoreEnabledForProfile(Profile* profile, bool enabled) {
+  DCHECK(IsArcAllowedForProfile(profile));
+  if (IsArcPlayStoreEnabledPreferenceManagedForProfile(profile)) {
+    VLOG(1) << "Do nothing, if the Google-Play-Store-enabled pref is managed.";

[Luis Héctor Chávez, 2017/02/21 16:50:02]

nit: "Doing nothing since the Google-Play-Store-enabled pref is managed."

[hidehiko, 2017/02/21 17:25:14]

Done.

+    return;
+  }
+  profile->GetPrefs()->SetBoolean(prefs::kArcEnabled, enabled);
+}
+
 }  // namespace arc