Skip to content

Commit

Permalink
The changes are as follows:
Browse files Browse the repository at this point in the history
1. InputDeviceManager becomes a thread-local singleton instead of a global
   singleton. UI Service instantiates DeviceDataManager, which is the service-
   side implementation of InputDeviceManager, while the browser uses
   InputDeviceClient, which is the client-side implementation.

2. PlatformEventSource also becomes a thread-local singleton. This is to prevent
   ash and the browser from executing code on the UI Service's thread by
   registering PlatformEventObservers (e.g. via UserActivityDetector). In the
   futurer we may need a client-side implementation of PlatformEventSource.

3. GPU Service checks if a PowerMonitor singleton exists before creating one.
   This is temporary until GPU Service runs in a separate process from the UI
   Service. (crbug.com/609317).

BUG=722527

Review-Url: https://codereview.chromium.org/2978873002
Cr-Commit-Position: refs/heads/master@{#486534}
  • Loading branch information
mfomitchev authored and Commit Bot committed Jul 13, 2017
1 parent 591e1f8 commit f76cad2
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 21 deletions.
9 changes: 8 additions & 1 deletion services/ui/gpu/gpu_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,15 @@ namespace ui {
GpuMain::GpuMain(mojom::GpuMainRequest request)
: gpu_thread_("GpuThread"),
io_thread_("GpuIOThread"),
power_monitor_(base::MakeUnique<base::PowerMonitorDeviceSource>()),
binding_(this) {
// TODO: crbug.com/609317: Remove this when Mus Window Server and GPU are
// split into separate processes. Until then this is necessary to be able to
// run Mushrome (chrome --mus) with Mus running in the browser process.
if (!base::PowerMonitor::Get()) {
power_monitor_ = base::MakeUnique<base::PowerMonitor>(
base::MakeUnique<base::PowerMonitorDeviceSource>());
}

base::Thread::Options thread_options;

#if defined(OS_WIN)
Expand Down
2 changes: 1 addition & 1 deletion services/ui/gpu/gpu_main.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class GpuMain : public gpu::GpuSandboxHelper, public mojom::GpuMain {

scoped_refptr<base::SingleThreadTaskRunner> compositor_thread_task_runner_;

base::PowerMonitor power_monitor_;
std::unique_ptr<base::PowerMonitor> power_monitor_;
mojo::Binding<mojom::GpuMain> binding_;

DISALLOW_COPY_AND_ASSIGN(GpuMain);
Expand Down
24 changes: 17 additions & 7 deletions ui/events/devices/input_device_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,41 @@
// found in the LICENSE file.

#include "ui/events/devices/input_device_manager.h"
#include "base/lazy_instance.h"
#include "base/threading/thread_local.h"

namespace ui {
namespace {

InputDeviceManager* InputDeviceManager::instance_ = nullptr;
// InputDeviceManager singleton is thread-local so that different instances can
// be used on different threads (eg. UI Service thread vs. browser UI thread).
base::LazyInstance<base::ThreadLocalPointer<InputDeviceManager>>::Leaky
lazy_tls_ptr = LAZY_INSTANCE_INITIALIZER;

} // namespace

// static
InputDeviceManager* InputDeviceManager::GetInstance() {
DCHECK(instance_);
return instance_;
InputDeviceManager* instance = lazy_tls_ptr.Pointer()->Get();
DCHECK(instance) << "InputDeviceManager::SetInstance must be called before "
"getting the instance of InputDeviceManager.";
return instance;
}

// static
bool InputDeviceManager::HasInstance() {
return instance_ != nullptr;
return lazy_tls_ptr.Pointer()->Get() != nullptr;
}

// static
void InputDeviceManager::SetInstance(InputDeviceManager* instance) {
DCHECK(!instance_);
instance_ = instance;
DCHECK(!lazy_tls_ptr.Pointer()->Get());
lazy_tls_ptr.Pointer()->Set(instance);
}

// static
void InputDeviceManager::ClearInstance() {
instance_ = nullptr;
lazy_tls_ptr.Pointer()->Set(nullptr);
}

} // namespace ui
6 changes: 3 additions & 3 deletions ui/events/devices/input_device_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@

namespace ui {

// Interface to query available input devices. Holds a static pointer to an
// implementation that provides this service. The implementation could be
// Interface to query available input devices. Holds a thread-local pointer to
// an implementation that provides this service. The implementation could be
// DeviceDataManager or something that mirrors the necessary state if
// DeviceDataManager is in a different process.
class EVENTS_DEVICES_EXPORT InputDeviceManager {
Expand All @@ -39,7 +39,7 @@ class EVENTS_DEVICES_EXPORT InputDeviceManager {
virtual void RemoveObserver(InputDeviceEventObserver* observer) = 0;

protected:
// Sets the instance. This should only be set once per process.
// Sets the instance. This should only be set once per thread.
static void SetInstance(InputDeviceManager* instance);

// Clears the instance. InputDeviceManager doesn't own the instance and won't
Expand Down
26 changes: 19 additions & 7 deletions ui/events/platform/platform_event_source.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,42 @@

#include <algorithm>

#include "base/lazy_instance.h"
#include "base/memory/ptr_util.h"
#include "base/message_loop/message_loop.h"
#include "base/threading/thread_local.h"
#include "ui/events/platform/platform_event_dispatcher.h"
#include "ui/events/platform/platform_event_observer.h"
#include "ui/events/platform/scoped_event_dispatcher.h"

namespace ui {

// static
PlatformEventSource* PlatformEventSource::instance_ = NULL;
namespace {

// PlatformEventSource singleton is thread local so that different instances
// can be used on different threads (e.g. browser thread should be able to
// access PlatformEventSource owned by the UI Service's thread).
base::LazyInstance<base::ThreadLocalPointer<PlatformEventSource>>::Leaky
lazy_tls_ptr = LAZY_INSTANCE_INITIALIZER;

} // namespace

PlatformEventSource::PlatformEventSource()
: overridden_dispatcher_(NULL),
overridden_dispatcher_restored_(false) {
CHECK(!instance_) << "Only one platform event source can be created.";
instance_ = this;
CHECK(!lazy_tls_ptr.Pointer()->Get())
<< "Only one platform event source can be created.";
lazy_tls_ptr.Pointer()->Set(this);
}

PlatformEventSource::~PlatformEventSource() {
CHECK_EQ(this, instance_);
instance_ = NULL;
CHECK_EQ(this, lazy_tls_ptr.Pointer()->Get());
lazy_tls_ptr.Pointer()->Set(nullptr);
}

PlatformEventSource* PlatformEventSource::GetInstance() { return instance_; }
PlatformEventSource* PlatformEventSource::GetInstance() {
return lazy_tls_ptr.Pointer()->Get();
}

void PlatformEventSource::AddPlatformEventDispatcher(
PlatformEventDispatcher* dispatcher) {
Expand Down
4 changes: 2 additions & 2 deletions ui/events/platform/platform_event_source.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class EVENTS_EXPORT PlatformEventSource {
public:
virtual ~PlatformEventSource();

// Returns the thread-local singleton.
static PlatformEventSource* GetInstance();

// Adds a dispatcher to the dispatcher list. If a dispatcher is added during
Expand Down Expand Up @@ -64,6 +65,7 @@ class EVENTS_EXPORT PlatformEventSource {
void AddPlatformEventObserver(PlatformEventObserver* observer);
void RemovePlatformEventObserver(PlatformEventObserver* observer);

// Creates PlatformEventSource and sets it as a thread-local singleton.
static std::unique_ptr<PlatformEventSource> CreateDefault();

protected:
Expand All @@ -80,8 +82,6 @@ class EVENTS_EXPORT PlatformEventSource {
friend class ScopedEventDispatcher;
friend class test::PlatformEventSourceTestAPI;

static PlatformEventSource* instance_;

// This is invoked when the list of dispatchers changes (i.e. a new dispatcher
// is added, or a dispatcher is removed).
virtual void OnDispatcherListChanged();
Expand Down

0 comments on commit f76cad2

Please sign in to comment.