Update Suspicious Site Safety Tip UI for SDD experiment

For the purposes of measuring the Simplified Domain Display
experiment, we're using a Suspicious Site Safety Tip, but we're
removing the call to action buttons because we don't want to push the
user towards what to do; rather, we want to direct their attention to
the omnibox and then use the simplified domain UI to make a decision
about what to do.

This change therefore removes the buttons and Learn More link for the
Suspicious Site Safety Tip.

Bug: 1146471
Change-Id: I6101d1af604571a8b439f7096dab356795794962
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2532871
Commit-Queue: Emily Stark <estark@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Joe DeBlasio <jdeblasio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826974}

chrome/app/theme/theme_resources.grd

@@ -256,10 +256,8 @@
       </if>
       <structure type="chrome_scaled_image" name="IDR_RESTORE_BUTTON_MASK" file="common/restore_button_mask.png" />
       <if expr="not is_android">
-        <structure type="chrome_scaled_image" name="IDR_SAFETY_TIP_LOOKALIKE_ILLUSTRATION_DARK" file="common/safety_tip_lookalike_illustration_dark.png" />
-        <structure type="chrome_scaled_image" name="IDR_SAFETY_TIP_LOOKALIKE_ILLUSTRATION_LIGHT" file="common/safety_tip_lookalike_illustration_light.png" />
-        <structure type="chrome_scaled_image" name="IDR_SAFETY_TIP_SUSPICIOUS_ILLUSTRATION_DARK" file="common/safety_tip_suspicious_illustration_dark.png" />
-        <structure type="chrome_scaled_image" name="IDR_SAFETY_TIP_SUSPICIOUS_ILLUSTRATION_LIGHT" file="common/safety_tip_suspicious_illustration_light.png" />
+        <structure type="chrome_scaled_image" name="IDR_SAFETY_TIP_ILLUSTRATION_DARK" file="common/safety_tip_illustration_dark.png" />
+        <structure type="chrome_scaled_image" name="IDR_SAFETY_TIP_ILLUSTRATION_LIGHT" file="common/safety_tip_illustration_light.png" />
         <structure type="chrome_scaled_image" name="IDR_SAVED_PASSWORDS_NEUTRAL_STATE" file="common/passwords_neutral_state.png" />
         <structure type="chrome_scaled_image" name="IDR_SAVED_PASSWORDS_NEUTRAL_STATE_DARK" file="common/passwords_neutral_state_dark.png" />
         <structure type="chrome_scaled_image" name="IDR_SAVED_PASSWORDS_SAFE_STATE" file="common/passwords_safe_state.png" />

chrome/browser/reputation/safety_tip_ui_helper.cc

