Skip ARC initial screen when everything is set up by policy

chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc

Index: chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc
diff --git a/chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc b/chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc
index b52c9cac018f5bcfe45e98757899b1efb867c2e4..ae2dfd417d90c88c72b502d6bcd402633d5303e2 100644
--- a/chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc
+++ b/chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc
@@ -115,8 +115,19 @@ class ArcSettingsServiceImpl
   // Retrieves Chrome's state for the settings that need to be synced on each
   // Android boot and send it to Android.
   void SyncRuntimeSettings() const;
-  // Send settings that need to be synced only on Android first start to
-  // Android.
+  // Determines whether the particular setting should be synced to Android. This
+  // should happen if the pref is managed or if it transitions from the managed
+  // to unmanaged state (which is captured by unsetting the user pref when the
+  // pref is managed and by setting it to |default_value| when the pref is
+  // unset).
+  bool ShouldSyncPolicyOrUserControlledPref(
+      const std::string& pref,
+      const base::Value& default_value) const;
+  // Determine whether a particular setting needs to be synced to Android.
+  // Keep these lines ordered lexicographically.
+  bool ShouldSyncBackupEnabled() const;
+  bool ShouldSyncLocationServiceEnabled() const;
+  // Send particular settings to Android.
   // Keep these lines ordered lexicographically.
   void SyncAccessibilityLargeMouseCursorEnabled() const;
   void SyncAccessibilityVirtualKeyboardEnabled() const;
@@ -204,10 +215,11 @@ void ArcSettingsServiceImpl::OnPrefChanged(const std::string& pref_name) const {
     SyncSpokenFeedbackEnabled();
   } else if (pref_name == prefs::kAccessibilityVirtualKeyboardEnabled) {
     SyncAccessibilityVirtualKeyboardEnabled();
+  } else if (pref_name == prefs::kArcBackupRestoreEnabled) {
+    if (ShouldSyncBackupEnabled())
+      SyncBackupEnabled();
   } else if (pref_name == prefs::kArcLocationServiceEnabled) {
-    const PrefService* const prefs =
-        ProfileManager::GetActiveUserProfile()->GetPrefs();
-    if (prefs->IsManagedPreference(prefs::kArcLocationServiceEnabled))
+    if (ShouldSyncLocationServiceEnabled())
       SyncLocationServiceEnabled();
   } else if (pref_name == prefs::kUse24HourClock) {
     SyncUse24HourClock();
@@ -306,14 +318,40 @@ void ArcSettingsServiceImpl::SyncRuntimeSettings() const {
   SyncTimeZone();
   SyncUse24HourClock();
 
-  const PrefService* const prefs =
-      ProfileManager::GetActiveUserProfile()->GetPrefs();
-  if (prefs->IsManagedPreference(prefs::kArcBackupRestoreEnabled))
+  if (ShouldSyncBackupEnabled())
     SyncBackupEnabled();
-  if (prefs->IsManagedPreference(prefs::kArcLocationServiceEnabled))
+  if (ShouldSyncLocationServiceEnabled())
     SyncLocationServiceEnabled();
 }
 
+bool ArcSettingsServiceImpl::ShouldSyncPolicyOrUserControlledPref(
+    const std::string& pref,
+    const base::Value& default_value) const {
+  PrefService* const prefs = ProfileManager::GetActiveUserProfile()->GetPrefs();
+  if (prefs->IsManagedPreference(pref)) {
+    // Unset the user pref so that if the pref becomes unmanaged at some point,
+    // this change will be synced.
+    prefs->ClearPref(pref);

[hidehiko, 2017/02/14 11:51:36]

In general, side effect in a function to check the condition (e.g. IsSomething,
HasSomething, CanSomething, ShouldSomething etc. in Chrome) is practically
confusing. Actually, it is also marked as "const".

Could you avoid them?
Maybe renaming somehow?

[emaxx, 2017/02/15 03:40:59]

Done. I left only non-modifying operations in the Should* methods, and moved
modifications into the Sync* methods.
(This required to move away from sharing this bit of code, but the duplication
probably made the code even clearer.)

+    return true;
+  }
+  if (!prefs->HasPrefPath(pref)) {
+    // Set the pref value in order to prevent the subsequent syncing.
+    prefs->Set(pref, default_value);
+    return true;
+  }
+  return false;
+}
+
+bool ArcSettingsServiceImpl::ShouldSyncBackupEnabled() const {
+  return ShouldSyncPolicyOrUserControlledPref(prefs::kArcBackupRestoreEnabled,
+                                              base::Value(false));
+}
+
+bool ArcSettingsServiceImpl::ShouldSyncLocationServiceEnabled() const {
+  return ShouldSyncPolicyOrUserControlledPref(prefs::kArcLocationServiceEnabled,
+                                              base::Value(false));
+}
+
 void ArcSettingsServiceImpl::SyncAccessibilityLargeMouseCursorEnabled() const {
   SendBoolPrefSettingsBroadcast(
       prefs::kAccessibilityLargeCursorEnabled,