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

Unified Diff: components/arc/arc_bridge_service_impl.cc

Issue 2194193002: Fix ArcBridgeBootstrap race issues. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Fix ArcBridgeBootstrap race issues. 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
Index: components/arc/arc_bridge_service_impl.cc
diff --git a/components/arc/arc_bridge_service_impl.cc b/components/arc/arc_bridge_service_impl.cc
index 7b8a11edc690d059690070b1817503759c0ae4f0..09067ab3712c6943bacf4cffbb82a818955ee4bf 100644
--- a/components/arc/arc_bridge_service_impl.cc
+++ b/components/arc/arc_bridge_service_impl.cc
@@ -4,24 +4,9 @@
#include "components/arc/arc_bridge_service_impl.h"
-#include <string>
-#include <utility>
-
-#include "base/command_line.h"
-#include "base/json/json_writer.h"
#include "base/message_loop/message_loop.h"
#include "base/sequenced_task_runner.h"
-#include "base/sys_info.h"
-#include "base/task_runner_util.h"
-#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
-#include "chromeos/chromeos_switches.h"
-#include "chromeos/dbus/dbus_method_call_status.h"
-#include "chromeos/dbus/dbus_thread_manager.h"
-#include "chromeos/dbus/session_manager_client.h"
-#include "components/arc/arc_bridge_host_impl.h"
-#include "components/prefs/pref_registry_simple.h"
-#include "components/prefs/pref_service.h"
namespace arc {
@@ -31,14 +16,9 @@ namespace {
constexpr int64_t kReconnectDelayInSeconds = 5;
} // namespace
-ArcBridgeServiceImpl::ArcBridgeServiceImpl(
- std::unique_ptr<ArcBridgeBootstrap> bootstrap)
- : bootstrap_(std::move(bootstrap)),
- session_started_(false),
- weak_factory_(this) {
+ArcBridgeServiceImpl::ArcBridgeServiceImpl() : weak_factory_(this) {
DCHECK(!g_arc_bridge_service);
g_arc_bridge_service = this;
- bootstrap_->set_delegate(this);
}
ArcBridgeServiceImpl::~ArcBridgeServiceImpl() {
@@ -78,7 +58,11 @@ void ArcBridgeServiceImpl::PrerequisitesChanged() {
return;
VLOG(0) << "Prerequisites met, starting ARC";
SetStopReason(StopReason::SHUTDOWN);
+
SetState(State::CONNECTING);
+ bootstrap_ =
+ delegate_ ? delegate_->CreateBootstrap() : ArcBridgeBootstrap::Create();
+ bootstrap_->set_delegate(this);
bootstrap_->Start();
} else {
if (session_started_)
@@ -95,25 +79,24 @@ void ArcBridgeServiceImpl::StopInstance() {
return;
}
+ // We were explicitly asked to stop, so do not reconnect.
+ reconnect_ = false;
+
VLOG(1) << "Stopping ARC";
+ DCHECK(bootstrap_.get());
SetState(State::STOPPING);
- arc_bridge_host_.reset();
- bootstrap_->Stop();
- // We were explicitly asked to stop, so do not reconnect.
- reconnect_ = false;
+ // Note: this can call OnStopped() internally as a callback.
+ bootstrap_->Stop();
}
-void ArcBridgeServiceImpl::OnConnectionEstablished(
- mojom::ArcBridgeInstancePtr instance) {
+void ArcBridgeServiceImpl::OnReady() {
DCHECK(CalledOnValidThread());
if (state() != State::CONNECTING) {
VLOG(1) << "StopInstance() called while connecting";
return;
}
- arc_bridge_host_.reset(new ArcBridgeHostImpl(std::move(instance)));
-
// The container can be considered to have been successfully launched, so
// restart if the connection goes down without being requested.
reconnect_ = true;
@@ -123,10 +106,10 @@ void ArcBridgeServiceImpl::OnConnectionEstablished(
void ArcBridgeServiceImpl::OnStopped(StopReason stop_reason) {
DCHECK(CalledOnValidThread());
- arc_bridge_host_.reset();
+ VLOG(0) << "ARC stopped: " << static_cast<int>(stop_reason);
+ bootstrap_.reset();
SetStopReason(stop_reason);
SetState(State::STOPPED);
- VLOG(0) << "ARC stopped";
if (reconnect_) {
// There was a previous invocation and it crashed for some reason. Try

Powered by Google App Engine
This is Rietveld 408576698