Desktop Page Info/Harmony: Replace Cookie/Certificate/Settings links w/ buttons.

The "Cookies", "Certificate" and "Site settings" links are all buttons in
Harmony mode, so update the Page Info bubble to match.

Screenshot - https://drive.google.com/open?id=1DdlG3W-a7K5E73PRLCo0u_9cT5I_JYbV

Bug: 535074
Change-Id: I4f715ccae231dbadfdbceee2408753671e17829c
Reviewed-on: https://chromium-review.googlesource.com/754468
Commit-Queue: Patti <patricialor@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517758}

chrome/browser/ui/page_info/page_info_ui.cc

@@ -474,6 +474,11 @@ int PageInfoUI::GetConnectionIconID(PageInfo::SiteConnectionStatus status) {
 const gfx::ImageSkia PageInfoUI::GetCertificateIcon() {
   return gfx::CreateVectorIcon(kCertificateIcon, 16, gfx::kChromeIconGrey);
 }
+
+// static
+const gfx::ImageSkia PageInfoUI::GetSiteSettingsIcon() {
+  return gfx::CreateVectorIcon(kSettingsIcon, 16, gfx::kChromeIconGrey);
+}
 #endif
 
 // static

chrome/browser/ui/page_info/page_info_ui.h

@@ -196,8 +196,11 @@ class PageInfoUI {
   // Returns the connection icon ID for the given connection |status|.
   static int GetConnectionIconID(PageInfo::SiteConnectionStatus status);
 #else
-  // Returns the icon for the Certificate area.
+  // Returns the icon for the page Certificate.
   static const gfx::ImageSkia GetCertificateIcon();
+
+  // Returns the icon for the button / link to Site settings.
+  static const gfx::ImageSkia GetSiteSettingsIcon();
 #endif
 
   // Return true if the given ContentSettingsType is in PageInfoUI.

chrome/browser/ui/views/hover_button.cc

@@ -13,6 +13,7 @@
 #include "ui/views/animation/ink_drop_ripple.h"
 #include "ui/views/background.h"
 #include "ui/views/border.h"
+#include "ui/views/controls/styled_label.h"
 #include "ui/views/focus/focus_manager.h"
 #include "ui/views/layout/grid_layout.h"
 
@@ -26,6 +27,34 @@ std::unique_ptr<views::Border> CreateBorderWithVerticalSpacing(
                                   horz_spacing);
 }
 
+// Sets the accessible name of |parent| to the text from |first| and |second|.
+// Also set the combined text as the tooltip for |parent| if either |first| or
+// |second|'s text is cut off or elided.
+void SetTooltipAndAccessibleName(views::Button* parent,
+                                 views::StyledLabel* first,
+                                 views::Label* second,
+                                 const gfx::Rect& available_space,
+                                 int taken_width) {
+  const base::string16 accessible_name =
+      second == nullptr ? first->text()
+                        : base::JoinString({first->text(), second->text()},
+                                           base::ASCIIToUTF16("\n"));
+
+  // |views::StyledLabel|s only add tooltips for any links they may have.
+  // However, since |HoverButton| will never insert a link inside its child
+  // |StyledLabel|, decide whether it needs a tooltip by checking whether the
+  // available space is smaller than its preferred size.
+  bool first_truncated = first->GetPreferredSize().width() >
+                         (available_space.width() - taken_width);
+  base::string16 second_tooltip;
+  if (second != nullptr)
+    second->GetTooltipText(gfx::Point(), &second_tooltip);
+  parent->SetTooltipText(first_truncated || !second_tooltip.empty()
+                             ? accessible_name
+                             : base::string16());
+  parent->SetAccessibleName(accessible_name);
+}
+
 }  // namespace
 
 HoverButton::HoverButton(views::ButtonListener* button_listener,
@@ -102,9 +131,14 @@ HoverButton::HoverButton(views::ButtonListener* button_listener,
   columns->AddColumn(views::GridLayout::CENTER, views::GridLayout::CENTER,
                      kFixed, views::GridLayout::USE_PREF, 0, 0);
   columns->AddPaddingColumn(kFixed, icon_label_spacing);
-  columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::CENTER,
+  columns->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL,
                      kStretchy, views::GridLayout::USE_PREF, 0, 0);
 
+  taken_width_ = GetInsets().width() + icon_view->GetPreferredSize().width() +
+                 icon_label_spacing;
+
+  // Make sure hovering over the icon also hovers the |HoverButton|.
+  icon_view->set_can_process_events_within_subtree(false);
   // Don't cover |icon_view| when the ink drops are being painted. |LabelButton|
   // already does this with its own image.
   icon_view->SetPaintToLayer();
@@ -112,36 +146,36 @@ HoverButton::HoverButton(views::ButtonListener* button_listener,
   // Split the two rows evenly between the total height minus the padding.
   const int row_height =
       (total_height - remaining_vert_spacing * 2) / num_labels;
-  grid_layout->StartRow(0, 0, row_height);
+  grid_layout->StartRow(0, kColumnSetId, row_height);
   grid_layout->AddView(icon_view.release(), 1, num_labels);
 
-  title_ = new views::Label(title);
-  grid_layout->AddView(title_);
+  title_ = new views::StyledLabel(title, nullptr);
+  // Size without a maximum width to get a single line label.
+  title_->SizeToFit(0);
+  // |views::StyledLabel|s are all multi-line. With a layout manager,
+  // |StyledLabel| will try use the available space to size itself, and long
+  // titles will wrap to the next line (for smaller |HoverButton|s, this will
+  // also cover up |subtitle_|). Wrap it in a parent view with no layout manager
+  // to ensure it keeps its original size set by SizeToFit() above. Long titles
+  // will then be truncated.
+  views::View* title_wrapper = new views::View;
+  title_wrapper->AddChildView(title_);
+  // Hover the whole button when hovering |title_|. This is OK because |title_|
+  // will never have a link in it.
+  title_wrapper->set_can_process_events_within_subtree(false);
+  grid_layout->AddView(title_wrapper);
 
   if (!subtitle.empty()) {
-    grid_layout->StartRow(0, 0, row_height);
+    grid_layout->StartRow(0, kColumnSetId, row_height);
     subtitle_ = new views::Label(subtitle, views::style::CONTEXT_BUTTON,
                                  STYLE_SECONDARY);
     subtitle_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
     subtitle_->SetAutoColorReadabilityEnabled(false);
     grid_layout->SkipColumns(1);
     grid_layout->AddView(subtitle_);
-
-    const base::string16 accessible_name =
-        base::JoinString({title, subtitle}, base::ASCIIToUTF16("\n"));
-    // Only set a tooltip if either |title_| or |subtitle_| are too long.
-    base::string16 title_tooltip, subtitle_tooltip;
-    title_->GetTooltipText(gfx::Point(), &title_tooltip);
-    subtitle_->GetTooltipText(gfx::Point(), &subtitle_tooltip);
-    if (!title_tooltip.empty() || !subtitle_tooltip.empty()) {
-      // Setting the tooltip text also happens to set the accessible name.
-      SetTooltipText(accessible_name);
-    } else {
-      SetAccessibleName(accessible_name);
-    }
-  } else {
-    SetAccessibleName(title);
   }
+  SetTooltipAndAccessibleName(this, title_, subtitle_, GetLocalBounds(),
+                              taken_width_);
 
   // Since this constructor doesn't use the |LabelButton|'s image / label Views,
   // make sure the minimum size is correct according to the |grid_layout|.
@@ -156,6 +190,21 @@ void HoverButton::SetSubtitleElideBehavior(gfx::ElideBehavior elide_behavior) {
     subtitle_->SetElideBehavior(elide_behavior);
 }
 
+void HoverButton::SetTitleTextWithHintRange(const base::string16& title_text,
+                                            const gfx::Range& range) {
+  DCHECK(title_);
+  title_->SetText(title_text);
+
+  if (range.IsValid()) {
+    views::StyledLabel::RangeStyleInfo style_info;
+    style_info.text_style = STYLE_SECONDARY;
+    title_->AddStyleRange(range, style_info);
+  }
+  title_->SizeToFit(0);
+  SetTooltipAndAccessibleName(this, title_, subtitle_, GetLocalBounds(),
+                              taken_width_);
+}
+
 void HoverButton::StateChanged(ButtonState old_state) {
   LabelButton::StateChanged(old_state);
 
@@ -190,13 +239,31 @@ std::unique_ptr<views::InkDropHighlight> HoverButton::CreateInkDropHighlight()
   return highlight;
 }
 
+void HoverButton::Layout() {
+  LabelButton::Layout();
+
+  // Vertically center |title_| manually since it doesn't have a LayoutManager.
+  if (title_) {
+    DCHECK(title_->parent());
+    int y_center = title_->parent()->height() / 2 - title_->size().height() / 2;
+    title_->SetPosition(gfx::Point(title_->x(), y_center));
+  }
+}
+
 views::View* HoverButton::GetTooltipHandlerForPoint(const gfx::Point& point) {
   if (!HitTestPoint(point))
     return nullptr;
 
   // If possible, take advantage of the |views::Label| tooltip behavior, which
   // only sets the tooltip when the text is too long.
-  if (title_ && subtitle_)
+  if (title_)
     return this;
-  return title_ ? title_ : label();
+  return label();
+}
+
+void HoverButton::OnBoundsChanged(const gfx::Rect& previous_bounds) {
+  if (title_) {
+    SetTooltipAndAccessibleName(this, title_, subtitle_, GetLocalBounds(),
+                                taken_width_);
+  }
 }

chrome/browser/ui/views/hover_button.h

@@ -16,6 +16,7 @@ class ImageSkia;
 namespace views {
 class ButtonListener;
 class Label;
+class StyledLabel;
 class View;
 }  // namespace views
 
@@ -42,6 +43,15 @@ class HoverButton : public views::LabelButton {
 
   ~HoverButton() override;
 
+  // Updates the title text, and applies the secondary style to the text
+  // specified by |range|. If |range| is invalid, no style is applied. This
+  // method is only supported for |HoverButton|s created with a title and
+  // subtitle.
+  void SetTitleTextWithHintRange(const base::string16& title_text,
+                                 const gfx::Range& range);
+
+  // This method is only supported for |HoverButton|s created with a title and
+  // non-empty subtitle.
   void SetSubtitleElideBehavior(gfx::ElideBehavior elide_behavior);
 
  protected:
@@ -55,12 +65,18 @@ class HoverButton : public views::LabelButton {
       const override;
 
   // views::View:
+  void Layout() override;
   views::View* GetTooltipHandlerForPoint(const gfx::Point& point) override;
+  void OnBoundsChanged(const gfx::Rect& previous_bounds) override;
 
  private:
-  views::Label* title_;
+  views::StyledLabel* title_;
   views::Label* subtitle_;
 
+  // The horizontal space the padding and icon take up. Used for calculating the
+  // available space for |title_|, if it exists.
+  int taken_width_ = 0;
+
   DISALLOW_COPY_AND_ASSIGN(HoverButton);
 };
 

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

@@ -25,6 +25,7 @@
 class GURL;
 class Browser;
 class BubbleHeaderView;
+class HoverButton;
 class Profile;
 
 namespace content {
@@ -81,9 +82,9 @@ class PageInfoBubbleView : public content::WebContentsObserver,
     VIEW_ID_PAGE_INFO_BUTTON_WHITELIST_PASSWORD_REUSE,
     VIEW_ID_PAGE_INFO_LABEL_SECURITY_DETAILS,
     VIEW_ID_PAGE_INFO_LABEL_RESET_CERTIFICATE_DECISIONS,
-    VIEW_ID_PAGE_INFO_LINK_COOKIE_DIALOG,
-    VIEW_ID_PAGE_INFO_LINK_SITE_SETTINGS,
-    VIEW_ID_PAGE_INFO_LINK_CERTIFICATE_VIEWER,
+    VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_COOKIE_DIALOG,
+    VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_SITE_SETTINGS,
+    VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_CERTIFICATE_VIEWER,
   };
 
   // Creates the appropriate page info bubble for the given |url|.
@@ -152,12 +153,15 @@ class PageInfoBubbleView : public content::WebContentsObserver,
 
   // Creates the contents of the |site_settings_view_|. The ownership of the
   // returned view is transferred to the caller.
-  views::View* CreateSiteSettingsView(int side_margin) WARN_UNUSED_RESULT;
+  views::View* CreateSiteSettingsView() WARN_UNUSED_RESULT;
+
+  // Posts a task to HandleMoreInfoRequestAsync() below.
+  void HandleMoreInfoRequest(views::View* source);
 
   // Used to asynchronously handle clicks since these calls may cause the
   // destruction of the settings view and the base class window still needs to
   // be alive to finish handling the mouse or keyboard click.
-  void HandleLinkClickedAsync(views::Link* source);
+  void HandleMoreInfoRequestAsync(int view_id);
 
   // The presenter that controls the Page Info UI.
   std::unique_ptr<PageInfo> presenter_;
@@ -173,8 +177,10 @@ class PageInfoBubbleView : public content::WebContentsObserver,
   // The view that contains the certificate, cookie, and permissions sections.
   views::View* site_settings_view_;
 
-  // The link that opens the "Cookies" dialog.
-  views::Link* cookie_dialog_link_;
+  // The link that opens the "Cookies" dialog. Non-harmony mode only.
+  views::Link* cookie_link_legacy_;
+  // The bubble that opens the "Cookies" dialog. Harmony mode only.
+  HoverButton* cookie_button_;
 
   // The view that contains the "Permissions" table of the bubble.
   views::View* permissions_view_;

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

@@ -92,7 +92,8 @@ const GURL OpenSiteSettingsForUrl(Browser* browser, const GURL& url) {
   OpenPageInfoBubble(browser);
   // Get site settings button.
   views::View* site_settings_button = GetView(
-      browser, PageInfoBubbleView::VIEW_ID_PAGE_INFO_LINK_SITE_SETTINGS);
+      browser,
+      PageInfoBubbleView::VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_SITE_SETTINGS);
   ClickAndWaitForSettingsPageToOpen(site_settings_button);
 
   return browser->tab_strip_model()

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

@@ -9,6 +9,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/ui/exclusive_access/exclusive_access_manager.h"
 #include "chrome/browser/ui/views/harmony/chrome_layout_provider.h"
+#include "chrome/browser/ui/views/hover_button.h"
 #include "chrome/browser/ui/views/page_info/chosen_object_row.h"
 #include "chrome/browser/ui/views/page_info/permission_selector_row.h"
 #include "chrome/browser/usb/usb_chooser_context.h"
@@ -23,6 +24,7 @@
 #include "device/usb/mock_usb_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/material_design/material_design_controller.h"
 #include "ui/base/ui_base_features.h"
 #include "ui/events/event_utils.h"
 #include "ui/views/controls/button/menu_button.h"
@@ -66,11 +68,19 @@ class PageInfoBubbleViewTestApi {
     return view_->selector_rows_[index].get();
   }
 
-  // Returns the number of cookies shown on the link to open the collected
-  // cookies dialog. This link is always shown, so fail if it's not there.
+  // Returns the number of cookies shown on the link or button to open the
+  // collected cookies dialog. This should always be shown.
   base::string16 GetCookiesLinkText() {
-    EXPECT_TRUE(view_->cookie_dialog_link_);
-    return view_->cookie_dialog_link_->text();
+    if (ui::MaterialDesignController::IsSecondaryUiMaterial()) {
+      EXPECT_TRUE(view_->cookie_button_);
+      ui::AXNodeData data;
+      view_->cookie_button_->GetAccessibleNodeData(&data);
+      std::string name;
+      data.GetStringAttribute(ui::AX_ATTR_NAME, &name);
+      return base::ASCIIToUTF16(name);
+    }
+    EXPECT_TRUE(view_->cookie_link_legacy_);
+    return view_->cookie_link_legacy_->text();
   }
 
   // Returns the permission label text of the |index|th permission selector row.
@@ -334,5 +344,6 @@ TEST_F(PageInfoBubbleViewTest, UpdatingSiteDataRetainsLayout) {
   base::string16 expected = l10n_util::GetPluralStringFUTF16(
       IDS_PAGE_INFO_NUM_COOKIES,
       first_party_cookies.allowed + third_party_cookies.allowed);
-  EXPECT_EQ(expected, api_->GetCookiesLinkText());
+  size_t index = api_->GetCookiesLinkText().find(expected);
+  EXPECT_NE(std::string::npos, index);
 }