Start ARC and sign in after Chrome OS login

chrome/browser/chromeos/arc/arc_session_manager.h

 // Copyright 2016 The Chromium Authors. All rights reserved.

[Yusuke Sato, 2017/02/23 00:47:13]

In case you missed https://codereview.chromium.org/2702723002/ by hidehiko, you
might want to rebase this on top of hidehiko's once it lands.

[victorhsieh, 2017/02/24 00:44:59]

Thanks.

 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_CHROMEOS_ARC_ARC_SESSION_MANAGER_H_
 #define CHROME_BROWSER_CHROMEOS_ARC_ARC_SESSION_MANAGER_H_
 
 #include <memory>
 #include <ostream>
 #include <string>
 
@@ skipping 102 matching lines @@
   // Exposed here for unit_tests validation.
   static bool IsOobeOptInActive();
 
   // It is called from chrome/browser/prefs/browser_prefs.cc.
   static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
 
   static void DisableUIForTesting();
   static void SetShelfDelegateForTesting(ash::ShelfDelegate* shelf_delegate);
   static void EnableCheckAndroidManagementForTesting();
 
-  // Returns true if ARC is allowed to run for the current session.
+  // Returns true if Persistent ARC is enabled.
+  static bool IsPersistentArc();

[Yusuke Sato, 2017/02/23 00:47:13]

I haven't reviewed this CL in detail yet, but the word Persistent ARC seems a
bit too broad here because it is a project name that consists of a series of
features. Also, developers who are not familiar with the project won't
immediately get the meaning. The word "persistent" may also give a wrong
impression that the instance will outlive the user session when
IsPersistentArc() is true.

Could you use a little bit more specific function name (and flag name) for this?
As we chatted off line, what about something like ShouldAlwaysStartArc()?

hidehiko@, WDYT? Any preference on this?

[hidehiko, 2017/02/23 10:29:16]

Makes sense. Another candidate is ShouldAlwaysRunArc()?
Also, IMHO, it'd be nice if the flag name is also consistent with the function
name somehow.


Could you move this into components/arc/arc_util?

[hidehiko, 2017/02/23 10:30:54]

a bit bikeshedding: To be consistent with helper functions in arc_util, maybe
ShouldArc...() looks better?

[victorhsieh, 2017/02/24 00:44:59]

Done.

+
   // TODO(hidehiko): The name is very close to IsArcAllowedForProfile(), but
   // has different meaning. Clean this up.
   bool IsAllowed() const;
 
   void OnPrimaryUserProfilePrepared(Profile* profile);
   void Shutdown();
 
   Profile* profile() { return profile_; }
   const Profile* profile() const { return profile_; }
 
@@ skipping 110 matching lines @@
   void SetState(State state);
   void ShutdownSession();
   void OnOptInPreferenceChanged();
   void OnAndroidManagementPassed();
   void OnArcDataRemoved(bool success);
   void OnArcSignInTimeout();
   void FetchAuthCode();
   void PrepareContextForAuthCodeRequest();
 
   void StartArcAndroidManagementCheck();
+  void StartArcAndroidManagementCheckedInBackground();
   void MaybeReenableArc();
 
   // Called when the Android management check is done in opt-in flow or
   // re-auth flow.
   void OnAndroidManagementChecked(
       policy::AndroidManagementClient::Result result);
 
   // Called when the background Android management check is done. It is
   // triggered when the second or later ARC boot timing.
   void OnBackgroundAndroidManagementChecked(
@@ skipping 43 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(ArcSessionManager);
 };
 
 // Outputs the stringified |state| to |os|. This is only for logging purposes.
 std::ostream& operator<<(std::ostream& os,
                          const ArcSessionManager::State& state);
 
 }  // namespace arc
 
 #endif  // CHROME_BROWSER_CHROMEOS_ARC_ARC_SESSION_MANAGER_H_