Adding device selection menus to the content setting bubble

chrome/browser/ui/views/content_setting_bubble_contents.cc

 // Copyright (c) 2012 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/content_setting_bubble_contents.h"
 
 #include <algorithm>
 #include <set>
 #include <string>
 #include <vector>
 
+#include "base/bind.h"
+#include "base/prefs/pref_service.h"
+#include "base/stl_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/content_settings/host_content_settings_map.h"
 #include "chrome/browser/plugins/plugin_finder.h"
 #include "chrome/browser/plugins/plugin_metadata.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/chrome_style.h"
 #include "chrome/browser/ui/content_settings/content_setting_bubble_model.h"
+#include "chrome/browser/ui/content_settings/content_setting_media_menu_model.h"
 #include "chrome/browser/ui/views/browser_dialogs.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/plugin_service.h"
+#include "content/public/browser/web_contents.h"
 #include "grit/generated_resources.h"
+#include "grit/theme_resources.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/models/simple_menu_model.h"
+#include "ui/views/controls/button/menu_button.h"
 #include "ui/views/controls/button/radio_button.h"
 #include "ui/views/controls/button/text_button.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/link.h"
+#include "ui/views/controls/menu/menu.h"
+#include "ui/views/controls/menu/menu_model_adapter.h"
+#include "ui/views/controls/menu/menu_runner.h"
 #include "ui/views/controls/separator.h"
 #include "ui/views/layout/grid_layout.h"
 #include "ui/views/layout/layout_constants.h"
 
 #if defined(USE_AURA)
 #include "ui/base/cursor/cursor.h"
 #endif
 
+namespace {
+
 // If we don't clamp the maximum width, then very long URLs and titles can make
 // the bubble arbitrarily wide.
 const int kMaxContentsWidth = 500;
 
 // When we have multiline labels, we should set a minimum width lest we get very
 // narrow bubbles with lots of line-wrapping.
 const int kMinMultiLineContentsWidth = 250;
 
+// The width of the media menu button.

[Peter Kasting, 2013/02/16 02:55:13]

Nit: width -> minimum width

Also, I'm a fan of defining these constants locally where they're used, assuming
they're only used within one function, as opposed to at the top of the file.

[no longer working on chromium, 2013/02/18 17:20:04]

Done changing the comment to minimum width.

Regarding to a static local constant or a constant inside the namespace, I think
both are OK, but since this file puts all constants inside the namespace on top,
I will prefer to following the existing way. 

[Peter Kasting, 2013/02/18 18:23:18]

Yeah, I was really suggesting fixing all of these as appropriate.

+const int kMinMediaMenuButtonWidth = 100;
+
+}
+
 using content::PluginService;
 using content::WebContents;
 
 class ContentSettingBubbleContents::Favicon : public views::ImageView {

[Peter Kasting, 2013/02/16 02:55:13]

Nit: Since you have a number of classes defined in this file, consider putting
dividers between each, as we do elsewhere:

... previous code ...


// ClassName --------------------------...

... code for class ...

Stylistically, we generally make the "--"s extend out to 79 characters, and we
generally have two blank lines above each divider and one below.

[no longer working on chromium, 2013/02/18 17:20:04]

Done.

  public:
   Favicon(const gfx::Image& image,
           ContentSettingBubbleContents* parent,
           views::Link* link);
   virtual ~Favicon();
 
  private:
   // views::View overrides:
   virtual bool OnMousePressed(const ui::MouseEvent& event) OVERRIDE;
   virtual void OnMouseReleased(const ui::MouseEvent& event) OVERRIDE;
   virtual gfx::NativeCursor GetCursor(const ui::MouseEvent& event) OVERRIDE;
 
   ContentSettingBubbleContents* parent_;
   views::Link* link_;
 };
 
 ContentSettingBubbleContents::Favicon::Favicon(
     const gfx::Image& image,
     ContentSettingBubbleContents* parent,
     views::Link* link)
     : parent_(parent),
       link_(link) {
   SetImage(image.AsImageSkia());
 }
 
 ContentSettingBubbleContents::Favicon::~Favicon() {
 }
 
