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

Issue 618659 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug
Proj-Servicification

Blocking:
issue 576270
issue 611154



Sign in to add a comment

DidStartProvisionalLoad sent with un-upgraded URL (http vs https).

Project Member Reported by carlosk@chromium.org, Jun 9 2016

Issue description

I recently discovered that URL upgrades happen after the start of the provisional load is notified to the browser. This caused the URL stored by the NavigationHandle to be outdated, a problem that I will be fixing right away.

But there is a broader decision to made about making the URL upgrade call happen before the provisional load start notification is sent to the browser. So I'm creating this issue so that we can make this discussion public.
 
Blocking: 611154
After an offline meeting during BlinkOn this is the agreed course of action:
- For now we'll just run the HTTP upgrade code code before the call to dispatchDidStartProvisionalLoad in FrameLoader::startLoad.
- Later on we'll see about doing "the right thing" that seems to be running the upgrade code in the browser, what was already part of PlzNavigate's plan.
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 9 2016

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

commit 2ece5a75956f9df5c0a78939a8f06d4e09b6e860
Author: carlosk <carlosk@chromium.org>
Date: Sat Jul 09 19:35:59 2016

Executed HTTPS upgrade before notifying the start of the provisional load.

This makes so that the fetch URL sent to the browser process for
navigations with the start provisional load notification is already the
final, potentially HTTPS upgraded one. This caused some problems with
upcoming changes where the URL stored in the NavigationHandle differed
from the actual URL being navigated to. A new test is also added to confirm
the upgrade is executed as expected.

BUG= 618659 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/2ece5a75956f9df5c0a78939a8f06d4e09b6e860/content/browser/frame_host/navigation_handle_impl_browsertest.cc
[add] https://crrev.com/2ece5a75956f9df5c0a78939a8f06d4e09b6e860/content/test/data/https_upgrade_cross_site.html
[add] https://crrev.com/2ece5a75956f9df5c0a78939a8f06d4e09b6e860/content/test/data/https_upgrade_same_site.html
[modify] https://crrev.com/2ece5a75956f9df5c0a78939a8f06d4e09b6e860/testing/buildbot/filters/browser-side-navigation.linux.content_browsertests.filter
[modify] https://crrev.com/2ece5a75956f9df5c0a78939a8f06d4e09b6e860/third_party/WebKit/Source/core/fetch/FetchContext.cpp
[modify] https://crrev.com/2ece5a75956f9df5c0a78939a8f06d4e09b6e860/third_party/WebKit/Source/core/fetch/FetchContext.h
[modify] https://crrev.com/2ece5a75956f9df5c0a78939a8f06d4e09b6e860/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp
[modify] https://crrev.com/2ece5a75956f9df5c0a78939a8f06d4e09b6e860/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
[modify] https://crrev.com/2ece5a75956f9df5c0a78939a8f06d4e09b6e860/third_party/WebKit/Source/core/loader/DocumentLoader.h
[modify] https://crrev.com/2ece5a75956f9df5c0a78939a8f06d4e09b6e860/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/2ece5a75956f9df5c0a78939a8f06d4e09b6e860/third_party/WebKit/Source/core/loader/FrameFetchContext.h
[modify] https://crrev.com/2ece5a75956f9df5c0a78939a8f06d4e09b6e860/third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp
[modify] https://crrev.com/2ece5a75956f9df5c0a78939a8f06d4e09b6e860/third_party/WebKit/Source/core/loader/FrameLoader.cpp

Status: Fixed (was: Assigned)
With the patch that just landed this is now fixed. The 2nd step described in #2 will be executed with issue 576271 is done.

Comment 5 by laforge@google.com, Nov 7 2017

Components: Internals>Network>Service

Comment 6 by laforge@google.com, Nov 7 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.

Sign in to add a comment