arc: Reactivate OptIn flow on clicking Play Store.

chrome/browser/chromeos/arc/arc_util.cc

 // Copyright 2017 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/arc_util.h"
 
 #include "base/logging.h"
+#include "chrome/browser/chromeos/arc/arc_session_manager.h"
 #include "chrome/browser/chromeos/login/user_flow.h"
 #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"
 
@@ skipping 89 matching lines @@
   if (!IsArcAllowedForProfile(profile)) {
     LOG(DFATAL) << "ARC is not allowed for profile";
     return false;
   }
   return profile->GetPrefs()->IsManagedPreference(prefs::kArcEnabled);
 }
 
 void SetArcPlayStoreEnabledForProfile(Profile* profile, bool enabled) {
   DCHECK(IsArcAllowedForProfile(profile));
   if (IsArcPlayStoreEnabledPreferenceManagedForProfile(profile)) {
-    VLOG(1) << "Do nothing, since the Google-Play-Store-enabled pref is "
-            << "managed.";
+    VLOG(1) << "Google-Play-Store-enabled pref is managed. Request to "
+            << (enabled ? "enable" : "disable") << " Play Store is not stored";
+    // Need update ARC session manager manually for managed case in order to
+    // keep its state up to date, otherwise it may stuck with enabling
+    // request.
+    ArcSessionManager* arc_session_manager = ArcSessionManager::Get();

[hidehiko, 2017/03/13 21:46:47]

QQ: If this is only for launcher icon click case, can this move to there instead
of here?
This function is used in various places, I'm slightly worried about its
side-effect.
Also, with this change, we cannot rely on the assumption IsArcPlayStoreEnabled()
== ARC is running, any more.
So, e.g., settings page etc. may not work? Is this safely doable?

[khmel, 2017/03/13 21:58:49]

This is not only for launcher item case. My first attempt was to add
RequestEnable into arc_app_utils. However this was not enough. One problem is
when user closes OptIn UI. In this case this function is called from host
handler IIRC. We have to reset enable_request in session manager in order to get
a chance to reactivate negotiator. We should not worry about settings case
because user cannot change arc.enabled state (which settings represents
actually)

[hidehiko, 2017/03/15 00:45:00]

If this is short term fix that's ok, but please add TODO to remove this.
Actually this is not SetArcPlayStoreEnabledForProfile conceptually.
Also please make sure components IsArcPlayStoreEnabledForProfile() does not
break by this change.

[khmel, 2017/03/15 01:33:28]

TODO added, IsArcPlayStoreEnabledForProfile is always true for this case.

+    // |arc_session_manager| can be nullptr in unit_tests.
+    if (!arc_session_manager)
+      return;
+    if (enabled)
+      arc_session_manager->RequestEnable();
+    else
+      arc_session_manager->RequestDisable();
     return;
   }
   profile->GetPrefs()->SetBoolean(prefs::kArcEnabled, enabled);
 }
 
 bool AreArcAllOptInPreferencesManagedForProfile(const Profile* profile) {
   return profile->GetPrefs()->IsManagedPreference(
              prefs::kArcBackupRestoreEnabled) &&
          profile->GetPrefs()->IsManagedPreference(
              prefs::kArcLocationServiceEnabled);
 }
 
 }  // namespace arc