Commit 332f7d52 authored by Joe DeBlasio's avatar Joe DeBlasio Committed by Commit Bot

Reland "Hide URL from lookalike URL warning interstitial."

This is a reland of c9078153
which was reverted because it triggered a bug fixed in
http://crrev.com/c/1483732 .

Original change's description:
> Hide URL from lookalike URL warning interstitial.
>
> This CL adds support for hiding of URLs in the omnibox by security
> interstitials so as to enable it for the lookalike URL interstitial.
>
> Bug: 927924
> Change-Id: I2a7c7311480d6626d2ef27572e326c0a473dc20b
> Reviewed-on: https://chromium-review.googlesource.com/c/1478358
> Commit-Queue: Joe DeBlasio <jdeblasio@chromium.org>
> Reviewed-by: Michael Wasserman <msw@chromium.org>
> Reviewed-by: Carlos IL <carlosil@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#634067}

Bug: 927924
Change-Id: I14e5db4d7eec03b060df25569fa38ce1cd1e8522
Reviewed-on: https://chromium-review.googlesource.com/c/1482940Reviewed-by: default avatarEmily Stark <estark@chromium.org>
Reviewed-by: default avatarMichael Wasserman <msw@chromium.org>
Commit-Queue: Joe DeBlasio <jdeblasio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635221}
parent 8285be72
...@@ -73,6 +73,10 @@ void LookalikeUrlInterstitialPage::PopulateInterstitialStrings( ...@@ -73,6 +73,10 @@ void LookalikeUrlInterstitialPage::PopulateInterstitialStrings(
void LookalikeUrlInterstitialPage::OnInterstitialClosing() {} void LookalikeUrlInterstitialPage::OnInterstitialClosing() {}
bool LookalikeUrlInterstitialPage::ShouldDisplayURL() const {
return false;
}
// This handles the commands sent from the interstitial JavaScript. // This handles the commands sent from the interstitial JavaScript.
void LookalikeUrlInterstitialPage::CommandReceived(const std::string& command) { void LookalikeUrlInterstitialPage::CommandReceived(const std::string& command) {
if (command == "\"pageLoadComplete\"") { if (command == "\"pageLoadComplete\"") {
......
...@@ -45,6 +45,7 @@ class LookalikeUrlInterstitialPage ...@@ -45,6 +45,7 @@ class LookalikeUrlInterstitialPage
void PopulateInterstitialStrings( void PopulateInterstitialStrings(
base::DictionaryValue* load_time_data) override; base::DictionaryValue* load_time_data) override;
void OnInterstitialClosing() override; void OnInterstitialClosing() override;
bool ShouldDisplayURL() const override;
int GetHTMLTemplateId() override; int GetHTMLTemplateId() override;
private: private:
......
...@@ -187,6 +187,7 @@ class LookalikeUrlNavigationThrottleBrowserTest ...@@ -187,6 +187,7 @@ class LookalikeUrlNavigationThrottleBrowserTest
NavigateToURL(browser, navigated_url); NavigateToURL(browser, navigated_url);
navigation_observer.Wait(); navigation_observer.Wait();
EXPECT_EQ(nullptr, GetCurrentInterstitial(web_contents)); EXPECT_EQ(nullptr, GetCurrentInterstitial(web_contents));
EXPECT_TRUE(IsUrlShowing(browser));
} }
{ {
// Navigate to an empty page. This will happen after any // Navigate to an empty page. This will happen after any
...@@ -196,6 +197,7 @@ class LookalikeUrlNavigationThrottleBrowserTest ...@@ -196,6 +197,7 @@ class LookalikeUrlNavigationThrottleBrowserTest
NavigateToURL(browser, GURL("about:blank")); NavigateToURL(browser, GURL("about:blank"));
navigation_observer.Wait(); navigation_observer.Wait();
EXPECT_EQ(nullptr, GetCurrentInterstitial(web_contents)); EXPECT_EQ(nullptr, GetCurrentInterstitial(web_contents));
EXPECT_TRUE(IsUrlShowing(browser));
} }
} }
...@@ -215,6 +217,7 @@ class LookalikeUrlNavigationThrottleBrowserTest ...@@ -215,6 +217,7 @@ class LookalikeUrlNavigationThrottleBrowserTest
EXPECT_EQ(LookalikeUrlInterstitialPage::kTypeForTesting, EXPECT_EQ(LookalikeUrlInterstitialPage::kTypeForTesting,
GetInterstitialType(web_contents)); GetInterstitialType(web_contents));
EXPECT_FALSE(IsUrlShowing(browser));
} }
// Tests that the histogram event |expected_event| is recorded. If the UI is // Tests that the histogram event |expected_event| is recorded. If the UI is
...@@ -297,8 +300,12 @@ class LookalikeUrlNavigationThrottleBrowserTest ...@@ -297,8 +300,12 @@ class LookalikeUrlNavigationThrottleBrowserTest
base::SimpleTestClock* test_clock() { return &test_clock_; } base::SimpleTestClock* test_clock() { return &test_clock_; }
private: protected:
bool ui_enabled() const { return GetParam() == UIEnabled::kEnabled; } virtual bool ui_enabled() const { return GetParam() == UIEnabled::kEnabled; }
static bool IsUrlShowing(Browser* browser) {
return !browser->location_bar_model()->GetFormattedFullURL().empty();
}
// Simulates a link click navigation. We don't use // Simulates a link click navigation. We don't use
// ui_test_utils::NavigateToURL(const GURL&) because it simulates the user // ui_test_utils::NavigateToURL(const GURL&) because it simulates the user
...@@ -327,6 +334,7 @@ class LookalikeUrlNavigationThrottleBrowserTest ...@@ -327,6 +334,7 @@ class LookalikeUrlNavigationThrottleBrowserTest
browser->tab_strip_model()->GetActiveWebContents(); browser->tab_strip_model()->GetActiveWebContents();
EXPECT_EQ(nullptr, GetCurrentInterstitial(web_contents)); EXPECT_EQ(nullptr, GetCurrentInterstitial(web_contents));
EXPECT_TRUE(IsUrlShowing(browser));
{ {
content::TestNavigationObserver navigation_observer(web_contents, 1); content::TestNavigationObserver navigation_observer(web_contents, 1);
NavigateToURL(browser, navigated_url); NavigateToURL(browser, navigated_url);
...@@ -335,6 +343,7 @@ class LookalikeUrlNavigationThrottleBrowserTest ...@@ -335,6 +343,7 @@ class LookalikeUrlNavigationThrottleBrowserTest
EXPECT_EQ(LookalikeUrlInterstitialPage::kTypeForTesting, EXPECT_EQ(LookalikeUrlInterstitialPage::kTypeForTesting,
GetInterstitialType(web_contents)); GetInterstitialType(web_contents));
EXPECT_FALSE(IsUrlShowing(browser));
// Clicking the link in the interstitial should remove the interstitial and // Clicking the link in the interstitial should remove the interstitial and
// navigate to the suggested URL. // navigate to the suggested URL.
...@@ -347,6 +356,7 @@ class LookalikeUrlNavigationThrottleBrowserTest ...@@ -347,6 +356,7 @@ class LookalikeUrlNavigationThrottleBrowserTest
EXPECT_EQ(nullptr, GetCurrentInterstitial(web_contents)); EXPECT_EQ(nullptr, GetCurrentInterstitial(web_contents));
EXPECT_EQ(expected_suggested_url, web_contents->GetURL()); EXPECT_EQ(expected_suggested_url, web_contents->GetURL());
EXPECT_TRUE(IsUrlShowing(browser));
// Clicking the link in the interstitial should also remove the original URL // Clicking the link in the interstitial should also remove the original URL
// from history. // from history.
...@@ -376,6 +386,7 @@ class LookalikeUrlNavigationThrottleBrowserTest ...@@ -376,6 +386,7 @@ class LookalikeUrlNavigationThrottleBrowserTest
EXPECT_EQ(LookalikeUrlInterstitialPage::kTypeForTesting, EXPECT_EQ(LookalikeUrlInterstitialPage::kTypeForTesting,
GetInterstitialType(web_contents)); GetInterstitialType(web_contents));
EXPECT_FALSE(IsUrlShowing(browser));
// Clicking the ignore button in the interstitial should remove the // Clicking the ignore button in the interstitial should remove the
// interstitial and navigate to the original URL. // interstitial and navigate to the original URL.
...@@ -388,6 +399,7 @@ class LookalikeUrlNavigationThrottleBrowserTest ...@@ -388,6 +399,7 @@ class LookalikeUrlNavigationThrottleBrowserTest
EXPECT_EQ(nullptr, GetCurrentInterstitial(web_contents)); EXPECT_EQ(nullptr, GetCurrentInterstitial(web_contents));
EXPECT_EQ(navigated_url, web_contents->GetURL()); EXPECT_EQ(navigated_url, web_contents->GetURL());
EXPECT_TRUE(IsUrlShowing(browser));
// Clicking the link should result in the original URL appearing in history. // Clicking the link should result in the original URL appearing in history.
ui_test_utils::HistoryEnumerator enumerator(browser->profile()); ui_test_utils::HistoryEnumerator enumerator(browser->profile());
...@@ -399,6 +411,54 @@ class LookalikeUrlNavigationThrottleBrowserTest ...@@ -399,6 +411,54 @@ class LookalikeUrlNavigationThrottleBrowserTest
base::SimpleTestClock test_clock_; base::SimpleTestClock test_clock_;
}; };
class LookalikeUrlInterstitialPageBrowserTest
: public LookalikeUrlNavigationThrottleBrowserTest {
public:
// Checks that navigating to |navigated_url| displays an interstitial with a
// hidden URL, and then navigating to other pages shows/hides the URLs
// appropriately both when a URL is expected to be hidden (by navigating to
// "chrome://newtab") and when expected to be shown (by navigating to
// |subsequent_url|).
static void TestInterstitialHidesUrlThenRestores(Browser* browser,
const GURL& navigated_url,
const GURL& subsequent_url) {
content::WebContents* web_contents =
browser->tab_strip_model()->GetActiveWebContents();
EXPECT_EQ(nullptr, GetCurrentInterstitial(web_contents));
EXPECT_TRUE(IsUrlShowing(browser));
{
content::TestNavigationObserver navigation_observer(web_contents, 1);
NavigateToURL(browser, navigated_url);
navigation_observer.Wait();
}
EXPECT_EQ(LookalikeUrlInterstitialPage::kTypeForTesting,
GetInterstitialType(web_contents));
EXPECT_FALSE(IsUrlShowing(browser));
{
content::TestNavigationObserver navigation_observer(web_contents, 1);
ui_test_utils::NavigateToURL(browser, subsequent_url);
navigation_observer.Wait();
}
EXPECT_EQ(nullptr, GetCurrentInterstitial(web_contents));
EXPECT_TRUE(IsUrlShowing(browser));
{
content::TestNavigationObserver navigation_observer(web_contents, 1);
ui_test_utils::NavigateToURL(browser, GURL("chrome://newtab"));
navigation_observer.Wait();
}
EXPECT_FALSE(IsUrlShowing(browser));
}
protected:
bool ui_enabled() const override { return true; }
};
INSTANTIATE_TEST_SUITE_P(, INSTANTIATE_TEST_SUITE_P(,
LookalikeUrlNavigationThrottleBrowserTest, LookalikeUrlNavigationThrottleBrowserTest,
::testing::Values(UIEnabled::kDisabled, ::testing::Values(UIEnabled::kDisabled,
...@@ -731,3 +791,12 @@ IN_PROC_BROWSER_TEST_P(LookalikeUrlNavigationThrottleBrowserTest, ...@@ -731,3 +791,12 @@ IN_PROC_BROWSER_TEST_P(LookalikeUrlNavigationThrottleBrowserTest,
TestOnlyInterstitialShown(browser(), kNavigatedUrl); TestOnlyInterstitialShown(browser(), kNavigatedUrl);
} }
} }
// Verify that the URL shows normally on pages after a lookalike interstitial.
IN_PROC_BROWSER_TEST_F(LookalikeUrlInterstitialPageBrowserTest,
UrlShownAfterInterstitial) {
const GURL kNavigatedUrl = GetURL("goooglé.com");
const GURL kSubsequentUrl = GetURL("example.com");
TestInterstitialHidesUrlThenRestores(browser(), kNavigatedUrl,
kSubsequentUrl);
}
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
#include "components/offline_pages/buildflags/buildflags.h" #include "components/offline_pages/buildflags/buildflags.h"
#include "components/omnibox/browser/autocomplete_input.h" #include "components/omnibox/browser/autocomplete_input.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "components/security_interstitials/content/security_interstitial_tab_helper.h"
#include "components/security_state/core/security_state.h" #include "components/security_state/core/security_state.h"
#include "content/public/browser/navigation_controller.h" #include "content/public/browser/navigation_controller.h"
#include "content/public/browser/navigation_entry.h" #include "content/public/browser/navigation_entry.h"
...@@ -93,6 +94,12 @@ bool ChromeLocationBarModelDelegate::ShouldDisplayURL() const { ...@@ -93,6 +94,12 @@ bool ChromeLocationBarModelDelegate::ShouldDisplayURL() const {
if (!entry) if (!entry)
return true; return true;
security_interstitials::SecurityInterstitialTabHelper* tab_helper =
security_interstitials::SecurityInterstitialTabHelper::FromWebContents(
GetActiveWebContents());
if (tab_helper && tab_helper->IsDisplayingInterstitial())
return tab_helper->ShouldDisplayURL();
if (entry->IsViewSourceMode() || if (entry->IsViewSourceMode() ||
entry->GetPageType() == content::PAGE_TYPE_INTERSTITIAL) { entry->GetPageType() == content::PAGE_TYPE_INTERSTITIAL) {
return true; return true;
......
...@@ -66,6 +66,10 @@ void SecurityInterstitialPage::DontCreateViewForTesting() { ...@@ -66,6 +66,10 @@ void SecurityInterstitialPage::DontCreateViewForTesting() {
create_view_ = false; create_view_ = false;
} }
bool SecurityInterstitialPage::ShouldDisplayURL() const {
return true;
}
std::string SecurityInterstitialPage::GetHTMLContents() { std::string SecurityInterstitialPage::GetHTMLContents() {
base::DictionaryValue load_time_data; base::DictionaryValue load_time_data;
PopulateInterstitialStrings(&load_time_data); PopulateInterstitialStrings(&load_time_data);
......
...@@ -50,6 +50,10 @@ class SecurityInterstitialPage : public content::InterstitialPageDelegate { ...@@ -50,6 +50,10 @@ class SecurityInterstitialPage : public content::InterstitialPageDelegate {
// to e.g. update metrics. // to e.g. update metrics.
virtual void OnInterstitialClosing() = 0; virtual void OnInterstitialClosing() = 0;
// Whether a URL should be displayed on this interstitial page. This is
// respected by committed interstitials only.
virtual bool ShouldDisplayURL() const;
protected: protected:
// Returns true if the interstitial should create a new navigation entry. // Returns true if the interstitial should create a new navigation entry.
virtual bool ShouldCreateNewNavigation() const = 0; virtual bool ShouldCreateNewNavigation() const = 0;
......
...@@ -60,6 +60,15 @@ void SecurityInterstitialTabHelper::AssociateBlockingPage( ...@@ -60,6 +60,15 @@ void SecurityInterstitialTabHelper::AssociateBlockingPage(
helper->SetBlockingPage(navigation_id, std::move(blocking_page)); helper->SetBlockingPage(navigation_id, std::move(blocking_page));
} }
bool SecurityInterstitialTabHelper::ShouldDisplayURL() const {
CHECK(IsDisplayingInterstitial());
return blocking_page_for_currently_committed_navigation_->ShouldDisplayURL();
}
bool SecurityInterstitialTabHelper::IsDisplayingInterstitial() const {
return blocking_page_for_currently_committed_navigation_ != nullptr;
}
security_interstitials::SecurityInterstitialPage* security_interstitials::SecurityInterstitialPage*
SecurityInterstitialTabHelper:: SecurityInterstitialTabHelper::
GetBlockingPageForCurrentlyCommittedNavigationForTesting() { GetBlockingPageForCurrentlyCommittedNavigationForTesting() {
......
...@@ -43,6 +43,12 @@ class SecurityInterstitialTabHelper ...@@ -43,6 +43,12 @@ class SecurityInterstitialTabHelper
std::unique_ptr<security_interstitials::SecurityInterstitialPage> std::unique_ptr<security_interstitials::SecurityInterstitialPage>
blocking_page); blocking_page);
// Determines whether a URL should be shown on the current navigation page.
bool ShouldDisplayURL() const;
// Whether this tab helper is tracking a currently-displaying interstitial.
bool IsDisplayingInterstitial() const;
security_interstitials::SecurityInterstitialPage* security_interstitials::SecurityInterstitialPage*
GetBlockingPageForCurrentlyCommittedNavigationForTesting(); GetBlockingPageForCurrentlyCommittedNavigationForTesting();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment