BluetoothSocketCloseFunction: Fix accessing unpopulated params_.

CL d06a2579 stopped using
populating |params_| and started using local variables instead. However,
one reference to |params_| still remained that is causing null pointer
access.

Fix this and also remove all bluetooth APIs' params_ where they
are currently never used.

Add a regression (unit) test as well, along with a small sanity test.

Bug: 831651
Test: See bug for repro steps.
Change-Id: Ic2df916bb7175563e21c816890bac1c8624d334f
Reviewed-on: https://chromium-review.googlesource.com/1008065Reviewed-by: James Hawkins <jhawkins@chromium.org>
Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550019}

extensions/browser/BUILD.gn

@@ -517,6 +517,7 @@ source_set("unit_tests") {
     "api/alarms/alarms_api_unittest.cc",
     "api/api_resource_manager_unittest.cc",
     "api/bluetooth/bluetooth_event_router_unittest.cc",
+    "api/bluetooth_socket/bluetooth_socket_api_unittest.cc",
     "api/cast_channel/cast_channel_api_unittest.cc",
     "api/cast_channel/cast_channel_enum_util_unittest.cc",
     "api/declarative/declarative_rule_unittest.cc",

extensions/browser/api/bluetooth_socket/bluetooth_socket_api.cc

@@ -533,7 +533,7 @@ ExtensionFunction::ResponseAction BluetoothSocketCloseFunction::Run() {
   if (!socket)
     return RespondNow(Error(kSocketNotFoundError));
 
-  RemoveSocket(params_->socket_id);
+  RemoveSocket(params->socket_id);
   return RespondNow(ArgumentList(bluetooth_socket::Close::Results::Create()));
 }
 

extensions/browser/api/bluetooth_socket/bluetooth_socket_api.h

@@ -72,7 +72,7 @@ class BluetoothSocketCreateFunction : public BluetoothSocketAsyncApiFunction {
   ResponseAction Run() override;
 
  private:
-  std::unique_ptr<bluetooth_socket::Create::Params> params_;
+  DISALLOW_COPY_AND_ASSIGN(BluetoothSocketCreateFunction);
 };
 
 class BluetoothSocketUpdateFunction : public BluetoothSocketAsyncApiFunction {
@@ -88,7 +88,7 @@ class BluetoothSocketUpdateFunction : public BluetoothSocketAsyncApiFunction {
   ResponseAction Run() override;
 
  private:
-  std::unique_ptr<bluetooth_socket::Update::Params> params_;
+  DISALLOW_COPY_AND_ASSIGN(BluetoothSocketUpdateFunction);
 };
 
 class BluetoothSocketSetPausedFunction
@@ -106,7 +106,7 @@ class BluetoothSocketSetPausedFunction
   ResponseAction Run() override;
 
  private:
-  std::unique_ptr<bluetooth_socket::SetPaused::Params> params_;
+  DISALLOW_COPY_AND_ASSIGN(BluetoothSocketSetPausedFunction);
 };
 
 class BluetoothSocketListenFunction : public BluetoothSocketAsyncApiFunction {
@@ -258,7 +258,7 @@ class BluetoothSocketDisconnectFunction
  private:
   virtual void OnSuccess();
 
-  std::unique_ptr<bluetooth_socket::Disconnect::Params> params_;
+  DISALLOW_COPY_AND_ASSIGN(BluetoothSocketDisconnectFunction);
 };
 
 class BluetoothSocketCloseFunction : public BluetoothSocketAsyncApiFunction {
@@ -274,7 +274,7 @@ class BluetoothSocketCloseFunction : public BluetoothSocketAsyncApiFunction {
   ResponseAction Run() override;
 
  private:
-  std::unique_ptr<bluetooth_socket::Close::Params> params_;
+  DISALLOW_COPY_AND_ASSIGN(BluetoothSocketCloseFunction);
 };
 
 class BluetoothSocketSendFunction : public BluetoothSocketAsyncApiFunction {
@@ -294,9 +294,10 @@ class BluetoothSocketSendFunction : public BluetoothSocketAsyncApiFunction {
   void OnError(BluetoothApiSocket::ErrorReason reason,
                const std::string& message);
 
-  std::unique_ptr<bluetooth_socket::Send::Params> params_;
   scoped_refptr<net::IOBuffer> io_buffer_;
   size_t io_buffer_size_;
+
+  DISALLOW_COPY_AND_ASSIGN(BluetoothSocketSendFunction);
 };
 
 class BluetoothSocketGetInfoFunction : public BluetoothSocketAsyncApiFunction {
@@ -313,7 +314,7 @@ class BluetoothSocketGetInfoFunction : public BluetoothSocketAsyncApiFunction {
   ResponseAction Run() override;
 
  private:
-  std::unique_ptr<bluetooth_socket::GetInfo::Params> params_;
+  DISALLOW_COPY_AND_ASSIGN(BluetoothSocketGetInfoFunction);
 };
 
 class BluetoothSocketGetSocketsFunction

extensions/browser/api/bluetooth_socket/bluetooth_socket_api_unittest.cc

+// Copyright 2018 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 "extensions/browser/api/bluetooth_socket/bluetooth_socket_api.h"
+
+#include <memory>
+
+#include "base/strings/stringprintf.h"
+#include "extensions/browser/api_test_utils.h"
+#include "extensions/browser/api_unittest.h"
+#include "extensions/common/extension_builder.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace extensions {
+
+class BluetoothSocketApiUnittest : public ApiUnitTest {
+ public:
+  BluetoothSocketApiUnittest() = default;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BluetoothSocketApiUnittest);
+};
+
+// Tests that bluetoothSocket.create fails as expected when extension does not
+// have permission.
+TEST_F(BluetoothSocketApiUnittest, Permission) {
+  auto function = base::MakeRefCounted<api::BluetoothSocketCreateFunction>();
+  // Runs with an extension without bluetooth permission.
+  EXPECT_EQ("Permission denied",
+            RunFunctionAndReturnError(function.get(), "[]"));
+}
+
+// Tests bluetoothSocket.create() and bluetoothSocket.close().
+// Regression test for https://crbug.com/831651.
+TEST_F(BluetoothSocketApiUnittest, CreateThenClose) {
+  scoped_refptr<Extension> extension_with_socket_permitted =
+      ExtensionBuilder()
+          .SetManifest(
+              DictionaryBuilder()
+                  .Set("name", "bluetooth app")
+                  .Set("version", "1.0")
+                  .Set("bluetooth",
+                       DictionaryBuilder().SetBoolean("socket", true).Build())
+                  .Set("app",
+                       DictionaryBuilder()
+                           .Set("background",
+                                DictionaryBuilder()
+                                    .Set("scripts", ListBuilder()
+                                                        .Append("background.js")
+                                                        .Build())
+                                    .Build())
+                           .Build())
+                  .Build())
+          .SetLocation(Manifest::COMPONENT)
+          .Build();
+
+  ASSERT_TRUE(extension_with_socket_permitted);
+  set_extension(extension_with_socket_permitted);
+
+  auto create_function =
+      base::MakeRefCounted<api::BluetoothSocketCreateFunction>();
+  std::unique_ptr<base::DictionaryValue> result =
+      RunFunctionAndReturnDictionary(create_function.get(), "[]");
+  ASSERT_TRUE(result);
+
+  api::bluetooth_socket::CreateInfo create_info;
+  EXPECT_TRUE(
+      api::bluetooth_socket::CreateInfo::Populate(*result, &create_info));
+
+  const int socket_id = create_info.socket_id;
+  auto close_function =
+      base::MakeRefCounted<api::BluetoothSocketCloseFunction>();
+  RunFunction(close_function.get(), base::StringPrintf("[%d]", socket_id));
+}
+
+}  // namespace extensions