Fix Arc integration test.

chrome/browser/chromeos/arc/arc_session_manager.cc

 // Copyright 2016 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_session_manager.h"
 
 #include <utility>
 
 #include "ash/common/shelf/shelf_delegate.h"
 #include "ash/common/wm_shell.h"
@@ skipping 42 matching lines @@
 namespace arc {
 
 namespace {
 
 // Weak pointer.  This class is owned by ArcServiceManager.
 ArcSessionManager* g_arc_session_manager = nullptr;
 
 // Skip creating UI in unit tests
 bool g_disable_ui_for_testing = false;
 
+// Skip DCHECK(state() == State::REMOVING_DATA_DIR) in OnArcDataRemoved.
+bool g_is_session_manager_for_sync_test = false;
+
 // Use specified ash::ShelfDelegate for unit tests.
 ash::ShelfDelegate* g_shelf_delegate_for_testing = nullptr;
 
 // The Android management check is disabled by default, it's used only for
 // testing.
 bool g_enable_check_android_management_for_testing = false;
 
 // Maximum amount of time we'll wait for ARC to finish booting up. Once this
 // timeout expires, keep ARC running in case the user wants to file feedback,
 // but present the UI to try again.
@@ skipping 76 matching lines @@
 void ArcSessionManager::SetShelfDelegateForTesting(
     ash::ShelfDelegate* shelf_delegate) {
   g_shelf_delegate_for_testing = shelf_delegate;
 }
 
 // static
 void ArcSessionManager::EnableCheckAndroidManagementForTesting() {
   g_enable_check_android_management_for_testing = true;
 }
 
+// static
+void ArcSessionManager::SetSessionManagerForSyncTest() {
+  g_is_session_manager_for_sync_test = true;
+}
+
 void ArcSessionManager::OnSessionReady() {
   for (auto& observer : arc_session_observer_list_)
     observer.OnSessionReady();
 }
 
 void ArcSessionManager::OnSessionStopped(StopReason reason) {
   // TODO(crbug.com/625923): Use |reason| to report more detailed errors.
   if (arc_sign_in_timer_.IsRunning())
     OnProvisioningFinished(ProvisioningResult::ARC_STOPPED);
 
@@ skipping 46 matching lines @@
     LOG(ERROR) << "Request for ARC user data removal failed.";
 
   // TODO(khmel): Browser tests may shutdown profile by itself. Update browser
   // tests and remove this check.
   if (state() == State::NOT_INITIALIZED)
     return;
 
   for (auto& observer : observer_list_)
     observer.OnArcDataRemoved();
 
   profile_->GetPrefs()->SetBoolean(prefs::kArcDataRemoveRequested, false);

[lgcheng, 2017/02/22 23:36:44]

state() is confused when multiple profiles are created.

[hidehiko, 2017/02/23 06:31:45]

Instead, could you avoid confusing the state machine in ArcSessionManager,
because it means ArcSessionManager won't work properly then?
I think there are several possibilities;

ArcAppListPrefs is per profile (BrowserContextKeyedService), while ASM is a
global singleton (owned by ArcServiceLauncher).
So, maybe could you work on fixing ArcAppListPrefs so that it can do something
good with a Profile instance which is different from the one ASM knows?

Or, alternatively, ASM needs to be created per Profile. Though, IIUC, the
current plan is to keep it singleton, for persistent ARC and login-screen work.
Thus, this approach sounds a bit harder, I guess.

Another approach is, creating a primary Profile and do something, then destroy
it completely (incl. shutdown ARC), then creating another Profile and do
something.
In that way, Profile instance and tied ArcAppListPrefs can be one at once, so it
won't hit the ASM restriction?

WDYT?

-  DCHECK_EQ(state(), State::REMOVING_DATA_DIR);
+  DCHECK(state() == State::REMOVING_DATA_DIR ||
+         g_is_session_manager_for_sync_test);
   SetState(State::STOPPED);
 
   MaybeReenableArc();
 }
 
 void ArcSessionManager::MaybeReenableArc() {
   // Here check if |reenable_arc_| is marked or not.
   // The only case this happens should be in the special case for enterprise
   // "on managed lost" case. In that case, OnSessionStopped() should trigger
   // the RemoveArcData(), then this.
@@ skipping 771 matching lines @@
 
 #undef MAP_STATE
 
   // Some compilers report an error even if all values of an enum-class are
   // covered exhaustively in a switch statement.
   NOTREACHED() << "Invalid value " << static_cast<int>(state);
   return os;
 }
 
 }  // namespace arc