+struct ContentSettingBubbleContents::MediaMenuParts {
+  explicit MediaMenuParts(content::MediaStreamType type);
+  ~MediaMenuParts();
+
+  content::MediaStreamType type;
+  scoped_ptr<ui::SimpleMenuModel> menu_model;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MediaMenuParts);
+};
+
+ContentSettingBubbleContents::MediaMenuParts::MediaMenuParts(
+    content::MediaStreamType type)
+    : type(type) {}
+
+ContentSettingBubbleContents::MediaMenuParts::~MediaMenuParts() {}
+
 bool ContentSettingBubbleContents::Favicon::OnMousePressed(
     const ui::MouseEvent& event) {
   return event.IsLeftMouseButton() || event.IsMiddleMouseButton();
 }
 
 void ContentSettingBubbleContents::Favicon::OnMouseReleased(
     const ui::MouseEvent& event) {
   if ((event.IsLeftMouseButton() || event.IsMiddleMouseButton()) &&
      HitTestPoint(event.location())) {
     parent_->LinkClicked(link_, event.flags());
@@ skipping 22 matching lines @@
       manage_link_(NULL),
       close_button_(NULL) {
   // Compensate for built-in vertical padding in the anchor view's image.
   set_anchor_insets(gfx::Insets(5, 0, 5, 0));
 
   registrar_.Add(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
                  content::Source<WebContents>(web_contents));
 }
 
 ContentSettingBubbleContents::~ContentSettingBubbleContents() {
+  STLDeleteValues(&media_menus_);
 }
 
 gfx::Size ContentSettingBubbleContents::GetPreferredSize() {
   gfx::Size preferred_size(views::View::GetPreferredSize());
   int preferred_width =
       (!content_setting_bubble_model_->bubble_content().domain_lists.empty() &&
        (kMinMultiLineContentsWidth > preferred_size.width())) ?
       kMinMultiLineContentsWidth : preferred_size.width();
   preferred_size.set_width(std::min(preferred_width, kMaxContentsWidth));
   return preferred_size;
 }
 
+void ContentSettingBubbleContents::UpdateMenuLabel(
+    content::MediaStreamType type,
+    const std::string& label) {
+  for (MediaMenuPartsMap::const_iterator it = media_menus_.begin();
+       it != media_menus_.end(); ++it) {
+    if (it->second->type == type) {
+      it->first->SetText(UTF8ToUTF16(label));
+      return;
+    }
+  }
+  NOTREACHED();
+}
+
 void ContentSettingBubbleContents::Init() {
   using views::GridLayout;
 
   GridLayout* layout = new views::GridLayout(this);
   SetLayoutManager(layout);
 
   const int kSingleColumnSetId = 0;
   views::ColumnSet* column_set = layout->AddColumnSet(kSingleColumnSetId);
   column_set->AddColumn(GridLayout::LEADING, GridLayout::FILL, 1,
                         GridLayout::USE_PREF, 0, 0);
@@ skipping 77 matching lines @@
       layout->StartRow(0, indented_kSingleColumnSetId);
       layout->AddView(radio);
       bubble_content_empty = false;
     }
     DCHECK(!radio_group_.empty());
     // Now that the buttons have been added to the view hierarchy, it's safe
     // to call SetChecked() on them.
     radio_group_[radio_group.default_item]->SetChecked(true);
   }
 
+  // Layout code for the media device menus.
+  if (content_setting_bubble_model_->content_type() ==
+      CONTENT_SETTINGS_TYPE_MEDIASTREAM) {
+    const int kMediaMenuColumnSetId = 2;
+    views::ColumnSet* menu_column_set =
+        layout->AddColumnSet(kMediaMenuColumnSetId);
+    menu_column_set->AddPaddingColumn(
+        0, chrome_style::kCheckboxIndent);
+    menu_column_set->AddColumn(GridLayout::LEADING, GridLayout::FILL, 0,
+                               GridLayout::USE_PREF, 0, 0);
+    menu_column_set->AddPaddingColumn(
+        0, views::kRelatedControlHorizontalSpacing);
+    menu_column_set->AddColumn(GridLayout::LEADING, GridLayout::FILL, 1,
+                               GridLayout::USE_PREF, 0, 0);
+
+    int max_menu_label_width = 0;
+    int menu_width = 0;
+    for (ContentSettingBubbleModel::MediaMenuMap::const_iterator i(
+         bubble_content.media_menus.begin());
+         i != bubble_content.media_menus.end(); ++i) {
+      if (!bubble_content_empty)
+        layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
+      layout->StartRow(0, kMediaMenuColumnSetId);
+
+      views::Label* menu_label =
+          new views::Label(UTF8ToUTF16(i->second.label));
+      menu_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+      views::MenuButton* menu_button = new views::MenuButton(
+          NULL, UTF8ToUTF16((i->second.selected_device.name)), this, false);
+      menu_button->set_alignment(views::TextButton::ALIGN_LEFT);
+      menu_button->set_border(
+          new views::TextButtonNativeThemeBorder(menu_button));
+      menu_button->set_animate_on_state_change(false);
+      menu_button->SetEnabledColor(SK_ColorBLACK);

[Peter Kasting, 2013/02/16 02:55:13]

Why do we need to set all these?  Don't the default colors work?

I always worry when I see people hardcoding colors, that it will cause some
clash with themes or system colors in some case.

[no longer working on chromium, 2013/02/18 17:20:04]

It also works by using the default. I used those specific values since they were
used in other places.
Removed the hardcoded colors code.

+      menu_button->SetHighlightColor(SK_ColorBLACK);
+      menu_button->SetHoverColor(SK_ColorBLACK);
+
+      MediaMenuParts* menu_view = new MediaMenuParts(i->first);
+      menu_view->menu_model.reset(new ContentSettingMediaMenuModel(
+          Profile::FromBrowserContext(
+              web_contents_->GetBrowserContext())->GetPrefs(),
+          i->second.default_device.type,
+          content_setting_bubble_model_.get(),
+          base::Bind(&ContentSettingBubbleContents::UpdateMenuLabel,
+                     base::Unretained(this))));
+      media_menus_[menu_button] = menu_view;
+
+      // Calculate the optimal width for the menu buttons.

[Peter Kasting, 2013/02/16 02:55:13]

Nit: Maybe instead of "optimal width" you could say what we're ultimately
calculating (i.e. the width required by the longest item in any of the menus),
and that we'll ultimately set all the menus to that width below.

[no longer working on chromium, 2013/02/18 17:20:04]

Done with updating the comment.

+      int menu_preferred_width = GetPreferredMediaMenuWidth(
+          menu_button, menu_view->menu_model.get());
+      menu_width = std::max(menu_width, menu_preferred_width);
+      max_menu_label_width = std::max(max_menu_label_width,
+                                      menu_label->GetPreferredSize().width());
+      int avaialble_width =

[Peter Kasting, 2013/02/16 02:55:13]

Nit: typo

[no longer working on chromium, 2013/02/18 17:20:04]

Done.

+          layout->GetPreferredSize(this).width() -
+          chrome_style::kCheckboxIndent -
+          max_menu_label_width -
+          2 * views::kRelatedControlHorizontalSpacing;
+      menu_width = menu_width <= kMinMediaMenuButtonWidth ?

[Peter Kasting, 2013/02/16 02:55:13]

Nit: You can write this more simply using std::max(); plus, I think this isn't
quite correct, in that I think if |available_width| is smaller than
kMinMediaMenuButtonWidth, you want to use |available_width|, lest you extend
outside the bubble:

  menu_width = std::min(std::max(menu_width, kMinMediaMenuButtonWidth),
available_width);

[no longer working on chromium, 2013/02/18 17:20:04]

Done with the suggested change.

But just want to make sure it is what we want, when the minimum width is bigger
than the available width, don't we want to use the minimum width and force the
layout to get bigger instead?

[Peter Kasting, 2013/02/18 18:23:18]

? You're just restating what I said:

Me: "if |available_width| is smaller than kMinMediaMenuButtonWidth..."
You: "when the minimum width is bigger than the available width..."

Did you mistype?

If we don't have room for the minimum width, then clearly we don't want to force
things to the minimum width, or they'll escape the bubble.

Unless the preferred size depends on the menu width?  In which case making the
menus larger is fine and we'll just enlarge the whole bubble, which suggests in
turn that we shouldn't even be checking the available width.  I don't know, how
does layout here work?

[no longer working on chromium, 2013/02/19 14:37:35]

No, I asked the question precisely how we should handle the case. Since
personally I think we should set it to minimum width anyhow. And this will only
enlarge the bubble.
All right, I thought you knew how the layout work here.

The menus won't escape the bubble in any case, when we set them to a width more
than the existing layout, the layout will automatically become bigger to fit the
menus.

So I think we both agree that we should just use the longest width for the menu
here, in favor of avoiding the truncated strings.

+          kMinMediaMenuButtonWidth :
+          std::min(menu_width, avaialble_width);
+
+      layout->AddView(menu_label);
+      layout->AddView(menu_button);
+
+      bubble_content_empty = false;
+    }
+
+    // Set the optimal width for the menu buttons.

[Peter Kasting, 2013/02/16 02:55:13]

Nit: Perhaps "// Now set all the menus to the width we calculated above."

[no longer working on chromium, 2013/02/18 17:20:04]

Done.

+    for (MediaMenuPartsMap::const_iterator i = media_menus_.begin();
+         i != media_menus_.end(); ++i) {
+      i->first->set_min_width(menu_width);
+      i->first->set_max_width(menu_width);
+    }
+  }
+
   gfx::Font domain_font =
       views::Label().font().DeriveFont(0, gfx::Font::BOLD);
   for (std::vector<ContentSettingBubbleModel::DomainList>::const_iterator i(
        bubble_content.domain_lists.begin());
        i != bubble_content.domain_lists.end(); ++i) {
     layout->StartRow(0, kSingleColumnSetId);
     views::Label* section_title = new views::Label(UTF8ToUTF16(i->title));
     section_title->SetMultiLine(true);
     section_title->SetHorizontalAlignment(gfx::ALIGN_LEFT);
     layout->AddView(section_title, 1, 1, GridLayout::FILL, GridLayout::LEADING);
@@ skipping 75 matching lines @@
     // CAREFUL: Showing the settings window activates it, which deactivates the
     // info bubble, which causes it to close, which deletes us.
     return;
   }
 
   PopupLinks::const_iterator i(popup_links_.find(source));
   DCHECK(i != popup_links_.end());
   content_setting_bubble_model_->OnPopupClicked(i->second);
 }
 
