Start ARC and sign in after Chrome OS login

chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc

Index: chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc
diff --git a/chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc b/chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc
index 78512a0c0193e24f2dd0c8aa44b01fcd69cc693d..8cf669faa6399785d8a1876768e582cd75e39373 100644
--- a/chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc
+++ b/chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc
@@ -24,6 +24,7 @@
 #include "chrome/common/pref_names.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/arc/arc_service_manager.h"
+#include "components/arc/arc_util.h"
 #include "components/crx_file/id_util.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/scoped_user_pref_update.h"
@@ -151,6 +152,11 @@ bool IsArcAlive() {
   return arc_session_manager && arc_session_manager->IsAllowed();
 }
 
+bool IsArcAndroidEnabledForProfile(const Profile* profile) {

[hidehiko, 2017/03/02 15:57:51]

Could you add detailed document about what is "AndroidEnabled" state?

[victorhsieh, 2017/03/02 18:49:52]

Done.

+  return arc::ShouldArcAlwaysStart() ||
+         arc::IsArcPlayStoreEnabledForProfile(profile);
+}
+
 bool GetInt64FromPref(const base::DictionaryValue* dict,
                       const std::string& key,
                       int64_t* value) {
@@ -306,6 +312,12 @@ base::FilePath ArcAppListPrefs::GetIconPath(
 
 void ArcAppListPrefs::RequestIcon(const std::string& app_id,
                                   ui::ScaleFactor scale_factor) {
+  // ArcSessionManager can be terminated during test tear down, before callback
+  // into this function.
+  // TODO(victorhsieh): figure out the best way/place to handle this situation.
+  if (arc::ArcSessionManager::Get() == nullptr)
+    return;
+
   if (!IsRegistered(app_id)) {
     VLOG(2) << "Request to load icon for non-registered app: " << app_id << ".";
     return;
@@ -397,7 +409,7 @@ bool ArcAppListPrefs::HasObserver(Observer* observer) {
 
 std::unique_ptr<ArcAppListPrefs::PackageInfo> ArcAppListPrefs::GetPackage(
     const std::string& package_name) const {
-  if (!IsArcAlive() || !arc::IsArcPlayStoreEnabledForProfile(profile_))
+  if (!IsArcAlive() || !IsArcAndroidEnabledForProfile(profile_))
     return nullptr;
 
   const base::DictionaryValue* package = nullptr;
@@ -429,7 +441,8 @@ std::unique_ptr<ArcAppListPrefs::PackageInfo> ArcAppListPrefs::GetPackage(
 }
 
 std::vector<std::string> ArcAppListPrefs::GetAppIds() const {
-  if (!IsArcAlive() || !arc::IsArcPlayStoreEnabledForProfile(profile_)) {
+  if (!arc::ShouldArcAlwaysStart() &&

[hidehiko, 2017/03/02 15:57:51]

The condition looks complicated, because IsArcAndroidEnabledForProfile also
contains ShouldArcAlwaysStart(). Could you simplify?

[victorhsieh, 2017/03/02 18:49:52]

Now I shortcut ShouldArcAlwaysStart to return early.  The rest of the condition
is semantically more correct despite of duplicated call to ShouldArcAlwaysStart.
 I hope it's fine for now.  Also, at the end of the work, we will remove
--arc-always-start.

+      (!IsArcAlive() || !IsArcAndroidEnabledForProfile(profile_))) {
     // Default ARC apps available before OptIn.
     std::vector<std::string> ids;
     for (const auto& default_app : default_apps_.app_map()) {
@@ -443,7 +456,6 @@ std::vector<std::string> ArcAppListPrefs::GetAppIds() const {
 
 std::vector<std::string> ArcAppListPrefs::GetAppIdsNoArcEnabledCheck() const {
   std::vector<std::string> ids;
-
   const base::DictionaryValue* apps = prefs_->GetDictionary(prefs::kArcApps);
   DCHECK(apps);
 
@@ -462,7 +474,7 @@ std::vector<std::string> ArcAppListPrefs::GetAppIdsNoArcEnabledCheck() const {
 std::unique_ptr<ArcAppListPrefs::AppInfo> ArcAppListPrefs::GetApp(
     const std::string& app_id) const {
   // Information for default app is available before ARC enabled.
-  if ((!IsArcAlive() || !arc::IsArcPlayStoreEnabledForProfile(profile_)) &&
+  if ((!IsArcAlive() || !IsArcAndroidEnabledForProfile(profile_)) &&
       !default_apps_.HasApp(app_id))
     return std::unique_ptr<AppInfo>();
 
@@ -518,7 +530,7 @@ std::unique_ptr<ArcAppListPrefs::AppInfo> ArcAppListPrefs::GetApp(
 }
 
 bool ArcAppListPrefs::IsRegistered(const std::string& app_id) const {
-  if ((!IsArcAlive() || !arc::IsArcPlayStoreEnabledForProfile(profile_)) &&
+  if ((!IsArcAlive() || !IsArcAndroidEnabledForProfile(profile_)) &&
       !default_apps_.HasApp(app_id))
     return false;
 
@@ -609,6 +621,10 @@ void ArcAppListPrefs::RemoveAllApps() {
 void ArcAppListPrefs::OnArcPlayStoreEnabledChanged(bool enabled) {
   UpdateDefaultAppsHiddenState();
 
+  // TODO(victorhsieh): Implement opt-in and opt-out.
+  if (arc::ShouldArcAlwaysStart())
+    return;
+
   if (enabled)
     NotifyRegisteredApps();
   else
@@ -718,7 +734,7 @@ void ArcAppListPrefs::MaybeAddNonLaunchableApp(
     const base::Optional<std::string>& name,
     const std::string& package_name,
     const std::string& activity) {
-  DCHECK(arc::IsArcPlayStoreEnabledForProfile(profile_));
+  DCHECK(IsArcAndroidEnabledForProfile(profile_));
   if (IsRegistered(GetAppId(package_name, activity)))
     return;
 
@@ -855,7 +871,7 @@ void ArcAppListPrefs::RemoveApp(const std::string& app_id) {
 
 void ArcAppListPrefs::AddOrUpdatePackagePrefs(
     PrefService* prefs, const arc::mojom::ArcPackageInfo& package) {
-  DCHECK(arc::IsArcPlayStoreEnabledForProfile(profile_));
+  DCHECK(IsArcAndroidEnabledForProfile(profile_));
   const std::string& package_name = package.package_name;
   default_apps_.MaybeMarkPackageUninstalled(package_name, false);
   if (package_name.empty()) {
@@ -878,7 +894,7 @@ void ArcAppListPrefs::AddOrUpdatePackagePrefs(
 
 void ArcAppListPrefs::RemovePackageFromPrefs(PrefService* prefs,
                                              const std::string& package_name) {
-  DCHECK(arc::IsArcPlayStoreEnabledForProfile(profile_));
+  DCHECK(IsArcAndroidEnabledForProfile(profile_));
   default_apps_.MaybeMarkPackageUninstalled(package_name, true);
   if (!default_apps_.HasPackage(package_name)) {
     DictionaryPrefUpdate update(prefs, prefs::kArcPackages);
@@ -895,7 +911,7 @@ void ArcAppListPrefs::RemovePackageFromPrefs(PrefService* prefs,
 
 void ArcAppListPrefs::OnAppListRefreshed(
     std::vector<arc::mojom::AppInfoPtr> apps) {
-  DCHECK(arc::IsArcPlayStoreEnabledForProfile(profile_));
+  DCHECK(IsArcAndroidEnabledForProfile(profile_));
   std::vector<std::string> old_apps = GetAppIds();
 
   ready_apps_.clear();
@@ -1184,7 +1200,7 @@ bool ArcAppListPrefs::IsUnknownPackage(const std::string& package_name) const {
 
 void ArcAppListPrefs::OnPackageAdded(
     arc::mojom::ArcPackageInfoPtr package_info) {
-  DCHECK(arc::IsArcPlayStoreEnabledForProfile(profile_));
+  DCHECK(IsArcAndroidEnabledForProfile(profile_));
 
   // Ignore packages installed by internal sync.
   DCHECK(sync_service_);
@@ -1202,7 +1218,7 @@ void ArcAppListPrefs::OnPackageAdded(
 
 void ArcAppListPrefs::OnPackageModified(
     arc::mojom::ArcPackageInfoPtr package_info) {
-  DCHECK(arc::IsArcPlayStoreEnabledForProfile(profile_));
+  DCHECK(IsArcAndroidEnabledForProfile(profile_));
   AddOrUpdatePackagePrefs(prefs_, *package_info);
   for (auto& observer : observer_list_)
     observer.OnPackageModified(*package_info);
@@ -1210,7 +1226,7 @@ void ArcAppListPrefs::OnPackageModified(
 
 void ArcAppListPrefs::OnPackageListRefreshed(
     std::vector<arc::mojom::ArcPackageInfoPtr> packages) {
-  DCHECK(arc::IsArcPlayStoreEnabledForProfile(profile_));
+  DCHECK(IsArcAndroidEnabledForProfile(profile_));
 
   const std::vector<std::string> old_packages(GetPackagesFromPrefs());
   std::unordered_set<std::string> current_packages;
@@ -1242,8 +1258,7 @@ std::vector<std::string> ArcAppListPrefs::GetPackagesFromPrefs() const {
 std::vector<std::string> ArcAppListPrefs::GetPackagesFromPrefs(
     bool installed) const {
   std::vector<std::string> packages;
-  if ((!IsArcAlive() || !arc::IsArcPlayStoreEnabledForProfile(profile_)) &&
-      installed)
+  if ((!IsArcAlive() || !IsArcAndroidEnabledForProfile(profile_)) && installed)
     return packages;
 
   const base::DictionaryValue* package_prefs =