diff options
Diffstat (limited to 'chromium/services/device')
59 files changed, 900 insertions, 506 deletions
diff --git a/chromium/services/device/BUILD.gn b/chromium/services/device/BUILD.gn index d478ddab846..05d4696dc52 100644 --- a/chromium/services/device/BUILD.gn +++ b/chromium/services/device/BUILD.gn @@ -158,7 +158,6 @@ source_set("tests") { "//base/test:test_support", "//base/third_party/dynamic_annotations", "//device/base/synchronization", - "//mojo/core/embedder", "//mojo/public/cpp/bindings", "//net", "//net:test_support", diff --git a/chromium/services/device/generic_sensor/platform_sensor_and_provider_unittest_linux.cc b/chromium/services/device/generic_sensor/platform_sensor_and_provider_unittest_linux.cc index 3846c1587d8..b43c55c56b8 100644 --- a/chromium/services/device/generic_sensor/platform_sensor_and_provider_unittest_linux.cc +++ b/chromium/services/device/generic_sensor/platform_sensor_and_provider_unittest_linux.cc @@ -56,10 +56,6 @@ constexpr double kMagnetometerFrequencyValue = 7.0; constexpr double kMagnetometerOffsetValue = 3.0; constexpr double kMagnetometerScalingValue = 0.000001; -void DeleteFile(const base::FilePath& file) { - EXPECT_TRUE(base::DeleteFileRecursively(file)); -} - void WriteValueToFile(const base::FilePath& path, double value) { const std::string str = base::NumberToString(value); int bytes_written = base::WriteFile(path, str.data(), str.size()); @@ -90,9 +86,40 @@ double RoundGyroscopeValue(double value) { // to SensorDeviceManager. class MockSensorDeviceManager : public SensorDeviceManager { public: - MockSensorDeviceManager(base::WeakPtr<SensorDeviceManager::Delegate> delegate) - : SensorDeviceManager(std::move(delegate)) {} - ~MockSensorDeviceManager() override {} + ~MockSensorDeviceManager() override = default; + + // static + static std::unique_ptr<NiceMock<MockSensorDeviceManager>> Create( + base::WeakPtr<SensorDeviceManager::Delegate> delegate) { + auto device_manager = + std::make_unique<NiceMock<MockSensorDeviceManager>>(delegate); + { + base::ScopedAllowBlockingForTesting allow_blocking; + if (!device_manager->sensors_dir_.CreateUniqueTempDir()) + return nullptr; + } + + const base::FilePath& base_path = device_manager->GetSensorsBasePath(); + + ON_CALL(*device_manager, GetUdevDeviceGetSubsystem(IsNull())) + .WillByDefault(Invoke([](udev_device*) { return "iio"; })); + + ON_CALL(*device_manager, GetUdevDeviceGetSyspath(IsNull())) + .WillByDefault( + Invoke([base_path](udev_device*) { return base_path.value(); })); + + ON_CALL(*device_manager, GetUdevDeviceGetDevnode(IsNull())) + .WillByDefault(Invoke([](udev_device*) { return "/dev/test"; })); + + ON_CALL(*device_manager, GetUdevDeviceGetSysattrValue(IsNull(), _)) + .WillByDefault( + Invoke([base_path](udev_device*, const std::string& attribute) { + base::ScopedAllowBlockingForTesting allow_blocking; + return ReadValueFromFile(base_path, attribute); + })); + + return device_manager; + } MOCK_METHOD1(GetUdevDeviceGetSubsystem, std::string(udev_device*)); MOCK_METHOD1(GetUdevDeviceGetSyspath, std::string(udev_device*)); @@ -101,6 +128,10 @@ class MockSensorDeviceManager : public SensorDeviceManager { std::string(udev_device*, const std::string&)); MOCK_METHOD0(Start, void()); + const base::FilePath& GetSensorsBasePath() const { + return sensors_dir_.GetPath(); + } + void EnumerationReady() { bool success = delegate_task_runner_->PostTask( FROM_HERE, @@ -117,7 +148,14 @@ class MockSensorDeviceManager : public SensorDeviceManager { SensorDeviceManager::OnDeviceRemoved(nullptr /* unused */); } + protected: + explicit MockSensorDeviceManager( + base::WeakPtr<SensorDeviceManager::Delegate> delegate) + : SensorDeviceManager(std::move(delegate)) {} + private: + base::ScopedTempDir sensors_dir_; + DISALLOW_COPY_AND_ASSIGN(MockSensorDeviceManager); }; @@ -154,27 +192,16 @@ class PlatformSensorAndProviderLinuxTest : public ::testing::Test { public: void SetUp() override { provider_ = base::WrapUnique(new PlatformSensorProviderLinux); - - auto manager = std::make_unique<NiceMock<MockSensorDeviceManager>>( - provider_->weak_ptr_factory_.GetWeakPtr()); - manager_ = manager.get(); - provider_->SetSensorDeviceManagerForTesting(std::move(manager)); - - { - base::ScopedAllowBlockingForTesting allow_blocking; - ASSERT_TRUE(sensors_dir_.CreateUniqueTempDir()); - } + provider_->SetSensorDeviceManagerForTesting(MockSensorDeviceManager::Create( + provider_->weak_ptr_factory_.GetWeakPtr())); } - void TearDown() override { - { - base::ScopedAllowBlockingForTesting allow_blocking; - ASSERT_TRUE(sensors_dir_.Delete()); - } - base::RunLoop().RunUntilIdle(); + protected: + MockSensorDeviceManager* mock_sensor_device_manager() const { + return static_cast<MockSensorDeviceManager*>( + provider_->sensor_device_manager_.get()); } - protected: // Sensor creation is asynchronous, therefore inner loop is used to wait for // PlatformSensorProvider::CreateSensorCallback completion. scoped_refptr<PlatformSensor> CreateSensor(mojom::SensorType type) { @@ -204,7 +231,8 @@ class PlatformSensorAndProviderLinuxTest : public ::testing::Test { { base::ScopedAllowBlockingForTesting allow_blocking; - base::FilePath sensor_dir = sensors_dir_.GetPath(); + const base::FilePath& sensor_dir = + mock_sensor_device_manager()->GetSensorsBasePath(); if (!data.sensor_scale_name.empty() && scaling != 0) { base::FilePath sensor_scale_file = base::FilePath(sensor_dir).Append(data.sensor_scale_name); @@ -239,34 +267,13 @@ class PlatformSensorAndProviderLinuxTest : public ::testing::Test { } } - // Initializes mock udev methods that emulate system methods by - // just reading values from files, which SensorDeviceService has specified - // calling udev methods. - void InitializeMockUdevMethods(const base::FilePath& sensor_dir) { - ON_CALL(*manager_, GetUdevDeviceGetSubsystem(IsNull())) - .WillByDefault(Invoke([](udev_device* dev) { return "iio"; })); - - ON_CALL(*manager_, GetUdevDeviceGetSyspath(IsNull())) - .WillByDefault(Invoke( - [sensor_dir](udev_device* dev) { return sensor_dir.value(); })); - - ON_CALL(*manager_, GetUdevDeviceGetDevnode(IsNull())) - .WillByDefault(Invoke([](udev_device* dev) { return "/dev/test"; })); - - ON_CALL(*manager_, GetUdevDeviceGetSysattrValue(IsNull(), _)) - .WillByDefault(Invoke( - [sensor_dir](udev_device* dev, const std::string& attribute) { - base::ScopedAllowBlockingForTesting allow_blocking; - return ReadValueFromFile(sensor_dir, attribute); - })); - } - // Emulates device enumerations and initial udev events. Once all // devices are added, tells manager its ready. void SetServiceStart() { - EXPECT_CALL(*manager_, Start()).WillOnce(Invoke([this]() { - manager_->DeviceAdded(); - manager_->EnumerationReady(); + auto* manager = mock_sensor_device_manager(); + EXPECT_CALL(*manager, Start()).WillOnce(Invoke([manager]() { + manager->DeviceAdded(); + manager->EnumerationReady(); })); } @@ -292,8 +299,9 @@ class PlatformSensorAndProviderLinuxTest : public ::testing::Test { // been added. void GenerateDeviceAddedEvent() { bool success = provider_->blocking_task_runner_->PostTask( - FROM_HERE, base::BindOnce(&MockSensorDeviceManager::DeviceAdded, - base::Unretained(manager_))); + FROM_HERE, + base::BindOnce(&MockSensorDeviceManager::DeviceAdded, + base::Unretained(mock_sensor_device_manager()))); ASSERT_TRUE(success); // Make sure all tasks have been delivered (including SensorDeviceManager // notifying PlatformSensorProviderLinux of a device addition). @@ -305,11 +313,12 @@ class PlatformSensorAndProviderLinuxTest : public ::testing::Test { void GenerateDeviceRemovedEvent(const base::FilePath& sensor_dir) { { base::ScopedAllowBlockingForTesting allow_blocking; - DeleteFile(sensor_dir); + EXPECT_TRUE(base::DeleteFileRecursively(sensor_dir)); } bool success = provider_->blocking_task_runner_->PostTask( - FROM_HERE, base::BindOnce(&MockSensorDeviceManager::DeviceRemoved, - base::Unretained(manager_))); + FROM_HERE, + base::BindOnce(&MockSensorDeviceManager::DeviceRemoved, + base::Unretained(mock_sensor_device_manager()))); ASSERT_TRUE(success); // Make sure all tasks have been delivered (including SensorDeviceManager // notifying PlatformSensorProviderLinux of a device removal). @@ -318,10 +327,7 @@ class PlatformSensorAndProviderLinuxTest : public ::testing::Test { base::test::TaskEnvironment task_environment_; - MockSensorDeviceManager* manager_; std::unique_ptr<PlatformSensorProviderLinux> provider_; - // Holds base dir where a sensor dir is located. - base::ScopedTempDir sensors_dir_; // Used to simulate the non-test scenario where we're running in an IO thread // that forbids blocking operations. @@ -351,7 +357,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, SensorIsSupported) { double sensor_value[3] = {5}; InitializeSupportedSensor(SensorType::AMBIENT_LIGHT, kZero, kZero, kZero, sensor_value); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto sensor = CreateSensor(SensorType::AMBIENT_LIGHT); @@ -365,7 +370,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, StartFails) { double sensor_value[3] = {5}; InitializeSupportedSensor(SensorType::AMBIENT_LIGHT, kZero, kZero, kZero, sensor_value); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto sensor = CreateSensor(SensorType::AMBIENT_LIGHT); @@ -383,7 +387,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, SensorStarted) { double sensor_value[3] = {5}; InitializeSupportedSensor(SensorType::AMBIENT_LIGHT, kZero, kZero, kZero, sensor_value); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto sensor = CreateSensor(SensorType::AMBIENT_LIGHT); @@ -402,7 +405,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, SensorRemoved) { double sensor_value[3] = {1}; InitializeSupportedSensor(SensorType::AMBIENT_LIGHT, kZero, kZero, kZero, sensor_value); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto sensor = CreateSensor(SensorType::AMBIENT_LIGHT); @@ -412,7 +414,8 @@ TEST_F(PlatformSensorAndProviderLinuxTest, SensorRemoved) { std::make_unique<NiceMock<LinuxMockPlatformSensorClient>>(sensor); PlatformSensorConfiguration configuration(5); EXPECT_TRUE(sensor->StartListening(client.get(), configuration)); - GenerateDeviceRemovedEvent(sensors_dir_.GetPath()); + GenerateDeviceRemovedEvent( + mock_sensor_device_manager()->GetSensorsBasePath()); WaitOnSensorErrorEvent(client.get()); } @@ -422,7 +425,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, SensorAddedAndRemoved) { double sensor_value[3] = {1, 2, 4}; InitializeSupportedSensor(SensorType::AMBIENT_LIGHT, kZero, kZero, kZero, sensor_value); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto als_sensor = CreateSensor(SensorType::AMBIENT_LIGHT); @@ -453,7 +455,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, CheckAllSupportedSensors) { InitializeSupportedSensor( SensorType::MAGNETOMETER, kMagnetometerFrequencyValue, kMagnetometerOffsetValue, kMagnetometerScalingValue, sensor_value); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto als_sensor = CreateSensor(SensorType::AMBIENT_LIGHT); @@ -487,7 +488,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, GetMaximumSupportedFrequency) { InitializeSupportedSensor( SensorType::ACCELEROMETER, kAccelerometerFrequencyValue, kAccelerometerOffsetValue, kAccelerometerScalingValue, sensor_value); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto sensor = CreateSensor(SensorType::ACCELEROMETER); @@ -503,7 +503,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, double sensor_value[3] = {5}; InitializeSupportedSensor(SensorType::AMBIENT_LIGHT, kZero, kZero, kZero, sensor_value); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto sensor = CreateSensor(SensorType::AMBIENT_LIGHT); @@ -524,7 +523,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, CheckAmbientLightReadings) { InitializeSupportedSensor(SensorType::AMBIENT_LIGHT, kZero, kZero, kZero, sensor_value); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto sensor = CreateSensor(SensorType::AMBIENT_LIGHT); @@ -566,7 +564,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, kAccelerometerOffsetValue, kAccelerometerScalingValue, sensor_values); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto sensor = CreateSensor(SensorType::ACCELEROMETER); @@ -610,7 +607,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, // not available. TEST_F(PlatformSensorAndProviderLinuxTest, CheckLinearAccelerationSensorNotCreatedIfNoAccelerometer) { - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto sensor = CreateSensor(SensorType::LINEAR_ACCELERATION); @@ -633,7 +629,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, CheckLinearAcceleration) { kAccelerometerFrequencyValue, kZero, kZero, sensor_values); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto sensor = CreateSensor(SensorType::LINEAR_ACCELERATION); @@ -679,7 +674,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, CheckGyroscopeReadingConversion) { InitializeSupportedSensor(SensorType::GYROSCOPE, kZero, kGyroscopeOffsetValue, kGyroscopeScalingValue, sensor_values); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto sensor = CreateSensor(SensorType::GYROSCOPE); @@ -740,7 +734,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, CheckMagnetometerReadingConversion) { kMagnetometerOffsetValue, kMagnetometerScalingValue, sensor_values); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto sensor = CreateSensor(SensorType::MAGNETOMETER); @@ -784,7 +777,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, kAmbientLightFrequencyValue, kZero, kZero, sensor_value); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto sensor = CreateSensor(SensorType::AMBIENT_LIGHT); @@ -812,7 +804,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, TEST_F( PlatformSensorAndProviderLinuxTest, CheckAbsoluteOrientationSensorNotCreatedIfNoAccelerometerAndNoMagnetometer) { - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); { @@ -834,7 +825,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, InitializeSupportedSensor( SensorType::MAGNETOMETER, kMagnetometerFrequencyValue, kMagnetometerOffsetValue, kMagnetometerScalingValue, sensor_value); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); { @@ -856,7 +846,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, InitializeSupportedSensor( SensorType::ACCELEROMETER, kAccelerometerFrequencyValue, kAccelerometerOffsetValue, kAccelerometerScalingValue, sensor_value); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); { @@ -880,7 +869,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, InitializeSupportedSensor( SensorType::MAGNETOMETER, kMagnetometerFrequencyValue, kMagnetometerOffsetValue, kMagnetometerScalingValue, sensor_value); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto sensor = CreateSensor(SensorType::ABSOLUTE_ORIENTATION_EULER_ANGLES); @@ -897,7 +885,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, InitializeSupportedSensor( SensorType::MAGNETOMETER, kMagnetometerFrequencyValue, kMagnetometerOffsetValue, kMagnetometerScalingValue, sensor_value); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto sensor = CreateSensor(SensorType::ABSOLUTE_ORIENTATION_QUATERNION); @@ -909,7 +896,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, TEST_F( PlatformSensorAndProviderLinuxTest, CheckRelativeOrientationSensorNotCreatedIfNoAccelerometerAndNoGyroscope) { - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); { @@ -931,7 +917,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, InitializeSupportedSensor(SensorType::GYROSCOPE, kGyroscopeFrequencyValue, kGyroscopeOffsetValue, kGyroscopeScalingValue, sensor_value); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); { @@ -957,7 +942,6 @@ TEST_F( InitializeSupportedSensor( SensorType::ACCELEROMETER, kAccelerometerFrequencyValue, kAccelerometerOffsetValue, kAccelerometerScalingValue, sensor_value); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto sensor = CreateSensor(SensorType::RELATIVE_ORIENTATION_EULER_ANGLES); @@ -972,7 +956,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, InitializeSupportedSensor( SensorType::ACCELEROMETER, kAccelerometerFrequencyValue, kAccelerometerOffsetValue, kAccelerometerScalingValue, sensor_value); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto sensor = CreateSensor(SensorType::RELATIVE_ORIENTATION_EULER_ANGLES); @@ -990,7 +973,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, InitializeSupportedSensor( SensorType::ACCELEROMETER, kAccelerometerFrequencyValue, kAccelerometerOffsetValue, kAccelerometerScalingValue, sensor_value); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto sensor = CreateSensor(SensorType::RELATIVE_ORIENTATION_QUATERNION); @@ -1005,7 +987,6 @@ TEST_F(PlatformSensorAndProviderLinuxTest, InitializeSupportedSensor( SensorType::ACCELEROMETER, kAccelerometerFrequencyValue, kAccelerometerOffsetValue, kAccelerometerScalingValue, sensor_value); - InitializeMockUdevMethods(sensors_dir_.GetPath()); SetServiceStart(); auto sensor = CreateSensor(SensorType::RELATIVE_ORIENTATION_QUATERNION); diff --git a/chromium/services/device/generic_sensor/platform_sensor_provider_linux.cc b/chromium/services/device/generic_sensor/platform_sensor_provider_linux.cc index 1f951322af7..63e745911e9 100644 --- a/chromium/services/device/generic_sensor/platform_sensor_provider_linux.cc +++ b/chromium/services/device/generic_sensor/platform_sensor_provider_linux.cc @@ -8,7 +8,7 @@ #include <vector> #include "base/bind.h" -#include "base/memory/ref_counted.h" +#include "base/memory/scoped_refptr.h" #include "base/task/post_task.h" #include "base/task/thread_pool.h" #include "base/task_runner_util.h" @@ -60,6 +60,8 @@ void PlatformSensorProviderLinux::CreateSensorInternal( mojom::SensorType type, SensorReadingSharedBuffer* reading_buffer, CreateSensorCallback callback) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + if (!sensor_nodes_enumerated_) { if (!sensor_nodes_enumeration_started_) { // Unretained() is safe because the deletion of |sensor_device_manager_| @@ -84,20 +86,8 @@ void PlatformSensorProviderLinux::CreateSensorInternal( return; } - SensorDeviceFound(type, reading_buffer, std::move(callback), sensor_device); -} - -void PlatformSensorProviderLinux::SensorDeviceFound( - mojom::SensorType type, - SensorReadingSharedBuffer* reading_buffer, - PlatformSensorProviderBase::CreateSensorCallback callback, - const SensorInfoLinux* sensor_device) { - DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); - DCHECK(sensor_device); - - scoped_refptr<PlatformSensorLinux> sensor = - new PlatformSensorLinux(type, reading_buffer, this, sensor_device); - std::move(callback).Run(sensor); + std::move(callback).Run(base::MakeRefCounted<PlatformSensorLinux>( + type, reading_buffer, this, sensor_device)); } void PlatformSensorProviderLinux::FreeResources() { @@ -113,12 +103,6 @@ SensorInfoLinux* PlatformSensorProviderLinux::GetSensorDevice( return sensor->second.get(); } -void PlatformSensorProviderLinux::GetAllSensorDevices() { - DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); - // TODO(maksims): implement this method once we have discovery API. - NOTIMPLEMENTED(); -} - void PlatformSensorProviderLinux::SetSensorDeviceManagerForTesting( std::unique_ptr<SensorDeviceManager> sensor_device_manager) { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); diff --git a/chromium/services/device/generic_sensor/platform_sensor_provider_linux.h b/chromium/services/device/generic_sensor/platform_sensor_provider_linux.h index 4a564385269..820003f1235 100644 --- a/chromium/services/device/generic_sensor/platform_sensor_provider_linux.h +++ b/chromium/services/device/generic_sensor/platform_sensor_provider_linux.h @@ -42,21 +42,12 @@ class PlatformSensorProviderLinux : public PlatformSensorProvider, using SensorDeviceMap = std::unordered_map<mojom::SensorType, std::unique_ptr<SensorInfoLinux>>; - void SensorDeviceFound( - mojom::SensorType type, - SensorReadingSharedBuffer* reading_buffer, - PlatformSensorProviderBase::CreateSensorCallback callback, - const SensorInfoLinux* sensor_device); - // Returns SensorInfoLinux structure of a requested type. // If a request cannot be processed immediately, returns nullptr and // all the requests stored in |requests_map_| are processed after // enumeration is ready. SensorInfoLinux* GetSensorDevice(mojom::SensorType type); - // Returns all found iio devices. Currently not implemented. - void GetAllSensorDevices(); - // Processed stored requests in |request_map_|. void ProcessStoredRequests(); diff --git a/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.cc b/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.cc index fa980f8fddf..622d85f02ae 100644 --- a/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.cc +++ b/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.cc @@ -299,20 +299,6 @@ void PlatformSensorReaderWinrtBase< } } -template <wchar_t const* runtime_class_id, - class ISensorWinrtStatics, - class ISensorWinrtClass, - class ISensorReadingChangedHandler, - class ISensorReadingChangedEventArgs> -PlatformSensorReaderWinrtBase< - runtime_class_id, - ISensorWinrtStatics, - ISensorWinrtClass, - ISensorReadingChangedHandler, - ISensorReadingChangedEventArgs>::~PlatformSensorReaderWinrtBase() { - StopSensor(); -} - // static std::unique_ptr<PlatformSensorReaderWinBase> PlatformSensorReaderWinrtLightSensor::Create() { diff --git a/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.h b/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.h index cae0f6504be..0abc1c18628 100644 --- a/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.h +++ b/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.h @@ -74,7 +74,7 @@ class PlatformSensorReaderWinrtBase : public PlatformSensorReaderWinBase { protected: PlatformSensorReaderWinrtBase(); - virtual ~PlatformSensorReaderWinrtBase(); + virtual ~PlatformSensorReaderWinrtBase() { StopSensor(); } // Derived classes should implement this function to handle sensor specific // parsing of the sensor reading. diff --git a/chromium/services/device/geolocation/BUILD.gn b/chromium/services/device/geolocation/BUILD.gn index 45e6296d458..bc366b81794 100644 --- a/chromium/services/device/geolocation/BUILD.gn +++ b/chromium/services/device/geolocation/BUILD.gn @@ -70,8 +70,8 @@ source_set("geolocation") { deps = [ "//base", - "//mojo/core/embedder", "//mojo/public/cpp/bindings", + "//mojo/public/cpp/system", "//net", "//ui/gfx", ] diff --git a/chromium/services/device/geolocation/geolocation_context.cc b/chromium/services/device/geolocation/geolocation_context.cc index 379b7651ea6..a41139ad815 100644 --- a/chromium/services/device/geolocation/geolocation_context.cc +++ b/chromium/services/device/geolocation/geolocation_context.cc @@ -24,7 +24,8 @@ void GeolocationContext::Create( } void GeolocationContext::BindGeolocation( - mojo::PendingReceiver<mojom::Geolocation> receiver) { + mojo::PendingReceiver<mojom::Geolocation> receiver, + const GURL& requesting_origin) { GeolocationImpl* impl = new GeolocationImpl(std::move(receiver), this); impls_.push_back(base::WrapUnique<GeolocationImpl>(impl)); if (geoposition_override_) diff --git a/chromium/services/device/geolocation/geolocation_context.h b/chromium/services/device/geolocation/geolocation_context.h index d5863794b0f..b7c65a67a68 100644 --- a/chromium/services/device/geolocation/geolocation_context.h +++ b/chromium/services/device/geolocation/geolocation_context.h @@ -30,8 +30,8 @@ class GeolocationContext : public mojom::GeolocationContext { static void Create(mojo::PendingReceiver<mojom::GeolocationContext> receiver); // mojom::GeolocationContext implementation: - void BindGeolocation( - mojo::PendingReceiver<mojom::Geolocation> receiver) override; + void BindGeolocation(mojo::PendingReceiver<mojom::Geolocation> receiver, + const GURL& requesting_origin) override; void SetOverride(mojom::GeopositionPtr geoposition) override; void ClearOverride() override; diff --git a/chromium/services/device/geolocation/geolocation_service_unittest.cc b/chromium/services/device/geolocation/geolocation_service_unittest.cc index 52ebfee8e3a..70547aab16b 100644 --- a/chromium/services/device/geolocation/geolocation_service_unittest.cc +++ b/chromium/services/device/geolocation/geolocation_service_unittest.cc @@ -54,7 +54,7 @@ class GeolocationServiceUnitTest : public DeviceServiceTestBase { device_service()->BindGeolocationContext( geolocation_context_.BindNewPipeAndPassReceiver()); geolocation_context_->BindGeolocation( - geolocation_.BindNewPipeAndPassReceiver()); + geolocation_.BindNewPipeAndPassReceiver(), GURL::EmptyGURL()); } void TearDown() override { diff --git a/chromium/services/device/geolocation/public_ip_address_geolocator_unittest.cc b/chromium/services/device/geolocation/public_ip_address_geolocator_unittest.cc index 258ce53af1a..d6e7b4ddc15 100644 --- a/chromium/services/device/geolocation/public_ip_address_geolocator_unittest.cc +++ b/chromium/services/device/geolocation/public_ip_address_geolocator_unittest.cc @@ -9,9 +9,9 @@ #include "base/run_loop.h" #include "base/strings/string_util.h" #include "base/test/task_environment.h" -#include "mojo/core/embedder/embedder.h" #include "mojo/public/cpp/bindings/remote.h" #include "mojo/public/cpp/bindings/unique_receiver_set.h" +#include "mojo/public/cpp/system/functions.h" #include "net/traffic_annotation/network_traffic_annotation_test_helper.h" #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h" #include "services/network/test/test_network_connection_tracker.h" @@ -42,7 +42,7 @@ class PublicIpAddressGeolocatorTest : public testing::Test { protected: void SetUp() override { // Intercept Mojo bad-message errors. - mojo::core::SetDefaultProcessErrorCallback( + mojo::SetDefaultProcessErrorHandler( base::BindRepeating(&PublicIpAddressGeolocatorTest::OnMojoBadMessage, base::Unretained(this))); @@ -57,8 +57,7 @@ class PublicIpAddressGeolocatorTest : public testing::Test { void TearDown() override { // Stop intercepting Mojo bad-message errors. - mojo::core::SetDefaultProcessErrorCallback( - mojo::core::ProcessErrorCallback()); + mojo::SetDefaultProcessErrorHandler(base::NullCallback()); } // Deal with mojo bad message. diff --git a/chromium/services/device/geolocation/wifi_data_provider_chromeos.cc b/chromium/services/device/geolocation/wifi_data_provider_chromeos.cc index 6698a7f6588..37bc1d54eae 100644 --- a/chromium/services/device/geolocation/wifi_data_provider_chromeos.cc +++ b/chromium/services/device/geolocation/wifi_data_provider_chromeos.cc @@ -9,6 +9,7 @@ #include <stdint.h> #include "base/bind.h" +#include "base/logging.h" #include "base/strings/utf_string_conversions.h" #include "chromeos/network/geolocation_handler.h" #include "chromeos/network/network_handler.h" diff --git a/chromium/services/device/geolocation/wifi_data_provider_common.h b/chromium/services/device/geolocation/wifi_data_provider_common.h index 19a168eb4c6..2590e0a83ae 100644 --- a/chromium/services/device/geolocation/wifi_data_provider_common.h +++ b/chromium/services/device/geolocation/wifi_data_provider_common.h @@ -10,7 +10,6 @@ #include <memory> -#include "base/logging.h" #include "base/macros.h" #include "base/memory/weak_ptr.h" #include "base/strings/string16.h" diff --git a/chromium/services/device/geolocation/wifi_data_provider_linux.cc b/chromium/services/device/geolocation/wifi_data_provider_linux.cc index 549df44c578..ffb0cb08820 100644 --- a/chromium/services/device/geolocation/wifi_data_provider_linux.cc +++ b/chromium/services/device/geolocation/wifi_data_provider_linux.cc @@ -16,6 +16,7 @@ #include <utility> #include <vector> +#include "base/logging.h" #include "base/macros.h" #include "base/strings/string_number_conversions.h" #include "base/strings/utf_string_conversions.h" diff --git a/chromium/services/device/geolocation/wifi_data_provider_linux_unittest.cc b/chromium/services/device/geolocation/wifi_data_provider_linux_unittest.cc index af08db71f86..0552077b043 100644 --- a/chromium/services/device/geolocation/wifi_data_provider_linux_unittest.cc +++ b/chromium/services/device/geolocation/wifi_data_provider_linux_unittest.cc @@ -8,6 +8,7 @@ #include <memory> +#include "base/logging.h" #include "base/memory/ref_counted.h" #include "base/stl_util.h" #include "base/strings/utf_string_conversions.h" diff --git a/chromium/services/device/geolocation/wifi_data_provider_mac.mm b/chromium/services/device/geolocation/wifi_data_provider_mac.mm index 49cd5870f26..5764adf247c 100644 --- a/chromium/services/device/geolocation/wifi_data_provider_mac.mm +++ b/chromium/services/device/geolocation/wifi_data_provider_mac.mm @@ -7,6 +7,7 @@ #import <CoreWLAN/CoreWLAN.h> #import <Foundation/Foundation.h> +#include "base/logging.h" #include "base/mac/scoped_nsobject.h" #include "base/macros.h" #include "base/memory/ptr_util.h" diff --git a/chromium/services/device/geolocation/wifi_data_provider_win.cc b/chromium/services/device/geolocation/wifi_data_provider_win.cc index 0dd3a5f8875..e383a590996 100644 --- a/chromium/services/device/geolocation/wifi_data_provider_win.cc +++ b/chromium/services/device/geolocation/wifi_data_provider_win.cc @@ -8,6 +8,7 @@ #include <winioctl.h> #include <wlanapi.h> +#include "base/logging.h" #include "base/memory/free_deleter.h" #include "base/memory/ptr_util.h" #include "base/metrics/histogram_macros.h" diff --git a/chromium/services/device/hid/hid_service.h b/chromium/services/device/hid/hid_service.h index 1db1f5fbe51..49a49dd7ae9 100644 --- a/chromium/services/device/hid/hid_service.h +++ b/chromium/services/device/hid/hid_service.h @@ -11,7 +11,7 @@ #include <vector> #include "base/bind_helpers.h" -#include "base/logging.h" +#include "base/check.h" #include "base/macros.h" #include "base/memory/ref_counted.h" #include "base/memory/weak_ptr.h" diff --git a/chromium/services/device/hid/hid_service_linux.cc b/chromium/services/device/hid/hid_service_linux.cc index 55c636faf1e..28bf7f8641f 100644 --- a/chromium/services/device/hid/hid_service_linux.cc +++ b/chromium/services/device/hid/hid_service_linux.cc @@ -5,6 +5,7 @@ #include "services/device/hid/hid_service_linux.h" #include <fcntl.h> +#include <linux/input.h> #include <stdint.h> #include <limits> @@ -24,6 +25,7 @@ #include "base/sequenced_task_runner.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_split.h" +#include "base/strings/string_util.h" #include "base/task/thread_pool.h" #include "base/threading/scoped_blocking_call.h" #include "base/threading/sequenced_task_runner_handle.h" @@ -42,11 +44,132 @@ namespace device { namespace { -const char kHidrawSubsystem[] = "hidraw"; +const char kDevtypeUsbDevice[] = "usb_device"; +const char kSubsystemBluetooth[] = "bluetooth"; +const char kSubsystemHid[] = "hid"; +const char kSubsystemHidraw[] = "hidraw"; +const char kSubsystemUsb[] = "usb"; const char kHIDID[] = "HID_ID"; const char kHIDName[] = "HID_NAME"; const char kHIDUnique[] = "HID_UNIQ"; const char kSysfsReportDescriptorKey[] = "report_descriptor"; +const char kKernelHciPrefix[] = "hci"; + +// Walks up the sysfs device tree starting at |device| and returns the first +// ancestor in the "hid" subsystem. Returns nullptr on failure. +udev_device* FindFirstHidAncestor(udev_device* device) { + udev_device* ancestor = device; + do { + const char* subsystem = udev_device_get_subsystem(ancestor); + if (!subsystem) + return nullptr; + if (strcmp(subsystem, kSubsystemHid) == 0) + return ancestor; + } while ((ancestor = udev_device_get_parent(ancestor))); + return nullptr; +} + +// Walks up the sysfs device tree starting at |device| and returns the first +// ancestor not in the "hid" or "hidraw" subsystems. Returns nullptr on failure. +udev_device* FindFirstNonHidAncestor(udev_device* device) { + udev_device* ancestor = device; + do { + const char* subsystem = udev_device_get_subsystem(ancestor); + if (!subsystem) + return nullptr; + if (strcmp(subsystem, kSubsystemHid) != 0 && + strcmp(subsystem, kSubsystemHidraw) != 0) { + return ancestor; + } + } while ((ancestor = udev_device_get_parent(ancestor))); + return nullptr; +} + +// Returns the sysfs path for a USB device |usb_device|, or nullptr if the sysfs +// path could not be retrieved. |usb_device| must be a device in the "usb" +// subsystem. +// +// Some USB devices expose multiple interfaces. If |usb_device| refers to a +// single USB interface, walk up the device tree to find the ancestor that +// represents the physical device. +const char* GetUsbDeviceSyspath(udev_device* usb_device) { + do { + const char* subsystem = udev_device_get_subsystem(usb_device); + if (!subsystem || strcmp(subsystem, kSubsystemUsb) != 0) + return nullptr; + + const char* devtype = udev_device_get_devtype(usb_device); + if (!devtype) + return nullptr; + + // Use the syspath of the first ancestor with devtype "usb_device". + if (strcmp(devtype, kDevtypeUsbDevice) == 0) + return udev_device_get_syspath(usb_device); + } while ((usb_device = udev_device_get_parent(usb_device))); + return nullptr; +} + +// Returns the sysfs path for a Bluetooth Classic device |bt_device|, or nullptr +// if the sysfs path could not be retrieved. |bt_device| must be a device in the +// "bluetooth" subsystem. +const char* GetBluetoothDeviceSyspath(udev_device* bt_device) { + do { + const char* subsystem = udev_device_get_subsystem(bt_device); + if (!subsystem || strcmp(subsystem, kSubsystemBluetooth) != 0) + return nullptr; + + // Look for a sysname like "hci0:123". + const char* sysfs_name = udev_device_get_sysname(bt_device); + if (!sysfs_name) + return nullptr; + + std::vector<std::string> parts = base::SplitString( + sysfs_name, ":", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL); + if (parts.size() == 2 && base::StartsWith(parts[0], kKernelHciPrefix, + base::CompareCase::SENSITIVE)) { + return udev_device_get_syspath(bt_device); + } + } while ((bt_device = udev_device_get_parent(bt_device))); + return nullptr; +} + +// Returns the physical device ID for a device |hidraw_device|. On Linux, the +// physical device ID is the sysfs path to the device node that represents the +// physical device if it is available. When the physical device node is not +// available, the sysfs path of the HID interface is returned instead. Returns +// nullptr on failure. +const char* GetPhysicalDeviceId(udev_device* hidraw_device) { + const char* subsystem = udev_device_get_subsystem(hidraw_device); + if (!subsystem || strcmp(subsystem, kSubsystemHidraw) != 0) + return nullptr; + + udev_device* hid_ancestor = FindFirstHidAncestor(hidraw_device); + if (!hid_ancestor) + return nullptr; + const char* hid_sysfs_path = udev_device_get_syspath(hid_ancestor); + + udev_device* ancestor = FindFirstNonHidAncestor(hid_ancestor); + if (!ancestor) + return hid_sysfs_path; + + const char* ancestor_subsystem = udev_device_get_subsystem(ancestor); + if (!ancestor_subsystem) + return hid_sysfs_path; + + if (strcmp(ancestor_subsystem, kSubsystemUsb) == 0) { + const char* usb_sysfs_path = GetUsbDeviceSyspath(ancestor); + if (usb_sysfs_path) + return usb_sysfs_path; + } + + if (strcmp(ancestor_subsystem, kSubsystemBluetooth) == 0) { + const char* bt_sysfs_path = GetBluetoothDeviceSyspath(ancestor); + if (bt_sysfs_path) + return bt_sysfs_path; + } + + return hid_sysfs_path; +} } // namespace @@ -82,7 +205,8 @@ class HidServiceLinux::BlockingTaskRunnerHelper : public UdevWatcher::Observer { void Start() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - watcher_ = UdevWatcher::StartWatching(this); + watcher_ = UdevWatcher::StartWatching( + this, {UdevWatcher::Filter(kSubsystemHidraw, "")}); watcher_->EnumerateExistingDevices(); task_runner_->PostTask( FROM_HERE, @@ -101,9 +225,11 @@ class HidServiceLinux::BlockingTaskRunnerHelper : public UdevWatcher::Observer { return; HidPlatformDeviceId platform_device_id = device_path; +#if DCHECK_IS_ON() const char* subsystem = udev_device_get_subsystem(device.get()); - if (!subsystem || strcmp(subsystem, kHidrawSubsystem) != 0) - return; + DCHECK(subsystem); + DCHECK_EQ(base::StringPiece(subsystem), kSubsystemHidraw); +#endif const char* str_property = udev_device_get_devnode(device.get()); if (!str_property) @@ -155,14 +281,21 @@ class HidServiceLinux::BlockingTaskRunnerHelper : public UdevWatcher::Observer { if (!base::ReadFileToString(report_descriptor_path, &report_descriptor_str)) return; - scoped_refptr<HidDeviceInfo> device_info( - new HidDeviceInfo(platform_device_id, /*physical_device_id=*/"", - vendor_id, product_id, product_name, serial_number, - // TODO(reillyg): Detect Bluetooth. crbug.com/443335 - mojom::HidBusType::kHIDBusTypeUSB, - std::vector<uint8_t>(report_descriptor_str.begin(), - report_descriptor_str.end()), - device_node)); + const char* physical_device_id = GetPhysicalDeviceId(device.get()); + if (!physical_device_id) { + HID_LOG(EVENT) << "GetPhysicalDeviceId failed for '" << device_path + << "'"; + return; + } + + auto device_info = base::MakeRefCounted<HidDeviceInfo>( + platform_device_id, physical_device_id, vendor_id, product_id, + product_name, serial_number, + // TODO(reillyg): Detect Bluetooth. crbug.com/443335 + mojom::HidBusType::kHIDBusTypeUSB, + std::vector<uint8_t>(report_descriptor_str.begin(), + report_descriptor_str.end()), + device_node); task_runner_->PostTask( FROM_HERE, diff --git a/chromium/services/device/hid/hid_service_mac.cc b/chromium/services/device/hid/hid_service_mac.cc index f65c8afda57..5ef38b1e0ba 100644 --- a/chromium/services/device/hid/hid_service_mac.cc +++ b/chromium/services/device/hid/hid_service_mac.cc @@ -16,6 +16,7 @@ #include "base/logging.h" #include "base/mac/foundation_util.h" #include "base/stl_util.h" +#include "base/strings/string_number_conversions.h" #include "base/strings/stringprintf.h" #include "base/strings/sys_string_conversions.h" #include "base/task/thread_pool.h" @@ -78,8 +79,11 @@ scoped_refptr<HidDeviceInfo> CreateDeviceInfo( HID_LOG(DEBUG) << "Device report descriptor not available."; } + int32_t location_id = GetIntProperty(service, CFSTR(kIOHIDLocationIDKey)); + std::string physical_device_id = base::NumberToString(location_id); + return new HidDeviceInfo( - entry_id, /*physical_device_id=*/"", + entry_id, physical_device_id, GetIntProperty(service, CFSTR(kIOHIDVendorIDKey)), GetIntProperty(service, CFSTR(kIOHIDProductIDKey)), GetStringProperty(service, CFSTR(kIOHIDProductKey)), diff --git a/chromium/services/device/hid/hid_service_win.cc b/chromium/services/device/hid/hid_service_win.cc index b9a5e7a3ec6..80fff2d39a8 100644 --- a/chromium/services/device/hid/hid_service_win.cc +++ b/chromium/services/device/hid/hid_service_win.cc @@ -4,15 +4,16 @@ #include "services/device/hid/hid_service_win.h" -#include <memory> - #define INITGUID #include <dbt.h> +#include <devpkey.h> #include <setupapi.h> #include <stddef.h> +#include <wdmguid.h> #include <winioctl.h> +#include <memory> #include <utility> #include "base/bind.h" @@ -22,15 +23,112 @@ #include "base/memory/free_deleter.h" #include "base/sequenced_task_runner.h" #include "base/strings/string_util.h" -#include "base/strings/sys_string_conversions.h" +#include "base/strings/utf_string_conversions.h" #include "base/task/thread_pool.h" #include "base/threading/sequenced_task_runner_handle.h" +#include "base/win/scoped_devinfo.h" +#include "base/win/win_util.h" #include "components/device_event_log/device_event_log.h" #include "services/device/hid/hid_connection_win.h" #include "services/device/hid/hid_device_info.h" namespace device { +namespace { + +// Looks up the value of a GUID-type device property specified by |property| for +// the device described by |device_info_data|. On success, returns true and sets +// |property_buffer| to the property value. Returns false if the property is not +// present or has a different type. +bool GetDeviceGuidProperty(HDEVINFO device_info_set, + SP_DEVINFO_DATA& device_info_data, + const DEVPROPKEY& property, + GUID* property_buffer) { + DEVPROPTYPE property_type; + if (!SetupDiGetDeviceProperty( + device_info_set, &device_info_data, &property, &property_type, + reinterpret_cast<PBYTE>(property_buffer), sizeof(*property_buffer), + /*RequiredSize=*/nullptr, /*Flags=*/0) || + property_type != DEVPROP_TYPE_GUID) { + return false; + } + return true; +} + +// Looks up information about the device described by |device_interface_data| +// in |device_info_set|. On success, returns true and sets |device_info_data| +// and |device_path|. Returns false if an error occurred. +bool GetDeviceInfoAndPathFromInterface( + HDEVINFO device_info_set, + SP_DEVICE_INTERFACE_DATA& device_interface_data, + SP_DEVINFO_DATA* device_info_data, + base::string16* device_path) { + // Get the required buffer size. When called with + // DeviceInterfaceDetailData == nullptr and DeviceInterfaceDetailSize == 0, + // SetupDiGetDeviceInterfaceDetail returns the required buffer size at + // RequiredSize and fails with GetLastError() == ERROR_INSUFFICIENT_BUFFER. + DWORD required_size; + if (SetupDiGetDeviceInterfaceDetail(device_info_set, &device_interface_data, + /*DeviceInterfaceDetailData=*/nullptr, + /*DeviceInterfaceDetailSize=*/0, + &required_size, + /*DeviceInfoData=*/nullptr) || + GetLastError() != ERROR_INSUFFICIENT_BUFFER) { + return false; + } + + std::unique_ptr<SP_DEVICE_INTERFACE_DETAIL_DATA, base::FreeDeleter> + device_interface_detail_data( + static_cast<SP_DEVICE_INTERFACE_DETAIL_DATA*>(malloc(required_size))); + device_interface_detail_data->cbSize = sizeof(*device_interface_detail_data); + + // Call the function again with the correct buffer size to get the detailed + // data for this device. + if (!SetupDiGetDeviceInterfaceDetail(device_info_set, &device_interface_data, + device_interface_detail_data.get(), + required_size, /*RequiredSize=*/nullptr, + device_info_data)) { + return false; + } + + // Windows uses case-insensitive paths and may return paths that differ only + // by case. Canonicalize the device path by converting to lowercase. + base::string16 path = + base::string16(device_interface_detail_data->DevicePath); + DCHECK(base::IsStringASCII(path)); + *device_path = base::ToLowerASCII(path); + return true; +} + +// Returns a device info set containing only the device described by +// |device_path|, or an invalid ScopedDevInfo if there was an error while +// creating the device set. The device info is returned in |device_info_data|. +base::win::ScopedDevInfo GetDeviceInfoFromPath( + const base::string16& device_path, + SP_DEVINFO_DATA* device_info_data) { + base::win::ScopedDevInfo device_info_set(SetupDiGetClassDevs( + &GUID_DEVINTERFACE_HID, /*Enumerator=*/nullptr, + /*hwndParent=*/0, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT)); + if (!device_info_set.is_valid()) + return base::win::ScopedDevInfo(); + + SP_DEVICE_INTERFACE_DATA device_interface_data; + device_interface_data.cbSize = sizeof(device_interface_data); + if (!SetupDiOpenDeviceInterface(device_info_set.get(), device_path.c_str(), + /*OpenFlags=*/0, &device_interface_data)) { + return base::win::ScopedDevInfo(); + } + + base::string16 intf_device_path; + GetDeviceInfoAndPathFromInterface(device_info_set.get(), + device_interface_data, device_info_data, + &intf_device_path); + DCHECK_EQ(intf_device_path, device_path); + return device_info_set; +} + +} // namespace + HidServiceWin::HidServiceWin() : task_runner_(base::SequencedTaskRunnerHandle::Get()), blocking_task_runner_( @@ -81,41 +179,37 @@ base::WeakPtr<HidService> HidServiceWin::GetWeakPtr() { void HidServiceWin::EnumerateBlocking( base::WeakPtr<HidServiceWin> service, scoped_refptr<base::SequencedTaskRunner> task_runner) { - HDEVINFO device_info_set = - SetupDiGetClassDevs(&GUID_DEVINTERFACE_HID, NULL, NULL, - DIGCF_PRESENT | DIGCF_DEVICEINTERFACE); + base::win::ScopedDevInfo dev_info(SetupDiGetClassDevs( + &GUID_DEVINTERFACE_HID, /*Enumerator=*/nullptr, + /*hwndParent=*/nullptr, DIGCF_PRESENT | DIGCF_DEVICEINTERFACE)); - if (device_info_set != INVALID_HANDLE_VALUE) { - SP_DEVICE_INTERFACE_DATA device_interface_data; - device_interface_data.cbSize = sizeof(SP_DEVICE_INTERFACE_DATA); + if (dev_info.is_valid()) { + SP_DEVICE_INTERFACE_DATA device_interface_data = {0}; + device_interface_data.cbSize = sizeof(device_interface_data); for (int device_index = 0; SetupDiEnumDeviceInterfaces( - device_info_set, NULL, &GUID_DEVINTERFACE_HID, device_index, - &device_interface_data); + dev_info.get(), /*DeviceInfoData=*/nullptr, &GUID_DEVINTERFACE_HID, + device_index, &device_interface_data); ++device_index) { - DWORD required_size = 0; - - // Determime the required size of detail struct. - SetupDiGetDeviceInterfaceDetail(device_info_set, &device_interface_data, - NULL, 0, &required_size, NULL); + SP_DEVINFO_DATA dev_info_data = {0}; + dev_info_data.cbSize = sizeof(dev_info_data); + base::string16 device_path; + if (!GetDeviceInfoAndPathFromInterface(dev_info.get(), + device_interface_data, + &dev_info_data, &device_path)) { + continue; + } - std::unique_ptr<SP_DEVICE_INTERFACE_DETAIL_DATA, base::FreeDeleter> - device_interface_detail_data( - static_cast<SP_DEVICE_INTERFACE_DETAIL_DATA*>(malloc(required_size))); - device_interface_detail_data->cbSize = - sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA); - - // Get the detailed data for this device. - BOOL res = SetupDiGetDeviceInterfaceDetail( - device_info_set, &device_interface_data, - device_interface_detail_data.get(), required_size, NULL, NULL); - if (!res) { + // Get the container ID for the physical device. + GUID container_id; + if (!GetDeviceGuidProperty(dev_info.get(), dev_info_data, + DEVPKEY_Device_ContainerId, &container_id)) { continue; } + std::string physical_device_id = + base::UTF16ToUTF8(base::win::String16FromGUID(container_id)); - base::string16 device_path(device_interface_detail_data->DevicePath); - DCHECK(base::IsStringASCII(device_path)); - AddDeviceBlocking(service, task_runner, base::ToLowerASCII(device_path)); + AddDeviceBlocking(service, task_runner, device_path, physical_device_id); } } @@ -170,14 +264,15 @@ void HidServiceWin::CollectInfoFromValueCaps( void HidServiceWin::AddDeviceBlocking( base::WeakPtr<HidServiceWin> service, scoped_refptr<base::SequencedTaskRunner> task_runner, - const base::string16& device_path) { + const base::string16& device_path, + const std::string& physical_device_id) { base::win::ScopedHandle device_handle(OpenDevice(device_path)); if (!device_handle.IsValid()) { return; } HIDD_ATTRIBUTES attrib = {0}; - attrib.Size = sizeof(HIDD_ATTRIBUTES); + attrib.Size = sizeof(attrib); if (!HidD_GetAttributes(device_handle.Get(), &attrib)) { HID_LOG(EVENT) << "Failed to get device attributes."; return; @@ -237,24 +332,24 @@ void HidServiceWin::AddDeviceBlocking( // 1023 characters plus NULL terminator is more than enough for a USB string // descriptor which is limited to 126 characters. - wchar_t buffer[1024]; + base::char16 buffer[1024]; std::string product_name; if (HidD_GetProductString(device_handle.Get(), &buffer[0], sizeof(buffer))) { // NULL termination guaranteed by the API. - product_name = base::SysWideToUTF8(buffer); + product_name = base::UTF16ToUTF8(buffer); } std::string serial_number; if (HidD_GetSerialNumberString(device_handle.Get(), &buffer[0], sizeof(buffer))) { // NULL termination guaranteed by the API. - serial_number = base::SysWideToUTF8(buffer); + serial_number = base::UTF16ToUTF8(buffer); } // This populates the HidDeviceInfo instance without a raw report descriptor. // The descriptor is unavailable on Windows because HID devices are exposed to // user-space as individual top-level collections. scoped_refptr<HidDeviceInfo> device_info(new HidDeviceInfo( - device_path, /*physical_device_id=*/"", attrib.VendorID, attrib.ProductID, + device_path, physical_device_id, attrib.VendorID, attrib.ProductID, product_name, serial_number, // TODO(reillyg): Detect Bluetooth. crbug.com/443335 mojom::HidBusType::kHIDBusTypeUSB, std::move(collection_info), @@ -267,10 +362,24 @@ void HidServiceWin::AddDeviceBlocking( void HidServiceWin::OnDeviceAdded(const GUID& class_guid, const base::string16& device_path) { + SP_DEVINFO_DATA device_info_data = {0}; + device_info_data.cbSize = sizeof(device_info_data); + auto device_info_set = GetDeviceInfoFromPath(device_path, &device_info_data); + if (!device_info_set.is_valid()) + return; + + GUID container_id; + if (!GetDeviceGuidProperty(device_info_set.get(), device_info_data, + DEVPKEY_Device_ContainerId, &container_id)) { + return; + } + std::string physical_device_id = + base::UTF16ToUTF8(base::win::String16FromGUID(container_id)); + blocking_task_runner_->PostTask( - FROM_HERE, - base::BindOnce(&HidServiceWin::AddDeviceBlocking, - weak_factory_.GetWeakPtr(), task_runner_, device_path)); + FROM_HERE, base::BindOnce(&HidServiceWin::AddDeviceBlocking, + weak_factory_.GetWeakPtr(), task_runner_, + device_path, physical_device_id)); } void HidServiceWin::OnDeviceRemoved(const GUID& class_guid, diff --git a/chromium/services/device/hid/hid_service_win.h b/chromium/services/device/hid/hid_service_win.h index 4965888e989..ac6745f3aaf 100644 --- a/chromium/services/device/hid/hid_service_win.h +++ b/chromium/services/device/hid/hid_service_win.h @@ -59,7 +59,8 @@ class HidServiceWin : public HidService, public DeviceMonitorWin::Observer { static void AddDeviceBlocking( base::WeakPtr<HidServiceWin> service, scoped_refptr<base::SequencedTaskRunner> task_runner, - const base::string16& device_path); + const base::string16& device_path, + const std::string& physical_device_id); // DeviceMonitorWin::Observer implementation: void OnDeviceAdded(const GUID& class_guid, diff --git a/chromium/services/device/media_transfer_protocol/mtp_device_manager.h b/chromium/services/device/media_transfer_protocol/mtp_device_manager.h index 67991e5272c..5fcd34093f2 100644 --- a/chromium/services/device/media_transfer_protocol/mtp_device_manager.h +++ b/chromium/services/device/media_transfer_protocol/mtp_device_manager.h @@ -13,7 +13,6 @@ #include "base/containers/flat_map.h" #include "base/containers/flat_set.h" #include "base/containers/queue.h" -#include "base/logging.h" #include "base/memory/weak_ptr.h" #include "base/sequenced_task_runner.h" #include "base/threading/thread_checker.h" diff --git a/chromium/services/device/nfc/android/junit/src/org/chromium/device/nfc/NfcBlocklistTest.java b/chromium/services/device/nfc/android/junit/src/org/chromium/device/nfc/NfcBlocklistTest.java index a94863557f0..be7b36f3e0a 100644 --- a/chromium/services/device/nfc/android/junit/src/org/chromium/device/nfc/NfcBlocklistTest.java +++ b/chromium/services/device/nfc/android/junit/src/org/chromium/device/nfc/NfcBlocklistTest.java @@ -18,7 +18,7 @@ import org.chromium.base.test.util.Feature; * Unit tests for the {@link NfcBlocklist} class. */ @RunWith(BaseRobolectricTestRunner.class) -@Config(manifest = Config.NONE) +@Config(sdk = 21, manifest = Config.NONE) public class NfcBlocklistTest { // Static historical bytes private static final byte[] YUBIKEY_NEO_HISTORICAL_BYTES = new byte[] { @@ -87,4 +87,4 @@ public class NfcBlocklistTest { assertFalse(areHistoricalBytesBlocked(new byte[] {})); assertFalse(areHistoricalBytesBlocked(new byte[] {0x01, 0x02, 0x03})); } -}
\ No newline at end of file +} diff --git a/chromium/services/device/public/cpp/device_feature_list.cc b/chromium/services/device/public/cpp/device_feature_list.cc index 04d1858ae6a..28284fc701c 100644 --- a/chromium/services/device/public/cpp/device_feature_list.cc +++ b/chromium/services/device/public/cpp/device_feature_list.cc @@ -4,6 +4,7 @@ #include "base/android/jni_string.h" #include "base/feature_list.h" +#include "base/notreached.h" #include "base/stl_util.h" #include "services/device/device_service_jni_headers/DeviceFeatureList_jni.h" #include "services/device/public/cpp/device_features.h" diff --git a/chromium/services/device/public/cpp/generic_sensor/platform_sensor_configuration.h b/chromium/services/device/public/cpp/generic_sensor/platform_sensor_configuration.h index 93e8507e136..e75d20d6273 100644 --- a/chromium/services/device/public/cpp/generic_sensor/platform_sensor_configuration.h +++ b/chromium/services/device/public/cpp/generic_sensor/platform_sensor_configuration.h @@ -5,7 +5,6 @@ #ifndef SERVICES_DEVICE_PUBLIC_CPP_GENERIC_SENSOR_PLATFORM_SENSOR_CONFIGURATION_H_ #define SERVICES_DEVICE_PUBLIC_CPP_GENERIC_SENSOR_PLATFORM_SENSOR_CONFIGURATION_H_ -#include "base/logging.h" namespace device { diff --git a/chromium/services/device/public/cpp/hid/fake_hid_manager.cc b/chromium/services/device/public/cpp/hid/fake_hid_manager.cc index ca3c5c63465..eae9dede08c 100644 --- a/chromium/services/device/public/cpp/hid/fake_hid_manager.cc +++ b/chromium/services/device/public/cpp/hid/fake_hid_manager.cc @@ -92,7 +92,7 @@ void FakeHidConnection::SendFeatureReport(uint8_t report_id, } // Implementation of FakeHidManager. -FakeHidManager::FakeHidManager() {} +FakeHidManager::FakeHidManager() = default; FakeHidManager::~FakeHidManager() = default; void FakeHidManager::Bind(mojo::PendingReceiver<mojom::HidManager> receiver) { @@ -105,6 +105,9 @@ void FakeHidManager::GetDevicesAndSetClient( GetDevicesCallback callback) { GetDevices(std::move(callback)); + if (!client.is_valid()) + return; + clients_.Add(std::move(client)); } @@ -201,4 +204,9 @@ void FakeHidManager::RemoveDevice(const std::string& guid) { } } +void FakeHidManager::SimulateConnectionError() { + clients_.Clear(); + receivers_.Clear(); +} + } // namespace device diff --git a/chromium/services/device/public/cpp/hid/fake_hid_manager.h b/chromium/services/device/public/cpp/hid/fake_hid_manager.h index e4319ee221a..7c7f0c02774 100644 --- a/chromium/services/device/public/cpp/hid/fake_hid_manager.h +++ b/chromium/services/device/public/cpp/hid/fake_hid_manager.h @@ -20,6 +20,8 @@ namespace device { class FakeHidConnection : public mojom::HidConnection { public: explicit FakeHidConnection(mojom::HidDeviceInfoPtr device); + FakeHidConnection(FakeHidConnection&) = delete; + FakeHidConnection& operator=(FakeHidConnection&) = delete; ~FakeHidConnection() override; // mojom::HidConnection implementation: @@ -41,6 +43,8 @@ class FakeHidConnection : public mojom::HidConnection { class FakeHidManager : public mojom::HidManager { public: FakeHidManager(); + FakeHidManager(FakeHidManager&) = delete; + FakeHidManager& operator=(FakeHidManager&) = delete; ~FakeHidManager() override; void Bind(mojo::PendingReceiver<mojom::HidManager> receiver); @@ -74,6 +78,7 @@ class FakeHidManager : public mojom::HidManager { uint16_t usage); void AddDevice(mojom::HidDeviceInfoPtr device); void RemoveDevice(const std::string& guid); + void SimulateConnectionError(); private: std::map<std::string, mojom::HidDeviceInfoPtr> devices_; diff --git a/chromium/services/device/public/cpp/hid/hid_report_descriptor_item.cc b/chromium/services/device/public/cpp/hid/hid_report_descriptor_item.cc index fd406fe6d47..c40fd7d74bd 100644 --- a/chromium/services/device/public/cpp/hid/hid_report_descriptor_item.cc +++ b/chromium/services/device/public/cpp/hid/hid_report_descriptor_item.cc @@ -26,8 +26,8 @@ HidReportDescriptorItem::HidReportDescriptorItem( size_t size, HidReportDescriptorItem* previous) : previous_(previous), - next_(NULL), - parent_(NULL), + next_(nullptr), + parent_(nullptr), shortData_(0), payload_size_(0) { Header* header = (Header*)&bytes[0]; diff --git a/chromium/services/device/public/mojom/BUILD.gn b/chromium/services/device/public/mojom/BUILD.gn index 6a3a2ae2b6c..221576f2369 100644 --- a/chromium/services/device/public/mojom/BUILD.gn +++ b/chromium/services/device/public/mojom/BUILD.gn @@ -37,6 +37,7 @@ mojom("mojom") { "//mojo/public/mojom/base", "//services/network/public/mojom", "//services/network/public/mojom:mutable_network_traffic_annotation_interface", + "//url/mojom:url_mojom_gurl", ] if (is_chromeos) { diff --git a/chromium/services/device/public/mojom/geolocation_context.mojom b/chromium/services/device/public/mojom/geolocation_context.mojom index 2123c45c4ac..1917b339b9f 100644 --- a/chromium/services/device/public/mojom/geolocation_context.mojom +++ b/chromium/services/device/public/mojom/geolocation_context.mojom @@ -6,13 +6,18 @@ module device.mojom; import "services/device/public/mojom/geolocation.mojom"; import "services/device/public/mojom/geoposition.mojom"; +import "url/mojom/url.mojom"; // GeolocationContext provides methods to bind Geolocation instance and to // set/clear overrides of geoposition that will apply to all Geolocation // instances created by this context. interface GeolocationContext { // Creates a Geolocation instance that is bound to the |request|. - BindGeolocation(pending_receiver<Geolocation> receiver); + // The |origin| is the origin of the top-level frame which the |request| from, + // it is only used for |InstalledWebappGeolocationContext| to separate request + // from different Trusted Web Activity client apps. + BindGeolocation(pending_receiver<Geolocation> receiver, + url.mojom.Url origin); // Enables geolocation override. This method can be used to trigger possible // location-specific behavior in GeolocationImpl created by this diff --git a/chromium/services/device/public/mojom/nfc_provider.mojom b/chromium/services/device/public/mojom/nfc_provider.mojom index ae2e89241a5..b4638000cea 100644 --- a/chromium/services/device/public/mojom/nfc_provider.mojom +++ b/chromium/services/device/public/mojom/nfc_provider.mojom @@ -10,7 +10,7 @@ interface NFCProvider { // Gets an NFC that is associated with |host_id|. |host_id| // is used to obtain the Activity that this NFC instance should use on // Android (see NFCDelegate.java). - GetNFCForHost(int32 host_id, NFC& nfc); + GetNFCForHost(int32 host_id, pending_receiver<NFC> receiver); // Suspends all pending NFC operations. Could be used when web page // visibility is lost. diff --git a/chromium/services/device/public/mojom/serial.mojom b/chromium/services/device/public/mojom/serial.mojom index b4a089120b0..b24628c54ee 100644 --- a/chromium/services/device/public/mojom/serial.mojom +++ b/chromium/services/device/public/mojom/serial.mojom @@ -9,7 +9,19 @@ import "mojo/public/mojom/base/unguessable_token.mojom"; struct SerialPortInfo { mojo_base.mojom.UnguessableToken token; + + // This platform-specific identifier, if present, can be used to identify the + // device across restarts of the application and operating system. + string? persistent_id; + mojo_base.mojom.FilePath path; + + // On macOS a serial device may have two paths, one for the call-out device + // and one for the dial-in device. The call-out device is preferred. If + // there is also an associated dial-in device its path is provided here. If + // the dial-in device is the only option its path will be in |path|. + [EnableIf=is_mac] mojo_base.mojom.FilePath? alternate_path; + uint16 vendor_id; bool has_vendor_id = false; uint16 product_id; @@ -100,9 +112,11 @@ interface SerialPortManager { GetDevices() => (array<SerialPortInfo> devices); // Creates a SerialPort instance attached to the port represented by |token|. - // When the pipe passed in |port_receiver| is closed the optional pipe passed - // in |watcher| will also be closed. + // If |use_alternate_path| is specified then the |alternate_path| for the + // port will be used instead. When the pipe passed in |port_receiver| is + // closed the optional pipe passed in |watcher| will also be closed. GetPort(mojo_base.mojom.UnguessableToken token, + bool use_alternate_path, pending_receiver<SerialPort> port_receiver, pending_remote<SerialPortConnectionWatcher>? watcher); }; @@ -120,19 +134,17 @@ interface SerialPortManagerClient { interface SerialPort { // Initiates an Open of the device then returns result. Open(SerialConnectionOptions options, - handle<data_pipe_consumer> in_stream, - handle<data_pipe_producer> out_stream, pending_remote<SerialPortClient> client) => (bool success); - // Try to clear existing send error and reconnect the data pipe for send. - // This is supposed to be called after SerialPortClient#OnSendError is - // notified. - ClearSendError(handle<data_pipe_consumer> consumer); + // Start writing data from |consumer| to the port. This should be called after + // Open() or to restart data flow after when SerialPortClient#OnSendError is + // called on |client| to indicate an error. + StartWriting(handle<data_pipe_consumer> consumer); - // Try to clear existing read error and reconnect the data pipe for read. - // This is supposed to be called after SerialPortClient#OnReadError is - // notified. - ClearReadError(handle<data_pipe_producer> producer); + // Start reading data from the port into |producer|. This should be called + // after Open() or to restart data flow when SerialPortClient#OnReadError is + // called on |client| to indicate an error. + StartReading(handle<data_pipe_producer> producer); // Flushes input and output buffers. Flush() => (bool success); diff --git a/chromium/services/device/public/mojom/usb_manager.mojom b/chromium/services/device/public/mojom/usb_manager.mojom index a1ec731a19a..62a9bc61806 100644 --- a/chromium/services/device/public/mojom/usb_manager.mojom +++ b/chromium/services/device/public/mojom/usb_manager.mojom @@ -37,9 +37,11 @@ interface UsbDeviceManager { CheckAccess(string guid) => (bool success); // Attempt to open a USB device using permission_broker and return - // a file descriptor. + // a file descriptor. Allow access only to interfaces specified in + // |allowed_interfaces_mask|. [EnableIf=is_chromeos] - OpenFileDescriptor(string guid) => (mojo_base.mojom.File? fd); + OpenFileDescriptor(string guid, uint32 allowed_interfaces_mask) + => (mojo_base.mojom.File? fd); // Sets the client for this DeviceManager service. The service will notify its // client of device events such as connection and disconnection. diff --git a/chromium/services/device/serial/serial_device_enumerator.cc b/chromium/services/device/serial/serial_device_enumerator.cc index 781df170927..057efc7654a 100644 --- a/chromium/services/device/serial/serial_device_enumerator.cc +++ b/chromium/services/device/serial/serial_device_enumerator.cc @@ -58,13 +58,19 @@ void SerialDeviceEnumerator::RemoveObserver(Observer* observer) { } base::Optional<base::FilePath> SerialDeviceEnumerator::GetPathFromToken( - const base::UnguessableToken& token) { + const base::UnguessableToken& token, + bool use_alternate_path) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); auto it = ports_.find(token); if (it == ports_.end()) return base::nullopt; +#if defined(OS_MACOSX) + if (use_alternate_path) + return it->second->alternate_path; +#endif + return it->second->path; } diff --git a/chromium/services/device/serial/serial_device_enumerator.h b/chromium/services/device/serial/serial_device_enumerator.h index 5a0ef07c154..fe852ede549 100644 --- a/chromium/services/device/serial/serial_device_enumerator.h +++ b/chromium/services/device/serial/serial_device_enumerator.h @@ -41,8 +41,9 @@ class SerialDeviceEnumerator { void AddObserver(Observer* observer); void RemoveObserver(Observer* observer); - virtual base::Optional<base::FilePath> GetPathFromToken( - const base::UnguessableToken& token); + base::Optional<base::FilePath> GetPathFromToken( + const base::UnguessableToken& token, + bool use_alternate_path); protected: // These helper methods take care of managing |ports_| and notifying diff --git a/chromium/services/device/serial/serial_device_enumerator_linux.cc b/chromium/services/device/serial/serial_device_enumerator_linux.cc index 5553deb0528..a90aa6afded 100644 --- a/chromium/services/device/serial/serial_device_enumerator_linux.cc +++ b/chromium/services/device/serial/serial_device_enumerator_linux.cc @@ -11,32 +11,84 @@ #include <vector> #include "base/check_op.h" +#include "base/files/file_util.h" #include "base/strings/string_number_conversions.h" +#include "base/strings/string_split.h" #include "base/strings/string_util.h" +#include "base/strings/stringprintf.h" #include "base/threading/scoped_blocking_call.h" +#include "device/udev_linux/udev.h" namespace device { namespace { -const char kSerialSubsystem[] = "tty"; +constexpr char kSubsystemTty[] = "tty"; + +// Holds information about a TTY driver for serial devices. Each driver creates +// device nodes with a given major number and in a range of minor numbers. +struct SerialDriverInfo { + int major; + int minor_start; + int minor_end; // Inclusive. +}; + +std::vector<SerialDriverInfo> ReadSerialDriverInfo() { + std::string tty_drivers; + if (!base::ReadFileToString(base::FilePath("/proc/tty/drivers"), + &tty_drivers)) { + return {}; + } -const char kHostPathKey[] = "DEVNAME"; -const char kHostBusKey[] = "ID_BUS"; -const char kMajorKey[] = "MAJOR"; -const char kVendorIDKey[] = "ID_VENDOR_ID"; -const char kProductIDKey[] = "ID_MODEL_ID"; -const char kProductNameKey[] = "ID_MODEL"; + // Each line has information on a single TTY driver. + std::vector<SerialDriverInfo> serial_drivers; + for (const auto& line : + base::SplitStringPiece(tty_drivers, "\n", base::KEEP_WHITESPACE, + base::SPLIT_WANT_NONEMPTY)) { + std::vector<base::StringPiece> fields = base::SplitStringPiece( + line, " ", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY); + + // The format of each line is: + // + // driver name<SPACE>name<SPACE>major<SPACE>minor range<SPACE>type + // + // We only care about drivers that provide the "serial" type. The rest are + // things like pseudoterminals. + if (fields.size() < 5 || fields[4] != "serial") + continue; + + SerialDriverInfo info; + if (!base::StringToInt(fields[2], &info.major)) + continue; + + std::vector<base::StringPiece> minor_range = base::SplitStringPiece( + fields[3], "-", base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY); + if (minor_range.size() == 1) { + if (!base::StringToInt(minor_range[0], &info.minor_start)) + continue; + info.minor_end = info.minor_start; + } else if (minor_range.size() == 2) { + if (!base::StringToInt(minor_range[0], &info.minor_start) || + !base::StringToInt(minor_range[1], &info.minor_end)) { + continue; + } + } else { + continue; + } + + serial_drivers.push_back(info); + } -// The major number for an RFCOMM TTY device. -const char kRfcommMajor[] = "216"; + return serial_drivers; +} } // namespace SerialDeviceEnumeratorLinux::SerialDeviceEnumeratorLinux() { DETACH_FROM_SEQUENCE(sequence_checker_); - watcher_ = UdevWatcher::StartWatching(this); + watcher_ = UdevWatcher::StartWatching( + this, {UdevWatcher::Filter(kSubsystemTty, "")}); watcher_->EnumerateExistingDevices(); } @@ -49,34 +101,32 @@ void SerialDeviceEnumeratorLinux::OnDeviceAdded(ScopedUdevDevicePtr device) { base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, base::BlockingType::MAY_BLOCK); +#if DCHECK_IS_ON() const char* subsystem = udev_device_get_subsystem(device.get()); - if (!subsystem || strcmp(subsystem, kSerialSubsystem) != 0) - return; + DCHECK(subsystem); + DCHECK_EQ(base::StringPiece(subsystem), kSubsystemTty); +#endif const char* syspath_str = udev_device_get_syspath(device.get()); if (!syspath_str) return; std::string syspath(syspath_str); - // Platform serial ports. - if (base::StartsWith(syspath, "/sys/devices/platform/", - base::CompareCase::SENSITIVE)) { - CreatePort(std::move(device), syspath); - return; - } + const char* major_str = udev_device_get_property_value(device.get(), "MAJOR"); + const char* minor_str = udev_device_get_property_value(device.get(), "MINOR"); - // USB serial ports and others that have a proper bus identifier. - const char* bus = udev_device_get_property_value(device.get(), kHostBusKey); - if (bus) { - CreatePort(std::move(device), syspath); + int major, minor; + if (!major_str || !minor_str || !base::StringToInt(major_str, &major) || + !base::StringToInt(minor_str, &minor)) { return; } - // Bluetooth ports are virtual TTYs but have an identifiable major number. - const char* major = udev_device_get_property_value(device.get(), kMajorKey); - if (major && base::StringPiece(major) == kRfcommMajor) { - CreatePort(std::move(device), syspath); - return; + for (const auto& driver : ReadSerialDriverInfo()) { + if (major == driver.major && minor >= driver.minor_start && + minor <= driver.minor_end) { + CreatePort(std::move(device), syspath); + return; + } } } @@ -102,7 +152,7 @@ void SerialDeviceEnumeratorLinux::OnDeviceRemoved(ScopedUdevDevicePtr device) { void SerialDeviceEnumeratorLinux::CreatePort(ScopedUdevDevicePtr device, const std::string& syspath) { - const char* path = udev_device_get_property_value(device.get(), kHostPathKey); + const char* path = udev_device_get_property_value(device.get(), "DEVNAME"); if (!path) return; @@ -112,11 +162,13 @@ void SerialDeviceEnumeratorLinux::CreatePort(ScopedUdevDevicePtr device, info->token = token; const char* vendor_id = - udev_device_get_property_value(device.get(), kVendorIDKey); + udev_device_get_property_value(device.get(), "ID_VENDOR_ID"); const char* product_id = - udev_device_get_property_value(device.get(), kProductIDKey); - const char* product_name = - udev_device_get_property_value(device.get(), kProductNameKey); + udev_device_get_property_value(device.get(), "ID_PRODUCT_ID"); + const char* product_name_enc = + udev_device_get_property_value(device.get(), "ID_MODEL_ENC"); + const char* serial_number = + udev_device_get_property_value(device.get(), "ID_SERIAL_SHORT"); uint32_t int_value; if (vendor_id && base::HexStringToUInt(vendor_id, &int_value)) { @@ -127,8 +179,13 @@ void SerialDeviceEnumeratorLinux::CreatePort(ScopedUdevDevicePtr device, info->product_id = int_value; info->has_product_id = true; } - if (product_name) - info->display_name.emplace(product_name); + if (product_name_enc) + info->display_name = device::UdevDecodeString(product_name_enc); + + if (info->has_vendor_id && info->has_product_id && serial_number) { + info->persistent_id = base::StringPrintf("%04X-%04X-%s", info->vendor_id, + info->product_id, serial_number); + } paths_.insert(std::make_pair(syspath, token)); AddPort(std::move(info)); diff --git a/chromium/services/device/serial/serial_device_enumerator_mac.cc b/chromium/services/device/serial/serial_device_enumerator_mac.cc index 8d950297f60..3685b4bc42e 100644 --- a/chromium/services/device/serial/serial_device_enumerator_mac.cc +++ b/chromium/services/device/serial/serial_device_enumerator_mac.cc @@ -69,37 +69,28 @@ CFNumberRef GetCFNumberProperty(io_service_t service, const CFStringRef key) { return NULL; } -// Searches the specified service for a string property with the specified key, -// sets value to that property's value, and returns whether the operation was -// successful. -bool GetStringProperty(io_service_t service, - const CFStringRef key, - std::string* value) { +// Searches the specified service for a string property with the specified key. +base::Optional<std::string> GetStringProperty(io_service_t service, + const CFStringRef key) { CFStringRef propValue = GetCFStringProperty(service, key); - if (propValue) { - *value = base::SysCFStringRefToUTF8(propValue); - return true; - } + if (propValue) + return base::SysCFStringRefToUTF8(propValue); - return false; + return base::nullopt; } // Searches the specified service for a uint16_t property with the specified -// key, sets value to that property's value, and returns whether the operation -// was successful. -bool GetUInt16Property(io_service_t service, - const CFStringRef key, - uint16_t* value) { +// key. +base::Optional<uint16_t> GetUInt16Property(io_service_t service, + const CFStringRef key) { CFNumberRef propValue = GetCFNumberProperty(service, key); if (propValue) { int intValue; - if (CFNumberGetValue(propValue, kCFNumberIntType, &intValue)) { - *value = static_cast<uint16_t>(intValue); - return true; - } + if (CFNumberGetValue(propValue, kCFNumberIntType, &intValue)) + return static_cast<uint16_t>(intValue); } - return false; + return base::nullopt; } } // namespace @@ -165,51 +156,55 @@ void SerialDeviceEnumeratorMac::AddDevices() { if (result != kIOReturnSuccess) continue; - auto callout_info = mojom::SerialPortInfo::New(); - uint16_t vendorId; - if (GetUInt16Property(device.get(), CFSTR(kUSBVendorID), &vendorId)) { - callout_info->has_vendor_id = true; - callout_info->vendor_id = vendorId; + auto info = mojom::SerialPortInfo::New(); + base::Optional<uint16_t> vendor_id = + GetUInt16Property(device.get(), CFSTR(kUSBVendorID)); + if (vendor_id) { + info->has_vendor_id = true; + info->vendor_id = *vendor_id; } - uint16_t productId; - if (GetUInt16Property(device.get(), CFSTR(kUSBProductID), &productId)) { - callout_info->has_product_id = true; - callout_info->product_id = productId; + base::Optional<uint16_t> product_id = + GetUInt16Property(device.get(), CFSTR(kUSBProductID)); + if (product_id) { + info->has_product_id = true; + info->product_id = *product_id; } - std::string display_name; - if (GetStringProperty(device.get(), CFSTR(kUSBProductString), - &display_name)) { - callout_info->display_name = std::move(display_name); + info->display_name = + GetStringProperty(device.get(), CFSTR(kUSBProductString)); + + base::Optional<std::string> serial_number = + GetStringProperty(device.get(), CFSTR(kUSBSerialNumberString)); + if (vendor_id && product_id && serial_number) { + info->persistent_id = base::StringPrintf( + "%04X-%04X-%s", *vendor_id, *product_id, serial_number->c_str()); } - // Each serial device has two "paths" in /dev/ associated with it: a - // "dialin" path starting with "tty" and a "callout" path starting with - // "cu". Each of these is considered a different device from Chrome's - // standpoint, but both should share the device's USB properties. - std::string dialin_device; - if (GetStringProperty(device.get(), CFSTR(kIODialinDeviceKey), - &dialin_device)) { - auto token = base::UnguessableToken::Create(); - mojom::SerialPortInfoPtr dialin_info = callout_info.Clone(); - dialin_info->path = base::FilePath(dialin_device); - dialin_info->token = token; - - entries_[entry_id].first = token; - AddPort(std::move(dialin_info)); + // Each serial device has two paths associated with it: a "dialin" path + // starting with "tty" and a "callout" path starting with "cu". The + // call-out device is typically preferred but requesting the dial-in device + // is supported for the legacy Chrome Apps API. + base::Optional<std::string> dialin_device = + GetStringProperty(device.get(), CFSTR(kIODialinDeviceKey)); + base::Optional<std::string> callout_device = + GetStringProperty(device.get(), CFSTR(kIOCalloutDeviceKey)); + + if (callout_device) { + info->path = base::FilePath(*callout_device); + if (dialin_device) + info->alternate_path = base::FilePath(*dialin_device); + } else if (dialin_device) { + info->path = base::FilePath(*dialin_device); + } else { + continue; } - std::string callout_device; - if (GetStringProperty(device.get(), CFSTR(kIOCalloutDeviceKey), - &callout_device)) { - auto token = base::UnguessableToken::Create(); - callout_info->path = base::FilePath(callout_device); - callout_info->token = token; + auto token = base::UnguessableToken::Create(); + info->token = token; - entries_[entry_id].second = token; - AddPort(std::move(callout_info)); - } + entries_.insert({entry_id, token}); + AddPort(std::move(info)); } } @@ -227,14 +222,10 @@ void SerialDeviceEnumeratorMac::RemoveDevices() { if (it == entries_.end()) continue; - std::pair<base::UnguessableToken, base::UnguessableToken> tokens = - it->second; + base::UnguessableToken token = it->second; entries_.erase(it); - if (tokens.first) - RemovePort(tokens.first); - if (tokens.second) - RemovePort(tokens.second); + RemovePort(token); } } diff --git a/chromium/services/device/serial/serial_device_enumerator_mac.h b/chromium/services/device/serial/serial_device_enumerator_mac.h index 79b6e55b17c..5ba21837d53 100644 --- a/chromium/services/device/serial/serial_device_enumerator_mac.h +++ b/chromium/services/device/serial/serial_device_enumerator_mac.h @@ -33,10 +33,9 @@ class SerialDeviceEnumeratorMac : public SerialDeviceEnumerator { void AddDevices(); void RemoveDevices(); - // Each IORegistry entry potentially creates two serial ports for the dialin - // and callout device nodes. - std::map<uint64_t, std::pair<base::UnguessableToken, base::UnguessableToken>> - entries_; + // Map from IORegistry entry IDs to the token used to refer to the device + // internally. + std::map<uint64_t, base::UnguessableToken> entries_; base::mac::ScopedIONotificationPortRef notify_port_; base::mac::ScopedIOObject<io_iterator_t> devices_added_iterator_; diff --git a/chromium/services/device/serial/serial_device_enumerator_win.cc b/chromium/services/device/serial/serial_device_enumerator_win.cc index cecf66c047d..931173b8f55 100644 --- a/chromium/services/device/serial/serial_device_enumerator_win.cc +++ b/chromium/services/device/serial/serial_device_enumerator_win.cc @@ -11,6 +11,9 @@ #include <setupapi.h> #include <stdint.h> +#define INITGUID +#include <devpkey.h> + #include <algorithm> #include <memory> #include <string> @@ -26,45 +29,40 @@ #include "base/strings/utf_string_conversions.h" #include "base/threading/scoped_blocking_call.h" #include "base/win/registry.h" +#include "base/win/scoped_devinfo.h" #include "third_party/re2/src/re2/re2.h" namespace device { namespace { -struct DevInfoScopedTraits { - static HDEVINFO InvalidValue() { return INVALID_HANDLE_VALUE; } - static void Free(HDEVINFO h) { SetupDiDestroyDeviceInfoList(h); } -}; +base::Optional<std::string> GetProperty(HDEVINFO dev_info, + SP_DEVINFO_DATA* dev_info_data, + const DEVPROPKEY& property) { + // SetupDiGetDeviceProperty() makes an RPC which may block. + base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, + base::BlockingType::MAY_BLOCK); -using ScopedDevInfo = base::ScopedGeneric<HDEVINFO, DevInfoScopedTraits>; - -// Searches the specified device info for a property with the specified key, -// assigns the result to value, and returns whether the operation was -// successful. -bool GetProperty(HDEVINFO dev_info, - SP_DEVINFO_DATA* dev_info_data, - const int key, - std::string* value) { - // We don't know how much space the property's value will take up, so we call - // the property retrieval function once to fetch the size of the required - // value buffer, then again once we've allocated a sufficiently large buffer. - DWORD buffer_size = 0; - SetupDiGetDeviceRegistryProperty(dev_info, dev_info_data, key, nullptr, - nullptr, buffer_size, &buffer_size); - if (GetLastError() != ERROR_INSUFFICIENT_BUFFER) - return false; + DEVPROPTYPE property_type; + DWORD required_size; + if (SetupDiGetDeviceProperty(dev_info, dev_info_data, &property, + &property_type, /*PropertyBuffer=*/nullptr, + /*PropertyBufferSize=*/0, &required_size, + /*Flags=*/0) || + GetLastError() != ERROR_INSUFFICIENT_BUFFER || + property_type != DEVPROP_TYPE_STRING) { + return base::nullopt; + } base::string16 buffer; - if (!SetupDiGetDeviceRegistryProperty( - dev_info, dev_info_data, key, nullptr, - reinterpret_cast<PBYTE>(base::WriteInto(&buffer, buffer_size)), - buffer_size, nullptr)) { - return false; + if (!SetupDiGetDeviceProperty( + dev_info, dev_info_data, &property, &property_type, + reinterpret_cast<PBYTE>(base::WriteInto(&buffer, required_size)), + required_size, /*RequiredSize=*/nullptr, /*Flags=*/0)) { + return base::nullopt; } - *value = base::UTF16ToUTF8(buffer); - return true; + return base::UTF16ToUTF8(buffer); } base::FilePath FixUpPortName(base::StringPiece port_name) { @@ -84,19 +82,19 @@ bool GetDisplayName(const std::string friendly_name, display_name); } -// Searches for the vendor ID in the device's hardware ID, assigns its value to +// Searches for the vendor ID in the device's instance ID, assigns its value to // vendor_id, and returns whether the operation was successful. -bool GetVendorID(const std::string hardware_id, uint32_t* vendor_id) { +bool GetVendorID(const std::string& instance_id, uint32_t* vendor_id) { std::string vendor_id_str; - return RE2::PartialMatch(hardware_id, "VID_([0-9a-fA-F]+)", &vendor_id_str) && + return RE2::PartialMatch(instance_id, "VID_([0-9a-fA-F]+)", &vendor_id_str) && base::HexStringToUInt(vendor_id_str, vendor_id); } -// Searches for the product ID in the device's product ID, assigns its value to +// Searches for the product ID in the device's instance ID, assigns its value to // product_id, and returns whether the operation was successful. -bool GetProductID(const std::string hardware_id, uint32_t* product_id) { +bool GetProductID(const std::string& instance_id, uint32_t* product_id) { std::string product_id_str; - return RE2::PartialMatch(hardware_id, "PID_([0-9a-fA-F]+)", + return RE2::PartialMatch(instance_id, "PID_([0-9a-fA-F]+)", &product_id_str) && base::HexStringToUInt(product_id_str, product_id); } @@ -179,7 +177,8 @@ base::Optional<base::FilePath> SerialDeviceEnumeratorWin::GetPath( } void SerialDeviceEnumeratorWin::OnPathAdded(const base::string16& device_path) { - ScopedDevInfo dev_info(SetupDiCreateDeviceInfoList(nullptr, nullptr)); + base::win::ScopedDevInfo dev_info( + SetupDiCreateDeviceInfoList(nullptr, nullptr)); if (!dev_info.is_valid()) return; @@ -198,7 +197,8 @@ void SerialDeviceEnumeratorWin::OnPathAdded(const base::string16& device_path) { void SerialDeviceEnumeratorWin::OnPathRemoved( const base::string16& device_path) { - ScopedDevInfo dev_info(SetupDiCreateDeviceInfoList(nullptr, nullptr)); + base::win::ScopedDevInfo dev_info( + SetupDiCreateDeviceInfoList(nullptr, nullptr)); if (!dev_info.is_valid()) return; @@ -212,16 +212,15 @@ void SerialDeviceEnumeratorWin::OnPathRemoved( if (!SetupDiEnumDeviceInfo(dev_info.get(), 0, &dev_info_data)) return; - std::string friendly_name; - // SPDRP_FRIENDLYNAME looks like "USB_SERIAL_PORT (COM3)". + // The friendly name looks like "USB_SERIAL_PORT (COM3)". // In Windows, the COM port is the path used to uniquely identify the // serial device. If the COM can't be found, ignore the device. - if (!GetProperty(dev_info.get(), &dev_info_data, SPDRP_FRIENDLYNAME, - &friendly_name)) { + base::Optional<std::string> friendly_name = + GetProperty(dev_info.get(), &dev_info_data, DEVPKEY_Device_FriendlyName); + if (!friendly_name) return; - } - base::Optional<base::FilePath> path = GetPath(friendly_name); + base::Optional<base::FilePath> path = GetPath(*friendly_name); if (!path) return; @@ -239,7 +238,7 @@ void SerialDeviceEnumeratorWin::DoInitialEnumeration() { base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, base::BlockingType::MAY_BLOCK); // Make a device interface query to find all serial devices. - ScopedDevInfo dev_info( + base::win::ScopedDevInfo dev_info( SetupDiGetClassDevs(&GUID_DEVINTERFACE_COMPORT, nullptr, 0, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT)); if (!dev_info.is_valid()) @@ -255,40 +254,43 @@ void SerialDeviceEnumeratorWin::DoInitialEnumeration() { void SerialDeviceEnumeratorWin::EnumeratePort(HDEVINFO dev_info, SP_DEVINFO_DATA* dev_info_data) { - std::string friendly_name; - // SPDRP_FRIENDLYNAME looks like "USB_SERIAL_PORT (COM3)". + // The friendly name looks like "USB_SERIAL_PORT (COM3)". // In Windows, the COM port is the path used to uniquely identify the // serial device. If the COM can't be found, ignore the device. - if (!GetProperty(dev_info, dev_info_data, SPDRP_FRIENDLYNAME, - &friendly_name)) { + base::Optional<std::string> friendly_name = + GetProperty(dev_info, dev_info_data, DEVPKEY_Device_FriendlyName); + if (!friendly_name) return; - } - base::Optional<base::FilePath> path = GetPath(friendly_name); + base::Optional<base::FilePath> path = GetPath(*friendly_name); if (!path) return; - auto info = mojom::SerialPortInfo::New(); - info->path = *path; + base::Optional<std::string> instance_id = + GetProperty(dev_info, dev_info_data, DEVPKEY_Device_InstanceId); + if (!instance_id) + return; + base::UnguessableToken token = base::UnguessableToken::Create(); + auto info = mojom::SerialPortInfo::New(); info->token = token; + info->path = *path; + info->persistent_id = instance_id; + // TODO(https://crbug.com/1015074): Read the real USB strings here. std::string display_name; - if (GetDisplayName(friendly_name, &display_name)) + if (GetDisplayName(*friendly_name, &display_name)) info->display_name = std::move(display_name); - std::string hardware_id; - // SPDRP_HARDWAREID looks like "FTDIBUS\COMPORT&VID_0403&PID_6001". - if (GetProperty(dev_info, dev_info_data, SPDRP_HARDWAREID, &hardware_id)) { - uint32_t vendor_id, product_id; - if (GetVendorID(hardware_id, &vendor_id)) { - info->has_vendor_id = true; - info->vendor_id = vendor_id; - } - if (GetProductID(hardware_id, &product_id)) { - info->has_product_id = true; - info->product_id = product_id; - } + // The instance ID looks like "FTDIBUS\VID_0403+PID_6001+A703X87GA\0000". + uint32_t vendor_id, product_id; + if (GetVendorID(*instance_id, &vendor_id)) { + info->has_vendor_id = true; + info->vendor_id = vendor_id; + } + if (GetProductID(*instance_id, &product_id)) { + info->has_product_id = true; + info->product_id = product_id; } paths_.insert(std::make_pair(*path, token)); diff --git a/chromium/services/device/serial/serial_io_handler_posix.cc b/chromium/services/device/serial/serial_io_handler_posix.cc index 452d81538fd..7f2a2c421b3 100644 --- a/chromium/services/device/serial/serial_io_handler_posix.cc +++ b/chromium/services/device/serial/serial_io_handler_posix.cc @@ -126,7 +126,11 @@ scoped_refptr<SerialIoHandler> SerialIoHandler::Create( void SerialIoHandlerPosix::ReadImpl() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK(pending_read_buffer()); - DCHECK(file().IsValid()); + + if (!file().IsValid()) { + QueueReadCompleted(0, mojom::SerialReceiveError::DISCONNECTED); + return; + } // Try to read immediately. This is needed because on some platforms // (e.g., OSX) there may not be a notification from the message loop @@ -138,7 +142,11 @@ void SerialIoHandlerPosix::ReadImpl() { void SerialIoHandlerPosix::WriteImpl() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK(pending_write_buffer()); - DCHECK(file().IsValid()); + + if (!file().IsValid()) { + QueueWriteCompleted(0, mojom::SerialSendError::DISCONNECTED); + return; + } EnsureWatchingWrites(); } diff --git a/chromium/services/device/serial/serial_io_handler_win.cc b/chromium/services/device/serial/serial_io_handler_win.cc index 92d05b2cb16..12d91e68ea9 100644 --- a/chromium/services/device/serial/serial_io_handler_win.cc +++ b/chromium/services/device/serial/serial_io_handler_win.cc @@ -266,7 +266,11 @@ bool SerialIoHandlerWin::PostOpen() { void SerialIoHandlerWin::ReadImpl() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK(pending_read_buffer()); - DCHECK(file().IsValid()); + + if (!file().IsValid()) { + QueueReadCompleted(0, mojom::SerialReceiveError::DISCONNECTED); + return; + } if (!SetCommMask(file().GetPlatformFile(), EV_RXCHAR)) { VPLOG(1) << "Failed to set serial event flags"; @@ -285,7 +289,11 @@ void SerialIoHandlerWin::ReadImpl() { void SerialIoHandlerWin::WriteImpl() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK(pending_write_buffer()); - DCHECK(file().IsValid()); + + if (!file().IsValid()) { + QueueWriteCompleted(0, mojom::SerialSendError::DISCONNECTED); + return; + } BOOL ok = ::WriteFile(file().GetPlatformFile(), pending_write_buffer(), pending_write_buffer_len(), NULL, @@ -319,7 +327,7 @@ bool SerialIoHandlerWin::ConfigurePortImpl() { // Set up some sane default options that are not configurable. config.fBinary = TRUE; config.fParity = TRUE; - config.fAbortOnError = TRUE; + config.fAbortOnError = FALSE; config.fOutxDsrFlow = FALSE; config.fDtrControl = DTR_CONTROL_ENABLE; config.fDsrSensitivity = FALSE; @@ -371,9 +379,13 @@ void SerialIoHandlerWin::OnIOCompleted( DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); if (context == comm_context_.get()) { DWORD errors; - COMSTAT status; - if (!ClearCommError(file().GetPlatformFile(), &errors, &status) || - errors != 0) { + if (!ClearCommError(file().GetPlatformFile(), &errors, nullptr)) { + VPLOG(1) << "Failed to clear communication error"; + ReadCompleted(0, mojom::SerialReceiveError::SYSTEM_ERROR); + return; + } + + if (errors != 0) { if (errors & CE_BREAK) { ReadCompleted(0, mojom::SerialReceiveError::BREAK); } else if (errors & CE_FRAME) { @@ -385,7 +397,8 @@ void SerialIoHandlerWin::OnIOCompleted( } else if (errors & CE_RXPARITY) { ReadCompleted(0, mojom::SerialReceiveError::PARITY_ERROR); } else { - ReadCompleted(0, mojom::SerialReceiveError::SYSTEM_ERROR); + NOTIMPLEMENTED() << "Unexpected communication error: " << std::hex + << errors; } return; } @@ -393,6 +406,8 @@ void SerialIoHandlerWin::OnIOCompleted( if (read_canceled()) { ReadCompleted(bytes_transferred, read_cancel_reason()); } else if (error != ERROR_SUCCESS && error != ERROR_OPERATION_ABORTED) { + VLOG(1) << "Waiting for communcations event failed: " + << logging::SystemErrorCodeToString(error); ReadCompleted(0, mojom::SerialReceiveError::SYSTEM_ERROR); } else if (pending_read_buffer()) { BOOL ok = ::ReadFile(file().GetPlatformFile(), pending_read_buffer(), @@ -406,19 +421,20 @@ void SerialIoHandlerWin::OnIOCompleted( } else if (context == read_context_.get()) { if (read_canceled()) { ReadCompleted(bytes_transferred, read_cancel_reason()); - } else if (error != ERROR_SUCCESS && error != ERROR_OPERATION_ABORTED) { - ReadCompleted(0, mojom::SerialReceiveError::SYSTEM_ERROR); + } else if (error == ERROR_SUCCESS || error == ERROR_OPERATION_ABORTED) { + ReadCompleted(bytes_transferred, mojom::SerialReceiveError::NONE); } else { - ReadCompleted(bytes_transferred, - error == ERROR_SUCCESS - ? mojom::SerialReceiveError::NONE - : mojom::SerialReceiveError::SYSTEM_ERROR); + VLOG(1) << "Read failed: " << logging::SystemErrorCodeToString(error); + ReadCompleted(0, mojom::SerialReceiveError::SYSTEM_ERROR); } } else if (context == write_context_.get()) { DCHECK(pending_write_buffer()); if (write_canceled()) { WriteCompleted(0, write_cancel_reason()); - } else if (error != ERROR_SUCCESS && error != ERROR_OPERATION_ABORTED) { + } else if (error == ERROR_SUCCESS || error == ERROR_OPERATION_ABORTED) { + WriteCompleted(bytes_transferred, mojom::SerialSendError::NONE); + } else { + VLOG(1) << "Write failed: " << logging::SystemErrorCodeToString(error); WriteCompleted(0, mojom::SerialSendError::SYSTEM_ERROR); if (error == ERROR_GEN_FAILURE && IsReadPending()) { // For devices using drivers such as FTDI, CP2xxx, when device is @@ -432,11 +448,6 @@ void SerialIoHandlerWin::OnIOCompleted( // disconnection. CancelRead(mojom::SerialReceiveError::SYSTEM_ERROR); } - } else { - WriteCompleted(bytes_transferred, - error == ERROR_SUCCESS - ? mojom::SerialSendError::NONE - : mojom::SerialSendError::SYSTEM_ERROR); } } else { NOTREACHED() << "Invalid IOContext"; diff --git a/chromium/services/device/serial/serial_port_impl.cc b/chromium/services/device/serial/serial_port_impl.cc index 26c90886ed1..efef354e9d4 100644 --- a/chromium/services/device/serial/serial_port_impl.cc +++ b/chromium/services/device/serial/serial_port_impl.cc @@ -52,30 +52,22 @@ SerialPortImpl::~SerialPortImpl() { } void SerialPortImpl::Open(mojom::SerialConnectionOptionsPtr options, - mojo::ScopedDataPipeConsumerHandle in_stream, - mojo::ScopedDataPipeProducerHandle out_stream, mojo::PendingRemote<mojom::SerialPortClient> client, OpenCallback callback) { - DCHECK(in_stream); - DCHECK(out_stream); - in_stream_ = std::move(in_stream); - out_stream_ = std::move(out_stream); if (client) client_.Bind(std::move(client)); - io_handler_->Open(*options, base::BindOnce(&SerialPortImpl::OnOpenCompleted, - weak_factory_.GetWeakPtr(), - std::move(callback))); + + io_handler_->Open(*options, std::move(callback)); } -void SerialPortImpl::ClearSendError( - mojo::ScopedDataPipeConsumerHandle consumer) { - // Make sure |io_handler_| is still open and the |in_stream_| has been - // closed. - if (!io_handler_ || in_stream_) { +void SerialPortImpl::StartWriting(mojo::ScopedDataPipeConsumerHandle consumer) { + if (in_stream_) { + mojo::ReportBadMessage("Data pipe consumer still open."); return; } + in_stream_watcher_.Cancel(); - in_stream_.swap(consumer); + in_stream_ = std::move(consumer); in_stream_watcher_.Watch( in_stream_.get(), MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_PEER_CLOSED, @@ -85,15 +77,14 @@ void SerialPortImpl::ClearSendError( in_stream_watcher_.ArmOrNotify(); } -void SerialPortImpl::ClearReadError( - mojo::ScopedDataPipeProducerHandle producer) { - // Make sure |io_handler_| is still open and the |out_stream_| has been - // closed. - if (!io_handler_ || out_stream_) { +void SerialPortImpl::StartReading(mojo::ScopedDataPipeProducerHandle producer) { + if (out_stream_) { + mojo::ReportBadMessage("Data pipe producer still open."); return; } + out_stream_watcher_.Cancel(); - out_stream_.swap(producer); + out_stream_ = std::move(producer); out_stream_watcher_.Watch( out_stream_.get(), MOJO_HANDLE_SIGNAL_WRITABLE | MOJO_HANDLE_SIGNAL_PEER_CLOSED, @@ -132,27 +123,6 @@ void SerialPortImpl::Close(CloseCallback callback) { io_handler_->Close(std::move(callback)); } -void SerialPortImpl::OnOpenCompleted(OpenCallback callback, bool success) { - if (success) { - in_stream_watcher_.Watch( - in_stream_.get(), - MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_PEER_CLOSED, - MOJO_TRIGGER_CONDITION_SIGNALS_SATISFIED, - base::BindRepeating(&SerialPortImpl::WriteToPort, - weak_factory_.GetWeakPtr())); - in_stream_watcher_.ArmOrNotify(); - - out_stream_watcher_.Watch( - out_stream_.get(), - MOJO_HANDLE_SIGNAL_WRITABLE | MOJO_HANDLE_SIGNAL_PEER_CLOSED, - MOJO_TRIGGER_CONDITION_SIGNALS_SATISFIED, - base::BindRepeating(&SerialPortImpl::ReadFromPortAndWriteOut, - weak_factory_.GetWeakPtr())); - out_stream_watcher_.ArmOrNotify(); - } - std::move(callback).Run(success); -} - void SerialPortImpl::WriteToPort(MojoResult result, const mojo::HandleSignalsState& state) { const void* buffer; diff --git a/chromium/services/device/serial/serial_port_impl.h b/chromium/services/device/serial/serial_port_impl.h index bf5ac94571e..2ba5d31d45c 100644 --- a/chromium/services/device/serial/serial_port_impl.h +++ b/chromium/services/device/serial/serial_port_impl.h @@ -49,12 +49,10 @@ class SerialPortImpl : public mojom::SerialPort { // mojom::SerialPort methods: void Open(mojom::SerialConnectionOptionsPtr options, - mojo::ScopedDataPipeConsumerHandle in_stream, - mojo::ScopedDataPipeProducerHandle out_stream, mojo::PendingRemote<mojom::SerialPortClient> client, OpenCallback callback) override; - void ClearSendError(mojo::ScopedDataPipeConsumerHandle consumer) override; - void ClearReadError(mojo::ScopedDataPipeProducerHandle producer) override; + void StartWriting(mojo::ScopedDataPipeConsumerHandle consumer) override; + void StartReading(mojo::ScopedDataPipeProducerHandle producer) override; void Flush(FlushCallback callback) override; void GetControlSignals(GetControlSignalsCallback callback) override; void SetControlSignals(mojom::SerialHostControlSignalsPtr signals, @@ -64,7 +62,6 @@ class SerialPortImpl : public mojom::SerialPort { void GetPortInfo(GetPortInfoCallback callback) override; void Close(CloseCallback callback) override; - void OnOpenCompleted(OpenCallback callback, bool success); void WriteToPort(MojoResult result, const mojo::HandleSignalsState& state); void OnWriteToPortCompleted(uint32_t bytes_expected, uint32_t bytes_sent, diff --git a/chromium/services/device/serial/serial_port_impl_unittest.cc b/chromium/services/device/serial/serial_port_impl_unittest.cc index 3618b88e916..93c7f0aa317 100644 --- a/chromium/services/device/serial/serial_port_impl_unittest.cc +++ b/chromium/services/device/serial/serial_port_impl_unittest.cc @@ -21,9 +21,65 @@ class SerialPortImplTest : public DeviceServiceTestBase { ~SerialPortImplTest() override = default; protected: + void CreateDataPipe(mojo::ScopedDataPipeProducerHandle* producer, + mojo::ScopedDataPipeConsumerHandle* consumer) { + MojoCreateDataPipeOptions options; + options.struct_size = sizeof(MojoCreateDataPipeOptions); + options.flags = MOJO_CREATE_DATA_PIPE_FLAG_NONE; + options.element_num_bytes = 1; + options.capacity_num_bytes = 64; + + MojoResult result = mojo::CreateDataPipe(&options, producer, consumer); + DCHECK_EQ(result, MOJO_RESULT_OK); + } + + mojo::ScopedDataPipeConsumerHandle StartReading( + mojom::SerialPort* serial_port) { + mojo::ScopedDataPipeProducerHandle producer; + mojo::ScopedDataPipeConsumerHandle consumer; + CreateDataPipe(&producer, &consumer); + serial_port->StartReading(std::move(producer)); + return consumer; + } + + mojo::ScopedDataPipeProducerHandle StartWriting( + mojom::SerialPort* serial_port) { + mojo::ScopedDataPipeProducerHandle producer; + mojo::ScopedDataPipeConsumerHandle consumer; + CreateDataPipe(&producer, &consumer); + serial_port->StartWriting(std::move(consumer)); + return producer; + } + DISALLOW_COPY_AND_ASSIGN(SerialPortImplTest); }; +TEST_F(SerialPortImplTest, StartIoBeforeOpen) { + mojo::Remote<mojom::SerialPort> serial_port; + mojo::PendingRemote<mojom::SerialPortConnectionWatcher> watcher_remote; + mojo::SelfOwnedReceiverRef<mojom::SerialPortConnectionWatcher> watcher = + mojo::MakeSelfOwnedReceiver( + std::make_unique<mojom::SerialPortConnectionWatcher>(), + watcher_remote.InitWithNewPipeAndPassReceiver()); + SerialPortImpl::Create( + base::FilePath(FILE_PATH_LITERAL("/dev/fakeserialmojo")), + serial_port.BindNewPipeAndPassReceiver(), std::move(watcher_remote), + task_environment_.GetMainThreadTaskRunner()); + + mojo::ScopedDataPipeConsumerHandle consumer = StartReading(serial_port.get()); + mojo::ScopedDataPipeProducerHandle producer = StartWriting(serial_port.get()); + + // Write some data so that StartWriting() will cause a call to Write(). + static const char kBuffer[] = "test"; + uint32_t bytes_written = base::size(kBuffer); + MojoResult result = + producer->WriteData(&kBuffer, &bytes_written, MOJO_WRITE_DATA_FLAG_NONE); + DCHECK_EQ(result, MOJO_RESULT_OK); + DCHECK_EQ(bytes_written, base::size(kBuffer)); + + base::RunLoop().RunUntilIdle(); +} + TEST_F(SerialPortImplTest, WatcherClosedWhenPortClosed) { mojo::Remote<mojom::SerialPort> serial_port; mojo::PendingRemote<mojom::SerialPortConnectionWatcher> watcher; diff --git a/chromium/services/device/serial/serial_port_manager_impl.cc b/chromium/services/device/serial/serial_port_manager_impl.cc index d519b7eb546..b893f4e2e2a 100644 --- a/chromium/services/device/serial/serial_port_manager_impl.cc +++ b/chromium/services/device/serial/serial_port_manager_impl.cc @@ -50,13 +50,15 @@ void SerialPortManagerImpl::GetDevices(GetDevicesCallback callback) { void SerialPortManagerImpl::GetPort( const base::UnguessableToken& token, + bool use_alternate_path, mojo::PendingReceiver<mojom::SerialPort> receiver, mojo::PendingRemote<mojom::SerialPortConnectionWatcher> watcher) { if (!enumerator_) { enumerator_ = SerialDeviceEnumerator::Create(ui_task_runner_); observed_enumerator_.Add(enumerator_.get()); } - base::Optional<base::FilePath> path = enumerator_->GetPathFromToken(token); + base::Optional<base::FilePath> path = + enumerator_->GetPathFromToken(token, use_alternate_path); if (path) { io_task_runner_->PostTask( FROM_HERE, diff --git a/chromium/services/device/serial/serial_port_manager_impl.h b/chromium/services/device/serial/serial_port_manager_impl.h index 9d162a241c3..546e9a25fcb 100644 --- a/chromium/services/device/serial/serial_port_manager_impl.h +++ b/chromium/services/device/serial/serial_port_manager_impl.h @@ -46,6 +46,7 @@ class SerialPortManagerImpl : public mojom::SerialPortManager, void GetDevices(GetDevicesCallback callback) override; void GetPort( const base::UnguessableToken& token, + bool use_alternate_path, mojo::PendingReceiver<mojom::SerialPort> receiver, mojo::PendingRemote<mojom::SerialPortConnectionWatcher> watcher) override; diff --git a/chromium/services/device/serial/serial_port_manager_impl_unittest.cc b/chromium/services/device/serial/serial_port_manager_impl_unittest.cc index 8fe414196d8..3b5b4da545f 100644 --- a/chromium/services/device/serial/serial_port_manager_impl_unittest.cc +++ b/chromium/services/device/serial/serial_port_manager_impl_unittest.cc @@ -98,6 +98,7 @@ TEST_F(SerialPortManagerImplTest, SimpleConnectTest) { for (auto& device : results) { mojo::Remote<mojom::SerialPort> serial_port; port_manager->GetPort(device->token, + /*use_alternate_path=*/false, serial_port.BindNewPipeAndPassReceiver(), /*watcher=*/mojo::NullRemote()); // Send a message on the pipe and wait for the response to make sure @@ -189,6 +190,7 @@ TEST_F(SerialPortManagerImplTest, GetPort) { mojo::Remote<mojom::SerialPort> serial_port; port_manager->GetPort(results[0]->token, + /*use_alternate_path=*/false, serial_port.BindNewPipeAndPassReceiver(), /*watcher=*/mojo::NullRemote()); // Send a message on the pipe and wait for the response to make sure diff --git a/chromium/services/device/time_zone_monitor/time_zone_monitor_linux.cc b/chromium/services/device/time_zone_monitor/time_zone_monitor_linux.cc index 65ca480d017..c03a0d554c6 100644 --- a/chromium/services/device/time_zone_monitor/time_zone_monitor_linux.cc +++ b/chromium/services/device/time_zone_monitor/time_zone_monitor_linux.cc @@ -87,7 +87,7 @@ class TimeZoneMonitorLinuxImpl void StopWatching() { DCHECK(main_task_runner_->RunsTasksInCurrentSequence()); - owner_ = NULL; + owner_ = nullptr; file_task_runner_->PostTask( FROM_HERE, base::BindOnce(&TimeZoneMonitorLinuxImpl::StopWatchingOnFileThread, diff --git a/chromium/services/device/usb/mojo/device_manager_impl.cc b/chromium/services/device/usb/mojo/device_manager_impl.cc index 2c7b79af77c..8c3aa4ee25b 100644 --- a/chromium/services/device/usb/mojo/device_manager_impl.cc +++ b/chromium/services/device/usb/mojo/device_manager_impl.cc @@ -123,6 +123,7 @@ void DeviceManagerImpl::CheckAccess(const std::string& guid, void DeviceManagerImpl::OpenFileDescriptor( const std::string& guid, + uint32_t drop_privileges_mask, OpenFileDescriptorCallback callback) { scoped_refptr<UsbDevice> device = usb_service_->GetDevice(guid); if (!device) { @@ -133,8 +134,8 @@ void DeviceManagerImpl::OpenFileDescriptor( base::AdaptCallbackForRepeating(std::move(callback)); auto devpath = static_cast<device::UsbDeviceLinux*>(device.get())->device_path(); - chromeos::PermissionBrokerClient::Get()->OpenPath( - devpath, + chromeos::PermissionBrokerClient::Get()->OpenPathWithDroppedPrivileges( + devpath, drop_privileges_mask, base::BindOnce(&DeviceManagerImpl::OnOpenFileDescriptor, weak_factory_.GetWeakPtr(), copyable_callback), base::BindOnce(&DeviceManagerImpl::OnOpenFileDescriptorError, diff --git a/chromium/services/device/usb/mojo/device_manager_impl.h b/chromium/services/device/usb/mojo/device_manager_impl.h index 5684eab03cf..a048b379dff 100644 --- a/chromium/services/device/usb/mojo/device_manager_impl.h +++ b/chromium/services/device/usb/mojo/device_manager_impl.h @@ -70,6 +70,7 @@ class DeviceManagerImpl : public mojom::UsbDeviceManager, CheckAccessCallback callback) override; void OpenFileDescriptor(const std::string& guid, + uint32_t drop_privileges_mask, OpenFileDescriptorCallback callback) override; void OnOpenFileDescriptor(OpenFileDescriptorCallback callback, diff --git a/chromium/services/device/usb/usb_device_handle_usbfs.cc b/chromium/services/device/usb/usb_device_handle_usbfs.cc index 90e2be66708..68eb5658725 100644 --- a/chromium/services/device/usb/usb_device_handle_usbfs.cc +++ b/chromium/services/device/usb/usb_device_handle_usbfs.cc @@ -17,6 +17,7 @@ #include "base/files/file_descriptor_watcher_posix.h" #include "base/logging.h" #include "base/memory/ref_counted_memory.h" +#include "base/numerics/checked_math.h" #include "base/posix/eintr_wrapper.h" #include "base/sequence_checker.h" #include "base/stl_util.h" @@ -169,7 +170,7 @@ class UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper { DISALLOW_COPY_AND_ASSIGN(BlockingTaskRunnerHelper); }; -struct UsbDeviceHandleUsbfs::Transfer { +struct UsbDeviceHandleUsbfs::Transfer final { Transfer() = delete; Transfer(scoped_refptr<base::RefCountedBytes> buffer, TransferCallback callback); @@ -291,8 +292,12 @@ void UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper::SetInterface( USB_PLOG(DEBUG) << "Failed to set interface " << interface_number << " to alternate setting " << alternate_setting; } - task_runner_->PostTask(FROM_HERE, - base::BindOnce(std::move(callback), rc == 0)); + task_runner_->PostTask( + FROM_HERE, + base::BindOnce( + &UsbDeviceHandleUsbfs::SetAlternateInterfaceSettingComplete, + device_handle_, interface_number, alternate_setting, rc == 0, + std::move(callback))); } void UsbDeviceHandleUsbfs::BlockingTaskRunnerHelper::ResetDevice( @@ -386,9 +391,9 @@ UsbDeviceHandleUsbfs::Transfer::Transfer( scoped_refptr<base::RefCountedBytes> buffer, IsochronousTransferCallback callback) : buffer(buffer), isoc_callback(std::move(callback)) { - memset( - &urb, 0, - sizeof(urb) + sizeof(usbdevfs_iso_packet_desc) * urb.number_of_packets); + // This buffer size calculation is checked in operator new(). + memset(&urb, 0, + sizeof(urb) + sizeof(urb.iso_frame_desc[0]) * urb.number_of_packets); urb.usercontext = this; urb.buffer = buffer->front(); } @@ -396,10 +401,15 @@ UsbDeviceHandleUsbfs::Transfer::Transfer( UsbDeviceHandleUsbfs::Transfer::~Transfer() = default; void* UsbDeviceHandleUsbfs::Transfer::operator new( - std::size_t size, + size_t size, size_t number_of_iso_packets) { - void* p = ::operator new(size + sizeof(usbdevfs_iso_packet_desc) * - number_of_iso_packets); + // The checked math should pass as long as Mojo message size limits are being + // enforced. + size_t total_size = + base::CheckAdd(size, base::CheckMul(sizeof(urb.iso_frame_desc[0]), + number_of_iso_packets)) + .ValueOrDie(); + void* p = ::operator new(total_size); Transfer* transfer = static_cast<Transfer*>(p); transfer->urb.number_of_packets = number_of_iso_packets; return p; @@ -763,6 +773,19 @@ void UsbDeviceHandleUsbfs::SetConfigurationComplete(int configuration_value, std::move(callback).Run(success); } +void UsbDeviceHandleUsbfs::SetAlternateInterfaceSettingComplete( + int interface_number, + int alternate_setting, + bool success, + ResultCallback callback) { + DCHECK(sequence_checker_.CalledOnValidSequence()); + if (success && device_) { + interfaces_[interface_number].alternate_setting = alternate_setting; + RefreshEndpointInfo(); + } + std::move(callback).Run(success); +} + void UsbDeviceHandleUsbfs::ReleaseInterfaceComplete(int interface_number, ResultCallback callback) { DCHECK(sequence_checker_.CalledOnValidSequence()); diff --git a/chromium/services/device/usb/usb_device_handle_usbfs.h b/chromium/services/device/usb/usb_device_handle_usbfs.h index 33a09d00542..949ddd05327 100644 --- a/chromium/services/device/usb/usb_device_handle_usbfs.h +++ b/chromium/services/device/usb/usb_device_handle_usbfs.h @@ -103,6 +103,10 @@ class UsbDeviceHandleUsbfs : public UsbDeviceHandle { void SetConfigurationComplete(int configuration_value, bool success, ResultCallback callback); + void SetAlternateInterfaceSettingComplete(int interface_number, + int alternate_setting, + bool success, + ResultCallback callback); void ReleaseInterfaceComplete(int interface_number, ResultCallback callback); void IsochronousTransferInternal(uint8_t endpoint_address, scoped_refptr<base::RefCountedBytes> buffer, diff --git a/chromium/services/device/usb/usb_device_handle_win.cc b/chromium/services/device/usb/usb_device_handle_win.cc index 2cac1b0cc6d..e9364245b16 100644 --- a/chromium/services/device/usb/usb_device_handle_win.cc +++ b/chromium/services/device/usb/usb_device_handle_win.cc @@ -26,6 +26,7 @@ #include "base/stl_util.h" #include "base/strings/string16.h" #include "base/task/post_task.h" +#include "base/task/thread_pool.h" #include "base/threading/scoped_blocking_call.h" #include "base/threading/sequenced_task_runner_handle.h" #include "base/win/object_watcher.h" @@ -335,8 +336,8 @@ void UsbDeviceHandleWin::SetInterfaceAlternateSetting(int interface_number, // Use a strong reference to |this| rather than a weak pointer to prevent // |interface.handle| from being freed because |this| was destroyed. - base::PostTaskAndReplyWithResult( - FROM_HERE, {base::ThreadPool(), base::MayBlock()}, + base::ThreadPool::PostTaskAndReplyWithResult( + FROM_HERE, {base::MayBlock()}, base::BindOnce(&SetCurrentAlternateSettingBlocking, interface.handle.Get(), alternate_setting), base::BindOnce(&UsbDeviceHandleWin::OnSetAlternateInterfaceSetting, this, @@ -388,8 +389,8 @@ void UsbDeviceHandleWin::ClearHalt(mojom::UsbTransferDirection direction, // Use a strong reference to |this| rather than a weak pointer to prevent // |interface.handle| from being freed because |this| was destroyed. - base::PostTaskAndReplyWithResult( - FROM_HERE, {base::ThreadPool(), base::MayBlock()}, + base::ThreadPool::PostTaskAndReplyWithResult( + FROM_HERE, {base::MayBlock()}, base::BindOnce(&ResetPipeBlocking, interface.handle.Get(), endpoint_address), base::BindOnce(&UsbDeviceHandleWin::OnClearHalt, this, @@ -1017,13 +1018,22 @@ void UsbDeviceHandleWin::GotDescriptorFromNodeConnection( if (win32_result != ERROR_SUCCESS) { SetLastError(win32_result); USB_PLOG(ERROR) << "Failed to read descriptor from node connection"; - std::move(callback).Run(UsbTransferStatus::TRANSFER_ERROR, nullptr, 0); + std::move(callback).Run(UsbTransferStatus::TRANSFER_ERROR, + /*buffer=*/nullptr, /*length=*/0); + return; + } + + if (bytes_transferred < sizeof(USB_DESCRIPTOR_REQUEST)) { + USB_LOG(ERROR) << "Descriptor response too short (" << bytes_transferred + << " < " << sizeof(USB_DESCRIPTOR_REQUEST) << ")"; + std::move(callback).Run(UsbTransferStatus::TRANSFER_ERROR, + /*buffer=*/nullptr, /*length=*/0); return; } - DCHECK_GE(bytes_transferred, sizeof(USB_DESCRIPTOR_REQUEST)); bytes_transferred -= sizeof(USB_DESCRIPTOR_REQUEST); - DCHECK_LE(bytes_transferred, original_buffer->size()); + bytes_transferred = std::min(bytes_transferred, original_buffer->size()); + memcpy(original_buffer->front(), request_buffer->front() + sizeof(USB_DESCRIPTOR_REQUEST), bytes_transferred); diff --git a/chromium/services/device/usb/usb_service.h b/chromium/services/device/usb/usb_service.h index 97a44f0ccea..3e9df5c05ab 100644 --- a/chromium/services/device/usb/usb_service.h +++ b/chromium/services/device/usb/usb_service.h @@ -12,7 +12,7 @@ #include <vector> #include "base/bind_helpers.h" -#include "base/logging.h" +#include "base/check.h" #include "base/macros.h" #include "base/memory/ref_counted.h" #include "base/observer_list.h" diff --git a/chromium/services/device/usb/usb_service_linux.cc b/chromium/services/device/usb/usb_service_linux.cc index d5cef63fe30..7e7eb833b76 100644 --- a/chromium/services/device/usb/usb_service_linux.cc +++ b/chromium/services/device/usb/usb_service_linux.cc @@ -34,6 +34,8 @@ namespace device { namespace { +constexpr char kSubsystemUsb[] = "usb"; + // Standard USB requests and descriptor types: const uint16_t kUsbVersion2_1 = 0x0210; @@ -109,7 +111,8 @@ void UsbServiceLinux::BlockingTaskRunnerHelper::Start() { // Initializing udev for device enumeration and monitoring may fail. In that // case this service will continue to exist but no devices will be found. - watcher_ = UdevWatcher::StartWatching(this); + watcher_ = UdevWatcher::StartWatching( + this, {UdevWatcher::Filter(kSubsystemUsb, "")}); if (watcher_) watcher_->EnumerateExistingDevices(); @@ -123,9 +126,12 @@ void UsbServiceLinux::BlockingTaskRunnerHelper::OnDeviceAdded( base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, base::BlockingType::MAY_BLOCK); + +#if DCHECK_IS_ON() const char* subsystem = udev_device_get_subsystem(device.get()); - if (!subsystem || strcmp(subsystem, "usb") != 0) - return; + DCHECK(subsystem); + DCHECK_EQ(base::StringPiece(subsystem), kSubsystemUsb); +#endif const char* value = udev_device_get_devnode(device.get()); if (!value) diff --git a/chromium/services/device/usb/usb_service_win.cc b/chromium/services/device/usb/usb_service_win.cc index 6acaf445f91..4e8989f6d3d 100644 --- a/chromium/services/device/usb/usb_service_win.cc +++ b/chromium/services/device/usb/usb_service_win.cc @@ -24,8 +24,11 @@ #include "base/strings/string_util.h" #include "base/strings/sys_string_conversions.h" #include "base/strings/utf_string_conversions.h" +#include "base/threading/scoped_blocking_call.h" +#include "base/threading/scoped_thread_priority.h" #include "base/threading/thread_task_runner_handle.h" #include "base/win/registry.h" +#include "base/win/scoped_devinfo.h" #include "base/win/scoped_handle.h" #include "components/device_event_log/device_event_log.h" #include "services/device/usb/usb_descriptors.h" @@ -37,16 +40,13 @@ namespace device { namespace { -struct DevInfoScopedTraits { - static HDEVINFO InvalidValue() { return INVALID_HANDLE_VALUE; } - static void Free(HDEVINFO h) { SetupDiDestroyDeviceInfoList(h); } -}; - -using ScopedDevInfo = base::ScopedGeneric<HDEVINFO, DevInfoScopedTraits>; - base::Optional<uint32_t> GetDeviceUint32Property(HDEVINFO dev_info, SP_DEVINFO_DATA* dev_info_data, const DEVPROPKEY& property) { + // SetupDiGetDeviceProperty() makes an RPC which may block. + base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, + base::BlockingType::MAY_BLOCK); + DEVPROPTYPE property_type; uint32_t buffer; if (!SetupDiGetDeviceProperty( @@ -63,6 +63,10 @@ base::Optional<base::string16> GetDeviceStringProperty( HDEVINFO dev_info, SP_DEVINFO_DATA* dev_info_data, const DEVPROPKEY& property) { + // SetupDiGetDeviceProperty() makes an RPC which may block. + base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, + base::BlockingType::MAY_BLOCK); + DEVPROPTYPE property_type; DWORD required_size; if (SetupDiGetDeviceProperty(dev_info, dev_info_data, &property, @@ -87,6 +91,10 @@ base::Optional<std::vector<base::string16>> GetDeviceStringListProperty( HDEVINFO dev_info, SP_DEVINFO_DATA* dev_info_data, const DEVPROPKEY& property) { + // SetupDiGetDeviceProperty() makes an RPC which may block. + base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, + base::BlockingType::MAY_BLOCK); + DEVPROPTYPE property_type; DWORD required_size; if (SetupDiGetDeviceProperty(dev_info, dev_info_data, &property, @@ -236,7 +244,7 @@ bool GetDeviceInterfaceDetails(HDEVINFO dev_info, base::string16 GetDevicePath(const base::string16& instance_id, const GUID& device_interface_guid) { - ScopedDevInfo dev_info( + base::win::ScopedDevInfo dev_info( SetupDiGetClassDevs(&device_interface_guid, instance_id.c_str(), 0, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT)); if (!dev_info.is_valid()) { @@ -287,7 +295,8 @@ int GetInterfaceNumber(const base::string16& instance_id) { UsbDeviceWin::FunctionInfo GetFunctionInfo(const base::string16& instance_id) { UsbDeviceWin::FunctionInfo info; - ScopedDevInfo dev_info(SetupDiCreateDeviceInfoList(nullptr, nullptr)); + base::win::ScopedDevInfo dev_info( + SetupDiCreateDeviceInfoList(nullptr, nullptr)); if (!dev_info.is_valid()) { USB_PLOG(ERROR) << "SetupDiCreateDeviceInfoList"; return info; @@ -310,6 +319,10 @@ UsbDeviceWin::FunctionInfo GetFunctionInfo(const base::string16& instance_id) { if (!base::EqualsCaseInsensitiveASCII(info.driver, L"winusb")) return info; + // Boost priority while potentially loading Advapi32.dll on a background + // thread for the registry functions used below. + SCOPED_MAY_LOAD_LIBRARY_AT_BACKGROUND_PRIORITY(); + // There is no standard device interface GUID for USB functions and so we // must discover the set of GUIDs that have been set in the registry by // the INF file or Microsoft OS Compatibility descriptors before @@ -332,6 +345,10 @@ UsbDeviceWin::FunctionInfo GetFunctionInfo(const base::string16& instance_id) { } for (const auto& guid_string : device_interface_guids) { + // Boost priority while potentially loading Ole32.dll on a background + // thread for CLSIDFromString(). + SCOPED_MAY_LOAD_LIBRARY_AT_BACKGROUND_PRIORITY(); + GUID guid; if (FAILED(CLSIDFromString(guid_string.c_str(), &guid))) { USB_LOG(ERROR) << "Failed to parse device interface GUID: " @@ -357,7 +374,11 @@ class UsbServiceWin::BlockingTaskRunnerHelper { ~BlockingTaskRunnerHelper() {} void EnumerateDevices() { - ScopedDevInfo dev_info( + // Boost priority while potentially loading SetupAPI.dll for the following + // functions on a background thread. + SCOPED_MAY_LOAD_LIBRARY_AT_BACKGROUND_PRIORITY(); + + base::win::ScopedDevInfo dev_info( SetupDiGetClassDevs(&GUID_DEVINTERFACE_USB_DEVICE, nullptr, 0, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT)); if (!dev_info.is_valid()) { @@ -384,7 +405,11 @@ class UsbServiceWin::BlockingTaskRunnerHelper { } void OnDeviceAdded(const GUID& guid, const base::string16& device_path) { - ScopedDevInfo dev_info(SetupDiGetClassDevs( + // Boost priority while potentially loading SetupAPI.dll and Ole32.dll on a + // background thread for the following functions. + SCOPED_MAY_LOAD_LIBRARY_AT_BACKGROUND_PRIORITY(); + + base::win::ScopedDevInfo dev_info(SetupDiGetClassDevs( &guid, nullptr, 0, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT)); if (!dev_info.is_valid()) { USB_PLOG(ERROR) << "Failed to set up device enumeration"; @@ -407,6 +432,7 @@ class UsbServiceWin::BlockingTaskRunnerHelper { } } + private: void EnumerateDevice(HDEVINFO dev_info, SP_DEVICE_INTERFACE_DATA* device_interface_data, const base::Optional<base::string16>& opt_device_path) { @@ -499,7 +525,6 @@ class UsbServiceWin::BlockingTaskRunnerHelper { std::move(parent_path), interface_number, info)); } - private: std::unordered_map<base::string16, base::string16> hub_paths_; // Calls back to |service_| must be posted to |service_task_runner_|, which diff --git a/chromium/services/device/wake_lock/power_save_blocker/BUILD.gn b/chromium/services/device/wake_lock/power_save_blocker/BUILD.gn index a60e9f21e26..9365bd609d5 100644 --- a/chromium/services/device/wake_lock/power_save_blocker/BUILD.gn +++ b/chromium/services/device/wake_lock/power_save_blocker/BUILD.gn @@ -47,7 +47,6 @@ source_set("power_save_blocker") { "//ui/gfx", ] if (use_x11) { - configs += [ "//build/config/linux:xscrnsaver" ] deps += [ "//ui/gfx/x" ] } } else if (is_mac) { diff --git a/chromium/services/device/wake_lock/power_save_blocker/power_save_blocker_linux.cc b/chromium/services/device/wake_lock/power_save_blocker/power_save_blocker_linux.cc index ea4ffee26e5..6b28c96b7dc 100644 --- a/chromium/services/device/wake_lock/power_save_blocker/power_save_blocker_linux.cc +++ b/chromium/services/device/wake_lock/power_save_blocker/power_save_blocker_linux.cc @@ -26,9 +26,9 @@ #include "ui/gfx/switches.h" #if defined(USE_X11) -#include <X11/extensions/scrnsaver.h> - -#include "ui/gfx/x/x11_types.h" // nogncheck +#include "ui/gfx/x/connection.h" // nogncheck +#include "ui/gfx/x/screensaver.h" // nogncheck +#include "ui/gfx/x/x11_types.h" // nogncheck #endif namespace device { @@ -138,18 +138,16 @@ bool X11ScreenSaverAvailable() { // X Screen Saver isn't accessible in headless mode. if (base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kHeadless)) return false; - XDisplay* display = gfx::GetXDisplay(); - int dummy; - int major; - int minor; - - if (!XScreenSaverQueryExtension(display, &dummy, &dummy)) - return false; + auto* connection = x11::Connection::Get(); - if (!XScreenSaverQueryVersion(display, &major, &minor)) - return false; + auto version = connection->screensaver() + .QueryVersion({x11::ScreenSaver::major_version, + x11::ScreenSaver::minor_version}) + .Sync(); - return major > 1 || (major == 1 && minor >= 1); + return version && (version->server_major_version > 1 || + (version->server_major_version == 1 && + version->server_minor_version >= 1)); } // Wrapper for XScreenSaverSuspend. Checks whether the X11 Screen Saver @@ -159,8 +157,8 @@ void X11ScreenSaverSuspendSet(bool suspend) { if (!X11ScreenSaverAvailable()) return; - XDisplay* display = gfx::GetXDisplay(); - XScreenSaverSuspend(display, suspend); + auto* connection = x11::Connection::Get(); + connection->screensaver().Suspend({suspend}); } #endif |