Commit 10545a81 authored by Joe DeBlasio's avatar Joe DeBlasio Committed by Commit Bot

Reland "Update CCT to hide origin if requested by security interstitial."

This is a reland of ff13eb07 with a minor
tweak to prevent a flaky test.

Original change's description:
> Update CCT to hide origin if requested by security interstitial.
>
> This CL modifies the behavior of CCTs such that they hide the
> origin of a page and page info button if there is an active security
> interstitial that requests it. This brings the behavior in line with
> that of desktop and mobile.
>
> Bug: 939601
> Change-Id: I6c46cb4918871b9d2a760a72bee742fc7867e1b7
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1570415
> Reviewed-by: Jay Harris <harrisjay@chromium.org>
> Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
> Commit-Queue: Joe DeBlasio <jdeblasio@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#653038}

Bug: 939601
Change-Id: I9fd61d7689bcec23723ee2d7ecd6520ae2c201da
TBR: harrisjay@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1579915
Commit-Queue: Joe DeBlasio <jdeblasio@chromium.org>
Reviewed-by: default avatarMustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#654680}
parent 6ef82b87
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "chrome/browser/ui/views/chrome_typography.h" #include "chrome/browser/ui/views/chrome_typography.h"
#include "chrome/browser/ui/views/frame/browser_view.h" #include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/frame/hosted_app_button_container.h" #include "chrome/browser/ui/views/frame/hosted_app_button_container.h"
#include "components/security_interstitials/content/security_interstitial_tab_helper.h"
#include "components/url_formatter/url_formatter.h" #include "components/url_formatter/url_formatter.h"
#include "components/vector_icons/vector_icons.h" #include "components/vector_icons/vector_icons.h"
#include "content/public/browser/navigation_controller.h" #include "content/public/browser/navigation_controller.h"
...@@ -77,6 +78,15 @@ views::ImageButton* CreateCloseButton(views::ButtonListener* listener, ...@@ -77,6 +78,15 @@ views::ImageButton* CreateCloseButton(views::ButtonListener* listener,
return close_button; return close_button;
} }
bool ShouldDisplayUrl(content::WebContents* contents) {
auto* tab_helper =
security_interstitials::SecurityInterstitialTabHelper::FromWebContents(
contents);
if (tab_helper && tab_helper->IsDisplayingInterstitial())
return tab_helper->ShouldDisplayURL();
return true;
}
} // namespace } // namespace
// Container view for laying out and rendering the title/origin of the current // Container view for laying out and rendering the title/origin of the current
...@@ -119,6 +129,7 @@ class CustomTabBarTitleOriginView : public views::View { ...@@ -119,6 +129,7 @@ class CustomTabBarTitleOriginView : public views::View {
void Update(base::string16 title, base::string16 location) { void Update(base::string16 title, base::string16 location) {
title_label_->SetText(title); title_label_->SetText(title);
location_label_->SetText(location); location_label_->SetText(location);
location_label_->SetVisible(!location.empty());
} }
int GetMinimumWidth() const { int GetMinimumWidth() const {
...@@ -147,6 +158,10 @@ class CustomTabBarTitleOriginView : public views::View { ...@@ -147,6 +158,10 @@ class CustomTabBarTitleOriginView : public views::View {
return preferred_size; return preferred_size;
} }
bool IsShowingOriginForTesting() const {
return location_label_ != nullptr && location_label_->visible();
}
private: private:
views::Label* title_label_; views::Label* title_label_;
views::Label* location_label_; views::Label* location_label_;
...@@ -215,15 +230,19 @@ void CustomTabBarView::TabChangedAt(content::WebContents* contents, ...@@ -215,15 +230,19 @@ void CustomTabBarView::TabChangedAt(content::WebContents* contents,
base::string16 title, location; base::string16 title, location;
if (entry) { if (entry) {
title = Browser::FormatTitleForDisplay(entry->GetTitleForDisplay()); title = Browser::FormatTitleForDisplay(entry->GetTitleForDisplay());
location = url_formatter::FormatUrl(entry->GetVirtualURL().GetOrigin(), if (ShouldDisplayUrl(contents))
url_formatter::kFormatUrlOmitDefaults, location = url_formatter::FormatUrl(entry->GetVirtualURL().GetOrigin(),
net::UnescapeRule::NORMAL, nullptr, url_formatter::kFormatUrlOmitDefaults,
nullptr, nullptr); net::UnescapeRule::NORMAL, nullptr,
nullptr, nullptr);
} }
title_origin_view_->Update(title, location); title_origin_view_->Update(title, location);
location_icon_view_->Update(/*suppress animations = */ false); location_icon_view_->Update(/*suppress animations = */ false);
// Hide location icon if we're already hiding the origin.
location_icon_view_->SetVisible(!location.empty());
last_title_ = title; last_title_ = title;
last_location_ = location; last_location_ = location;
...@@ -364,3 +383,8 @@ void CustomTabBarView::GoBackToApp() { ...@@ -364,3 +383,8 @@ void CustomTabBarView::GoBackToApp() {
// Otherwise, go back to the first in scope url. // Otherwise, go back to the first in scope url.
controller.GoToOffset(offset); controller.GoToOffset(offset);
} }
bool CustomTabBarView::IsShowingOriginForTesting() const {
return title_origin_view_ != nullptr &&
title_origin_view_->IsShowingOriginForTesting();
}
...@@ -75,6 +75,7 @@ class CustomTabBarView : public views::AccessiblePaneView, ...@@ -75,6 +75,7 @@ class CustomTabBarView : public views::AccessiblePaneView,
base::string16 location_for_testing() const { return last_location_; } base::string16 location_for_testing() const { return last_location_; }
views::Button* close_button_for_testing() const { return close_button_; } views::Button* close_button_for_testing() const { return close_button_; }
void GoBackToAppForTesting(); void GoBackToAppForTesting();
bool IsShowingOriginForTesting() const;
private: private:
// Takes the web contents for the custom tab bar back to the app scope. // Takes the web contents for the custom tab bar back to the app scope.
......
...@@ -14,7 +14,12 @@ ...@@ -14,7 +14,12 @@
#include "chrome/common/web_application_info.h" #include "chrome/common/web_application_info.h"
#include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h" #include "chrome/test/base/ui_test_utils.h"
#include "components/security_interstitials/content/security_interstitial_controller_client.h"
#include "components/security_interstitials/content/security_interstitial_page.h"
#include "components/security_interstitials/content/security_interstitial_tab_helper.h"
#include "components/security_interstitials/core/metrics_helper.h"
#include "content/public/browser/navigation_entry.h" #include "content/public/browser/navigation_entry.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/notification_service.h" #include "content/public/browser/notification_service.h"
#include "content/public/test/browser_test_utils.h" #include "content/public/test/browser_test_utils.h"
#include "content/public/test/test_navigation_observer.h" #include "content/public/test/test_navigation_observer.h"
...@@ -109,6 +114,54 @@ void SetTitleAndLocation(content::WebContents* web_contents, ...@@ -109,6 +114,54 @@ void SetTitleAndLocation(content::WebContents* web_contents,
title_observer.Wait(); title_observer.Wait();
} }
// An interstitial page that requests URL hiding
class UrlHidingInterstitialPage
: public security_interstitials::SecurityInterstitialPage {
public:
UrlHidingInterstitialPage(content::WebContents* web_contents,
const GURL& request_url)
: security_interstitials::SecurityInterstitialPage(
web_contents,
request_url,
std::make_unique<
security_interstitials::SecurityInterstitialControllerClient>(
web_contents,
nullptr,
nullptr,
base::i18n::GetConfiguredLocale(),
GURL())) {}
void OnInterstitialClosing() override {}
bool ShouldDisplayURL() const override { return false; }
protected:
bool ShouldCreateNewNavigation() const override { return false; }
void PopulateInterstitialStrings(
base::DictionaryValue* load_time_data) override {}
};
// An observer that associates a URL-hiding interstitial when a page loads when
// |install_interstitial| is true.
class UrlHidingWebContentsObserver : public content::WebContentsObserver {
public:
UrlHidingWebContentsObserver(content::WebContents* contents)
: content::WebContentsObserver(contents), install_interstitial_(true) {}
void DidFinishNavigation(content::NavigationHandle* handle) override {
if (!install_interstitial_)
return;
security_interstitials::SecurityInterstitialTabHelper::
AssociateBlockingPage(web_contents(), handle->GetNavigationId(),
std::make_unique<UrlHidingInterstitialPage>(
web_contents(), handle->GetURL()));
}
void StopBlocking() { install_interstitial_ = false; }
private:
bool install_interstitial_;
};
} // namespace } // namespace
class CustomTabBarViewBrowserTest : public extensions::ExtensionBrowserTest { class CustomTabBarViewBrowserTest : public extensions::ExtensionBrowserTest {
...@@ -528,3 +581,37 @@ IN_PROC_BROWSER_TEST_F(CustomTabBarViewBrowserTest, ...@@ -528,3 +581,37 @@ IN_PROC_BROWSER_TEST_F(CustomTabBarViewBrowserTest,
->close_button_for_testing() ->close_button_for_testing()
->visible()); ->visible());
} }
// Verify that interstitials that hide origin have their preference respected.
IN_PROC_BROWSER_TEST_F(CustomTabBarViewBrowserTest, InterstitialCanHideOrigin) {
ASSERT_TRUE(embedded_test_server()->Start());
ASSERT_TRUE(https_server()->Start());
InstallPWA(https_server()->GetURL("app.com", "/ssl/google.html"));
EXPECT_TRUE(app_browser_);
BrowserView* app_view = BrowserView::GetBrowserViewForBrowser(app_browser_);
EXPECT_NE(app_view, browser_view_);
content::WebContents* contents = app_view->GetActiveWebContents();
// Verify origin is blanked on interstitial.
UrlHidingWebContentsObserver blocker(contents);
SetTitleAndLocation(contents, base::ASCIIToUTF16("FooBar"),
https_server()->GetURL("/simple.html"));
EXPECT_EQ(base::string16(),
app_view->toolbar()->custom_tab_bar()->location_for_testing());
EXPECT_FALSE(
app_view->toolbar()->custom_tab_bar()->IsShowingOriginForTesting());
// Verify origin returns when interstitial is gone.
blocker.StopBlocking();
SetTitleAndLocation(contents, base::ASCIIToUTF16("FooBar2"),
https_server()->GetURL("/title1.html"));
EXPECT_NE(base::string16(),
app_view->toolbar()->custom_tab_bar()->location_for_testing());
EXPECT_TRUE(
app_view->toolbar()->custom_tab_bar()->IsShowingOriginForTesting());
}
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