Skip to content

Commit

Permalink
Relanding addProfile/removeProfile patch with heapchecker fix.
Browse files Browse the repository at this point in the history 
This is a relanding of r197554 with heapchecker fix.

Some profile-dependent services depend on UI thread to clean up, so I had to add
UI message loop in bluetooth_event_router_unittest.cc to make sure that all the
objects are released at the end of test.

I verified with valgrind that this patch does not introduce any new memory leak.

BUG=229636

Review URL: https://chromiumcodereview.appspot.com/14569007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@197741 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
youngki@chromium.org committed May 1, 2013
1 parent af8fae7 commit ec00099
Show file tree
Hide file tree
Showing 17 changed files with 580 additions and 33 deletions.
105 changes: 100 additions & 5 deletions chrome/browser/extensions/api/bluetooth/bluetooth_api.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "device/bluetooth/bluetooth_adapter.h"
#include "device/bluetooth/bluetooth_device.h"
#include "device/bluetooth/bluetooth_out_of_band_pairing_data.h"
#include "device/bluetooth/bluetooth_profile.h"
#include "device/bluetooth/bluetooth_service_record.h"
#include "device/bluetooth/bluetooth_socket.h"
#include "device/bluetooth/bluetooth_utils.h"
Expand Down Expand Up @@ -50,6 +51,10 @@ const char kInvalidDevice[] = "Invalid device";
const char kInvalidUuid[] = "Invalid UUID";
const char kPlatformNotSupported[] =
"This operation is not supported on your platform";
const char kProfileAlreadyRegistered[] =
"This profile has already been registered";
const char kProfileNotFound[] = "Profile not found: invalid uuid";
const char kProfileRegistrationFailed[] = "Profile registration failed";
const char kServiceDiscoveryFailed[] = "Service discovery failed";
const char kSocketNotFoundError[] = "Socket not found: invalid socket id";
const char kStartDiscoveryFailed[] = "Starting discovery failed";
Expand Down Expand Up @@ -107,16 +112,106 @@ void BluetoothAPI::OnListenerRemoved(const EventListenerInfo& details) {

namespace api {

// TOOD(youngki): Implement.
BluetoothAddProfileFunction::BluetoothAddProfileFunction() {
}

bool BluetoothAddProfileFunction::RunImpl() {
return false;
scoped_ptr<AddProfile::Params> params(AddProfile::Params::Create(*args_));
EXTENSION_FUNCTION_VALIDATE(params.get() != NULL);

if (!BluetoothDevice::IsUUIDValid(params->profile.uuid)) {
SetError(kInvalidUuid);
return false;
}

uuid_ = device::bluetooth_utils::CanonicalUuid(params->profile.uuid);

if (GetEventRouter(profile())->HasProfile(uuid_)) {
SetError(kProfileAlreadyRegistered);
return false;
}

device::BluetoothProfile::Options options;
if (params->profile.name.get())
options.name = *params->profile.name.get();
if (params->profile.channel.get())
options.channel = *params->profile.channel.get();
if (params->profile.psm.get())
options.psm = *params->profile.psm.get();
if (params->profile.require_authentication.get()) {
options.require_authentication =
*params->profile.require_authentication.get();
}
if (params->profile.require_authorization.get()) {
options.require_authorization =
*params->profile.require_authorization.get();
}
if (params->profile.auto_connect.get())
options.auto_connect = *params->profile.auto_connect.get();
if (params->profile.version.get())
options.version = *params->profile.version.get();
if (params->profile.features.get())
options.features = *params->profile.features.get();

RegisterProfile(
options,
base::Bind(&BluetoothAddProfileFunction::OnProfileRegistered, this));

return true;
}

void BluetoothAddProfileFunction::RegisterProfile(
const device::BluetoothProfile::Options& options,
const device::BluetoothProfile::ProfileCallback& callback) {
device::BluetoothProfile::Register(uuid_, options, callback);
}

void BluetoothAddProfileFunction::OnProfileRegistered(
device::BluetoothProfile* bluetooth_profile) {
if (!bluetooth_profile) {
SetError(kProfileRegistrationFailed);
SendResponse(false);
return;
}

if (GetEventRouter(profile())->HasProfile(uuid_)) {
bluetooth_profile->Unregister();
SetError(kProfileAlreadyRegistered);
SendResponse(false);
return;
}

bluetooth_profile->SetConnectionCallback(
base::Bind(&ExtensionBluetoothEventRouter::DispatchConnectionEvent,
base::Unretained(GetEventRouter(profile())),
extension_id(),
uuid_));
GetEventRouter(profile())->AddProfile(uuid_, bluetooth_profile);
SendResponse(true);
}

// TODO(youngki): Implement.
bool BluetoothRemoveProfileFunction::RunImpl() {
return false;
scoped_ptr<RemoveProfile::Params> params(
RemoveProfile::Params::Create(*args_));

if (!BluetoothDevice::IsUUIDValid(params->profile.uuid)) {
SetError(kInvalidUuid);
return false;
}

std::string uuid =
device::bluetooth_utils::CanonicalUuid(params->profile.uuid);

if (!GetEventRouter(profile())->HasProfile(uuid)) {
SetError(kProfileNotFound);
return false;
}

GetEventRouter(profile())->RemoveProfile(uuid);
return true;
}

// TODO(youngki): Implement.
bool BluetoothGetProfilesFunction::DoWork(
scoped_refptr<device::BluetoothAdapter> adapter) {
scoped_ptr<GetProfiles::Params> params(GetProfiles::Params::Create(*args_));
Expand Down Expand Up @@ -273,7 +368,7 @@ void BluetoothConnectFunction::ConnectToServiceCallback(

bluetooth::Socket result_socket;
bluetooth::BluetoothDeviceToApiDevice(*device, &result_socket.device);
result_socket.service_uuid = service_uuid;
result_socket.profile.uuid = service_uuid;
result_socket.id = socket_id;
SetResult(result_socket.ToValue().release());
SendResponse(true);
Expand Down
17 changes: 14 additions & 3 deletions chrome/browser/extensions/api/bluetooth/bluetooth_api.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "chrome/browser/extensions/event_router.h"
#include "chrome/browser/profiles/profile_keyed_service.h"
#include "device/bluetooth/bluetooth_device.h"
#include "device/bluetooth/bluetooth_profile.h"

namespace device {

Expand Down Expand Up @@ -60,20 +61,30 @@ class BluetoothAddProfileFunction : public AsyncExtensionFunction {
public:
DECLARE_EXTENSION_FUNCTION("bluetooth.addProfile", BLUETOOTH_ADDPROFILE)

virtual bool RunImpl() OVERRIDE;
BluetoothAddProfileFunction();

protected:
virtual ~BluetoothAddProfileFunction() {}
virtual bool RunImpl() OVERRIDE;

virtual void RegisterProfile(
const device::BluetoothProfile::Options& options,
const device::BluetoothProfile::ProfileCallback& callback);

private:
void OnProfileRegistered(device::BluetoothProfile* bluetooth_profile);

std::string uuid_;
};

class BluetoothRemoveProfileFunction : public AsyncExtensionFunction {
class BluetoothRemoveProfileFunction : public SyncExtensionFunction {
public:
DECLARE_EXTENSION_FUNCTION("bluetooth.removeProfile",
BLUETOOTH_REMOVEPROFILE)
virtual bool RunImpl() OVERRIDE;

protected:
virtual ~BluetoothRemoveProfileFunction() {}
virtual bool RunImpl() OVERRIDE;
};

class BluetoothGetProfilesFunction : public BluetoothExtensionFunction {
Expand Down
127 changes: 115 additions & 12 deletions chrome/browser/extensions/api/bluetooth/bluetooth_apitest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,17 @@
#include "device/bluetooth/bluetooth_out_of_band_pairing_data.h"
#include "device/bluetooth/test/mock_bluetooth_adapter.h"
#include "device/bluetooth/test/mock_bluetooth_device.h"
#include "device/bluetooth/test/mock_bluetooth_profile.h"
#include "device/bluetooth/test/mock_bluetooth_socket.h"
#include "testing/gmock/include/gmock/gmock.h"

using device::BluetoothAdapter;
using device::BluetoothDevice;
using device::BluetoothOutOfBandPairingData;
using device::BluetoothProfile;
using device::MockBluetoothAdapter;
using device::MockBluetoothDevice;
using device::MockBluetoothProfile;
using extensions::Extension;

namespace utils = extension_function_test_utils;
Expand All @@ -48,6 +51,8 @@ class BluetoothApiTest : public ExtensionApiTest {

virtual void SetUpOnMainThread() OVERRIDE {
SetUpMockAdapter();
profile1_.reset(new testing::NiceMock<MockBluetoothProfile>());
profile2_.reset(new testing::NiceMock<MockBluetoothProfile>());
}

virtual void CleanUpOnMainThread() OVERRIDE {
Expand Down Expand Up @@ -78,6 +83,8 @@ class BluetoothApiTest : public ExtensionApiTest {
testing::StrictMock<MockBluetoothAdapter>* mock_adapter_;
scoped_ptr<testing::NiceMock<MockBluetoothDevice> > device1_;
scoped_ptr<testing::NiceMock<MockBluetoothDevice> > device2_;
scoped_ptr<testing::NiceMock<MockBluetoothProfile> > profile1_;
scoped_ptr<testing::NiceMock<MockBluetoothProfile> > profile2_;

extensions::ExtensionBluetoothEventRouter* event_router() {
return extensions::BluetoothAPI::Get(browser()->profile())
Expand All @@ -88,6 +95,29 @@ class BluetoothApiTest : public ExtensionApiTest {
scoped_refptr<Extension> empty_extension_;
};

class TestBluetoothAddProfileFunction
: public api::BluetoothAddProfileFunction {
public:
explicit TestBluetoothAddProfileFunction(BluetoothProfile* profile)
: BluetoothAddProfileFunction(), profile_(profile) {
}

protected:
virtual ~TestBluetoothAddProfileFunction() {
}

// BluetoothAddProfileFunction override.
virtual void RegisterProfile(
const device::BluetoothProfile::Options& options,
const device::BluetoothProfile::ProfileCallback& callback) {
callback.Run(profile_);
}

private:
// TestBluetoothAddProfileFunction does not own |profile_|.
BluetoothProfile* profile_;
};

// This is the canonical UUID for the short UUID 0010.
static const char kOutOfBandPairingDataHash[] = "0123456789ABCDEh";
static const char kOutOfBandPairingDataRandomizer[] = "0123456789ABCDEr";
Expand Down Expand Up @@ -141,15 +171,63 @@ static void CallConnectToServiceCallback(

} // namespace

IN_PROC_BROWSER_TEST_F(BluetoothApiTest, AddProfile) {
// TODO(youngki): Implement it to test BluetoothAddProfileFunction.
}

IN_PROC_BROWSER_TEST_F(BluetoothApiTest, RemoveProfile) {
// TODO(youngki): Implement it to test BluetoothRemoveProfileFunction.
IN_PROC_BROWSER_TEST_F(BluetoothApiTest, Profiles) {
EXPECT_CALL(*profile1_, SetConnectionCallback(testing::_));
scoped_refptr<TestBluetoothAddProfileFunction> add_profile_function;
add_profile_function = setupFunction(
new TestBluetoothAddProfileFunction(profile1_.get()));
std::string error(
utils::RunFunctionAndReturnError(
add_profile_function,
"[{\"uuid\": \"1234\"}]",
browser()));
ASSERT_TRUE(error.empty());

// Registering the profile for the same uuid again will throw an error.
add_profile_function = setupFunction(
new TestBluetoothAddProfileFunction(profile2_.get()));
error = utils::RunFunctionAndReturnError(
add_profile_function,
"[{\"uuid\": \"1234\"}]",
browser());
ASSERT_FALSE(error.empty());

add_profile_function = setupFunction(
new TestBluetoothAddProfileFunction(profile2_.get()));
error = utils::RunFunctionAndReturnError(
add_profile_function,
"[{\"uuid\": \"5678\"}]",
browser());
ASSERT_TRUE(error.empty());

scoped_refptr<api::BluetoothRemoveProfileFunction> remove_profile_function;
remove_profile_function = setupFunction(
new api::BluetoothRemoveProfileFunction());
error = utils::RunFunctionAndReturnError(
remove_profile_function,
"[{\"uuid\": \"1234\"}]",
browser());
ASSERT_TRUE(error.empty());

remove_profile_function = setupFunction(
new api::BluetoothRemoveProfileFunction());
error = utils::RunFunctionAndReturnError(
remove_profile_function,
"[{\"uuid\": \"5678\"}]",
browser());
ASSERT_TRUE(error.empty());

// Removing the same profile again will throw an error.
remove_profile_function = setupFunction(
new api::BluetoothRemoveProfileFunction());
error = utils::RunFunctionAndReturnError(
remove_profile_function,
"[{\"uuid\": \"5678\"}]",
browser());
ASSERT_FALSE(error.empty());
}

IN_PROC_BROWSER_TEST_F(BluetoothApiTest, OnAdapterStateChanged) {
IN_PROC_BROWSER_TEST_F(BluetoothApiTest, GetAdapterState) {
EXPECT_CALL(*mock_adapter_, GetAddress())
.WillOnce(testing::Return(kAdapterAddress));
EXPECT_CALL(*mock_adapter_, GetName())
Expand Down Expand Up @@ -262,7 +340,7 @@ IN_PROC_BROWSER_TEST_F(BluetoothApiTest, Discovery) {
start_function = setupFunction(new api::BluetoothStartDiscoveryFunction);
std::string error(
utils::RunFunctionAndReturnError(start_function, "[]", browser()));
ASSERT_TRUE(!error.empty());
ASSERT_FALSE(error.empty());

// Reset for a successful start
SetUpMockAdapter();
Expand All @@ -276,7 +354,7 @@ IN_PROC_BROWSER_TEST_F(BluetoothApiTest, Discovery) {
testing::Mock::VerifyAndClearExpectations(mock_adapter_);
EXPECT_CALL(*mock_adapter_, StopDiscovering(testing::_, testing::_))
.WillOnce(testing::Invoke(CallDiscoveryCallback));
// StopDiscovery success will remove the apapter that is no longer used.
// StopDiscovery success will remove the adapter that is no longer used.
EXPECT_CALL(*mock_adapter_, RemoveObserver(testing::_));
scoped_refptr<api::BluetoothStopDiscoveryFunction> stop_function;
stop_function = setupFunction(new api::BluetoothStopDiscoveryFunction);
Expand All @@ -289,7 +367,7 @@ IN_PROC_BROWSER_TEST_F(BluetoothApiTest, Discovery) {
EXPECT_CALL(*mock_adapter_, RemoveObserver(testing::_));
stop_function = setupFunction(new api::BluetoothStopDiscoveryFunction);
error = utils::RunFunctionAndReturnError(stop_function, "[]", browser());
ASSERT_TRUE(!error.empty());
ASSERT_FALSE(error.empty());
SetUpMockAdapter();
}

Expand Down Expand Up @@ -368,13 +446,15 @@ IN_PROC_BROWSER_TEST_F(BluetoothApiTest, DiscoveryInProgress) {
EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
}

IN_PROC_BROWSER_TEST_F(BluetoothApiTest, Events) {
IN_PROC_BROWSER_TEST_F(BluetoothApiTest, OnAdapterStateChanged) {
ResultCatcher catcher;
catcher.RestrictToProfile(browser()->profile());

// Load and wait for setup
ExtensionTestMessageListener listener("ready", true);
ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("bluetooth/events")));
ASSERT_TRUE(
LoadExtension(
test_data_dir_.AppendASCII("bluetooth/on_adapter_state_changed")));
EXPECT_TRUE(listener.WaitUntilSatisfied());

EXPECT_CALL(*mock_adapter_, GetAddress())
Expand Down Expand Up @@ -418,6 +498,29 @@ IN_PROC_BROWSER_TEST_F(BluetoothApiTest, Events) {
EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
}

IN_PROC_BROWSER_TEST_F(BluetoothApiTest, OnConnection) {
ResultCatcher catcher;
catcher.RestrictToProfile(browser()->profile());

// Load and wait for setup
ExtensionTestMessageListener listener("ready", true);
scoped_refptr<const Extension> extension(
LoadExtension(test_data_dir_.AppendASCII("bluetooth/on_connection")));
ASSERT_TRUE(extension.get());
EXPECT_TRUE(listener.WaitUntilSatisfied());

scoped_refptr<device::MockBluetoothSocket> socket =
new device::MockBluetoothSocket();

event_router()->AddProfile("1234", profile1_.get());
event_router()->DispatchConnectionEvent(
extension->id(), "1234", device1_.get(), socket);

listener.Reply("go");
EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
event_router()->RemoveProfile("1234");
}

IN_PROC_BROWSER_TEST_F(BluetoothApiTest, GetProfiles) {
ResultCatcher catcher;
catcher.RestrictToProfile(browser()->profile());
Expand Down
Loading

0 comments on commit ec00099

Please sign in to comment.