Fix ArcSessionManager state machine, part 2.

chrome/browser/chromeos/arc/arc_session_manager.h

 // 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.
 
 #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 149 matching lines @@
   // enabled or disabled. Please see also TODO of
   // SetArcPlayStoreEnabledForProfile().
   void RequestEnable();
 
   // Requests to disable ARC session. This stops ARC instance, or quits Terms
   // Of Service negotiation if it is the middle of the process (e.g. closing
   // UI for manual negotiation if it is shown).
   // If it is already requested to disable, no-op.
   void RequestDisable();
 
+  // Requests to remove the ARC data.
+  // If ARC is stopped, triggers to remove the data. Otherwise, queues to
+  // remove the data on ARC stopping.

[Luis Héctor Chávez, 2017/03/07 21:52:00]

nit: "after ARC stops" is a bit more precise (otherwise it can be interpreted as
being leading-edge triggered).

[hidehiko, 2017/03/09 09:25:51]

Done.

+  // A log statement with the removal reason must be added prior to calling
+  // this.
+  void RequestArcDataRemoval();
+
   // Called from the Chrome OS metrics provider to record Arc.State
   // periodically.
   void RecordArcState();
 
   // ArcSupportHost::Observer:
   void OnWindowClosed() override;
   void OnTermsAgreed(bool is_metrics_enabled,
                      bool is_backup_and_restore_enabled,
                      bool is_location_service_enabled) override;
   void OnRetryClicked() override;
   void OnSendFeedbackClicked() override;
 
   // StopArc(), then restart. Between them data clear may happens.
   // This is a special method to support enterprise device lost case.
   // This can be called only when ARC is running.
   void StopAndEnableArc();
 
-  // Removes the data if ARC is stopped. Otherwise, queue to remove the data
-  // on ARC is stopped. A log statement with the removal reason must be added
-  // prior to calling RemoveArcData().
-  void RemoveArcData();
-
   ArcSupportHost* support_host() { return support_host_.get(); }
 
   // TODO(hidehiko): Get rid of the getter by migration between ArcAuthContext
   // and ArcAuthInfoFetcher.
   ArcAuthContext* auth_context() { return context_.get(); }
 
   // On provisioning completion (regardless of whether successfully done or
   // not), this is called with its status. On success, called with
   // ProvisioningResult::SUCCESS, otherwise |result| is the error reason.
   void OnProvisioningFinished(ProvisioningResult result);
@@ skipping 23 matching lines @@
   // RequestEnable() has a check in order not to trigger starting procedure
   // twice. This method can be called to bypass that check when restarting.
   void RequestEnableImpl();
 
   // Negotiates the terms of service to user.
   void StartTermsOfServiceNegotiation();
   void OnTermsOfServiceNegotiated(bool accepted);
 
   void SetState(State state);
   void ShutdownSession();
-  void OnArcDataRemoved(bool success);
   void OnArcSignInTimeout();
 
   // Starts Android management check. This is for first boot case (= Opt-in
   // or OOBE flow case). In secondary or later ARC enabling, the check should
   // run in background.
   void StartAndroidManagementCheck();
 
   // Called when the Android management check is done in opt-in flow or
   // OOBE flow.
   void OnAndroidManagementChecked(
@@ skipping 20 matching lines @@
   // Requests to stop ARC instnace. This resets two persistent flags:
   // kArcSignedIn and kArcTermsAccepted, so that, in next enabling,
   // it is started from Terms of Service negotiation.
   // TODO(hidehiko): Introduce STOPPING state, and this function should
   // transition to it.
   void StopArc();
 
   // ArcSessionRunner::Observer:
   void OnSessionStopped(ArcStopReason reason, bool restarting) override;
 
+  // Starts to remove ARC data, if it is requested via RequestArcDataRemoval().
+  // On completion, OnArcDataRemoved() is called.
+  // If not requested, OnArcDataRemoved() is called as if the data removal

[Yusuke Sato, 2017/03/07 21:03:08]

Please update the comment too.

[hidehiko, 2017/03/09 09:25:51]

Done.

+  // is done successfully, synchronously.
+  void StartArcDataRemoval();

[Luis Héctor Chávez, 2017/03/07 21:52:00]

Another vote to call this "MaybeStartArcDataRemoval" since it does not always
actually start the data removal.

[hidehiko, 2017/03/09 09:25:51]

Done.

+  void OnArcDataRemoved(bool success);
+
   // On ARC session stopped and/or data removal completion, this is called
   // so that, if necessary, ARC session is restarted.
   // TODO(hidehiko): This can be removed after the racy state machine
   // is fixed.
   void MaybeReenableArc();
 
   std::unique_ptr<ArcSessionRunner> arc_session_runner_;
 
   // Unowned pointer. Keeps current profile.
   Profile* profile_ = nullptr;
@@ skipping 29 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_