Skip to content

Commit

Permalink
Introduce Net.SharedDictionaryTransaction.NetResultWithDict.* UMAs
Browse files Browse the repository at this point in the history
https://crrev.com/c/5036162 introduces UMAs
Net.SharedDictionaryUsedResponseErrorCodes.* which record the
NetErrorCodes in URLLoader::NotifyCompleted() if the request used a
stored compression dictionary.

This cl intdocudes new UMAs
Net.SharedDictionaryTransaction.NetResultWithDict.*. They are similar
to Net.SharedDictionaryUsedResponseErrorCodes.*. But these new UMAs are
recorded in SharedDictionaryNetworkTransaction. So, by using this UMA,
we can detect errors closer to the network layer.

Bug: 1413922
Change-Id: Iab1f92d786d218f49b31304a2dceacdfbf5edc57
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5038912
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1228646}
  • Loading branch information
horo-t authored and Chromium LUCI CQ committed Nov 24, 2023
1 parent 9b1e1fa commit 699bdd0
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

#include <string>

#include "base/feature_list.h"
#include "base/functional/bind.h"
#include "base/functional/callback_helpers.h"
#include "base/metrics/histogram_functions.h"
#include "base/notreached.h"
Expand All @@ -17,6 +19,7 @@
#include "net/base/io_buffer.h"
#include "net/base/load_flags.h"
#include "net/base/net_errors.h"
#include "net/base/transport_info.h"
#include "net/cert/x509_certificate.h"
#include "net/extras/shared_dictionary/shared_dictionary_isolation_key.h"
#include "net/filter/brotli_source_stream.h"
Expand Down Expand Up @@ -88,7 +91,11 @@ SharedDictionaryNetworkTransaction::SharedDictionaryNetworkTransaction(
SharedDictionaryManager& shared_dictionary_manager,
std::unique_ptr<net::HttpTransaction> network_transaction)
: shared_dictionary_manager_(shared_dictionary_manager),
network_transaction_(std::move(network_transaction)) {}
network_transaction_(std::move(network_transaction)) {
network_transaction_->SetConnectedCallback(
base::BindRepeating(&SharedDictionaryNetworkTransaction::OnConnected,
base::Unretained(this)));
}

SharedDictionaryNetworkTransaction::~SharedDictionaryNetworkTransaction() =
default;
Expand Down Expand Up @@ -141,6 +148,15 @@ SharedDictionaryNetworkTransaction::ParseSharedDictionaryEncodingType(
void SharedDictionaryNetworkTransaction::OnStartCompleted(
net::CompletionOnceCallback callback,
int result) {
if (shared_dictionary_) {
base::UmaHistogramSparse(
base::StrCat({"Net.SharedDictionaryTransaction.NetResultWithDict.",
cert_is_issued_by_known_root_
? ".KnownRootCert"
: ".UnknownRootCertOrNoCert"}),
-result);
}

if (result == net::OK && shared_dictionary_) {
shared_dictionary_encoding_type_ = ParseSharedDictionaryEncodingType(
*network_transaction_->GetResponseInfo()->headers);
Expand Down Expand Up @@ -400,7 +416,7 @@ void SharedDictionaryNetworkTransaction::SetEarlyResponseHeadersCallback(

void SharedDictionaryNetworkTransaction::SetConnectedCallback(
const ConnectedCallback& callback) {
network_transaction_->SetConnectedCallback(callback);
connected_callback_ = callback;
}

int SharedDictionaryNetworkTransaction::ResumeNetworkStart() {
Expand Down Expand Up @@ -428,4 +444,15 @@ void SharedDictionaryNetworkTransaction::CloseConnectionOnDestruction() {
network_transaction_->CloseConnectionOnDestruction();
}

int SharedDictionaryNetworkTransaction::OnConnected(
const net::TransportInfo& info,
net::CompletionOnceCallback callback) {
cert_is_issued_by_known_root_ = info.cert_is_issued_by_known_root;

if (connected_callback_) {
return connected_callback_.Run(info, std::move(callback));
}
return net::OK;
}

} // namespace network
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@
#include "base/memory/raw_ref.h"
#include "base/memory/weak_ptr.h"
#include "base/time/time.h"
#include "net/base/completion_once_callback.h"
#include "net/http/http_transaction.h"
#include "third_party/abseil-cpp/absl/types/optional.h"

class GURL;

namespace net {
class SourceStream;
struct TransportInfo;
} // namespace net

namespace network {
Expand Down Expand Up @@ -141,6 +143,9 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) SharedDictionaryNetworkTransaction

void OnReadSharedDictionary(base::Time read_start_time, int result);

int OnConnected(const net::TransportInfo& info,
net::CompletionOnceCallback callback);

raw_ref<SharedDictionaryManager> shared_dictionary_manager_;
scoped_refptr<SharedDictionaryStorage> shared_dictionary_storage_;
std::unique_ptr<SharedDictionary> shared_dictionary_;
Expand All @@ -162,6 +167,10 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) SharedDictionaryNetworkTransaction
// This is set only when a shared dictionary is used for decoding the body.
std::unique_ptr<net::HttpResponseInfo> shared_dictionary_used_response_info_;

ConnectedCallback connected_callback_;

bool cert_is_issued_by_known_root_ = false;

base::WeakPtrFactory<SharedDictionaryNetworkTransaction> weak_factory_{this};
};

Expand Down
16 changes: 16 additions & 0 deletions tools/metrics/histograms/metadata/net/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4905,6 +4905,22 @@ chromium-metrics-reviews@google.com.
<token key="SuccessOrFailure" variants="SuccessOrFailure"/>
</histogram>

<histogram name="Net.SharedDictionaryTransaction.NetResultWithDict.{CertState}"
enum="NetErrorCodes" expires_after="2024-02-15">
<owner>horo@chromium.org</owner>
<owner>src/net/extras/shared_dictionary/OWNERS</owner>
<summary>
Positive net error codes returned from the network layer for requests with
Sec-Available-Dictionary header. Recorded when a
SharedDictionaryNetworkTransaction received the result from the network
layer.
</summary>
<token key="CertState">
<variant name="KnownRootCert"/>
<variant name="UnknownRootCertOrNoCert"/>
</token>
</histogram>

<histogram
name="Net.SharedDictionaryUsedByResponseWhenAvailable.MainFrame.{ConnectionType}.{CertState}"
enum="Boolean" expires_after="2024-02-15">
Expand Down

0 comments on commit 699bdd0

Please sign in to comment.