[bytesize] Migrate the ui/base byte formatting code from the old ByteCount to ByteSize. [chromium/src : main]
0 views
Skip to first unread message
Avi Drissman (Gerrit)
1:13 AM (21 hours ago) 1:13 AM
to Avi Drissman, Peng Zhou, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Andrew Rayskiy, Rijubrata Bhaumik, Simon Hangl, cros-setti...@google.com, jackshira+wat...@google.com, hidehik...@chromium.org, rrsilva+wat...@google.com, phshah...@chromium.org, tbarzi...@chromium.org, yhanada+...@chromium.org, croissant-...@chromium.org, crost...@chromium.org, arc-review...@google.com, alandin...@chromium.org, tote-eng...@google.com, chfreme...@chromium.org, chromium-a...@chromium.org, dtraino...@chromium.org, dullweb...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, msrame...@chromium.org, pdf-r...@chromium.org, rmcelra...@chromium.org
Attention needed from Peng Zhou
Avi Drissman voted and added 2 comments![]( "Open in Gerrit")
Votes added by Avi Drissman
| Commit-Queue | +1 |
2 comments
Patchset-level comments
File-level comment, Patchset 6:
Avi Drissman . resolved
Another one for you! This should be straightforward. I’m owner for most of these, but I’ll use the OO for the season for the few files I’m not owner of.
File chrome/browser/ash/login/screens/encryption_migration_screen.cc
Line 412, Patchset 6 (Parent):
ui::FormatBytes(base::ByteCount(size.value_or(-1))),Avi Drissman . unresolved
If `size` was ever `nullopt`, passing in `-1` into `FormatBytes()` would have hard-crashed. Therefore, it’s OK to call `value()` instead, as that trades one crash for another.
Related details
Attention is currently required from:
- Peng Zhou
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Peng Zhou (Gerrit)
8:01 AM (14 hours ago) 8:01 AM
to Avi Drissman, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Andrew Rayskiy, Rijubrata Bhaumik, Simon Hangl, cros-setti...@google.com, jackshira+wat...@google.com, hidehik...@chromium.org, rrsilva+wat...@google.com, phshah...@chromium.org, tbarzi...@chromium.org, yhanada+...@chromium.org, croissant-...@chromium.org, crost...@chromium.org, arc-review...@google.com, alandin...@chromium.org, tote-eng...@google.com, chfreme...@chromium.org, chromium-a...@chromium.org, dtraino...@chromium.org, dullweb...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, msrame...@chromium.org, pdf-r...@chromium.org, rmcelra...@chromium.org
Attention needed from Avi Drissman
Peng Zhou voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Avi Drissman
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Avi Drissman (Gerrit)
11:01 AM (11 hours ago) 11:01 AM
to Avi Drissman, Lei Zhang, Solomon Kinard, Peng Zhou, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Andrew Rayskiy, Rijubrata Bhaumik, Simon Hangl, cros-setti...@google.com, jackshira+wat...@google.com, hidehik...@chromium.org, rrsilva+wat...@google.com, phshah...@chromium.org, tbarzi...@chromium.org, yhanada+...@chromium.org, croissant-...@chromium.org, crost...@chromium.org, arc-review...@google.com, alandin...@chromium.org, tote-eng...@google.com, chfreme...@chromium.org, chromium-a...@chromium.org, dtraino...@chromium.org, dullweb...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, msrame...@chromium.org, pdf-r...@chromium.org, rmcelra...@chromium.org
Attention needed from Lei Zhang and Solomon Kinard
Avi Drissman voted and added 2 comments![]( "Open in Gerrit")
Votes added by Avi Drissman
| Commit-Queue | +2 |
| Owners-Override | +1 |
2 comments
Patchset-level comments
File-level comment, Patchset 7 (Latest):
Avi Drissman . resolved
+owner of unowned files
OO as is the season.
File chrome/browser/ash/login/screens/encryption_migration_screen.cc
Line 412, Patchset 6 (Parent):
ui::FormatBytes(base::ByteCount(size.value_or(-1))),Avi Drissman . resolved
If `size` was ever `nullopt`, passing in `-1` into `FormatBytes()` would have hard-crashed. Therefore, it’s OK to call `value()` instead, as that trades one crash for another.
Attention is currently required from:
- Lei Zhang
- Solomon Kinard
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
11:04 AM (11 hours ago) 11:04 AM
to Avi Drissman, Lei Zhang, Solomon Kinard, Peng Zhou, AyeAye, chromium...@chromium.org, Andrew Rayskiy, Rijubrata Bhaumik, Simon Hangl, cros-setti...@google.com, jackshira+wat...@google.com, hidehik...@chromium.org, rrsilva+wat...@google.com, phshah...@chromium.org, tbarzi...@chromium.org, yhanada+...@chromium.org, croissant-...@chromium.org, crost...@chromium.org, arc-review...@google.com, alandin...@chromium.org, tote-eng...@google.com, chfreme...@chromium.org, chromium-a...@chromium.org, dtraino...@chromium.org, dullweb...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, msrame...@chromium.org, pdf-r...@chromium.org, rmcelra...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[bytesize] Migrate the ui/base byte formatting code from the old ByteCount to ByteSize.
Part 2: one-for-one swaps
Doing these swaps is safe because ui::FormatBytes has always
hard-crashed on negative input values, so the usage of checked_cast is
just enforcing that in a different location.
Replaces [u]int64_t and ByteCount with stronger types to increase
clarity and type safety:
- ByteSize: A non-negative amount of bytes.
- ByteSizeDelta: A delta of bytes, which can be positive or negative.
Bug: 448661443
Change-Id: I6e1e7a3c4474eb4ff3b25085d547290a7abc5df6
Commit-Queue: Avi Drissman <a...@chromium.org>
Reviewed-by: Peng Zhou <zhoupe...@bytedance.com>
Owners-Override: Avi Drissman <a...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1562895}
Files:
- M chrome/browser/ash/app_list/search/system_info/system_info_card_provider.cc
- M chrome/browser/ash/app_list/search/system_info/system_info_card_provider_unittest.cc
- M chrome/browser/ash/arc/nearby_share/ui/low_disk_space_dialog_view.cc
- M chrome/browser/ash/crostini/crostini_disk.cc
- M chrome/browser/ash/crostini/crostini_export_import_status_tracker.cc
- M chrome/browser/ash/login/screens/drive_pinning_screen.cc
- M chrome/browser/ash/login/screens/drive_pinning_screen_browsertest.cc
- M chrome/browser/ash/login/screens/encryption_migration_screen.cc
- M chrome/browser/browsing_data/counters/browsing_data_counter_utils.cc
- M chrome/browser/download/download_item_model.cc
- M chrome/browser/download/download_ui_model.cc
- M chrome/browser/download/notification/download_item_notification.cc
- M chrome/browser/download/status_text_builder_utils.cc
- M chrome/browser/error_reporting/chrome_js_error_report_processor_unittest.cc
- M chrome/browser/media/webrtc/webrtc_event_log_uploader.cc
- M chrome/browser/memory/oom_memory_details.cc
- M chrome/browser/ui/ash/holding_space/holding_space_downloads_delegate.cc
- M chrome/browser/ui/task_manager/task_manager_table_model.cc
- M chrome/browser/ui/views/performance_controls/memory_saver_bubble_view.cc
- M chrome/browser/ui/views/performance_controls/memory_saver_chip_view.cc
- M chrome/browser/ui/views/plugin_vm/plugin_vm_installer_view.cc
- M chrome/browser/ui/views/tabs/fade_footer_view.cc
- M chrome/browser/ui/webui/app_management/app_management_page_handler_base.cc
- M chrome/browser/ui/webui/ash/crostini_installer/crostini_installer_ui.cc
- M chrome/browser/ui/webui/ash/settings/pages/crostini/crostini_section.cc
- M chrome/browser/ui/webui/ash/settings/pages/files/google_drive_handler_browsertest.cc
- M chrome/browser/ui/webui/ash/settings/pages/files/google_drive_page_handler.cc
- M chrome/browser/ui/webui/ash/settings/pages/storage/device_storage_handler.cc
- M chrome/browser/ui/webui/ash/settings/pages/storage/device_storage_handler_unittest.cc
- M chrome/browser/ui/webui/settings/site_settings_handler.cc
- M chrome/browser/ui/webui/settings/site_settings_handler_unittest.cc
- M components/storage_monitor/storage_info.cc
- M extensions/browser/path_util.cc
- M pdf/pdf_view_web_plugin.cc
Change size: L
Delta: 34 files changed, 217 insertions(+), 177 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Peng Zhou
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages