Use SimpleURLLoader::DownloadHeadersOnly in components/metrics [chromium/src : main]
0 views
Skip to first unread message
Andrew Williams (Gerrit)
Dec 17, 2025, 2:55:03 PM (3 days ago) Dec 17
to chromium...@chromium.org, net-r...@chromium.org, asvitki...@chromium.org, chromiumme...@microsoft.com
Andrew Williams has uploaded the change for review![]( "Open in Gerrit")
Commit message
Use SimpleURLLoader::DownloadHeadersOnly in components/metrics
This change replaces calls to SimpleURLLoader's
DownloadToStringOfUnboundedSizeUntilCrashAndDie with DownloadHeadersOnly
in various parts of the Chrome codebase. The body of the response was
not being used in these instances, so having SimpleURLLoader not store
the response body is more efficient and better conveys how the callers
interact with the request responses.
No major changes in functionality are expected as a result of this other
than SimpleURLLoader no longer saving the response bodies while the
request is being downloaded.
This CL was uploaded by git cl split.
Bug: None
Change-Id: I549e9b9bf8fdc03ac244be0fce1e61b2b0874490
Change diff
Change information
Files:
- M components/metrics/net/net_metrics_log_uploader.cc
- M components/metrics/net/net_metrics_log_uploader.h
Change size: S
Delta: 2 files changed, 10 insertions(+), 7 deletions(-)
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrew Williams (Gerrit)
Dec 17, 2025, 4:26:25 PM (3 days ago) Dec 17
to Chromium Metrics Reviews, chromium...@chromium.org, asvitki...@chromium.org, chromiumme...@microsoft.com, net-r...@chromium.org
Andrew Williams voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
gwsq (Gerrit)
Dec 17, 2025, 4:27:28 PM (3 days ago) Dec 17
to Andrew Williams, Chromium Metrics Reviews, Mark Pearson, Chromium LUCI CQ, chromium...@chromium.org, asvitki...@chromium.org, chromiumme...@microsoft.com, net-r...@chromium.org
Attention needed from Mark Pearson
Message from gwsq![]( "Open in Gerrit")
Reviewer source(s):
mpea...@chromium.org is from context(analysis/uma/chrome-metrics.gwsq)
Related details
Attention is currently required from:
- Mark Pearson
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrew Williams (Gerrit)
Dec 17, 2025, 4:30:59 PM (3 days ago) Dec 17
to Chromium Metrics Reviews, Mark Pearson, Chromium LUCI CQ, chromium...@chromium.org, asvitki...@chromium.org, chromiumme...@microsoft.com, net-r...@chromium.org
Attention needed from Mark Pearson
Andrew Williams added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 1 (Latest):
Andrew Williams . resolved
Note that this was also reviewed via https://chromium-review.googlesource.com/c/chromium/src/+/7266460, but split out here for owners approval
Mark Pearson (Gerrit)
Dec 18, 2025, 1:36:24 PM (2 days ago) Dec 18
to Andrew Williams, Chromium Metrics Reviews, Chromium LUCI CQ, chromium...@chromium.org, asvitki...@chromium.org, chromiumme...@microsoft.com, net-r...@chromium.org
Attention needed from Andrew Williams
Mark Pearson voted and added 2 comments![]( "Open in Gerrit")
Votes added by Mark Pearson
| Code-Review | +1 |
2 comments
Patchset-level comments
Andrew Williams . unresolved
Note that this was also reviewed via https://chromium-review.googlesource.com/c/chromium/src/+/7266460, but split out here for owners approval
Mark Pearson
I'm curious: is the aggregate of this change going to be a performance win?
File components/metrics/net/net_metrics_log_uploader.h
Line 12, Patchset 1 (Latest):
#include "components/metrics/metrics_log.h"Mark Pearson . unresolved
Please explicitly
#include "base/memory/scoped_refptr.h"
(unless this is a style violation that I'm not aware of)
Related details
Attention is currently required from:
- Andrew Williams
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. |
Andrew Williams (Gerrit)
Dec 19, 2025, 11:49:53 AM (2 days ago) Dec 19
to Mark Pearson, Chromium Metrics Reviews, Chromium LUCI CQ, chromium...@chromium.org, asvitki...@chromium.org, chromiumme...@microsoft.com, net-r...@chromium.org
Andrew Williams voted and added 3 comments![]( "Open in Gerrit")
Votes added by Andrew Williams
| Commit-Queue | +2 |
Mark PearsonNote that this was also reviewed via https://chromium-review.googlesource.com/c/chromium/src/+/7266460, but split out here for owners approval
I'm curious: is the aggregate of this change going to be a performance win?
Andrew Williams
I'm not sure... I think it would depend on how frequently the DownloadHeadersOnly calls are made and how big the responses tend to be, and I don't have any data on this. My guess would be that these calls aren't happening too frequently, and also since the responses aren't used today it seems likely that the responses, if any, are small.
Still, I think using DownloadHeadersOnly when no response data is needed is a slight simplification in a many cases and better conveys how the code interacts with the request responses.
File components/metrics/net/net_metrics_log_uploader.h
Please explicitly
#include "base/memory/scoped_refptr.h"(unless this is a style violation that I'm not aware of)
Andrew Williams
good catch! I missed this one
Related details
Attention set is empty
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)
Dec 19, 2025, 12:10:22 PM (2 days ago) Dec 19
to Andrew Williams, Mark Pearson, Chromium Metrics Reviews, chromium...@chromium.org, asvitki...@chromium.org, chromiumme...@microsoft.com, net-r...@chromium.org
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
1 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: components/metrics/net/net_metrics_log_uploader.h
Insertions: 1, Deletions: 0.
@@ -9,6 +9,7 @@
#include <string>
#include <string_view>
+#include "base/memory/scoped_refptr.h"
#include "components/metrics/metrics_log.h"
#include "components/metrics/metrics_log_uploader.h"
#include "third_party/metrics_proto/reporting_info.pb.h"
```
Change information
Commit message:
Use SimpleURLLoader::DownloadHeadersOnly in components/metrics
This change replaces calls to SimpleURLLoader's
DownloadToStringOfUnboundedSizeUntilCrashAndDie with DownloadHeadersOnly
in various parts of the Chrome codebase. The body of the response was
not being used in these instances, so having SimpleURLLoader not store
the response body is more efficient and better conveys how the callers
interact with the request responses.
No major changes in functionality are expected as a result of this other
than SimpleURLLoader no longer saving the response bodies while the
request is being downloaded.
This CL was uploaded by git cl split.
Bug: None
Change-Id: I549e9b9bf8fdc03ac244be0fce1e61b2b0874490
Reviewed-by: Mark Pearson <mpea...@chromium.org>
Commit-Queue: Andrew Williams <awi...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1561190}
Files:
- M components/metrics/net/net_metrics_log_uploader.cc
- M components/metrics/net/net_metrics_log_uploader.h
Change size: S
Delta: 2 files changed, 11 insertions(+), 7 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Mark Pearson
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages