cros: Skip need-dircrypto-migration check if not needed

chrome/browser/chromeos/login/screens/user_selection_screen.cc

 // Copyright 2014 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/login/screens/user_selection_screen.h"
 
 #include <stddef.h>
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/command_line.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_process_platform_part.h"
 #include "chrome/browser/chromeos/login/lock/screen_locker.h"
 #include "chrome/browser/chromeos/login/quick_unlock/quick_unlock_factory.h"
 #include "chrome/browser/chromeos/login/quick_unlock/quick_unlock_storage.h"
 #include "chrome/browser/chromeos/login/reauth_stats.h"
 #include "chrome/browser/chromeos/login/ui/login_display_host.h"
 #include "chrome/browser/chromeos/login/ui/views/user_board_view.h"
 #include "chrome/browser/chromeos/login/users/chrome_user_manager.h"
 #include "chrome/browser/chromeos/login/users/multi_profile_user_controller.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/signin/easy_unlock_service.h"
 #include "chrome/browser/ui/webui/chromeos/login/l10n_util.h"
 #include "chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h"
 #include "chrome/grit/generated_resources.h"
+#include "chromeos/chromeos_switches.h"
 #include "chromeos/cryptohome/cryptohome_parameters.h"
 #include "chromeos/dbus/cryptohome_client.h"
 #include "chromeos/dbus/dbus_method_call_status.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
+#include "components/arc/arc_util.h"
 #include "components/prefs/pref_service.h"
 #include "components/proximity_auth/screenlock_bridge.h"
 #include "components/signin/core/account_id/account_id.h"
 #include "components/user_manager/known_user.h"
 #include "components/user_manager/user_manager.h"
 #include "components/user_manager/user_type.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/user_activity/user_activity_detector.h"
 
 namespace chromeos {
@@ skipping 85 matching lines @@
     return false;
 
   quick_unlock::QuickUnlockStorage* quick_unlock_storage =
       quick_unlock::QuickUnlockFactory::GetForUser(user);
   if (!quick_unlock_storage)
     return false;
 
   return quick_unlock_storage->IsFingerprintAuthenticationAvailable();
 }
 
+// Returns true if dircrypto migration check should be performed.
+bool ShouldCheckNeedDircryptoMigration() {
+  return !base::CommandLine::ForCurrentProcess()->HasSwitch(
+             switches::kDisableEncryptionMigration) &&
+         arc::IsArcAvailable();

[hidehiko, 2017/05/02 05:58:57]

Just clarification:
Is it ok to pass this flow in the situation where;
- ARC++ is not officially supported, but
- ARC++ KIOSK is supported.

This is just a notification, IIUC, so maybe ok to keep as is, but just in case.

[xiyuan, 2017/05/02 06:06:04]

I skipped the ARC++ KIOSK test because the banner is meant for a real user. So
for such devices, a user would not get ARC so the banner should not show up
either.

Please confirm whether this is the desired behavior. I will hold off committing
for now.
Thanks. 

+}
+
 }  // namespace
 
 // Helper class to call cryptohome to check whether a user needs dircrypto
 // migration. The check results are cached to limit calls to cryptohome.
 class UserSelectionScreen::DircryptoMigrationChecker {
  public:
   explicit DircryptoMigrationChecker(UserSelectionScreen* owner)
       : owner_(owner), weak_ptr_factory_(this) {}
   ~DircryptoMigrationChecker() = default;
 
@@ skipping 377 matching lines @@
   if (!token_handle_util_.get()) {
     token_handle_util_.reset(new TokenHandleUtil());
   }
 
   if (token_handle_util_->HasToken(account_id)) {
     token_handle_util_->CheckToken(
         account_id, base::Bind(&UserSelectionScreen::OnUserStatusChecked,
                                weak_factory_.GetWeakPtr()));
   }
 
-  // Run dircrypto migration check only on the login screen.
-  if (display_type_ == OobeUI::kLoginDisplay) {
+  // Run dircrypto migration check only on the login screen when necessary.
+  if (display_type_ == OobeUI::kLoginDisplay &&
+      ShouldCheckNeedDircryptoMigration()) {
     if (!dircrypto_migration_checker_) {
       dircrypto_migration_checker_ =
           base::MakeUnique<DircryptoMigrationChecker>(this);
     }
     dircrypto_migration_checker_->Check(account_id);

[Junichi Uekawa, 2017/05/02 05:02:24]

From what I can see from
src/chrome/browser/chromeos/login/screens/user_selection_screen.cc 
DircryptoMigrationChecker::Check
sends a request to cryptohome, where cryptohome already should do a check ....

but from what I read in
https://chromium-review.googlesource.com/#/c/479475/
I guess it doesn't have a concept of experiement or Arc enablement check.

[xiyuan, 2017/05/02 05:48:57]

Correct. The CL is needed because cryptohomed does not have knowledge of ARC. It
only knows whether the user is on eCryptFS and whether dircrypto is supported. I
missed this because I assumed all dircrypto enabled device would have ARC. 

   }
 }
 
 void UserSelectionScreen::OnUserStatusChecked(
     const AccountId& account_id,
     TokenHandleUtil::TokenHandleStatus status) {
   if (status == TokenHandleUtil::INVALID) {
     RecordReauthReason(account_id, ReauthReason::INVALID_TOKEN_HANDLE);
     token_handle_util_->MarkHandleInvalid(account_id);
     SetAuthType(account_id, ONLINE_SIGN_IN, base::string16());
@@ skipping 123 matching lines @@
   // The user profile should exist if and only if this is the lock screen.
   DCHECK_EQ(!!profile, GetScreenType() == LOCK_SCREEN);
 
   if (!profile)
     profile = profile_helper->GetSigninProfile();
 
   return EasyUnlockService::Get(profile);
 }
 
 }  // namespace chromeos