@@ -96,8 +96,6 @@ base::string16 GetSafetyTipDescription(
     const GURL& suggested_url) {
   switch (warning_type) {
     case security_state::SafetyTipStatus::kBadReputation:
-      return l10n_util::GetStringUTF16(
-          IDS_PAGE_INFO_SAFETY_TIP_BAD_REPUTATION_DESCRIPTION);
     case security_state::SafetyTipStatus::kLookalike:
       return l10n_util::GetStringUTF16(
           IDS_PAGE_INFO_SAFETY_TIP_LOOKALIKE_DESCRIPTION);

chrome/browser/ui/views/page_info/safety_tip_page_info_bubble_view.cc

@@ -38,11 +38,9 @@ int GetSafetyTipBannerId(security_state::SafetyTipStatus safety_tip_status,
                          bool is_dark) {
   switch (safety_tip_status) {
     case security_state::SafetyTipStatus::kBadReputation:
-      return is_dark ? IDR_SAFETY_TIP_SUSPICIOUS_ILLUSTRATION_DARK
-                     : IDR_SAFETY_TIP_SUSPICIOUS_ILLUSTRATION_LIGHT;
     case security_state::SafetyTipStatus::kLookalike:
-      return is_dark ? IDR_SAFETY_TIP_LOOKALIKE_ILLUSTRATION_DARK
-                     : IDR_SAFETY_TIP_LOOKALIKE_ILLUSTRATION_LIGHT;
+      return is_dark ? IDR_SAFETY_TIP_ILLUSTRATION_DARK
+                     : IDR_SAFETY_TIP_ILLUSTRATION_LIGHT;
     case security_state::SafetyTipStatus::kBadReputationIgnored:
     case security_state::SafetyTipStatus::kLookalikeIgnored:
     case security_state::SafetyTipStatus::kBadKeyword:
@@ -153,55 +151,8 @@ SafetyTipPageInfoBubbleView::SafetyTipPageInfoBubbleView(
                         insets.left() - insets.right());
   bottom_layout->AddView(std::move(text_label));
 
-  // Add buttons.
-  // To make the rest of the layout simpler, they live in their own grid layout.
-  auto button_view = std::make_unique<views::View>();
-  views::GridLayout* button_layout =
-      button_view->SetLayoutManager(std::make_unique<views::GridLayout>());
-  views::ColumnSet* button_column_set = button_layout->AddColumnSet(0);
-  button_column_set->AddColumn(
-      views::GridLayout::LEADING, views::GridLayout::CENTER, 0.0,
-      views::GridLayout::ColumnSize::kUsePreferred, 0, 0);
-  button_column_set->AddPaddingColumn(1.f, 1);
-  button_column_set->AddColumn(
-      views::GridLayout::TRAILING, views::GridLayout::FILL, 0.0,
-      views::GridLayout::ColumnSize::kUsePreferred, 0, 0);
-
-  button_layout->StartRow(views::GridLayout::kFixedSize, kColumnId);
+  MaybeAddButtons(safety_tip_status, bottom_layout, spacing, kColumnId, insets);
 
-  // More info button.
-  auto info_text =
-      l10n_util::GetStringUTF16(IDS_PAGE_INFO_SAFETY_TIP_MORE_INFO_LINK);
-  auto info_link = std::make_unique<views::StyledLabel>();
-  info_link->SetText(info_text);
-  views::StyledLabel::RangeStyleInfo link_style =
-      views::StyledLabel::RangeStyleInfo::CreateForLink(
-          base::BindRepeating(&SafetyTipPageInfoBubbleView::OpenHelpCenter,
-                              base::Unretained(this)));
-  gfx::Range details_range(0, info_text.length());
-  info_link->AddStyleRange(details_range, link_style);
-  info_link->SizeToFit(0);
-  info_button_ = button_layout->AddView(std::move(info_link));
-  // Leave site button.
-  auto leave_button = std::make_unique<views::MdTextButton>(
-      base::BindRepeating(
-          [](SafetyTipPageInfoBubbleView* view) {
-            view->ExecuteLeaveCommand();
-          },
-          this),
-      l10n_util::GetStringUTF16(GetSafetyTipLeaveButtonId(safety_tip_status)));
-  leave_button->SetProminent(true);
-  leave_button->SetID(PageInfoBubbleView::VIEW_ID_PAGE_INFO_BUTTON_LEAVE_SITE);
-  leave_button_ = button_layout->AddView(std::move(leave_button));
-
-  bottom_layout->StartRowWithPadding(views::GridLayout::kFixedSize, kColumnId,
-                                     views::GridLayout::kFixedSize, spacing);
-  bottom_layout->AddView(std::move(button_view), 1, 1,
-                         views::GridLayout::LEADING, views::GridLayout::LEADING,
-                         layout_provider->GetDistanceMetric(
-                             views::DISTANCE_BUBBLE_PREFERRED_WIDTH) -
-                             insets.left() - insets.right(),
-                         0);
   bubble_layout->StartRow(views::GridLayout::kFixedSize, kColumnId);
   bubble_layout->AddView(std::move(bottom_view));
 
@@ -304,6 +255,69 @@ void SafetyTipPageInfoBubbleView::DidChangeVisibleSecurityState() {
   // Do nothing. (Base class closes the bubble.)
 }
 
+void SafetyTipPageInfoBubbleView::MaybeAddButtons(
+    security_state::SafetyTipStatus safety_tip_status,
+    views::GridLayout* bottom_layout,
+    int spacing,
+    int column_id,
+    const gfx::Insets& insets) {
+  // Suspicious site safety tips don't have a call to action, as they are used
+  // for drawing users' attention to the omnibox to see if they leave the site
+  // on their own once they notice the omnibox. (https://crbug.com/1146471)
+  if (safety_tip_status == security_state::SafetyTipStatus::kBadReputation)
+    return;
+
+  ChromeLayoutProvider* layout_provider = ChromeLayoutProvider::Get();
+  // To make the rest of the layout simpler, they live in their own grid layout.
+  auto button_view = std::make_unique<views::View>();
+  views::GridLayout* button_layout =
+      button_view->SetLayoutManager(std::make_unique<views::GridLayout>());
+  views::ColumnSet* button_column_set = button_layout->AddColumnSet(0);
+  button_column_set->AddColumn(
+      views::GridLayout::LEADING, views::GridLayout::CENTER, 0.0,
+      views::GridLayout::ColumnSize::kUsePreferred, 0, 0);
+  button_column_set->AddPaddingColumn(1.f, 1);
+  button_column_set->AddColumn(
+      views::GridLayout::TRAILING, views::GridLayout::FILL, 0.0,
+      views::GridLayout::ColumnSize::kUsePreferred, 0, 0);
+
+  button_layout->StartRow(views::GridLayout::kFixedSize, column_id);
+
+  // More info button.
+  auto info_text =
+      l10n_util::GetStringUTF16(IDS_PAGE_INFO_SAFETY_TIP_MORE_INFO_LINK);
+  auto info_link = std::make_unique<views::StyledLabel>();
+  info_link->SetText(info_text);
+  views::StyledLabel::RangeStyleInfo link_style =
+      views::StyledLabel::RangeStyleInfo::CreateForLink(
+          base::BindRepeating(&SafetyTipPageInfoBubbleView::OpenHelpCenter,
+                              base::Unretained(this)));
+  gfx::Range details_range(0, info_text.length());
+  info_link->AddStyleRange(details_range, link_style);
+  info_link->SizeToFit(0);
+  info_button_ = button_layout->AddView(std::move(info_link));
+  // Leave site button.
+  auto leave_button = std::make_unique<views::MdTextButton>(
+      base::BindRepeating(
+          [](SafetyTipPageInfoBubbleView* view) {
+            view->ExecuteLeaveCommand();
+          },
+          this),
+      l10n_util::GetStringUTF16(GetSafetyTipLeaveButtonId(safety_tip_status)));
+  leave_button->SetProminent(true);
+  leave_button->SetID(PageInfoBubbleView::VIEW_ID_PAGE_INFO_BUTTON_LEAVE_SITE);
+  leave_button_ = button_layout->AddView(std::move(leave_button));
+
+  bottom_layout->StartRowWithPadding(views::GridLayout::kFixedSize, column_id,
+                                     views::GridLayout::kFixedSize, spacing);
+  bottom_layout->AddView(std::move(button_view), 1, 1,
+                         views::GridLayout::LEADING, views::GridLayout::LEADING,
+                         layout_provider->GetDistanceMetric(
+                             views::DISTANCE_BUBBLE_PREFERRED_WIDTH) -
+                             insets.left() - insets.right(),
+                         0);
+}
+
 void ShowSafetyTipDialog(
     content::WebContents* web_contents,
     security_state::SafetyTipStatus safety_tip_status,

chrome/browser/ui/views/page_info/safety_tip_page_info_bubble_view.h

@@ -21,6 +21,7 @@ class Rect;
 }  // namespace gfx
 
 namespace views {
+class GridLayout;
 class View;
 class Widget;
 }  // namespace views
@@ -62,15 +63,21 @@ class SafetyTipPageInfoBubbleView : public PageInfoBubbleViewBase {
   void DidStartNavigation(content::NavigationHandle* handle) override;
   void DidChangeVisibleSecurityState() override;
 
+  void MaybeAddButtons(security_state::SafetyTipStatus safety_tip_status,
+                       views::GridLayout* bottom_layout,
+                       int spacing,
+                       int column_id,
+                       const gfx::Insets& insets);
+
   const security_state::SafetyTipStatus safety_tip_status_;
 
   // The URL of the page the Safety Tip suggests you intended to go to, when
   // applicable (for SafetyTipStatus::kLookalike).
   const GURL suggested_url_;
 
-  views::StyledLabel* info_button_;
-  views::Button* ignore_button_;
-  views::Button* leave_button_;
+  views::StyledLabel* info_button_ = nullptr;
+  views::Button* ignore_button_ = nullptr;
+  views::Button* leave_button_ = nullptr;
   base::OnceCallback<void(SafetyTipInteraction)> close_callback_;
   SafetyTipInteraction action_taken_ = SafetyTipInteraction::kNoAction;
 

chrome/browser/ui/views/page_info/safety_tip_page_info_bubble_view_browsertest.cc

@@ -311,6 +311,14 @@ class SafetyTipPageInfoBubbleViewBrowserTest
     bubble->OpenHelpCenter();
   }
 
+  void CheckNoButtons() {
+    auto* bubble = static_cast<SafetyTipPageInfoBubbleView*>(
+        PageInfoBubbleViewBase::GetPageInfoBubbleForTesting());
+    EXPECT_FALSE(bubble->info_button_);
+    EXPECT_FALSE(bubble->ignore_button_);
+    EXPECT_FALSE(bubble->leave_button_);
+  }
+
   void CloseWarningLeaveSite(Browser* browser) {
     if (ui_status() == UIStatus::kDisabled) {
       return;
@@ -592,13 +600,17 @@ IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest,
 // After the user clicks 'leave site', the user should end up on a safe domain.
 IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest,
                        LeaveSiteLeavesSite) {
-  auto kNavigatedUrl = GetURL("site1.com");
-  if (!IsSuspiciousSiteWarningEnabled()) {
+  // The suspicious site warning doesn't have call-to-action buttons, so this
+  // test only applies to lookalike warnings.
+  if (!AreLookalikeWarningsEnabled()) {
     return;
   }
 
-  TriggerWarningFromBlocklist(browser(), kNavigatedUrl,
-                              WindowOpenDisposition::CURRENT_TAB);
+  // This domain is a lookalike of a top domain not in the top 500.
+  const GURL kNavigatedUrl = GetURL("googlé.sk");
+  SetEngagementScore(browser(), kNavigatedUrl, kLowEngagement);
+  NavigateToURL(browser(), kNavigatedUrl, WindowOpenDisposition::CURRENT_TAB);
+  ASSERT_TRUE(IsUIShowing());
 
   CloseWarningLeaveSite(browser());
   EXPECT_FALSE(IsUIShowing());
@@ -624,6 +636,24 @@ IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest,
   EXPECT_NE(kNavigatedUrl, new_tab_observer.GetWebContents()->GetURL());
 }
 
+// Test that the Suspicious Site Safety Tip has no buttons and has the correct
+// strings.
+IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest,
+                       SuspiciousSiteUI) {
+  if (!IsSuspiciousSiteWarningEnabled()) {
+    return;
+  }
+
+  auto kNavigatedUrl = GetURL("site1.com");
+  TriggerWarningFromBlocklist(browser(), kNavigatedUrl,
+                              WindowOpenDisposition::CURRENT_TAB);
+  ASSERT_NO_FATAL_FAILURE(CheckNoButtons());
+  auto* page_info = PageInfoBubbleViewBase::GetPageInfoBubbleForTesting();
+  EXPECT_EQ(
+      page_info->GetWindowTitle(),
+      l10n_util::GetStringUTF16(IDS_PAGE_INFO_SAFETY_TIP_BAD_REPUTATION_TITLE));
+}
+
 // If the user clicks 'leave site', the warning should re-appear when the user
 // re-visits the page.
 // Flaky on Mac: https://crbug.com/1139955

components/page_info/page_info_ui.cc

@@ -211,16 +211,25 @@ std::unique_ptr<PageInfoUI::SecurityDescription> CreateSecurityDescription(
 }
 
 std::unique_ptr<PageInfoUI::SecurityDescription>
-CreateSecurityDescriptionForLookalikeSafetyTip(const GURL& safe_url) {
+CreateSecurityDescriptionForSafetyTip(
+    const security_state::SafetyTipStatus& safety_tip_status,
+    const GURL& safe_url) {
   auto security_description =
       std::make_unique<PageInfoUI::SecurityDescription>();
   security_description->summary_style = PageInfoUI::SecuritySummaryColor::RED;
 
+  if (safety_tip_status == security_state::SafetyTipStatus::kBadReputation ||
+      safety_tip_status ==
+          security_state::SafetyTipStatus::kBadReputationIgnored) {
+    security_description->summary = l10n_util::GetStringUTF16(
+        IDS_PAGE_INFO_SAFETY_TIP_BAD_REPUTATION_TITLE);
+  } else {
     const base::string16 safe_host =
         security_interstitials::common_string_util::GetFormattedHostName(
             safe_url);
     security_description->summary = l10n_util::GetStringFUTF16(
         IDS_PAGE_INFO_SAFETY_TIP_LOOKALIKE_TITLE, safe_host);
+  }
   security_description->details =
       l10n_util::GetStringUTF16(IDS_PAGE_INFO_SAFETY_TIP_LOOKALIKE_DESCRIPTION);
   security_description->type = PageInfoUI::SecurityDescriptionType::SAFETY_TIP;
@@ -831,14 +840,9 @@ PageInfoUI::CreateSafetyTipSecurityDescription(
   switch (info.status) {
     case security_state::SafetyTipStatus::kBadReputation:
     case security_state::SafetyTipStatus::kBadReputationIgnored:
-      return CreateSecurityDescription(
-          SecuritySummaryColor::RED,
-          IDS_PAGE_INFO_SAFETY_TIP_BAD_REPUTATION_TITLE,
-          IDS_PAGE_INFO_SAFETY_TIP_BAD_REPUTATION_DESCRIPTION,
-          PageInfoUI::SecurityDescriptionType::SAFETY_TIP);
     case security_state::SafetyTipStatus::kLookalike:
     case security_state::SafetyTipStatus::kLookalikeIgnored:
-      return CreateSecurityDescriptionForLookalikeSafetyTip(info.safe_url);
+      return CreateSecurityDescriptionForSafetyTip(info.status, info.safe_url);
 
     case security_state::SafetyTipStatus::kBadKeyword:
       // Keyword safety tips are only used to collect metrics for now and are

components/page_info_strings.grdp

@@ -48,9 +48,6 @@
   <message name="IDS_PAGE_INFO_SAFETY_TIP_BAD_REPUTATION_TITLE" desc="Message to display in the page info bubble when the page you are on triggered a safety tip.">
     Suspicious site
   </message>
-  <message name="IDS_PAGE_INFO_SAFETY_TIP_BAD_REPUTATION_DESCRIPTION" desc="Body of message to display in the page info bubble when you are viewing a page that triggered a safety tip.">
-    This site could be fake or fraudulent. Chrome recommends leaving now.
-  </message>
   <message name="IDS_PAGE_INFO_SAFETY_TIP_BAD_REPUTATION_LEAVE_BUTTON" desc="Text of button to leave a suspicious page. Shown on the safety tip page info bubble.">
     Leave site
   </message>
@@ -63,7 +60,7 @@
   <message name="IDS_PAGE_INFO_SAFETY_TIP_LOOKALIKE_TITLE" desc="Title of Safety Tip infobar on a domain that looks like another domain.">
     Did you mean <ph name='LOOKALIKE_DOMAIN'>$1<ex>google.com</ex></ph>?
   </message>
-  <message name="IDS_PAGE_INFO_SAFETY_TIP_LOOKALIKE_DESCRIPTION" desc="Body of a warning when the user visits a page that triggered a Safety Tip because the domain looked like another domain.">
+  <message name="IDS_PAGE_INFO_SAFETY_TIP_LOOKALIKE_DESCRIPTION" desc="Body of a warning when the user visits a page that triggered a Safety Tip.">
     Attackers sometimes mimic sites by making hard-to-see changes to the web address.
   </message>
 

components/page_info_strings_grdp/IDS_PAGE_INFO_SAFETY_TIP_BAD_REPUTATION_DESCRIPTION.png.sha1

-b34b7304a8ffb374af8016eb3068dbbb88ba4ede
\ No newline at end of file