Give platform apps control over launcher right-click context menu.

chrome/browser/extensions/context_menu_manager.h

+// 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_EXTENSIONS_CONTEXT_MENU_MANAGER_H_
+#define CHROME_BROWSER_EXTENSIONS_CONTEXT_MENU_MANAGER_H_
+
+#include <map>
+
+#include "base/callback.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
+#include "chrome/browser/extensions/menu_manager.h"
+#include "ui/base/models/simple_menu_model.h"
+
+class Profile;
+
+namespace extensions {
+
+// Shared code between various extension context menu related things
+class ContextMenuManager {

[asargent_no_longer_on_chrome, 2012/09/10 20:25:20]

I keep trying to come up with a better name to suggest for this class, since we
already have the MenuManager class in extensions which serves a somewhat
different purpose. The MenuManager class is all about holding the logical model
of items that might apply in a concrete situation, and this class is a helper
for populating a context menu in a concrete situation. 

Some ideas I've come up with:

ContextMenuItems
ContextMenuDelegate
ContextMenuAppender
ContextMenuMatcher

I think I might like ContextMenuMatcher a little better than the others, but
none of them seem great. Got any better ideas?

[Marijn Kruisselbrink, 2012/09/10 23:16:57]

I agree that the name of the class sucks. Unfortunately I don't have better
ideas (otherwise I would have used a different name to begin with :) ). Not sure
I like Matcher better... maybe ContextMenuHandler? Although that's also pretty
vague in what it does...
Oh, and is browser/extensions/ the right location for this class, considering
that it is used i code in browser/tab_contents, browser/ui/app_list and
browser/ui/ash/launcher?

[asargent_no_longer_on_chrome, 2012/09/21 19:44:22]

I think I have a slight preference for ContextMenuMatcher, but
ContextMenuHandler seems ok to me too. Pick whichever one you like better.

Also, I think it's ok to leave this code defined here in
chrome/browser/extensions/. It seems pretty focused on extension specific stuff.

+ public:
+  static const size_t kMaxExtensionItemTitleLength;
+
+  ContextMenuManager(Profile* profile,
+                     ui::SimpleMenuModel::Delegate* delegate,
+                     ui::SimpleMenuModel* menu_model,
+                     const base::Callback<bool(const MenuItem*)>& filter);

[asargent_no_longer_on_chrome, 2012/09/10 20:25:20]

It's probably worth a comment here that |filter| will be called on possibly
matching items, and its result used to make the final decision about inclusion. 

[Marijn Kruisselbrink, 2012/09/10 23:16:57]

Done.

+
+  // This is a helper function to append items for one particular extension.
+  // The |index| parameter is used for assigning id's, and is incremented for
+  // each item actually added.
+  void AppendExtensionItems(const std::string& extension_id,
+                            const string16& selection_text,
+                            int* index);
+
+  void Clear();
+
+  bool IsCommandIdChecked(int command_id) const;
+  bool IsCommandIdEnabled(int command_id) const;
+  void ExecuteCommand(int command_id,
+    const content::ContextMenuParams& params);
+
+  static bool ContextIsLauncher(const MenuItem* item);

[Marijn Kruisselbrink, 2012/09/06 17:54:50]

This method should probably instead of being here just be implemented separately
by the two launcher context menu related code paths where it is currently used
(it is only used to pass as filter when creating this class).

[asargent_no_longer_on_chrome, 2012/09/10 20:25:20]

Yes, it might make more sense to implemented in an anonymous namespace just
above ExtensionAppItem::GetContextMenuModel. You might also consider renaming it
to "HasLauncherContext", since it looks like |item| could have multiple contexts
and this will return true if LAUNCHER is one of them. 

[Marijn Kruisselbrink, 2012/09/10 23:16:57]

Done.

+
+  // These two methods should only be used for tests

[asargent_no_longer_on_chrome, 2012/09/10 20:25:20]

Can you put them in a protected: section and make the tests in question friend
classes?

Alternatively, sometimes we give methods a name like FooForTest to emphasize
that regular consumers should not use them (this is to prevent the situation of
a chrome developer copy/pasting from existing usage in a test somewhere and
never having come by the .h file to see a comment telling them they shouldn't be
using the method)

[Marijn Kruisselbrink, 2012/09/10 23:16:57]

Unfortunately the test code that uses these methods is (currently) in a class in
an anonymous namespace, not in the test class itself. Which makes adding a
friend declaration a bit harder (it's what I tried initially). I'm not sure what
the best way to go here is. The easiest would be of course to go with the
FooForTest naming and not change anything else, alternatively I guess I can have
this class in the anonymous namespace call a method in
ExtensionContextMenuBrowserTest which then will call these methods making it
possible to add that class as a friend, or move the entire implementation of
these methods that are currently in <anonymous>::TestRenderViewContextMenu to
ExtensionContextMenuBrowserTest.
I think I'm leaning towards that last option, since it seems the cleanest.

[asargent_no_longer_on_chrome, 2012/09/21 19:44:22]

Ok - the last option sounds fine to me. 

+  std::map<int, MenuItem::Id>::iterator map_begin() {
+    return extension_item_map_.begin();
+  }
+
+  std::map<int, MenuItem::Id>::iterator map_end() {
+    return extension_item_map_.end();
+  }
+
+ private:
+  MenuItem::List GetRelevantExtensionItems(
+      const MenuItem::List& items,
+      bool can_cross_incognito);
+
+  // Used for recursively adding submenus of extension items.
+  void RecursivelyAppendExtensionItems(
+    const MenuItem::List& items,
+    bool can_cross_incognito,
+    const string16& selection_text,
+    ui::SimpleMenuModel* menu_model,
+    int* index);
+
+  // Attempts to get an MenuItem given the id of a context menu item.
+  extensions::MenuItem* GetExtensionMenuItem(int id) const;
+
+  // This will set the icon on the most recently-added item in the menu_model_.
+  void SetExtensionIcon(const std::string& extension_id);
+
+  Profile* profile_;
+  ui::SimpleMenuModel* menu_model_;
+  ui::SimpleMenuModel::Delegate* delegate_;
+
+  base::Callback<bool(const MenuItem*)> filter_;
+
+  // Maps the id from a context menu item to the MenuItem's internal id.
+  std::map<int, extensions::MenuItem::Id> extension_item_map_;
+
+  // Keep track of and clean up menu models for submenus.
+  ScopedVector<ui::SimpleMenuModel> extension_menu_models_;
+
+  DISALLOW_COPY_AND_ASSIGN(ContextMenuManager);
+};
+
+}  // namespace extensions
+
+#endif  // CHROME_BROWSER_EXTENSIONS_CONTEXT_MENU_MANAGER_H_