Move free disk space check to session_manager.

components/arc/arc_bridge_bootstrap.cc

 // Copyright 2015 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 "components/arc/arc_bridge_bootstrap.h"
 
 #include <fcntl.h>
 #include <grp.h>
 #include <poll.h>
 #include <unistd.h>
@@ skipping 28 matching lines @@
 
 namespace arc {
 
 namespace {
 
 const base::FilePath::CharType kArcBridgeSocketPath[] =
     FILE_PATH_LITERAL("/var/run/chrome/arc_bridge.sock");
 
 const char kArcBridgeSocketGroup[] = "arc-bridge";
 
-const base::FilePath::CharType kDiskCheckPath[] = "/home";
-
-const int64_t kCriticalDiskFreeBytes = 64 << 20;  // 64MB
+// TODO(hidehiko): Share the constant between Chrome and ChromeOS.
+constexpr char kArcLowDiskError[] =
+    "org.chromium.SessionManagerInterface.LowFreeDisk";
 
 // This is called when StopArcInstance D-Bus method completes. Since we have the
 // ArcInstanceStopped() callback and are notified if StartArcInstance fails, we
 // don't need to do anything when StopArcInstance completes.
 void DoNothingInstanceStopped(bool) {}
 
 chromeos::SessionManagerClient* GetSessionManagerClient() {
   // If the DBusThreadManager or the SessionManagerClient aren't available,
   // there isn't much we can do. This should only happen when running tests.
   if (!chromeos::DBusThreadManager::IsInitialized() ||
@@ skipping 46 matching lines @@
 // TODO(hidehiko): Refactor more to make this class unittest-able, for at least
 // state-machine part.
 class ArcBridgeBootstrapImpl : public ArcBridgeBootstrap,
                                public chromeos::SessionManagerClient::Observer {
  public:
   // The possible states of the bootstrap connection.  In the normal flow,
   // the state changes in the following sequence:
   //
   // NOT_STARTED
   //   Start() ->
   // CHECKING_DISK_SPACE

[Shuhei Takahashi, 2016/10/06 08:21:03]

Please update these comments to remove CHECKING_DISK_SPACE.

[hidehiko, 2016/10/06 08:33:21]

Good catch. Done.

   //   OnDiskSpaceChecked() ->
   // CREATING_SOCKET
   //   CreateSocket() -> OnSocketCreated() ->
   // STARTING_INSTANCE
   //   -> OnInstanceStarted() ->
   // CONNECTING_MOJO
   //   ConnectMojo() -> OnMojoConnected() ->
   // RUNNING
   //
   // At any state, Stop() can be called. It does not immediately stop the
@@ skipping 50 matching lines @@
   // Specifically, in STOPPED state, there may be inflight operations or
   // pending callbacks. Though, what they do is just do-nothing conceptually
   // and they can be safely ignored.
   //
   // Note: Order of constants below matters. Please make sure to sort them
   // in chronological order.
   enum class State {
     // ARC is not yet started.
     NOT_STARTED,
 
-    // Checking the disk space.
-    CHECKING_DISK_SPACE,
-
     // An UNIX socket is being created.
     CREATING_SOCKET,
 
     // The request to start the instance has been sent.
     STARTING_INSTANCE,
 
     // The instance has started. Waiting for it to connect to the IPC bridge.
     CONNECTING_MOJO,
 
     // The instance is fully set up.
     RUNNING,
 
     // ARC is terminated.
     STOPPED,
   };
 
   ArcBridgeBootstrapImpl();
   ~ArcBridgeBootstrapImpl() override;
 
   // ArcBridgeBootstrap:
   void Start() override;
   void Stop() override;
 
  private:
-  // Called after getting the device free disk space.
-  void OnFreeDiskSpaceObtained(int64_t disk_free_bytes);
-
   // Creates the UNIX socket on the bootstrap thread and then processes its
   // file descriptor.
   static base::ScopedFD CreateSocket();
   void OnSocketCreated(base::ScopedFD fd);
 
   // DBus callback for StartArcInstance().
-  void OnInstanceStarted(base::ScopedFD socket_fd, bool success);
+  void OnInstanceStarted(base::ScopedFD socket_fd,
+                         bool success,
+                         const std::string& error);
 
   // Synchronously accepts a connection on |socket_fd| and then processes the
   // connected socket's file descriptor.
   static base::ScopedFD ConnectMojo(base::ScopedFD socket_fd,
                                     base::ScopedFD cancel_fd);
   void OnMojoConnected(base::ScopedFD fd);
 
   // Request to stop ARC instance via DBus.
   void StopArcInstance();
 
@@ skipping 40 matching lines @@
   chromeos::SessionManagerClient* client = GetSessionManagerClient();
   if (client == nullptr)
     return;
   client->RemoveObserver(this);
 }
 
 void ArcBridgeBootstrapImpl::Start() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_EQ(state_, State::NOT_STARTED);
   VLOG(2) << "Starting ARC session.";
-  VLOG(2) << "Checking disk space...";
-  state_ = State::CHECKING_DISK_SPACE;
+  VLOG(2) << "Disk space check is done. Creating socket...";

[Shuhei Takahashi, 2016/10/06 08:21:03]

Could you remove this line?

[hidehiko, 2016/10/06 08:33:21]

Done.

 
-  // TODO(crbug.com/628124): Move disk space checking logic to session_manager.
-  base::PostTaskAndReplyWithResult(
-      base::WorkerPool::GetTaskRunner(true).get(), FROM_HERE,
-      base::Bind(&base::SysInfo::AmountOfFreeDiskSpace,
-                 base::FilePath(kDiskCheckPath)),
-      base::Bind(&ArcBridgeBootstrapImpl::OnFreeDiskSpaceObtained,
-                 weak_factory_.GetWeakPtr()));
-}
-
-void ArcBridgeBootstrapImpl::OnFreeDiskSpaceObtained(int64_t disk_free_bytes) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK_EQ(state_, State::CHECKING_DISK_SPACE);
-
-  if (stop_requested_) {
-    VLOG(1) << "Stop() called while checking disk space";
-    OnStopped(ArcBridgeService::StopReason::SHUTDOWN);
-    return;
-  }
-
-  if (disk_free_bytes < 0) {
-    LOG(ERROR) << "ARC: Failed to get free disk space";
-    OnStopped(ArcBridgeService::StopReason::GENERIC_BOOT_FAILURE);
-    return;
-  }
-  if (disk_free_bytes < kCriticalDiskFreeBytes) {
-    LOG(ERROR) << "ARC: The device is too low on disk space to start ARC";
-    OnStopped(ArcBridgeService::StopReason::LOW_DISK_SPACE);
-    return;
-  }
-
-  VLOG(2) << "Disk space check is done. Creating socket...";
   state_ = State::CREATING_SOCKET;
   base::PostTaskAndReplyWithResult(
       base::WorkerPool::GetTaskRunner(true).get(), FROM_HERE,
       base::Bind(&ArcBridgeBootstrapImpl::CreateSocket),
       base::Bind(&ArcBridgeBootstrapImpl::OnSocketCreated,
                  weak_factory_.GetWeakPtr()));
 }
 
 // static
 base::ScopedFD ArcBridgeBootstrapImpl::CreateSocket() {
@@ skipping 62 matching lines @@
   chromeos::SessionManagerClient* session_manager_client =
       chromeos::DBusThreadManager::Get()->GetSessionManagerClient();
   session_manager_client->StartArcInstance(
       cryptohome_id,
       disable_boot_completed_broadcast,
       base::Bind(&ArcBridgeBootstrapImpl::OnInstanceStarted,
                  weak_factory_.GetWeakPtr(), base::Passed(&socket_fd)));
 }
 
 void ArcBridgeBootstrapImpl::OnInstanceStarted(base::ScopedFD socket_fd,
-                                               bool success) {
+                                               bool success,
+                                               const std::string& error) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (state_ == State::STOPPED) {
     // This is the case that error is notified via DBus before the
     // OnInstanceStarted() callback is invoked. The stopping procedure has
     // been run, so do nothing.
     return;
   }
 
   DCHECK_EQ(state_, State::STARTING_INSTANCE);
 
   if (stop_requested_) {
     if (success) {
       // The ARC instance has started to run. Request to stop.
       StopArcInstance();
       return;
     }
     OnStopped(ArcBridgeService::StopReason::SHUTDOWN);
     return;
   }
 
   if (!success) {
     LOG(ERROR) << "Failed to start ARC instance";
-    OnStopped(ArcBridgeService::StopReason::GENERIC_BOOT_FAILURE);
+    OnStopped(error == kArcLowDiskError
+                  ? ArcBridgeService::StopReason::LOW_DISK_SPACE
+                  : ArcBridgeService::StopReason::GENERIC_BOOT_FAILURE);
     return;
   }
 
   VLOG(2) << "ARC instance is successfully started. Connecting Mojo...";
   state_ = State::CONNECTING_MOJO;
 
   // Prepare a pipe so that AcceptInstanceConnection can be interrupted on
   // Stop().
   base::ScopedFD cancel_fd;
   if (!CreatePipe(&cancel_fd, &accept_cancel_pipe_)) {
@@ skipping 97 matching lines @@
   if (stop_requested_)
     return;
 
   stop_requested_ = true;
   arc_bridge_host_.reset();
   switch (state_) {
     case State::NOT_STARTED:
       OnStopped(ArcBridgeService::StopReason::SHUTDOWN);
       return;
 
-    case State::CHECKING_DISK_SPACE:
     case State::CREATING_SOCKET:
     case State::STARTING_INSTANCE:
       // Before starting the ARC instance, we do nothing here.
       // At some point, a callback will be invoked on UI thread,
       // and stopping procedure will be run there.
       // On Chrome shutdown, it is not the case because the message loop is
       // already stopped here. Practically, it is not a problem because;
       // - On disk space checking or on socket creating, it is ok to simply
       // ignore such cases, because we no-longer continue the bootstrap
       // procedure.
@@ skipping 83 matching lines @@
 void ArcBridgeBootstrap::RemoveObserver(Observer* observer) {
   observer_list_.RemoveObserver(observer);
 }
 
 // static
 std::unique_ptr<ArcBridgeBootstrap> ArcBridgeBootstrap::Create() {
   return base::MakeUnique<ArcBridgeBootstrapImpl>();
 }
 
 }  // namespace arc