Skip ARC initial screen when everything is set up by policy

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

 // Copyright 2015 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/chromeos/arc/intent_helper/arc_settings_service.h"
 
 #include <string>
 
 #include "base/gtest_prod_util.h"
 #include "base/json/json_writer.h"
@@ skipping 97 matching lines @@
  private:
   // Registers to observe changes for Chrome settings we care about.
   void StartObservingSettingsChanges();
 
   // Stops listening for Chrome settings changes.
   void StopObservingSettingsChanges();
 
   // 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;
   void SyncBackupEnabled() const;
   void SyncFocusHighlightEnabled() const;
   void SyncFontSize() const;
   void SyncInitialSettings() const;
   void SyncLocale() const;
   void SyncLocationServiceEnabled() const;
   void SyncProxySettings() const;
@@ skipping 67 matching lines @@
     }
     SyncProxySettings();
   } else if (pref_name == prefs::kAccessibilityFocusHighlightEnabled) {
     SyncFocusHighlightEnabled();
   } else if (pref_name == prefs::kAccessibilityLargeCursorEnabled) {
     SyncAccessibilityLargeMouseCursorEnabled();
   } else if (pref_name == prefs::kAccessibilitySpokenFeedbackEnabled) {
     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();
   } else if (pref_name == prefs::kWebKitDefaultFixedFontSize ||
              pref_name == prefs::kWebKitDefaultFontSize ||
              pref_name == prefs::kWebKitMinimumFontSize) {
     SyncFontSize();
   } else if (pref_name == proxy_config::prefs::kProxy) {
     SyncProxySettings();
   } else {
@@ skipping 78 matching lines @@
   SyncAccessibilityVirtualKeyboardEnabled();
   SyncFocusHighlightEnabled();
   SyncFontSize();
   SyncLocale();
   SyncProxySettings();
   SyncReportingConsent();
   SyncSpokenFeedbackEnabled();
   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,
       "org.chromium.arc.intent_helper.ACCESSIBILITY_LARGE_POINTER_ICON");
 }
 
 void ArcSettingsServiceImpl::SyncAccessibilityVirtualKeyboardEnabled() const {
   SendBoolPrefSettingsBroadcast(
       prefs::kAccessibilityVirtualKeyboardEnabled,
       "org.chromium.arc.intent_helper.SET_SHOW_IME_WITH_HARD_KEYBOARD");
@@ skipping 208 matching lines @@
 
 void ArcSettingsService::OnInstanceReady() {
   impl_.reset(new ArcSettingsServiceImpl(arc_bridge_service()));
 }
 
 void ArcSettingsService::OnInstanceClosed() {
   impl_.reset();
 }
 
 }  // namespace arc