Improve Android-12 bluetooth permission error reporting

The original commit to add support for new Android-12 (API Level 31)
Bluetooth permissions covered the possible codepaths the QtBluetooth
module user might use. This commit seeks to improve the error reporting
insofar as they are due to missing permissions.

Since there are many possible codepaths to take, the following
elaborates function-by-function the details for easier understanding.

QBluetoothDeviceDiscoveryAgent::start()
This is the main entry point for using device discovery and checks the
needed permissions.

QBluetoothServiceDiscoveryAgent::start()
This is the main entry point for using service discovery. In most use
cases it uses device discovery first which ensures adequate permissions
/ failure if not granted. There is one codepath bypassing this when
setRemoteAddress() has been called, for which reason there is a
permission check.

QBluetoothSocket::localName() localAddress() connectToServiceHelper()
These functions require permissions and can be called in any order so
all of them check for permissions. Notably this commit moves the
permission check from the constructor to these functions.

QBluetoothServiceInfo::registerService()
This function creates a local device and initiates active listening.
The local device creation in this function will fail if it is not
granted permissions, but this commit adds an earlier permission check
for slightly improved error reporting.

QBluetoothServer::listen()
This is the main entry point for using the server and it checks for
permission. The local device iteration in the function would also fail
without permissions, but the permission check is added for slightly
improved error reporting. Notably the initiateActiveListening() does not
have permission check as there is no code path to it without already
having the permission.

QBluetoothLocalDevice::allDevices() constructor() setHostMode
The local device needs permissions already at construction time. This
commit documents this on the local device documentation. In addition
the local device has a widely used allDevices() static function for
iterating devices which needs its own permission checks. Also the
setHostMode() function requires Advertise permission as it ultimately
uses ACTION_REQUEST_DISCOVERABLE Intent.

QLowEnergyController::connectToDevice() startAdvertising() addService()
This commit moves the permission check from the construction time to
these individual entry functions, as the creation itself does not seem
to require permissions (tested this with a somewhat contrived setup
which dodges earlier permission checks *). The aim here is to
slightly improve the error reporting if an error is suspected to be due
to missing permissions. User may decline the first permission requests
and call subsequent functions which may or may not trigger a new
permission query. For this reason this commit also adds another
permission to startAdvertising().
*) In most if not all practical scenarios a local device and a device
discovery has been made which both perform the permission checks.

In addition the header include order was reorganized to preferred order
where this commit touches them

This commit amends a0542cff15d58db83a835f1a378a55a0ec117c9c

Task-number: QTBUG-99590
Change-Id: Ic34a0184a79f6ea7c4a6643b9603e8ddaa18e28e
Reviewed-by: Assam Boudjelthia <assam.boudjelthia@qt.io>
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
(cherry picked from commit cc121cdef160a4ef528d1483ac365ac8319a1e42)
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>

9 files changed, 110 insertions, 31 deletions

