Support keyboard modifiers when opening blocked contents

Bug:46448

Change-Id: I9f1ea66e4cc9732741378e067d2b420a1136b885
Reviewed-on: https://chromium-review.googlesource.com/575805
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491929}

chrome/browser/ui/android/content_settings/popup_blocked_infobar_delegate.cc

@@ -117,7 +117,8 @@ bool PopupBlockedInfoBarDelegate::Accept() {
       popup_blocker_helper->GetBlockedPopupRequests();
   for (PopupBlockerTabHelper::PopupIdMap::iterator it = blocked_popups.begin();
       it != blocked_popups.end(); ++it)
-    popup_blocker_helper->ShowBlockedPopup(it->first);
+    popup_blocker_helper->ShowBlockedPopup(it->first,
+                                           WindowOpenDisposition::CURRENT_TAB);
 
   content_settings::RecordPopupsAction(
       content_settings::POPUPS_ACTION_CLICKED_ALWAYS_SHOW_ON_MOBILE);

chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc

@@ -142,18 +142,25 @@ void PopupBlockerTabHelper::AddBlockedPopup(
       OnContentBlocked(CONTENT_SETTINGS_TYPE_POPUPS);
 }
 
-void PopupBlockerTabHelper::ShowBlockedPopup(int32_t id) {
+void PopupBlockerTabHelper::ShowBlockedPopup(
+    int32_t id,
+    WindowOpenDisposition disposition) {
   BlockedRequest* popup = blocked_popups_.Lookup(id);
   if (!popup)
     return;
+
   // We set user_gesture to true here, so the new popup gets correctly focused.
   popup->params.user_gesture = true;
+  if (disposition != WindowOpenDisposition::CURRENT_TAB)
+    popup->params.disposition = disposition;
+
 #if defined(OS_ANDROID)
   TabModelList::HandlePopupNavigation(&popup->params);
 #else
   chrome::Navigate(&popup->params);
 #endif
-  if (popup->params.target_contents) {
+  if (popup->params.disposition == WindowOpenDisposition::NEW_POPUP &&
+      popup->params.target_contents) {
     popup->params.target_contents->Send(new ChromeViewMsg_SetWindowFeatures(
         popup->params.target_contents->GetRenderViewHost()->GetRoutingID(),
         popup->window_features));

chrome/browser/ui/blocked_content/popup_blocker_tab_helper.h

@@ -15,6 +15,7 @@
 #include "chrome/browser/ui/blocked_content/blocked_window_params.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
+#include "ui/base/window_open_disposition.h"
 
 namespace chrome {
 struct NavigateParams;
@@ -51,8 +52,10 @@ class PopupBlockerTabHelper
   // Adds a popup request to the |blocked_popups_| container.
   void AddBlockedPopup(const BlockedWindowParams& params);
 
-  // Creates the blocked popup with |popup_id|.
-  void ShowBlockedPopup(int32_t popup_id);
+  // Creates the blocked popup with |popup_id| in given |dispostion|.
+  // Note that if |disposition| is WindowOpenDisposition::CURRENT_TAB,
+  // blocked popup will be opened as it was specified by renderer.
+  void ShowBlockedPopup(int32_t popup_id, WindowOpenDisposition disposition);
 
   // Returns the number of blocked popups.
   size_t GetBlockedPopupsCount() const;

chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm

@@ -35,6 +35,7 @@
 #import "ui/base/cocoa/controls/hyperlink_button_cell.h"
 #import "ui/base/cocoa/touch_bar_util.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/events/cocoa/cocoa_event_utils.h"
 
 using content::PluginService;
 
@@ -994,7 +995,8 @@ const ContentTypeToNibPath kNibPaths[] = {
 - (void)popupLinkClicked:(id)sender {
   content_setting_bubble::PopupLinks::iterator i(popupLinks_.find(sender));
   DCHECK(i != popupLinks_.end());
-  contentSettingBubbleModel_->OnListItemClicked(i->second);
+  const int event_flags = ui::EventFlagsFromModifiers([NSEvent modifierFlags]);
+  contentSettingBubbleModel_->OnListItemClicked(i->second, event_flags);
 }
 
 - (void)clearGeolocationForCurrentHost:(id)sender {

chrome/browser/ui/content_settings/content_setting_bubble_model.cc

@@ -62,6 +62,7 @@
 #include "services/service_manager/public/cpp/interface_provider.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/base/window_open_disposition.h"
 #include "ui/resources/grit/ui_resources.h"
 
 using base::UserMetricsAction;
@@ -552,7 +553,7 @@ class ContentSettingPopupBubbleModel : public ContentSettingSingleRadioGroup {
   ~ContentSettingPopupBubbleModel() override;
 
  private:
-  void OnListItemClicked(int index) override;
+  void OnListItemClicked(int index, int event_flags) override;
 
   int32_t item_id_from_item_index(int index) const {
     return bubble_content().list_items[index].item_id;
@@ -592,11 +593,13 @@ ContentSettingPopupBubbleModel::ContentSettingPopupBubbleModel(
       content_settings::POPUPS_ACTION_DISPLAYED_BUBBLE);
 }
 
-void ContentSettingPopupBubbleModel::OnListItemClicked(int index) {
+void ContentSettingPopupBubbleModel::OnListItemClicked(int index,
+                                                       int event_flags) {
   if (web_contents()) {
     auto* helper = PopupBlockerTabHelper::FromWebContents(web_contents());
-    helper->ShowBlockedPopup(item_id_from_item_index(index));
-
+    helper->ShowBlockedPopup(item_id_from_item_index(index),
+                             ui::DispositionFromEventFlags(event_flags));
+    remove_list_item(index);
     content_settings::RecordPopupsAction(
         content_settings::POPUPS_ACTION_CLICKED_LIST_ITEM_CLICKED);
   }

chrome/browser/ui/content_settings/content_setting_bubble_model.h

@@ -153,7 +153,7 @@ class ContentSettingBubbleModel : public content::NotificationObserver {
                const content::NotificationDetails& details) override;
 
   virtual void OnRadioClicked(int radio_index) {}
-  virtual void OnListItemClicked(int index) {}
+  virtual void OnListItemClicked(int index, int event_flags) {}
   virtual void OnCustomLinkClicked() {}
   virtual void OnManageLinkClicked() {}
   virtual void OnManageCheckboxChecked(bool is_checked) {}
@@ -208,6 +208,10 @@ class ContentSettingBubbleModel : public content::NotificationObserver {
   void add_list_item(const ListItem& item) {
     bubble_content_.list_items.push_back(item);
   }
+  void remove_list_item(int index) {
+    bubble_content_.list_items.erase(
+        bubble_content_.list_items.begin() + index);
+  }
   void set_radio_group(const RadioGroup& radio_group) {
     bubble_content_.radio_group = radio_group;
   }

chrome/browser/ui/content_settings/content_setting_bubble_model_browsertest.cc

@@ -26,6 +26,7 @@
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/test/embedded_test_server/request_handler_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/events/event_constants.h"
 
 const base::FilePath::CharType kDocRoot[] =
     FILE_PATH_LITERAL("chrome/test/data");
@@ -265,7 +266,7 @@ IN_PROC_BROWSER_TEST_F(ContentSettingBubbleModelPopupTest,
         "ContentSettings.Popups",
         content_settings::POPUPS_ACTION_DISPLAYED_BUBBLE, 1);
 
-  model->OnListItemClicked(0);
+  model->OnListItemClicked(0, ui::EF_LEFT_MOUSE_BUTTON);
   histograms.ExpectBucketCount(
         "ContentSettings.Popups",
         content_settings::POPUPS_ACTION_CLICKED_LIST_ITEM_CLICKED, 1);

chrome/browser/ui/views/content_setting_bubble_contents.cc

@@ -7,6 +7,7 @@
 #include <algorithm>
 #include <set>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "base/bind.h"
@@ -165,6 +166,118 @@ gfx::NativeCursor ContentSettingBubbleContents::Favicon::GetCursor(
   return views::GetNativeHandCursor();
 }
 
+// ContentSettingBubbleContents::ListItemContainer -----------------------------
+
+class ContentSettingBubbleContents::ListItemContainer : public views::View {
+ public:
+  explicit ListItemContainer(ContentSettingBubbleContents* parent);
+
+  // Creates and adds child views representing |item|.
+  void AddItem(const ContentSettingBubbleModel::ListItem& item);
+
+  // Calling this will delete related children.
+  void RemoveRowAtIndex(int index);
+
+  // Returns row index of |link| among list items.
+  int GetRowIndexOf(const views::Link* link) const;
+
+ private:
+  using Row = std::pair<views::ImageView*, views::Label*>;
+
+  void ResetLayout();
+  void AddRowToLayout(const Row& row, bool padding_above);
+
+  ContentSettingBubbleContents* parent_;
+
+  int icon_column_width_;
+
+  // Our controls representing list items, so we can add or remove
+  // these dynamically. Each pair represetns one list item.
+  std::vector<Row> list_item_views_;
+
+  DISALLOW_COPY_AND_ASSIGN(ListItemContainer);
+};
+
+ContentSettingBubbleContents::ListItemContainer::ListItemContainer(
+    ContentSettingBubbleContents* parent)
+        : parent_(parent), icon_column_width_(0) {
+  ResetLayout();
+}
+
+void ContentSettingBubbleContents::ListItemContainer::AddItem(
+    const ContentSettingBubbleModel::ListItem& item) {
+  views::ImageView* icon = nullptr;
+  views::Label* label = nullptr;
+  if (item.has_link) {
+    views::Link* link = new views::Link(item.title);
+    link->set_listener(parent_);
+    link->SetElideBehavior(gfx::ELIDE_MIDDLE);
+    icon = new Favicon(item.image, parent_, link);
+    label = link;
+  } else {
+    icon = new views::ImageView();
+    icon->SetImage(item.image.AsImageSkia());
+    label = new views::Label(item.title);
+  }
+  icon_column_width_ = std::max(icon->GetPreferredSize().width(),
+                                icon_column_width_);
+  list_item_views_.push_back(Row(icon, label));
+  AddRowToLayout(list_item_views_.back(), list_item_views_.size() > 1);
+}
+
+void ContentSettingBubbleContents::ListItemContainer::RemoveRowAtIndex(
+    int index) {
+  auto& children = list_item_views_[index];
+  delete children.first;
+  delete children.second;
+  list_item_views_.erase(list_item_views_.begin() + index);
+
+  // As views::GridLayout can't remove rows, we have to rebuild it entirely.
+  ResetLayout();
+  for (size_t i = 0; i < list_item_views_.size(); i++)
+    AddRowToLayout(list_item_views_[i], i > 0);
+}
+
+int ContentSettingBubbleContents::ListItemContainer::GetRowIndexOf(
+    const views::Link* link) const {
+  auto has_link = [link](const Row& row) { return row.second == link; };
+  auto iter = std::find_if(list_item_views_.begin(), list_item_views_.end(),
+                           has_link);
+  return (iter == list_item_views_.end())
+             ? -1
+             : std::distance(list_item_views_.begin(), iter);
+}
+
+void ContentSettingBubbleContents::ListItemContainer::ResetLayout() {
+  using views::GridLayout;
+  GridLayout* layout = new GridLayout(this);
+  SetLayoutManager(layout);
+  views::ColumnSet* item_list_column_set = layout->AddColumnSet(0);
+  item_list_column_set->AddColumn(GridLayout::LEADING, GridLayout::FILL, 0,
+                                  GridLayout::USE_PREF, 0, 0);
+  const int related_control_horizontal_spacing =
+      ChromeLayoutProvider::Get()->GetDistanceMetric(
+          views::DISTANCE_RELATED_CONTROL_HORIZONTAL);
+  item_list_column_set->AddPaddingColumn(0, related_control_horizontal_spacing);
+  item_list_column_set->AddColumn(GridLayout::LEADING, GridLayout::FILL, 1,
+                                  GridLayout::USE_PREF, 0, 0);
+}
+
+void ContentSettingBubbleContents::ListItemContainer::AddRowToLayout(
+    const Row& row,
+    bool padding_above) {
+  views::GridLayout* layout =
+      static_cast<views::GridLayout*>(GetLayoutManager());
+  DCHECK(layout);
+  if (padding_above) {
+    const int vertical_padding = ChromeLayoutProvider::Get()->GetDistanceMetric(
+        views::DISTANCE_RELATED_CONTROL_VERTICAL);
+    layout->AddPaddingRow(0, vertical_padding);
+  }
+  layout->StartRow(0, 0);
+  layout->AddView(row.first);
+  layout->AddView(row.second);
+}
 
 // ContentSettingBubbleContents -----------------------------------------------
 
@@ -176,6 +289,7 @@ ContentSettingBubbleContents::ContentSettingBubbleContents(
     : content::WebContentsObserver(web_contents),
       BubbleDialogDelegateView(anchor_view, arrow),
       content_setting_bubble_model_(content_setting_bubble_model),
+      list_item_container_(nullptr),
       custom_link_(nullptr),
       manage_link_(nullptr),
       manage_checkbox_(nullptr),
@@ -270,35 +384,22 @@ void ContentSettingBubbleContents::Init() {
   // Layout for the item list (blocked plugins and popups).
   if (!bubble_content.list_items.empty()) {
     const int kItemListColumnSetId = 2;
-    views::ColumnSet* item_list_column_set =
-        layout->AddColumnSet(kItemListColumnSetId);
-    item_list_column_set->AddColumn(GridLayout::LEADING, GridLayout::FILL, 0,
-                                    GridLayout::USE_PREF, 0, 0);
-    item_list_column_set->AddPaddingColumn(0,
-                                           related_control_horizontal_spacing);
-    item_list_column_set->AddColumn(GridLayout::LEADING, GridLayout::FILL, 1,
-                                    GridLayout::USE_PREF, 0, 0);
-
-    int row = 0;
+    views::ColumnSet* column_set = layout->AddColumnSet(kItemListColumnSetId);
+    column_set->AddColumn(GridLayout::LEADING, GridLayout::FILL, 1,
+                          GridLayout::USE_PREF, 0, 0);
+
     for (const ContentSettingBubbleModel::ListItem& list_item :
          bubble_content.list_items) {
-      if (!bubble_content_empty)
-        layout->AddPaddingRow(0, related_control_vertical_spacing);
-      layout->StartRow(0, kItemListColumnSetId);
-      if (list_item.has_link) {
-        views::Link* link = new views::Link(list_item.title);
-        link->set_listener(this);
-        link->SetElideBehavior(gfx::ELIDE_MIDDLE);
-        list_item_links_[link] = row;
-        layout->AddView(new Favicon(list_item.image, this, link));
-        layout->AddView(link);
-      } else {
-        views::ImageView* icon = new views::ImageView();
-        icon->SetImage(list_item.image.AsImageSkia());
-        layout->AddView(icon);
-        layout->AddView(new views::Label(list_item.title));
+      if (!list_item_container_) {
+        if (!bubble_content_empty)
+          layout->AddPaddingRow(0, related_control_vertical_spacing);
+
+        list_item_container_ = new ListItemContainer(this);
+        layout->StartRow(0, kItemListColumnSetId);
+        layout->AddView(list_item_container_);
       }
-      row++;
+
+      list_item_container_->AddItem(list_item);
       bubble_content_empty = false;
     }
   }
@@ -520,9 +621,14 @@ void ContentSettingBubbleContents::LinkClicked(views::Link* source,
     return;
   }
 
-  ListItemLinks::const_iterator i(list_item_links_.find(source));
-  DCHECK(i != list_item_links_.end());
-  content_setting_bubble_model_->OnListItemClicked(i->second);
+  int row = list_item_container_->GetRowIndexOf(source);
+  DCHECK_NE(row, -1);
+  content_setting_bubble_model_->OnListItemClicked(row, event_flags);
+
+  // TODO(bug 35097) Need more work here. There's no way to be notified changes
+  // in |content_setting_bubble_model_| dynamically for now.
+  list_item_container_->RemoveRowAtIndex(row);
+  SizeToContents();
 }
 
 void ContentSettingBubbleContents::OnPerformAction(views::Combobox* combobox) {

chrome/browser/ui/views/content_setting_bubble_contents.h

@@ -91,12 +91,11 @@ class ContentSettingBubbleContents : public content::WebContentsObserver,
   };
 
   class Favicon;
+  class ListItemContainer;
 
   // This allows ContentSettingBubbleViewsBridge to call SetAnchorRect().
   friend class chrome::ContentSettingBubbleViewsBridge;
 
-  typedef std::map<views::Link*, int> ListItemLinks;
-
   // Applies the colors appropriate for |theme| to the learn more button.
   void StyleLearnMoreButton(const ui::NativeTheme* theme);
 
@@ -116,9 +115,8 @@ class ContentSettingBubbleContents : public content::WebContentsObserver,
   // Provides data for this bubble.
   std::unique_ptr<ContentSettingBubbleModel> content_setting_bubble_model_;
 
-  // Some of our controls, so we can tell what's been clicked when we get a
-  // message.
-  ListItemLinks list_item_links_;
+  ListItemContainer* list_item_container_;
+
   typedef std::vector<views::RadioButton*> RadioGroup;
   RadioGroup radio_group_;
   views::Link* custom_link_;