Finalize pruning of services/device peripheral modules#10778
Conversation
This change completes the pruning of unused peripheral hardware modules (WebUSB, Web Bluetooth, WebHID, Web Serial, and Web NFC) in content/browser and services/device. Gated StoragePartition maps and browser binders under !BUILDFLAG(IS_COBALT). This additional pruning achieves the following binary size savings on Android: - libchrobalt.so: -237,568 bytes (-232.0 KB) - Cobalt.apk: -261,708 bytes (-255.5 KB)
🤖 Gemini Suggested Commit Message💡 Pro Tips for a Better Commit Message:
|
There was a problem hiding this comment.
Code Review
This pull request introduces build-time exclusions and conditional preprocessor blocks under the is_cobalt flag to disable several web APIs and services (such as Bluetooth, USB, HID, NFC, and Serial) on the Cobalt platform. The review feedback highlights a critical issue in services/device/BUILD.gn where the accidental removal of the IS_SERIAL_ENABLED_PLATFORM definition disables the Serial service globally across all platforms. Additionally, the feedback advises replacing deprecated streaming usage of NOTREACHED() with the standard non-streaming NOTREACHED(); macro to prevent compilation failures in newer milestones.
| if (is_serial_enabled_platform) { | ||
| deps += [ "//services/device/serial" ] | ||
| defines = [ "IS_SERIAL_ENABLED_PLATFORM" ] | ||
| } |
There was a problem hiding this comment.
Removing defines = [ "IS_SERIAL_ENABLED_PLATFORM" ] will prevent IS_SERIAL_ENABLED_PLATFORM from being defined during compilation. This will disable the Serial service on all platforms (including non-Cobalt platforms like Windows, Mac, Linux, and Android) because the C++ code (e.g., in device_service.cc) relies on this macro to include and bind the serial port manager. Please restore the defines statement inside the is_serial_enabled_platform block.
| if (is_serial_enabled_platform) { | |
| deps += [ "//services/device/serial" ] | |
| defines = [ "IS_SERIAL_ENABLED_PLATFORM" ] | |
| } | |
| if (is_serial_enabled_platform) { | |
| deps += [ "//services/device/serial" ] | |
| defines = [ "IS_SERIAL_ENABLED_PLATFORM" ] | |
| } |
|
|
||
| usb_device_manager_->AddReceiver(std::move(receiver)); | ||
| #else | ||
| NOTREACHED() << "WebUSB is not supported on Cobalt."; |
There was a problem hiding this comment.
In modern Chromium (and thus Cobalt), NOTREACHED() has been updated to be a non-streaming macro (or behaves as [[noreturn]] depending on the build configuration). Using the operator<< stream syntax with NOTREACHED() is deprecated and will cause compilation errors in newer milestones. Please use NOTREACHED(); without any stream arguments.
| NOTREACHED() << "WebUSB is not supported on Cobalt."; | |
| NOTREACHED(); |
|
|
||
| usb_device_manager_test_->BindReceiver(std::move(receiver)); | ||
| #else | ||
| NOTREACHED() << "WebUSB is not supported on Cobalt."; |
There was a problem hiding this comment.
In modern Chromium (and thus Cobalt), NOTREACHED() has been updated to be a non-streaming macro (or behaves as [[noreturn]] depending on the build configuration). Using the operator<< stream syntax with NOTREACHED() is deprecated and will cause compilation errors in newer milestones. Please use NOTREACHED(); without any stream arguments.
| NOTREACHED() << "WebUSB is not supported on Cobalt."; | |
| NOTREACHED(); |
This change completes the pruning of unused peripheral hardware modules (WebUSB, Web Bluetooth, WebHID, Web Serial, and Web NFC) in content/browser and services/device. Gated StoragePartition maps and browser binders under !BUILDFLAG(IS_COBALT).
This additional pruning achieves the following binary size savings on Android: