Skip to content

Commit

Permalink
Add browser tests for USB device add/remove events.
Browse files Browse the repository at this point in the history
These tests are essentially copies of the HID device add and remove
event tests. In order to reconcile issues with the MockUsbService
lifetime it is now tracked by a MessageLoop::DestructionObserver at the
generic UsbService level. This is similar to the pattern used by the
HidService until it was moved to the UI thread and could use a
LazyInstance.

BUG=411715

Review URL: https://codereview.chromium.org/800963005

Cr-Commit-Position: refs/heads/master@{#312538}
  • Loading branch information
reillyeon authored and Commit bot committed Jan 22, 2015
1 parent 8acd5be commit 4591981
Show file tree
Hide file tree
Showing 13 changed files with 353 additions and 187 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -123,20 +123,18 @@ class DevicePermissionsManagerTest : public testing::Test {
protected:
void SetUp() override {
testing::Test::SetUp();
env_.reset(new TestExtensionEnvironment());
env_->GetExtensionPrefs(); // Force creation before adding extensions.
extension_ =
env_->MakeExtension(*base::test::ParseJson(
"{"
" \"app\": {"
" \"background\": {"
" \"scripts\": [\"background.js\"]"
" }"
" },"
" \"permissions\": ["
" \"usb\""
" ]"
"}"));
env_.GetExtensionPrefs(); // Force creation before adding extensions.
extension_ = env_.MakeExtension(*base::test::ParseJson(
"{"
" \"app\": {"
" \"background\": {"
" \"scripts\": [\"background.js\"]"
" }"
" },"
" \"permissions\": ["
" \"usb\""
" ]"
"}"));
device0_ = new MockUsbDevice("ABCDE");
device1_ = new MockUsbDevice("");
device2_ = new MockUsbDevice("12345");
Expand All @@ -145,12 +143,7 @@ class DevicePermissionsManagerTest : public testing::Test {
UsbService::SetInstanceForTest(usb_service_);
}

void TearDown() override {
env_.reset(nullptr);
UsbService::SetInstanceForTest(nullptr);
}

scoped_ptr<extensions::TestExtensionEnvironment> env_;
extensions::TestExtensionEnvironment env_;
const extensions::Extension* extension_;
MockUsbService* usb_service_;
scoped_refptr<MockUsbDevice> device0_;
Expand All @@ -161,7 +154,7 @@ class DevicePermissionsManagerTest : public testing::Test {

TEST_F(DevicePermissionsManagerTest, AllowAndClearDevices) {
DevicePermissionsManager* manager =
DevicePermissionsManager::Get(env_->profile());
DevicePermissionsManager::Get(env_.profile());
AllowUsbDevice(manager, extension_, device0_);
AllowUsbDevice(manager, extension_, device1_);

Expand Down Expand Up @@ -205,7 +198,7 @@ TEST_F(DevicePermissionsManagerTest, AllowAndClearDevices) {

TEST_F(DevicePermissionsManagerTest, SuspendExtension) {
DevicePermissionsManager* manager =
DevicePermissionsManager::Get(env_->profile());
DevicePermissionsManager::Get(env_.profile());
AllowUsbDevice(manager, extension_, device0_);
AllowUsbDevice(manager, extension_, device1_);

Expand All @@ -232,7 +225,7 @@ TEST_F(DevicePermissionsManagerTest, SuspendExtension) {

TEST_F(DevicePermissionsManagerTest, DisconnectDevice) {
DevicePermissionsManager* manager =
DevicePermissionsManager::Get(env_->profile());
DevicePermissionsManager::Get(env_.profile());
AllowUsbDevice(manager, extension_, device0_);
AllowUsbDevice(manager, extension_, device1_);

Expand Down Expand Up @@ -260,7 +253,7 @@ TEST_F(DevicePermissionsManagerTest, DisconnectDevice) {

TEST_F(DevicePermissionsManagerTest, RevokeAndRegrantAccess) {
DevicePermissionsManager* manager =
DevicePermissionsManager::Get(env_->profile());
DevicePermissionsManager::Get(env_.profile());
AllowUsbDevice(manager, extension_, device0_);
AllowUsbDevice(manager, extension_, device1_);

Expand Down Expand Up @@ -296,7 +289,7 @@ TEST_F(DevicePermissionsManagerTest, RevokeAndRegrantAccess) {

TEST_F(DevicePermissionsManagerTest, UpdateLastUsed) {
DevicePermissionsManager* manager =
DevicePermissionsManager::Get(env_->profile());
DevicePermissionsManager::Get(env_.profile());
AllowUsbDevice(manager, extension_, device0_);

scoped_ptr<DevicePermissions> device_permissions =
Expand All @@ -321,11 +314,11 @@ TEST_F(DevicePermissionsManagerTest, LoadPrefs) {
" \"vendor_id\": 0"
" }"
"]");
env_->GetExtensionPrefs()->UpdateExtensionPref(extension_->id(), "devices",
prefs_value.release());
env_.GetExtensionPrefs()->UpdateExtensionPref(extension_->id(), "devices",
prefs_value.release());

DevicePermissionsManager* manager =
DevicePermissionsManager::Get(env_->profile());
DevicePermissionsManager::Get(env_.profile());
scoped_ptr<DevicePermissions> device_permissions =
manager->GetForExtension(extension_->id());
ASSERT_TRUE(FindEntry(device_permissions.get(), device0_).get());
Expand Down
2 changes: 2 additions & 0 deletions device/usb/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ source_set("usb") {
"usb_error.h",
"usb_ids.cc",
"usb_ids.h",
"usb_service.cc",
"usb_service.h",
"usb_service_impl.cc",
"usb_service_impl.h",
generated_ids,
]

Expand Down
2 changes: 2 additions & 0 deletions device/usb/usb.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@
'usb_error.h',
'usb_ids.cc',
'usb_ids.h',
'usb_service.cc',
'usb_service.h',
'usb_service_impl.cc',
'usb_service_impl.h',
],
'actions': [
{
Expand Down
90 changes: 90 additions & 0 deletions device/usb/usb_service.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// 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 "device/usb/usb_service.h"

#include "base/message_loop/message_loop.h"
#include "device/usb/usb_device.h"
#include "device/usb/usb_service_impl.h"

namespace device {

namespace {

UsbService* g_service;

} // namespace

// This class manages the lifetime of the global UsbService instance so that
// it is destroyed when the current message loop is destroyed. A lazy instance
// cannot be used because this object does not live on the main thread.
class UsbService::Destroyer : private base::MessageLoop::DestructionObserver {
public:
explicit Destroyer(UsbService* usb_service) : usb_service_(usb_service) {
base::MessageLoop::current()->AddDestructionObserver(this);
}
~Destroyer() override {}

private:
// base::MessageLoop::DestructionObserver implementation.
void WillDestroyCurrentMessageLoop() override {
base::MessageLoop::current()->RemoveDestructionObserver(this);
delete usb_service_;
delete this;
g_service = nullptr;
}

UsbService* usb_service_;
};

void UsbService::Observer::OnDeviceAdded(scoped_refptr<UsbDevice> device) {
}

void UsbService::Observer::OnDeviceRemoved(scoped_refptr<UsbDevice> device) {
}

// static
UsbService* UsbService::GetInstance(
scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner) {
if (!g_service) {
g_service = UsbServiceImpl::Create(ui_task_runner);
// This object will clean itself up when the message loop is destroyed.
new Destroyer(g_service);
}
return g_service;
}

// static
void UsbService::SetInstanceForTest(UsbService* instance) {
g_service = instance;
new Destroyer(instance);
}

UsbService::UsbService() {
}

UsbService::~UsbService() {
}

void UsbService::AddObserver(Observer* observer) {
DCHECK(CalledOnValidThread());
observer_list_.AddObserver(observer);
}

void UsbService::RemoveObserver(Observer* observer) {
DCHECK(CalledOnValidThread());
observer_list_.RemoveObserver(observer);
}

void UsbService::NotifyDeviceAdded(scoped_refptr<UsbDevice> device) {
DCHECK(CalledOnValidThread());
FOR_EACH_OBSERVER(Observer, observer_list_, OnDeviceAdded(device));
}

void UsbService::NotifyDeviceRemoved(scoped_refptr<UsbDevice> device) {
DCHECK(CalledOnValidThread());
FOR_EACH_OBSERVER(Observer, observer_list_, OnDeviceRemoved(device));
}

} // namespace device
5 changes: 3 additions & 2 deletions device/usb/usb_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ class UsbService : public base::NonThreadSafe {
void RemoveObserver(Observer* observer);

protected:
friend struct base::DefaultDeleter<UsbService>;

UsbService();
virtual ~UsbService();

Expand All @@ -65,6 +63,9 @@ class UsbService : public base::NonThreadSafe {

ObserverList<Observer, true> observer_list_;

private:
class Destroyer;

DISALLOW_COPY_AND_ASSIGN(UsbService);
};

Expand Down
Loading

0 comments on commit 4591981

Please sign in to comment.