Remove the Allow/Block drop-down from grouped permission requests

chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/permission_bubble/permission_prompt_impl.h"
 
 #include <stddef.h>
 
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string16.h"
 #include "chrome/browser/permissions/permission_request.h"
 #include "chrome/browser/platform_util.h"
-#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/views/exclusive_access_bubble_views.h"
 #include "chrome/browser/ui/views/harmony/chrome_layout_provider.h"
 #include "chrome/browser/ui/views/page_info/permission_selector_row.h"
 #include "chrome/browser/ui/views/page_info/permission_selector_row_observer.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/strings/grit/components_strings.h"
 #include "components/url_formatter/elide_url.h"
 #include "ui/accessibility/ax_node_data.h"
 #include "ui/base/l10n/l10n_util.h"
-#include "ui/base/models/combobox_model.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/gfx/text_constants.h"
 #include "ui/views/background.h"
 #include "ui/views/bubble/bubble_dialog_delegate.h"
 #include "ui/views/bubble/bubble_frame_view.h"
 #include "ui/views/controls/button/checkbox.h"
-#include "ui/views/controls/button/menu_button.h"
-#include "ui/views/controls/button/menu_button_listener.h"
-#include "ui/views/controls/combobox/combobox.h"
-#include "ui/views/controls/combobox/combobox_listener.h"
 #include "ui/views/controls/label.h"
-#include "ui/views/controls/menu/menu_runner.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/grid_layout.h"
 
 namespace {
 
 // (Square) pixel size of icon.
 const int kIconSize = 18;
 
 }  // namespace
 