+void ContentSettingBubbleContents::OnMenuButtonClicked(
+    views::View* source,
+    const gfx::Point& point) {
+  MediaMenuPartsMap::iterator i(media_menus_.find(
+      static_cast<views::MenuButton*>(source)));
+  DCHECK(i != media_menus_.end());
+
+  views::MenuModelAdapter menu_model_adapter(i->second->menu_model.get());
+  menu_runner_.reset(new views::MenuRunner(menu_model_adapter.CreateMenu()));
+
+  gfx::Point screen_location;
+  views::View::ConvertPointToScreen(i->first, &screen_location);
+  ignore_result(menu_runner_->RunMenuAt(
+      source->GetWidget(),
+      i->first,
+      gfx::Rect(screen_location, i->first->size()),
+      views::MenuItemView::TOPLEFT,
+      views::MenuRunner::HAS_MNEMONICS));
+}
+
 void ContentSettingBubbleContents::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   DCHECK_EQ(content::NOTIFICATION_WEB_CONTENTS_DESTROYED, type);
   DCHECK(source == content::Source<WebContents>(web_contents_));
   web_contents_ = NULL;
 }
+
+int ContentSettingBubbleContents::GetPreferredMediaMenuWidth(
+    views::MenuButton* button, ui::SimpleMenuModel* menu_model) {

[Peter Kasting, 2013/02/16 02:55:13]

Nit: One arg per line

[no longer working on chromium, 2013/02/18 17:20:04]

Done.

+  // Store the current title for the button.

[Peter Kasting, 2013/02/16 02:55:13]

Nit: The comment here and below are unnecessary, the code alone is clear.

[no longer working on chromium, 2013/02/18 17:20:04]

Done.

+  string16 title = button->text();
+
+  int width = button->GetPreferredSize().width();
+  for (int i = 0; i < menu_model->GetItemCount(); ++i) {
+    button->SetText(menu_model->GetLabelAt(i));
+    int item_width = button->GetPreferredSize().width();
+    width = width > item_width ? width : item_width;

[Peter Kasting, 2013/02/16 02:55:13]

Nit: Simpler:

  width = std::max(width, button->GetPreferredSize().width());

[no longer working on chromium, 2013/02/18 17:20:04]

Done.

+  }
+
+  // Recover the title for the menu button.
+  button->SetText(title);
+  return width;
+}