Adding device selection menus to the content setting bubble

chrome/browser/ui/content_settings/content_setting_media_menu_model.h

+// Copyright (c) 2013 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.
+
+#ifndef CHROME_BROWSER_UI_CONTENT_SETTINGS_CONTENT_SETTING_MEDIA_MENU_MODEL_H_
+#define CHROME_BROWSER_UI_CONTENT_SETTINGS_CONTENT_SETTING_MEDIA_MENU_MODEL_H_
+
+#include <map>
+
+#include "content/public/common/media_stream_request.h"
+#include "ui/base/models/simple_menu_model.h"
+
+class ContentSettingBubbleModel;
+class Profile;
+
+// A menu model that builds the contents of the media capture devices menu in
+// the content setting bubble.
+class ContentSettingMediaMenuModel
+    : public ui::SimpleMenuModel,
+      public ui::SimpleMenuModel::Delegate {
+ public:
+  // This is the observer interface class that the client implements to
+  // observe the change on the menu model.
+  class Observer {

[tfarina, 2013/02/06 13:31:36]

Don't use nested classes for Observer/Listener (whatever you call it)!

https://codereview.chromium.org/12039058/diff/1/content/public/browser/host_z...

When you inherit from it in
chrome/browser/ui/views/content_setting_bubble_contents.h
you see that none of them are using this pattern? Are all
FooObserver/FooListener rather than Foo::Observer, Foo::Listener.

I hope John's recommendation above is strong enough to support my suggestion
here.

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

Thanks tfarina, I got hint from the thread you attached that a callback is
preferred since there is one interface.
Please check the code again if you want to.

+   public:
+    // Derived implementations must call shared_memory_.Map appropriately
+    // before Process can be called.
+    virtual void UpdateMenuLabel(content::MediaStreamType type,
+                                 const std::string& label) = 0;
+  };
+
+  ContentSettingMediaMenuModel(
+      Profile* profile,
+      content::MediaStreamType type,
+      ContentSettingBubbleModel* bubble_model,
+      Observer* observer);
+
+  virtual ~ContentSettingMediaMenuModel();
+
+  // ui::SimpleMenuModel::Delegate implementation:
+  virtual bool IsCommandIdChecked(int command_id) const OVERRIDE;
+  virtual bool IsCommandIdEnabled(int command_id) const OVERRIDE;
+  virtual bool GetAcceleratorForCommandId(
+      int command_id,
+      ui::Accelerator* accelerator) OVERRIDE;
+  virtual void ExecuteCommand(int command_id) OVERRIDE;
+
+ private:
+  typedef std::map<int, content::MediaStreamDevice> CommandMap;
+
+  // Internal method to build the menu.
+  void BuildMenu();
+
+  Profile* profile_;
+  content::MediaStreamType type_;
+  ContentSettingBubbleModel* media_bubble_model_;  // Weak.
+  Observer* observer_;

[markusheintz_, 2013/02/06 11:03:26]

This is a weak pointer too.

[no longer working on chromium, 2013/02/06 13:31:52]

Done.

+
+  // Map of command IDs to devices.
+  CommandMap commands_;
+
+  DISALLOW_COPY_AND_ASSIGN(ContentSettingMediaMenuModel);
+};
+
+#endif  // CHROME_BROWSER_UI_CONTENT_SETTINGS_CONTENT_SETTING_MEDIA_MENU_MODEL_H_