New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 721981 link

Starred by 1 user
Status: Fixed
Owner:
█ xunji...@chromium.org
Last visit > 30 days ago
Closed: May 2017
Cc:
█ ramyan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocking:
issue 475060



Sign in to add a comment

Network Predictor passes empty urls to HttpStreamFactory::PreconnectStreams

Project Member Reported by xunji...@chromium.org, May 13 2017 
Predictor::PreconnectUrlAndSubresources() is called with url = "chrome-native://newtab/". 
Predictor::CanonicalizeUrl(url) will return an empty GURL because chrome-native:// is neither http nor https. Predictor then passes that empty url to net::HttpStreamFactory::PreconnectStreams(). crrev.com/2882763002 is a CL that shows this behavior when running NTP Java tests on linux_android_rel_ng bot (the DCHECK(!canonicalized_url.is_empty()) will fail).

Passing an empty url to HttpStreamFactory is bad, because HttpStreamFactory layer and lower assume that the url is valid.

Not sure why we are preconnecting to chrome-native://newtab, but we should fix Predictor to not pass along empty urls. 

This is blocking a refactoring that I am doing, so I will upload a patch.
Project Member

Comment 1 by bugdroid1@chromium.org, May 15 2017 

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

commit cb7c661c1e2dbeb007a02832f9305ebc7199b845
Author: xunjieli <xunjieli@chromium.org>
Date: Mon May 15 18:26:05 2017

Make Network Predictor skip empty urls

Predictor::CanonicalizeUrl() can return empty urls. This causes
HttpStreamFactoryImpl::PreconnectStream() to preconnect empty urls, which
is bad because HttpStreamFactoryImpl layer and lower (e.g. proxy resolution)
assume that the urls are valid.

This CL makes Predictor to skip empty urls.

BUG= 721981 

Review-Url: https://codereview.chromium.org/2871323006
Cr-Commit-Position: refs/heads/master@{#471837}

[modify] https://crrev.com/cb7c661c1e2dbeb007a02832f9305ebc7199b845/chrome/browser/net/predictor.cc
[modify] https://crrev.com/cb7c661c1e2dbeb007a02832f9305ebc7199b845/chrome/browser/net/predictor_browsertest.cc
[modify] https://crrev.com/cb7c661c1e2dbeb007a02832f9305ebc7199b845/net/http/http_stream_factory_impl.cc

Comment 2 by xunji...@chromium.org, May 15 2017

Blocking: 475060
Status: Fixed (was: Assigned)
►

Sign in to add a comment