bluetooth: Move mock creation out of BluetoothDispatcherHost to LayoutTestBluetoothAdapterProvider.

content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.cc

Index: content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.cc
diff --git a/content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.cc b/content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d8d1493d23e514d8015d636af550705973c54461
--- /dev/null
+++ b/content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.cc
@@ -0,0 +1,101 @@
+// 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 "content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.h"
+
+#include "device/bluetooth/bluetooth_discovery_session.h"
+#include "testing/gmock/include/gmock/gmock.h"
+

[scheib, 2015/05/12 01:16:28]

Add using statements to simplify code below, such as:
using testing::Invoke;
using testing::Return;
using testing::NiceMock;
using testing::_;

[ortuno, 2015/05/12 19:33:20]

Done.

+namespace content {
+
+const uint32 kUnspecifiedDeviceClass =
+    0x1F00;  // bluetooth.org/en-us/specification/assigned-numbers/baseband
+
+LayoutTestBluetoothAdapterProvider::LayoutTestBluetoothAdapterProvider()
+    : uuid1(new BluetoothUUID("1800")), uuid2(new BluetoothUUID("1801")) {
+}
+LayoutTestBluetoothAdapterProvider::~LayoutTestBluetoothAdapterProvider() {

[scheib, 2015/05/11 20:19:26]

Insert blank line above.

[ortuno, 2015/05/12 19:33:19]

Done.

+}
+
+void LayoutTestBluetoothAdapterProvider::GetBluetoothAdapter(
+    const BluetoothAdapterFactory::AdapterCallback& callback,
+    const std::string& adapter) {
+  // TODO(ortuno): Remove RejectRequestDevice once LayoutTests are modified
+  if (adapter == "RejectRequestDevice_NotFoundError" ||
+      adapter == "EmptyAdapter")
+    GetEmptyAdapter(callback);
+  // TODO(ortuno): Remove "Single Empty Device" once LayoutTests are modified
+  else if (adapter == "Single Empty Device" ||
+           adapter == "SingleEmptyDeviceAdapter")
+    GetSingleEmptyDeviceAdapter(callback);
+  else if (adapter == "") {
+    if (BluetoothAdapterFactory::IsBluetoothAdapterAvailable())
+      BluetoothAdapterFactory::GetAdapter(callback);
+  } else
+    NOTREACHED();

[scheib, 2015/05/12 01:16:28]

"if one part of an if-else statement uses curly braces, the other part must
too:"
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Conditionals

[ortuno, 2015/05/12 19:33:19]

Done.

+}
+
+void LayoutTestBluetoothAdapterProvider::GetEmptyAdapter(
+    const BluetoothAdapterFactory::AdapterCallback& callback) {
+  testing::NiceMock<MockBluetoothAdapter>* mock_adapter_ =

[scheib, 2015/05/12 01:16:28]

Let's keep a scoped_refptr in this class to the adapter, but as written here
with a local variable the suffix '_' is incorrect. Later in Setup* calls instead
of passing the adapter we can just reference the member variable.

Rename mock_adapter_ to adapter_ to keep things as simple as possible, this
whole class is in the context of providing mock / fake objects.

[ortuno, 2015/05/12 19:33:20]

Done.

[Jeffrey Yasskin, 2015/05/12 22:37:49]

We shouldn't stash single-function-call objects into member variables unless
there's a reason to do it. Is there a reason here?

[scheib, 2015/05/13 04:11:05]

The reason I discussed with ortuno in person was to reduce code verbosity with
parameter passing. Everything in this class is and will be designed to
coordinate to create a set of fakes for the single data set under test. The
already dense test code seems easier to read to me to have the objects stored as
members in a single line and referenced directly as needed. This is a weak
preference though, so if you feel strongly then it's fine to marshal objects
around.

[Jeffrey Yasskin, 2015/05/13 19:40:50]

Yeah, even if it increases the size of a line or two, it's easier to read data
flow that's all local than flow that goes through class variables. When I see
the use of a member variable, it makes me want to know what else it's used for,
which requires more work.

+      new testing::NiceMock<MockBluetoothAdapter>();
+  ON_CALL(*mock_adapter_, StartDiscoverySession(testing::_, testing::_))
+      .WillByDefault(
+          testing::Invoke(this, &LayoutTestBluetoothAdapterProvider::

[scheib, 2015/05/12 01:16:28]

The LayoutTestBluetoothAdapterProvider:: is redundant I believe, as this code is
already in that name scope.

[ortuno, 2015/05/12 19:33:19]

It is not :(

+                                    SuccessfulDiscoverySessionCallback));
+  ON_CALL(*mock_adapter_, GetDevices())
+      .WillByDefault(testing::Return(BluetoothAdapter::ConstDeviceList()));
+  callback.Run(scoped_refptr<BluetoothAdapter>(mock_adapter_));

[scheib, 2015/05/12 01:16:27]

These should then be able to collapse to just callback.Run(adapter_);

[ortuno, 2015/05/12 19:33:19]

Done.

+}
+
+void LayoutTestBluetoothAdapterProvider::GetSingleEmptyDeviceAdapter(
+    const BluetoothAdapterFactory::AdapterCallback& callback) {
+  testing::NiceMock<MockBluetoothAdapter>* mock_adapter_ =
+      new testing::NiceMock<MockBluetoothAdapter>();
+  ON_CALL(*mock_adapter_, StartDiscoverySession(testing::_, testing::_))
+      .WillByDefault(
+          testing::Invoke(this, &LayoutTestBluetoothAdapterProvider::
+                                    SuccessfulDiscoverySessionCallback));
+  SetUpMockEmptyDevice(mock_adapter_);
+  BluetoothAdapter::ConstDeviceList devices;
+  devices.push_back(empty_device_.get());
+  ON_CALL(*mock_adapter_, GetDevices()).WillByDefault(testing::Return(devices));
+  callback.Run(scoped_refptr<BluetoothAdapter>(mock_adapter_));
+}
+void LayoutTestBluetoothAdapterProvider::SetUpMockEmptyDevice(
+    MockBluetoothAdapter* adapter) {
+  testing::NiceMock<MockBluetoothDevice>* empty_device =
+      new testing::NiceMock<MockBluetoothDevice>(
+          adapter, kUnspecifiedDeviceClass, "Empty Mock Device name",
+          "Empty Mock Device instanceID", true, /* Paired */
+          true /* Connected */);
+  ON_CALL(*empty_device, GetVendorIDSource())
+      .WillByDefault(testing::Return(BluetoothDevice::VENDOR_ID_BLUETOOTH));
+  ON_CALL(*empty_device, GetVendorID()).WillByDefault(testing::Return(0xFFFF));
+  ON_CALL(*empty_device, GetProductID()).WillByDefault(testing::Return(1));
+  ON_CALL(*empty_device, GetDeviceID()).WillByDefault(testing::Return(2));
+  BluetoothDevice::UUIDList list;
+  list.push_back(*uuid1.get());
+  list.push_back(*uuid2.get());
+  ON_CALL(*empty_device, GetUUIDs()).WillByDefault(testing::Return(list));
+  empty_device_.reset(empty_device);

[scheib, 2015/05/12 01:16:28]

Just set the device member at the top of this method when it is created.

[ortuno, 2015/05/12 19:33:20]

Done.

+}
+
+void LayoutTestBluetoothAdapterProvider::SuccessfulDiscoverySessionCallback(
+    const BluetoothAdapter::DiscoverySessionCallback& callback,
+    const BluetoothAdapter::ErrorCallback& error_callback) {
+  testing::NiceMock<MockBluetoothDiscoverySession>* discovery_session =
+      new testing::NiceMock<MockBluetoothDiscoverySession>();
+  ON_CALL(*discovery_session, Stop(testing::_, testing::_))
+      .WillByDefault(testing::Invoke(SuccessfulStopDiscoverySessionCallback));
+  callback.Run(scoped_ptr<BluetoothDiscoverySession>(discovery_session));
+}
+
+void LayoutTestBluetoothAdapterProvider::SuccessfulStopDiscoverySessionCallback(
+    const base::Closure& callback,
+    const base::Closure& error_callback) {
+  callback.Run();
+}
+
+}  // namespace content