[net] Populate DNS resolution details from ConnectJob [chromium/src : main]
Kenichi Ishibashi (Gerrit)
Kenichi Ishibashi added 3 comments![]( "Open in Gerrit")
ricea@: The 3rd CL for exposing DNS resolution details. This is the last one I want you to review before branch cut.
if (state_.connection()) {Kenichi IshibashiConsider adding `CHECK(load_timing_internal_info);` here before dereferencing the pointer, matching the pattern in `SpdyHttpStream::PopulateLoadTimingInternalInfo` (which currently protects from potential crashes if this is called with a null pointer).
(This is AI review comment, I'll address later)
std::optional<ResolutionDetails> GetResolutionDetails() const;Kenichi IshibashiThis header relies on the transitive inclusion of `net/dns/resolution_details.h` (via `stream_socket_handle.h`) to resolve `ResolutionDetails`. Consider including `net/dns/resolution_details.h` explicitly since it's used in the return type here.
(This is AI review comment, I'll address later)
Related details
- Adam Rice
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)
Andrew Williams added 4 comments![]( "Open in Gerrit")
Leaving some feedback
default_resolution_details_ = details;nit: std::move
// Returns the details of the host resolution if available.nit: can we elaborate on when this will return std::nullopt?
}It might be worth having a second stream test for SPDY as well
Related details
- Adam Rice
- Kenichi Ishibashi
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)
Andrew Williams added 1 comment![]( "Open in Gerrit")
void set_resolution_details(std::optional<ResolutionDetails> details) {optional: if you did want to mirror how set_connect_timing takes a reference as its parameter, this could use `base::optional_ref`
Kouhei Ueno (Gerrit)
Kouhei Ueno voted and added 1 comment![]( "Open in Gerrit")
Votes added by Kouhei Ueno
| Code-Review | +1 |
1 comment
if (spdy_session_) {newbie q: is PopulateLoadTimingInternalInfo called at most once? if that's not the case, with this logic, would the `load_timing_internal_info->resolution_details` get stale if the previous call filled it but was null on the second run?
Related details
- Adam Rice
- Kenichi Ishibashi
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. |