Issue metadata
Sign in to add a comment
|
DidStartProvisionalLoad sent with un-upgraded URL (http vs https). |
||||||||||||||||||||||||
Issue descriptionI 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.
,
Jun 24 2016
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.
,
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
,
Jul 11 2016
With the patch that just landed this is now fixed. The 2nd step described in #2 will be executed with issue 576271 is done.
,
Nov 7 2017
,
Nov 7 2017
Apologies, applied the wrong component in bulk. |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by alex...@chromium.org
, Jun 10 2016