diff --git a/src/bluetooth/qbluetoothdevicediscoveryagent_android.cpp b/src/bluetooth/qbluetoothdevicediscoveryagent_android.cpp
index 788b819d..21187e5e 100644
--- a/src/bluetooth/qbluetoothdevicediscoveryagent_android.cpp
+++ b/src/bluetooth/qbluetoothdevicediscoveryagent_android.cpp
@@ -223,8 +223,8 @@ void QBluetoothDeviceDiscoveryAgentPrivate::start(QBluetoothDeviceDiscoveryAgent
 
     if (!(ensureAndroidPermission(BluetoothPermission::Scan) &&
           ensureAndroidPermission(BluetoothPermission::Connect))) {
-        errorString = QBluetoothDeviceDiscoveryAgent::tr(
-                    "Search is not possible because of missing permissions.");
+        qCWarning(QT_BT_ANDROID) << "Device discovery start() failed due to missing permissions";
+        errorString = QBluetoothDeviceDiscoveryAgent::tr("Bluetooth adapter error");
         lastError = QBluetoothDeviceDiscoveryAgent::UnknownError;
         emit q->error(lastError);
         return;
diff --git a/src/bluetooth/qbluetoothlocaldevice.cpp b/src/bluetooth/qbluetoothlocaldevice.cpp
index 36ccbb4b..7f4de876 100644
--- a/src/bluetooth/qbluetoothlocaldevice.cpp
+++ b/src/bluetooth/qbluetoothlocaldevice.cpp
@@ -137,6 +137,13 @@ QBluetoothLocalDevice::~QBluetoothLocalDevice()
     remains invalid even if the same Bluetooth adapter is returned to
     the system.
 
+//! [android-permissions-valid]
+    \note Starting from Android 12 (API level 31), the construction of this class requires
+    \l {https://developer.android.com/guide/topics/connectivity/bluetooth/permissions}
+    {bluetooth runtime permissions} (\e BLUETOOTH_SCAN and \e BLUETOOTH_CONNECT). If the
+    permissions are not granted, the device will not be valid.
+//! [android-permissions-valid]
+
     \sa allDevices()
 */
 bool QBluetoothLocalDevice::isValid() const
@@ -206,6 +213,9 @@ bool QBluetoothLocalDevice::isValid() const
 /*!
   \fn QBluetoothLocalDevice::QBluetoothLocalDevice(QObject *parent)
     Constructs a QBluetoothLocalDevice with \a parent.
+
+  \include qbluetoothlocaldevice.cpp android-permissions-valid
+  \sa isValid()
 */
 
 /*!
@@ -324,6 +334,9 @@ bool QBluetoothLocalDevice::isValid() const
 
   Construct new QBluetoothLocalDevice for \a address. If \a address is default constructed
   the resulting local device selects the local default device.
+
+  \include qbluetoothlocaldevice.cpp android-permissions-valid
+  \sa isValid()
 */
 
 QT_END_NAMESPACE
diff --git a/src/bluetooth/qbluetoothlocaldevice_android.cpp b/src/bluetooth/qbluetoothlocaldevice_android.cpp
index 002d699f..fce88838 100644
--- a/src/bluetooth/qbluetoothlocaldevice_android.cpp
+++ b/src/bluetooth/qbluetoothlocaldevice_android.cpp
@@ -112,7 +112,7 @@ void QBluetoothLocalDevicePrivate::initialize(const QBluetoothAddress &address)
 
     if (!(ensureAndroidPermission(BluetoothPermission::Scan) &&
           ensureAndroidPermission(BluetoothPermission::Connect))) {
-        qCWarning(QT_BT_ANDROID) <<  "Local device unable to get permissions.";
+        qCWarning(QT_BT_ANDROID) <<  "Local device initialize() failed due to missing permissions";
         return;
     }
 
@@ -287,6 +287,11 @@ void QBluetoothLocalDevice::setHostMode(QBluetoothLocalDevice::HostMode requeste
         }
     } else if (mode == QBluetoothLocalDevice::HostDiscoverable
                || mode == QBluetoothLocalDevice::HostDiscoverableLimitedInquiry) {
+        if (!ensureAndroidPermission(BluetoothPermission::Advertise)) {
+            qCWarning(QT_BT_ANDROID) << "Local device setHostMode() failed due to"
+                                        "missing permissions";
+            return;
+        }
         QAndroidJniObject::callStaticMethod<void>(
             "org/qtproject/qt5/android/bluetooth/QtBluetoothBroadcastReceiver", "setDiscoverable");
     }
@@ -316,7 +321,8 @@ QList<QBluetoothHostInfo> QBluetoothLocalDevice::allDevices()
 {
     // As a static class function we need to ensure permissions here (in addition to initialize())
     if (!ensureAndroidPermission(BluetoothPermission::Connect)) {
-        qCWarning(QT_BT_ANDROID) <<  "allDevices() unable to get permission.";
+        qCWarning(QT_BT_ANDROID) <<  "Local device allDevices() failed due to"
+                                     "missing permissions";
         return {};
     }
     // Android only supports max of one device (so far)
diff --git a/src/bluetooth/qbluetoothserver_android.cpp b/src/bluetooth/qbluetoothserver_android.cpp
index f749058d..ab5c19fe 100644
--- a/src/bluetooth/qbluetoothserver_android.cpp
+++ b/src/bluetooth/qbluetoothserver_android.cpp
@@ -37,15 +37,16 @@
 **
 ****************************************************************************/
 
-#include <QtCore/QLoggingCategory>
+#include "android/serveracceptancethread_p.h"
+#include "android/androidutils_p.h"
 #include "qbluetoothserver.h"
 #include "qbluetoothserver_p.h"
 #include "qbluetoothsocket.h"
 #include "qbluetoothsocket_android_p.h"
 #include "qbluetoothlocaldevice.h"
-#include "android/serveracceptancethread_p.h"
 
 #include <QCoreApplication>
+#include <QtCore/QLoggingCategory>
 
 QT_BEGIN_NAMESPACE
 
@@ -133,6 +134,13 @@ bool QBluetoothServer::listen(const QBluetoothAddress &localAdapter, quint16 por
         return false;
     }
 
+    if (!ensureAndroidPermission(BluetoothPermission::Connect)) {
+        qCWarning(QT_BT_ANDROID) << "Bluetooth server listen() failed due to missing permissions";
+        d->m_lastError = QBluetoothServer::UnknownError;
+        emit error(d->m_lastError);
+        return false;
+    }
+
     const QList<QBluetoothHostInfo> localDevices = QBluetoothLocalDevice::allDevices();
     if (!localDevices.count()) {
         qCWarning(QT_BT_ANDROID) << "Device does not support Bluetooth";
diff --git a/src/bluetooth/qbluetoothservicediscoveryagent_android.cpp b/src/bluetooth/qbluetoothservicediscoveryagent_android.cpp
index 8947dc0c..79527852 100644
--- a/src/bluetooth/qbluetoothservicediscoveryagent_android.cpp
+++ b/src/bluetooth/qbluetoothservicediscoveryagent_android.cpp
@@ -37,6 +37,13 @@
 **
 ****************************************************************************/
 
+
+#include "qbluetoothservicediscoveryagent_p.h"
+#include "qbluetoothsocket_android_p.h"
+#include "android/servicediscoverybroadcastreceiver_p.h"
+#include "android/localdevicebroadcastreceiver_p.h"
+#include "android/androidutils_p.h"
+
 #include <QtCore/qcoreapplication.h>
 #include <QtCore/QLoggingCategory>
 #include <QtCore/QTimer>
@@ -46,11 +53,6 @@
 #include <QtBluetooth/QBluetoothLocalDevice>
 #include <QtBluetooth/QBluetoothServiceDiscoveryAgent>
 
-#include "qbluetoothservicediscoveryagent_p.h"
-#include "qbluetoothsocket_android_p.h"
-#include "android/servicediscoverybroadcastreceiver_p.h"
-#include "android/localdevicebroadcastreceiver_p.h"
-
 QT_BEGIN_NAMESPACE
 
 Q_DECLARE_LOGGING_CATEGORY(QT_BT_ANDROID)
@@ -123,6 +125,15 @@ void QBluetoothServiceDiscoveryAgentPrivate::start(const QBluetoothAddress &addr
 {
     Q_Q(QBluetoothServiceDiscoveryAgent);
 
+    if (!ensureAndroidPermission(BluetoothPermission::Connect)) {
+        qCWarning(QT_BT_ANDROID) << "Service discovery start() failed due to missing permissions";
+        error = QBluetoothServiceDiscoveryAgent::UnknownError;
+        errorString = QBluetoothServiceDiscoveryAgent::tr("Unable to perform SDP scan");
+        emit q->error(error);
+        _q_serviceDiscoveryFinished();
+        return;
+    }
+
     if (!btAdapter.isValid()) {
         if (m_deviceAdapterAddress.isNull()) {
             error = QBluetoothServiceDiscoveryAgent::UnknownError;
diff --git a/src/bluetooth/qbluetoothserviceinfo_android.cpp b/src/bluetooth/qbluetoothserviceinfo_android.cpp
index 78f94137..c00b2f92 100644
--- a/src/bluetooth/qbluetoothserviceinfo_android.cpp
+++ b/src/bluetooth/qbluetoothserviceinfo_android.cpp
@@ -37,8 +37,8 @@
 **
 ****************************************************************************/
 
-#include <QtCore/QLoggingCategory>
 
+#include "android/androidutils_p.h"
 #include "qbluetoothhostinfo.h"
 #include "qbluetoothlocaldevice.h"
 #include "qbluetoothserviceinfo.h"
@@ -46,6 +46,8 @@
 #include "qbluetoothserver_p.h"
 #include "qbluetoothserver.h"
 
+#include <QtCore/QLoggingCategory>
+
 QT_BEGIN_NAMESPACE
 
 Q_DECLARE_LOGGING_CATEGORY(QT_BT_ANDROID)
@@ -89,6 +91,12 @@ bool QBluetoothServiceInfoPrivate::unregisterService()
 
 bool QBluetoothServiceInfoPrivate::registerService(const QBluetoothAddress& localAdapter)
 {
+    if (!ensureAndroidPermission(BluetoothPermission::Connect)) {
+        qCWarning(QT_BT_ANDROID) << "Serviceinfo registerService() failed due to"
+                                    "missing permissions";
+        return false;
+    }
+
     const QList<QBluetoothHostInfo> localDevices = QBluetoothLocalDevice::allDevices();
     if (!localDevices.count())
         return false; //no Bluetooth device
diff --git a/src/bluetooth/qbluetoothsocket_android.cpp b/src/bluetooth/qbluetoothsocket_android.cpp
index ed1d6373..75d075dc 100644
--- a/src/bluetooth/qbluetoothsocket_android.cpp
+++ b/src/bluetooth/qbluetoothsocket_android.cpp
@@ -193,10 +193,6 @@ QBluetoothSocketPrivateAndroid::QBluetoothSocketPrivateAndroid()
                                                         "()Landroid/bluetooth/BluetoothAdapter;");
     qRegisterMetaType<QBluetoothSocket::SocketError>();
     qRegisterMetaType<QBluetoothSocket::SocketState>();
-
-    // Many functions of this class need connect permission; request already in ctor
-    if (!ensureAndroidPermission(BluetoothPermission::Connect))
-        qCWarning(QT_BT_ANDROID) << "Bluetooth socket unable to get permission.";
 }
 
 QBluetoothSocketPrivateAndroid::~QBluetoothSocketPrivateAndroid()
@@ -412,7 +408,15 @@ void QBluetoothSocketPrivateAndroid::connectToServiceHelper(const QBluetoothAddr
 
     qCDebug(QT_BT_ANDROID) << "connectToServiceHelper()" << address.toString() << uuid.toString();
 
-    q->setSocketState(QBluetoothSocket::ConnectingState);
+    if (!ensureAndroidPermission(BluetoothPermission::Connect)) {
+        qCWarning(QT_BT_ANDROID) << "Bluetooth socket connect failed due to missing permissions";
+        errorString = QBluetoothSocket::tr("Unknown socket error");
+        q->setSocketError(QBluetoothSocket::SocketError::UnknownSocketError);
+        q->setSocketState(QBluetoothSocket::SocketState::UnconnectedState);
+        return;
+    }
+
+    q->setSocketState(QBluetoothSocket::SocketState::ConnectingState);
 
     if (!adapter.isValid()) {
         qCWarning(QT_BT_ANDROID) << "Device does not support Bluetooth";
@@ -424,7 +428,7 @@ void QBluetoothSocketPrivateAndroid::connectToServiceHelper(const QBluetoothAddr
 
     const int state = adapter.callMethod<jint>("getState");
     if (state != 12 ) { //BluetoothAdapter.STATE_ON
-        qCWarning(QT_BT_ANDROID) << "Bt device offline";
+        qCWarning(QT_BT_ANDROID) << "Bluetooth device offline";
         errorString = QBluetoothSocket::tr("Device is powered off");
         q->setSocketError(QBluetoothSocket::NetworkError);
         q->setSocketState(QBluetoothSocket::UnconnectedState);
@@ -462,7 +466,7 @@ void QBluetoothSocketPrivateAndroid::connectToServiceHelper(const QBluetoothAddr
                                                  "(Ljava/util/UUID;)Landroid/bluetooth/BluetoothSocket;",
                                                  uuidObject.object<jobject>());
     } else {
-        qCDebug(QT_BT_ANDROID) << "Connnecting via secure rfcomm";
+        qCDebug(QT_BT_ANDROID) << "Connecting via secure rfcomm";
         socketObject = remoteDevice.callObjectMethod("createRfcommSocketToServiceRecord",
                                                  "(Ljava/util/UUID;)Landroid/bluetooth/BluetoothSocket;",
                                                  uuidObject.object<jobject>());
@@ -729,8 +733,12 @@ void QBluetoothSocketPrivateAndroid::abort()
 
 QString QBluetoothSocketPrivateAndroid::localName() const
 {
-    if (adapter.isValid())
+    if (!ensureAndroidPermission(BluetoothPermission::Connect)) {
+        qCWarning(QT_BT_ANDROID) << "Bluetooth socket localName() failed due to"
+                                    "missing permissions";
+    } else if (adapter.isValid()) {
         return adapter.callObjectMethod<jstring>("getName").toString();
+    }
 
     return QString();
 }
@@ -738,8 +746,13 @@ QString QBluetoothSocketPrivateAndroid::localName() const
 QBluetoothAddress QBluetoothSocketPrivateAndroid::localAddress() const
 {
     QString result;
-    if (adapter.isValid())
+
+    if (!ensureAndroidPermission(BluetoothPermission::Connect)) {
+        qCWarning(QT_BT_ANDROID) << "Bluetooth socket localAddress() failed due to"
+                                    "missing permissions";
+    } else if (adapter.isValid()) {
         result = adapter.callObjectMethod("getAddress", "()Ljava/lang/String;").toString();
+    }
 
     return QBluetoothAddress(result);
 }
diff --git a/src/bluetooth/qlowenergycontroller.cpp b/src/bluetooth/qlowenergycontroller.cpp
index 46d92c4e..a0fbd1ca 100644
--- a/src/bluetooth/qlowenergycontroller.cpp
+++ b/src/bluetooth/qlowenergycontroller.cpp
@@ -54,6 +54,7 @@
 #include "qlowenergycontroller_bluez_p.h"
 #elif defined(QT_ANDROID_BLUETOOTH)
 #include "qlowenergycontroller_android_p.h"
+#include "android/androidutils_p.h"
 #elif defined(QT_WINRT_BLUETOOTH)
 #include "qtbluetoothglobal_p.h"
 #include "qlowenergycontroller_winrt_p.h"
@@ -74,6 +75,9 @@ QT_BEGIN_NAMESPACE
 
 Q_DECLARE_LOGGING_CATEGORY(QT_BT)
 Q_DECLARE_LOGGING_CATEGORY(QT_BT_WINRT)
+#if defined(QT_ANDROID_BLUETOOTH)
+Q_DECLARE_LOGGING_CATEGORY(QT_BT_ANDROID)
+#endif
 
 /*!
     \class QLowEnergyController
@@ -635,6 +639,7 @@ void QLowEnergyController::connectToDevice()
     }
 
     if (!d->isValidLocalAdapter()) {
+        qCWarning(QT_BT) << "connectToDevice() LE controller has invalid adapter";
         d->setError(QLowEnergyController::InvalidBluetoothAdapterError);
         return;
     }
@@ -842,6 +847,13 @@ QLowEnergyService *QLowEnergyController::addService(const QLowEnergyServiceData
         return nullptr;
     }
 
+#if defined(QT_ANDROID_BLUETOOTH)
+    if (!ensureAndroidPermission(BluetoothPermission::Connect)) {
+        qCWarning(QT_BT_ANDROID) << "addService() failed due to missing permissions";
+        return nullptr;
+    }
+#endif
+
     Q_D(QLowEnergyController);
     QLowEnergyService *newService = d->addServiceHelper(service);
     if (newService)
diff --git a/src/bluetooth/qlowenergycontroller_android.cpp b/src/bluetooth/qlowenergycontroller_android.cpp
index 2bb40e1b..a16ee7f9 100644
--- a/src/bluetooth/qlowenergycontroller_android.cpp
+++ b/src/bluetooth/qlowenergycontroller_android.cpp
@@ -93,11 +93,6 @@ void QLowEnergyControllerPrivateAndroid::init()
     const bool isPeripheral = (role == QLowEnergyController::PeripheralRole);
     const jint version = QtAndroidPrivate::androidSdkVersion();
 
-    if (!ensureAndroidPermission(BluetoothPermission::Connect)) {
-        qCWarning(QT_BT_ANDROID) << "Unable to get needed permissions";
-        return;
-    }
-
     if (isPeripheral) {
         if (version < 21) {
             qWarning() << "Qt Bluetooth LE Peripheral support not available"
@@ -150,8 +145,17 @@ void QLowEnergyControllerPrivateAndroid::init()
 
 void QLowEnergyControllerPrivateAndroid::connectToDevice()
 {
-    if (!hub)
-        return; // Android version below v18
+    if (!hub) {
+        qCCritical(QT_BT_ANDROID) << "connectToDevice() LE controller has not been initialized";
+        return;
+    }
+
+    if (!ensureAndroidPermission(BluetoothPermission::Connect)) {
+        // This is unlikely to happen as a valid local adapter is a precondition
+        setError(QLowEnergyController::AuthorizationError);
+        qCWarning(QT_BT_ANDROID) << "connectToDevice() failed due to missing permissions";
+        return;
+    }
 
     // required to pass unit test on default backend
     if (remoteDevice.isNull()) {
@@ -201,6 +205,8 @@ void QLowEnergyControllerPrivateAndroid::disconnectFromDevice()
 
 void QLowEnergyControllerPrivateAndroid::discoverServices()
 {
+    // No need to check bluetooth permissions here as 'connected' is a precondition
+
     if (hub && hub->javaObject().callMethod<jboolean>("discoverServices")) {
         qCDebug(QT_BT_ANDROID) << "Service discovery initiated";
     } else {
@@ -1019,14 +1025,16 @@ void QLowEnergyControllerPrivateAndroid::startAdvertising(const QLowEnergyAdvert
 {
     setState(QLowEnergyController::AdvertisingState);
 
-    if (!hub->javaObject().isValid()) {
-        qCWarning(QT_BT_ANDROID) << "Cannot initiate QtBluetoothLEServer";
+    if (!(ensureAndroidPermission(BluetoothPermission::Advertise) &&
+          ensureAndroidPermission(BluetoothPermission::Connect))) {
+        qCWarning(QT_BT_ANDROID) << "startAdvertising() failed due to missing permissions";
         setError(QLowEnergyController::AdvertisingError);
         setState(QLowEnergyController::UnconnectedState);
         return;
     }
 
-    if (!ensureAndroidPermission(BluetoothPermission::Advertise)) {
+    if (!hub || !hub->javaObject().isValid()) {
+        qCWarning(QT_BT_ANDROID) << "Cannot initiate QtBluetoothLEServer";
         setError(QLowEnergyController::AdvertisingError);
         setState(QLowEnergyController::UnconnectedState);
         return;