0

[Reland] Don't attempt to upgrade zero-suggest autocomplete inputs to HTTPS

This CL fixes an issue with the Defaulting Typed Navigations to HTTPS
feature. Consider the following scenario:

1. User types https://example.com
2. Navigation completes successfully
3. User clicks the omnibox and hits enter
4. Omnibox attempts to upgrade this navigation to HTTPS.

In step 3, OmniboxEditModel::StartZeroSuggestRequest() creates an
AutocompleteInput using its client's
ShouldDefaultTypedNavigationsToHttps() value and "example.com" as the
input text. This causes AutocompleteInput to enable the HTTP fallback
logic if the HTTPS navigation times out, thus potentially downgrading
the connection.

This CL fixes this by disabling HTTPS upgrades when starting a zero
suggest request.

Bug: 1251065
Change-Id: I70a00f00bd646058fd62d31e4da572971bde0dbb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3188350
Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/main@{#926926}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3203050
Cr-Original-Commit-Position: refs/heads/main@{#928315}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3370903
Cr-Commit-Position: refs/heads/main@{#956558}
This commit is contained in:
Mustafa Emre Acer
2022-01-07 17:28:18 +00:00
committed by Chromium LUCI CQ
parent b21cadd9f1
commit 6023d60f71
4 changed files with 88 additions and 1 deletions

@ -9,11 +9,14 @@
#include "base/command_line.h"
#include "base/feature_list.h"
#include "base/memory/raw_ptr.h"
#include "base/test/bind.h"
#include "build/build_config.h"
#include "build/chromeos_buildflags.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/external_protocol/external_protocol_handler.h"
#include "chrome/browser/interstitials/security_interstitial_page_test_utils.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "chrome/browser/ssl/typed_navigation_upgrade_throttle.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/browser_window.h"
@ -34,6 +37,8 @@
#include "content/public/browser/web_contents.h"
#include "content/public/common/url_constants.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/test_navigation_observer.h"
#include "content/public/test/url_loader_interceptor.h"
#include "third_party/blink/public/common/chrome_debug_urls.h"
#include "ui/accessibility/accessibility_switches.h"
#include "ui/accessibility/ax_action_data.h"
@ -129,6 +134,19 @@ class OmniboxViewViewsTest : public InProcessBrowserTest {
}
}
OmniboxView* omnibox() {
return browser()->window()->GetLocationBar()->GetOmniboxView();
}
void PressEnterAndWaitForNavigations(size_t num_expected_navigations) {
content::TestNavigationObserver navigation_observer(
browser()->tab_strip_model()->GetActiveWebContents(),
num_expected_navigations);
EXPECT_TRUE(ui_test_utils::SendKeyPressSync(browser(), ui::VKEY_RETURN,
false, false, false, false));
navigation_observer.Wait();
}
private:
// InProcessBrowserTest:
void SetUpOnMainThread() override {
@ -989,3 +1007,69 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewViewsTest, MAYBE_HandleExternalProtocolURLs) {
#endif // !defined(OS_MAC) || defined(USE_AURA)
}
// SendKeyPressSync times out on Mac, probably due to https://crbug.com/824418.
#if defined(OS_MAC)
#define MAYBE_DefaultTypedNavigationsToHttps_ZeroSuggest_NoUpgrade \
DISABLED_DefaultTypedNavigationsToHttps_ZeroSuggest_NoUpgrade
#else
#define MAYBE_DefaultTypedNavigationsToHttps_ZeroSuggest_NoUpgrade \
DefaultTypedNavigationsToHttps_ZeroSuggest_NoUpgrade
#endif
// Test that triggers a zero suggest autocomplete request by clicking on the
// omnibox. These should never attempt an https upgrade or involve the typed
// navigation upgrade throttle.
// This is a regression test for https://crbug.com/1251065
IN_PROC_BROWSER_TEST_F(
OmniboxViewViewsTest,
MAYBE_DefaultTypedNavigationsToHttps_ZeroSuggest_NoUpgrade) {
// Since the embedded test server only works for URLs with non-default ports,
// use a URLLoaderInterceptor to mimic port-free operation. This allows the
// rest of the test to operate as if all URLs are using the default ports.
content::URLLoaderInterceptor interceptor(base::BindLambdaForTesting(
[&](content::URLLoaderInterceptor::RequestParams* params) {
if (params->url_request.url.host() == "site-with-good-https.com") {
std::string headers =
"HTTP/1.1 200 OK\nContent-Type: text/html; charset=utf-8\n";
std::string body = "<html><title>Success</title>Hello world</html>";
content::URLLoaderInterceptor::WriteResponse(headers, body,
params->client.get());
return true;
}
// Not handled by us.
return false;
}));
base::HistogramTester histograms;
const GURL url("https://site-with-good-https.com");
// Type "https://site-with-good-https.com". This should load fine without
// hitting TypedNavigationUpgradeThrottle.
omnibox()->SetUserText(base::UTF8ToUTF16(url.spec()), true);
PressEnterAndWaitForNavigations(1);
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
EXPECT_EQ(url, contents->GetLastCommittedURL());
EXPECT_FALSE(chrome_browser_interstitials::IsShowingInterstitial(contents));
histograms.ExpectTotalCount(TypedNavigationUpgradeThrottle::kHistogramName,
0);
ui_test_utils::HistoryEnumerator enumerator(browser()->profile());
EXPECT_TRUE(base::Contains(enumerator.urls(), url));
// Now click the omnibox. This should trigger a zero suggest request with the
// text "site-with-good-https.com" despite the omnibox URL being
// https://site-with-good-https.com. Autocomplete input class shouldn't try
// to upgrade this request.
const gfx::Rect omnibox_bounds =
BrowserView::GetBrowserViewForBrowser(browser())
->GetViewByID(VIEW_ID_OMNIBOX)
->GetBoundsInScreen();
const gfx::Point click_location = omnibox_bounds.CenterPoint();
ASSERT_NO_FATAL_FAILURE(
Click(ui_controls::LEFT, click_location, click_location));
PressEnterAndWaitForNavigations(1);
histograms.ExpectTotalCount(TypedNavigationUpgradeThrottle::kHistogramName,
0);
}

@ -1202,9 +1202,10 @@ void OmniboxEditModel::StartZeroSuggestRequest(
// Send the textfield contents exactly as-is, as otherwise the verbatim
// match can be wrong. The full page URL is anyways in set_current_url().
// Don't attempt to use https as the default scheme for these requests.
input_ = AutocompleteInput(view_->GetText(), GetPageClassification(),
client_->GetSchemeClassifier(),
client_->ShouldDefaultTypedNavigationsToHttps(),
/*should_use_https_as_default_scheme=*/false,
client_->GetHttpsPortForTesting());
input_.set_current_url(client_->GetURL());
input_.set_current_title(client_->GetTitle());

@ -15,6 +15,7 @@
-ExtensionApiTest.WindowOpenFocus
-MenuViewDragAndDropTestNestedDrag.MenuViewDragAndDropNestedDrag
-MenuViewDragAndDropTestTestInMenuDrag.TestInMenuDrag
-OmniboxViewViewsTest.DefaultTypedNavigationsToHttps_ZeroSuggest_NoUpgrade
-OmniboxViewViewsTest.SelectionClipboard
-SameSiteSubframe*
-SitePerProcessInteractiveBrowserTest.TabAndMouseFocusNavigation

@ -28,6 +28,7 @@
-KeyboardLockInteractiveBrowserTest.ActiveWithSomeKeysLocked
-MediaDialogViewBrowserTest.PictureInPicture
-NotificationsTestWithFakeMediaStream.ShouldQueueDuringScreenPresent
-OmniboxViewViewsTest.DefaultTypedNavigationsToHttps_ZeroSuggest_NoUpgrade
-OmniboxViewViewsTest.SelectionClipboard
-PasswordBubbleInteractiveUiTest.AutoSigninNoFocus
-PopupBlockerBrowserTest.ModalPopUnder