Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(612)

Unified Diff: components/arc/arc_bridge_bootstrap.cc

Issue 2133653002: arc: Notify ARC instance failures via callbacks. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Rebased to master. Created 4 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « components/arc/arc_bridge_bootstrap.h ('k') | components/arc/arc_bridge_service.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: components/arc/arc_bridge_bootstrap.cc
diff --git a/components/arc/arc_bridge_bootstrap.cc b/components/arc/arc_bridge_bootstrap.cc
index 77e15c42adb2ac02ae8a596d108ca3b0be1f95d6..c9cafb21c37bc8eb7d66cd01da3edb1689bedc96 100644
--- a/components/arc/arc_bridge_bootstrap.cc
+++ b/components/arc/arc_bridge_bootstrap.cc
@@ -73,11 +73,11 @@ class ArcBridgeBootstrapImpl : public ArcBridgeBootstrap,
// AcceptInstanceConnection() -> OnInstanceConnected() ->
// READY
//
- // When Stop() is called from any state, either because an operation
- // resulted in an error or because the user is logging out:
+ // When Stop() or AbortBoot() is called from any state, either because an
+ // operation resulted in an error or because the user is logging out:
//
// (any)
- // Stop() ->
+ // Stop()/AbortBoot() ->
// STOPPING
// StopInstance() ->
// STOPPED
@@ -86,6 +86,9 @@ class ArcBridgeBootstrapImpl : public ArcBridgeBootstrap,
// READY -> STOPPING -> STOPPED
// and then restarted:
// STOPPED -> SOCKET_CREATING -> ... -> READY).
+ //
+ // Note: Order of constants below matters. Please make sure to sort them
+ // in chronological order.
enum class State {
// ARC is not currently running.
STOPPED,
@@ -114,6 +117,10 @@ class ArcBridgeBootstrapImpl : public ArcBridgeBootstrap,
void Stop() override;
private:
+ // Aborts ARC instance boot. This is called from various state-machine
+ // functions when they encounter an error during boot.
+ void AbortBoot(ArcBridgeService::StopReason reason);
+
// Creates the UNIX socket on the bootstrap thread and then processes its
// file descriptor.
static base::ScopedFD CreateSocket();
@@ -135,6 +142,10 @@ class ArcBridgeBootstrapImpl : public ArcBridgeBootstrap,
// The state of the bootstrap connection.
State state_ = State::STOPPED;
+ // The reason the ARC instance is stopped.
+ ArcBridgeService::StopReason stop_reason_ =
+ ArcBridgeService::StopReason::SHUTDOWN;
+
base::ThreadChecker thread_checker_;
// WeakPtrFactory to use callbacks.
@@ -168,6 +179,7 @@ void ArcBridgeBootstrapImpl::Start() {
VLOG(1) << "Start() called when instance is not stopped";
return;
}
+ stop_reason_ = ArcBridgeService::StopReason::SHUTDOWN;
SetState(State::SOCKET_CREATING);
base::PostTaskAndReplyWithResult(
base::WorkerPool::GetTaskRunner(true).get(), FROM_HERE,
@@ -236,7 +248,7 @@ void ArcBridgeBootstrapImpl::OnSocketCreated(base::ScopedFD socket_fd) {
if (!socket_fd.is_valid()) {
LOG(ERROR) << "ARC: Error creating socket";
- Stop();
+ AbortBoot(ArcBridgeService::StopReason::GENERIC_BOOT_FAILURE);
return;
}
@@ -261,7 +273,7 @@ void ArcBridgeBootstrapImpl::OnInstanceStarted(base::ScopedFD socket_fd,
// Roll back the state to SOCKET_CREATING to avoid sending the D-Bus signal
// to stop the failed instance.
SetState(State::SOCKET_CREATING);
- Stop();
+ AbortBoot(ArcBridgeService::StopReason::GENERIC_BOOT_FAILURE);
return;
}
if (state_ != State::STARTING) {
@@ -318,12 +330,14 @@ void ArcBridgeBootstrapImpl::OnInstanceConnected(base::ScopedFD fd) {
}
if (!fd.is_valid()) {
LOG(ERROR) << "Invalid handle";
+ AbortBoot(ArcBridgeService::StopReason::GENERIC_BOOT_FAILURE);
return;
}
mojo::ScopedMessagePipeHandle server_pipe = mojo::edk::CreateMessagePipe(
mojo::edk::ScopedPlatformHandle(mojo::edk::PlatformHandle(fd.release())));
if (!server_pipe.is_valid()) {
LOG(ERROR) << "Invalid pipe";
+ AbortBoot(ArcBridgeService::StopReason::GENERIC_BOOT_FAILURE);
return;
}
SetState(State::READY);
@@ -339,11 +353,10 @@ void ArcBridgeBootstrapImpl::Stop() {
VLOG(1) << "Stop() called when ARC is not running";
return;
}
- if (state_ == State::SOCKET_CREATING) {
+ if (state_ < State::STARTING) {
// This was stopped before the D-Bus command to start the instance. Skip
// the D-Bus command to stop it.
- SetState(State::STOPPING);
- ArcInstanceStopped(true);
+ SetState(State::STOPPED);
return;
}
SetState(State::STOPPING);
@@ -354,13 +367,26 @@ void ArcBridgeBootstrapImpl::Stop() {
base::Bind(&DoNothingInstanceStopped));
}
+void ArcBridgeBootstrapImpl::AbortBoot(ArcBridgeService::StopReason reason) {
+ DCHECK(thread_checker_.CalledOnValidThread());
+ DCHECK(reason != ArcBridgeService::StopReason::SHUTDOWN);
+ // In case of multiple errors, report the first one.
+ if (stop_reason_ == ArcBridgeService::StopReason::SHUTDOWN) {
+ stop_reason_ = reason;
+ }
+ Stop();
+}
+
void ArcBridgeBootstrapImpl::ArcInstanceStopped(bool clean) {
DCHECK(thread_checker_.CalledOnValidThread());
- if (!clean)
+ if (!clean) {
LOG(ERROR) << "ARC instance crashed";
- DCHECK(delegate_);
+ // In case of multiple errors, report the first one.
+ if (stop_reason_ == ArcBridgeService::StopReason::SHUTDOWN) {
+ stop_reason_ = ArcBridgeService::StopReason::CRASH;
+ }
+ }
SetState(State::STOPPED);
- delegate_->OnStopped();
}
void ArcBridgeBootstrapImpl::SetState(State state) {
@@ -369,6 +395,10 @@ void ArcBridgeBootstrapImpl::SetState(State state) {
DCHECK(state_ != state);
state_ = state;
VLOG(2) << "State: " << static_cast<uint32_t>(state_);
+ if (state_ == State::STOPPED) {
+ DCHECK(delegate_);
+ delegate_->OnStopped(stop_reason_);
+ }
}
} // namespace
« no previous file with comments | « components/arc/arc_bridge_bootstrap.h ('k') | components/arc/arc_bridge_service.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698