-// This class is a MenuButton which is given a PermissionMenuModel. It
-// shows the current checked item in the menu model, and notifies its listener
-// about any updates to the state of the selection.
-// TODO(gbillock): refactor PermissionMenuButton to work like this and re-use?
-class PermissionCombobox : public views::MenuButton,
-                           public views::MenuButtonListener {
- public:
-  // Get notifications when the selection changes.
-  class Listener {
-   public:
-    virtual void PermissionSelectionChanged(int index, bool allowed) = 0;
-  };
-
-  PermissionCombobox(Profile* profile,
-                     Listener* listener,
-                     int index,
-                     const GURL& url,
-                     ContentSetting setting);
-  ~PermissionCombobox() override;
-
-  int index() const { return index_; }
-
-  void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
-
-  // MenuButtonListener:
-  void OnMenuButtonClicked(views::MenuButton* source,
-                           const gfx::Point& point,
-                           const ui::Event* event) override;
-
-  // Callback when a permission's setting is changed.
-  void PermissionChanged(const PageInfoUI::PermissionInfo& permission);
-
- private:
-  int index_;
-  Listener* listener_;
-  std::unique_ptr<PermissionMenuModel> model_;
-  std::unique_ptr<views::MenuRunner> menu_runner_;
-};
-
-PermissionCombobox::PermissionCombobox(Profile* profile,
-                                       Listener* listener,
-                                       int index,
-                                       const GURL& url,
-                                       ContentSetting setting)
-    : MenuButton(base::string16(), this, true),
-      index_(index),
-      listener_(listener),
-      model_(new PermissionMenuModel(
-          profile,
-          url,
-          setting,
-          base::Bind(&PermissionCombobox::PermissionChanged,
-                     base::Unretained(this)))) {
-  SetText(model_->GetLabelAt(model_->GetIndexOfCommandId(setting)));
-  SizeToPreferredSize();
-}
-
-PermissionCombobox::~PermissionCombobox() {}
-
-void PermissionCombobox::GetAccessibleNodeData(ui::AXNodeData* node_data) {
-  MenuButton::GetAccessibleNodeData(node_data);
-  node_data->SetValue(GetText());
-}
-
-void PermissionCombobox::OnMenuButtonClicked(views::MenuButton* source,
-                                             const gfx::Point& point,
-                                             const ui::Event* event) {
-  menu_runner_.reset(
-      new views::MenuRunner(model_.get(), views::MenuRunner::HAS_MNEMONICS));
-
-  gfx::Point p(point);
-  p.Offset(-source->width(), 0);
-  menu_runner_->RunMenuAt(source->GetWidget()->GetTopLevelWidget(), this,
-                          gfx::Rect(p, gfx::Size()), views::MENU_ANCHOR_TOPLEFT,
-                          ui::MENU_SOURCE_NONE);
-}
-
-void PermissionCombobox::PermissionChanged(
-    const PageInfoUI::PermissionInfo& permission) {
-  SetText(model_->GetLabelAt(model_->GetIndexOfCommandId(permission.setting)));
-  SizeToPreferredSize();
-
-  listener_->PermissionSelectionChanged(
-      index_, permission.setting == CONTENT_SETTING_ALLOW);
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 // View implementation for the permissions bubble.
 class PermissionsBubbleDialogDelegateView
-    : public views::BubbleDialogDelegateView,
-      public PermissionCombobox::Listener {
+    : public views::BubbleDialogDelegateView {
  public:
   PermissionsBubbleDialogDelegateView(
       PermissionPromptImpl* owner,
-      const std::vector<PermissionRequest*>& requests,
-      const std::vector<bool>& accept_state);
+      const std::vector<PermissionRequest*>& requests);
   ~PermissionsBubbleDialogDelegateView() override;
 
   void CloseBubble();
   void SizeToContents();
 
   // BubbleDialogDelegateView:
   bool ShouldShowCloseButton() const override;
   const gfx::FontList& GetTitleFontList() const override;
   base::string16 GetWindowTitle() const override;
   void OnWidgetDestroying(views::Widget* widget) override;
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
   bool Cancel() override;
   bool Accept() override;
   bool Close() override;
   int GetDefaultDialogButton() const override;
   int GetDialogButtons() const override;
   base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
 
-  // PermissionCombobox::Listener:
-  void PermissionSelectionChanged(int index, bool allowed) override;
-
   // Updates the anchor's arrow and view. Also repositions the bubble so it's
   // displayed in the correct location.
   void UpdateAnchor(views::View* anchor_view,
                     const gfx::Point& anchor_point,
                     views::BubbleBorder::Arrow anchor_arrow);
 
  private:
   PermissionPromptImpl* owner_;
-  bool multiple_requests_;
   base::string16 display_origin_;
-  std::unique_ptr<PermissionMenuModel> menu_button_model_;
-  std::vector<PermissionCombobox*> customize_comboboxes_;
   views::Checkbox* persist_checkbox_;
 
   DISALLOW_COPY_AND_ASSIGN(PermissionsBubbleDialogDelegateView);
 };
 
 PermissionsBubbleDialogDelegateView::PermissionsBubbleDialogDelegateView(
     PermissionPromptImpl* owner,
-    const std::vector<PermissionRequest*>& requests,
-    const std::vector<bool>& accept_state)
-    : owner_(owner),
-      multiple_requests_(requests.size() > 1),
-      persist_checkbox_(nullptr) {
+    const std::vector<PermissionRequest*>& requests)
+    : owner_(owner), persist_checkbox_(nullptr) {
   DCHECK(!requests.empty());
 
   set_close_on_deactivate(false);
 
   ChromeLayoutProvider* provider = ChromeLayoutProvider::Get();
   SetLayoutManager(new views::BoxLayout(
       views::BoxLayout::kVertical, 0, 0,
       provider->GetDistanceMetric(views::DISTANCE_RELATED_CONTROL_VERTICAL)));
 
   display_origin_ = url_formatter::FormatUrlForSecurityDisplay(
       requests[0]->GetOrigin(),
       url_formatter::SchemeDisplay::OMIT_CRYPTOGRAPHIC);
 
   bool show_persistence_toggle = true;
   for (size_t index = 0; index < requests.size(); index++) {
-    DCHECK(index < accept_state.size());
-    // The row is laid out containing a leading-aligned label area and a
-    // trailing column which will be filled if there are multiple permission
-    // requests.
+    // The row is laid out containing a leading-aligned label area.

[msw, 2017/06/05 23:14:10]

Actually, it seems like this layout could be greatly simplified:
-Remove |row| and |row_layout| and |columns|.
-Use |AddChildView(label_container)| to add that view directly.
-Optionally remove |label_container| and instead use
|label->SetBorder(views::CreateEmptyBorder(0, indent, 0, 0));|, then just use
|AddChildView(label)|

[raymes, 2017/06/05 23:23:54]

Done. I didn't do the optional bit because I wasn't sure what to do with the
icon. I'd rather not change that now though if possible :) 

     views::View* row = new views::View();
     views::GridLayout* row_layout = new views::GridLayout(row);
     row->SetLayoutManager(row_layout);
     views::ColumnSet* columns = row_layout->AddColumnSet(0);
     columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::FILL,
                        0, views::GridLayout::USE_PREF, 0, 0);
-    columns->AddColumn(views::GridLayout::TRAILING, views::GridLayout::FILL,
-                       100, views::GridLayout::USE_PREF, 0, 0);
     row_layout->StartRow(0, 0);
 
     views::View* label_container = new views::View();
     int indent =
         provider->GetDistanceMetric(DISTANCE_SUBSECTION_HORIZONTAL_INDENT);
     label_container->SetLayoutManager(new views::BoxLayout(
         views::BoxLayout::kHorizontal, indent, 0,
         provider->GetDistanceMetric(DISTANCE_RELATED_LABEL_HORIZONTAL)));
     views::ImageView* icon = new views::ImageView();
     const gfx::VectorIcon& vector_id = requests[index]->GetIconId();
     icon->SetImage(
         gfx::CreateVectorIcon(vector_id, kIconSize, gfx::kChromeIconGrey));
     icon->SetTooltipText(base::string16());  // Redundant with the text fragment
     label_container->AddChildView(icon);
     views::Label* label =
         new views::Label(requests.at(index)->GetMessageTextFragment());
     label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
     label_container->AddChildView(label);
     row_layout->AddView(label_container);
 
     // Only show the toggle if every request wants to show it.
     show_persistence_toggle = show_persistence_toggle &&
                               requests[index]->ShouldShowPersistenceToggle();
-    if (requests.size() > 1) {
-      PermissionCombobox* combobox = new PermissionCombobox(
-          owner->GetProfile(), this, index, requests[index]->GetOrigin(),
-          accept_state[index] ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK);
-      row_layout->AddView(combobox);
-      customize_comboboxes_.push_back(combobox);
-    } else {
-      row_layout->AddView(new views::View());
-    }
 
     AddChildView(row);
   }
 
   if (show_persistence_toggle) {
     persist_checkbox_ = new views::Checkbox(
         l10n_util::GetStringUTF16(IDS_PERMISSIONS_BUBBLE_PERSIST_TEXT));
     persist_checkbox_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
     persist_checkbox_->SetChecked(true);
     AddChildView(persist_checkbox_);
@@ skipping 46 matching lines @@
 }
 
 int PermissionsBubbleDialogDelegateView::GetDefaultDialogButton() const {
   // To prevent permissions being accepted accidentally, and as a security
   // measure against crbug.com/619429, permission prompts should not be accepted
   // as the default action.
   return ui::DIALOG_BUTTON_NONE;
 }
 
 int PermissionsBubbleDialogDelegateView::GetDialogButtons() const {
-  int buttons = ui::DIALOG_BUTTON_OK;
-  if (!multiple_requests_)
-    buttons |= ui::DIALOG_BUTTON_CANCEL;
-  return buttons;
+  return ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL;
 }
 
 base::string16 PermissionsBubbleDialogDelegateView::GetDialogButtonLabel(
     ui::DialogButton button) const {
   if (button == ui::DIALOG_BUTTON_CANCEL)
     return l10n_util::GetStringUTF16(IDS_PERMISSION_DENY);
 
   // The text differs based on whether OK is the only visible button.
   return l10n_util::GetStringUTF16(GetDialogButtons() == ui::DIALOG_BUTTON_OK
                                        ? IDS_OK
@@ skipping 14 matching lines @@
     owner_->Accept();
   }
   return true;
 }
 
 bool PermissionsBubbleDialogDelegateView::Close() {
   // Neither explicit accept nor explicit deny.
   return true;
 }
 
-void PermissionsBubbleDialogDelegateView::PermissionSelectionChanged(
-    int index,
-    bool allowed) {
-  owner_->ToggleAccept(index, allowed);
-}
-
 void PermissionsBubbleDialogDelegateView::UpdateAnchor(
     views::View* anchor_view,
     const gfx::Point& anchor_point,
     views::BubbleBorder::Arrow anchor_arrow) {
   set_arrow(anchor_arrow);
 
   // Update the border in the bubble: will either add or remove the arrow.
   views::BubbleFrameView* frame =
       views::BubbleDialogDelegateView::GetBubbleFrameView();
   views::BubbleBorder::Arrow adjusted_arrow = anchor_arrow;
@@ skipping 23 matching lines @@
   delegate_ = delegate;
 }
 
 void PermissionPromptImpl::Show() {
   DCHECK(browser_);
   DCHECK(browser_->window());
 
   if (bubble_delegate_)
     bubble_delegate_->CloseBubble();
 
-  bubble_delegate_ = new PermissionsBubbleDialogDelegateView(
-      this, delegate_->Requests(), delegate_->AcceptStates());
+  bubble_delegate_ =
+      new PermissionsBubbleDialogDelegateView(this, delegate_->Requests());
 
   // Set |parent_window| because some valid anchors can become hidden.
   bubble_delegate_->set_parent_window(
       platform_util::GetViewForWindow(browser_->window()->GetNativeWindow()));
 
   // Compensate for vertical padding in the anchor view's image. Note this is
   // ignored whenever the anchor view is null.
   bubble_delegate_->set_anchor_view_insets(gfx::Insets(
       GetLayoutConstant(LOCATION_BAR_BUBBLE_ANCHOR_VERTICAL_INSET), 0));
 
@@ skipping 58 matching lines @@
 
 void PermissionPromptImpl::Accept() {
   if (delegate_)
     delegate_->Accept();
 }
 
 void PermissionPromptImpl::Deny() {
   if (delegate_)
     delegate_->Deny();
 }
-
-Profile* PermissionPromptImpl::GetProfile() {
-  return browser_->profile();
-}