NTP5: Starting implementation of a native menu for showing other device sessions.

chrome/browser/ui/search/other_device_menu.h

Index: chrome/browser/ui/search/other_device_menu.h
diff --git a/chrome/browser/ui/search/other_device_menu.h b/chrome/browser/ui/search/other_device_menu.h
new file mode 100644
index 0000000000000000000000000000000000000000..e4b116ee3cef2637b83fe70ae594aaf40f45204f
--- /dev/null
+++ b/chrome/browser/ui/search/other_device_menu.h
@@ -0,0 +1,59 @@
+// 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.
+
+#ifndef CHROME_BROWSER_UI_SEARCH_OTHER_DEVICE_MENU_H_
+#define CHROME_BROWSER_UI_SEARCH_OTHER_DEVICE_MENU_H_
+
+#include "base/memory/linked_ptr.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/values.h"
+#include "ui/base/models/simple_menu_model.h"
+
+namespace content {
+class WebUI;
+}
+
+namespace gfx {
+class Point;
+}
+
+namespace views {
+class MenuRunner;
+}
+
+typedef std::map<int, linked_ptr<DictionaryValue> > SessionData;

[dhollowa, 2012/10/01 17:37:38]

A comment here would be nice.  What does the |int| represent?  What's in the
dictionary?

[dhollowa, 2012/10/01 17:37:38]

This should be pulled inside the class - possibly inside the private section if
it is not used externally.  Chrome generally follows the "principle of narrowest
scope".

[jeremycho, 2012/10/01 22:53:40]

Done.

[jeremycho, 2012/10/01 22:53:40]

Done.

+
+class OtherDeviceMenu : public ui::SimpleMenuModel::Delegate {

[dhollowa, 2012/10/01 17:37:38]

Seems like |OtherDevicesMenu| would be more appropriate since it can hold plural
devices.

[dhollowa, 2012/10/01 17:37:38]

A brief comment for the class would be helpful.  Why it needs to exist, its
relation to search_ntp, etc.

[jeremycho, 2012/10/01 22:53:40]

This menu will only hold the session of a single device.

[jeremycho, 2012/10/01 22:53:40]

Done.

+
+ public:
+  OtherDeviceMenu(content::WebUI* web_ui,
+                  const std::string& session_id,
+                  const gfx::Point& location);
+  virtual ~OtherDeviceMenu();
+
+  // Populates and displays the menu.
+  void ShowMenu();
+
+  // ui::SimpleMenuModel::Delegate overrides:
+  virtual bool IsCommandIdChecked(int command_id) const OVERRIDE;
+  virtual bool IsCommandIdEnabled(int command_id) const OVERRIDE;
+  virtual void ExecuteCommand(int command_id) OVERRIDE;
+  virtual void ExecuteCommand(int command_id, int event_flags) OVERRIDE;
+  virtual bool GetAcceleratorForCommandId(
+      int command_id,
+      ui::Accelerator* accelerator) OVERRIDE;
+
+ private:
+  content::WebUI* web_ui_;

[dhollowa, 2012/10/01 17:37:38]

Comment please, indicating ownership, such as "Weak."

[jeremycho, 2012/10/01 22:53:40]

Done.

+  const std::string& session_id_;
+  const gfx::Point& location_;

[dhollowa, 2012/10/01 17:37:38]

Comment please.  Origin of top-left of menu, or where mouse down occurred? 
Maybe same?

[jeremycho, 2012/10/01 22:53:40]

Done.

+  ui::SimpleMenuModel menu_model_;
+  scoped_ptr<views::MenuRunner> menu_runner_;
+  // Mapping of command_id to data for the associated tab of this session.
+  SessionData session_data_;
+
+  DISALLOW_COPY_AND_ASSIGN(OtherDeviceMenu);
+};
+
+#endif  // CHROME_BROWSER_UI_SEARCH_OTHER_DEVICE_MENU_H_