New issue
Advanced search Search tips

Issue 845648 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug
Proj-Servicification

Blocked on:
issue 821021

Blocking:
issue 809583



Sign in to add a comment

content::PreresolveUrl should return net::HostResolver::Request to a caller

Project Member Reported by alexilin@chromium.org, May 22 2018

Issue description

content::PreresolveUrl() stores a net::HostResolver::Request inside of the CompletionCallback, but the callback itself is stored inside of the request so the request just leaks if it's not resolved. content::PreresolveUrl() should be refactored to pass the request ownership to a caller.
 
Blocking: 809583
Blockedon: 821021
Cc: ericorth@chromium.org
PresolveUrl() accesses the HostResolver through net::URLRequestContext.
With network service, the browser process doesn't have access to net::URLRequestContext. 
+ericorth@ is working on mojo APIs for the HostResolver. Once that's ready, PresolveUrl() can be migrated to using it. 

Comment 3 by mmenke@chromium.org, May 22 2018

Components: Internals>Services>Network

Comment 4 by dxie@chromium.org, May 29 2018

Labels: Proj-Servicification-Canary
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/49527403bad77a77794d6dd669251f7343ad5818

commit 49527403bad77a77794d6dd669251f7343ad5818
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Wed Jun 06 12:45:12 2018

predictors: Return HostResolver::Request from content::PreresolveUrl()

This CL changes the signature of content::PreresolveUrl() so that the
method returns a net::HostResolver::Request object to a caller instead
of holding it inside of the callback state. It is done to guarantee
that there won't be hanging requests after a caller is shut down.

There are only two callers of this method:
chrome_browser_net::Predictor and predictors::PreconnectManager.
They become responsible for keeping Request objects alive.

Bug:  845648 
Change-Id: Ie057cde65288341b14813d4df4d7d90a02f588b6
Reviewed-on: https://chromium-review.googlesource.com/1080818
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Benoit L <lizeb@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564866}
[modify] https://crrev.com/49527403bad77a77794d6dd669251f7343ad5818/chrome/browser/net/predictor.cc
[modify] https://crrev.com/49527403bad77a77794d6dd669251f7343ad5818/chrome/browser/net/url_info.cc
[modify] https://crrev.com/49527403bad77a77794d6dd669251f7343ad5818/chrome/browser/net/url_info.h
[modify] https://crrev.com/49527403bad77a77794d6dd669251f7343ad5818/chrome/browser/predictors/preconnect_manager.cc
[modify] https://crrev.com/49527403bad77a77794d6dd669251f7343ad5818/chrome/browser/predictors/preconnect_manager.h
[modify] https://crrev.com/49527403bad77a77794d6dd669251f7343ad5818/chrome/browser/predictors/preconnect_manager_unittest.cc
[modify] https://crrev.com/49527403bad77a77794d6dd669251f7343ad5818/content/browser/loader/resource_hints_impl.cc
[modify] https://crrev.com/49527403bad77a77794d6dd669251f7343ad5818/content/public/browser/resource_hints.h

Status: Fixed (was: Assigned)
I think this issue is resolved. I've created a new bug to track the migration to mojo API for the HostResolver:  issue 850066 

Sign in to add a comment