Show icons for custom menuitems in contextmenu.

components/renderer_context_menu/render_view_context_menu_base.cc

Index: components/renderer_context_menu/render_view_context_menu_base.cc
diff --git a/components/renderer_context_menu/render_view_context_menu_base.cc b/components/renderer_context_menu/render_view_context_menu_base.cc
index cda6c4667aef1908774a90842753fd62598a7b95..3a4badd8fc1e04d6c1574b44f46ed9495ab3d114 100644
--- a/components/renderer_context_menu/render_view_context_menu_base.cc
+++ b/components/renderer_context_menu/render_view_context_menu_base.cc
@@ -16,6 +16,7 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/menu_item.h"
 #include "third_party/WebKit/public/web/WebContextMenuData.h"
+#include "ui/gfx/image/image.h"
 
 using blink::WebContextMenuData;
 using blink::WebString;
@@ -67,11 +68,29 @@ bool IsCustomItemCheckedInternal(const std::vector<content::MenuItem>& items,
 const size_t kMaxCustomMenuDepth = 5;
 const size_t kMaxCustomMenuTotalItems = 1000;
 
-void AddCustomItemsToMenu(const std::vector<content::MenuItem>& items,
-                          size_t depth,
-                          size_t* total_items,
-                          ui::SimpleMenuModel::Delegate* delegate,
-                          ui::SimpleMenuModel* menu_model) {
+}  // namespace
+
+void RenderViewContextMenuBase::OnIconDownloaded(
+    int id,
+    int http_status_code,
+    const GURL& image_url,
+    const std::vector<SkBitmap>& bitmaps,
+    const std::vector<gfx::Size>& original_bitmap_sizes) {
+  if (bitmaps.empty())
+    return;
+  content::MenuItem item = icon_map_.find(id)->second;
+  int command_id = RenderViewContextMenuBase::ConvertToContentCustomCommandId(
+      item.action);
+  gfx::Image* icon = new gfx::Image(gfx::Image::CreateFrom1xBitmap(bitmaps[0]));

[lazyboy, 2015/02/05 01:09:55]

|icon| is leaked after this function returns, right?
You'll need to find a better way to manage lifetime of this.

[pals, 2015/02/05 09:01:02]

Done.

+  UpdateMenuItem(command_id, item.enabled, false, item.label, icon);

[lazyboy, 2015/02/05 01:09:55]

The .enabled and .label can be retrieved from the menu_model_ instead of storing
it separately in the map.
This is also race-y, e.g. you're updating the label to the initial/original
value; What if the label already changed before tis download finishes.

Consider the following:
you have an item with label "a", has an icon
you initiate download of the icon
something else changes the label of the same item to "b"
download icon finishes and it updates the icon, but also incorrectly updates the
label to "a".

+}
+
+void RenderViewContextMenuBase::AddCustomItemsToMenu(
+    const std::vector<content::MenuItem>& items,
+    size_t depth,
+    size_t* total_items,
+    ui::SimpleMenuModel::Delegate* delegate,
+    ui::SimpleMenuModel* menu_model) {
   if (depth > kMaxCustomMenuDepth) {
     LOG(ERROR) << "Custom menu too deeply nested.";
     return;
@@ -79,6 +98,16 @@ void AddCustomItemsToMenu(const std::vector<content::MenuItem>& items,
   for (size_t i = 0; i < items.size(); ++i) {
     int command_id = RenderViewContextMenuBase::ConvertToContentCustomCommandId(
         items[i].action);
+    if (!items[i].icon.empty()) {
+      int download_id = source_web_contents_->DownloadImage(
+          GURL(items[i].icon),

[lazyboy, 2015/02/05 01:09:55]

this URL is coming from the renderer/ process, so we need to make sure we don't
do unexpected things. We can ask some security folks to chime in here, I'll do
that once other concerns are addressed.

[pals, 2015/02/05 09:01:02]

I think we should put a max size limitation on the icon's size.

+          false,  // is_favicon
+          0,      // no max size
+          base::Bind(&RenderViewContextMenuBase::OnIconDownloaded,
+                     weak_ptr_factory_.GetWeakPtr()));
+      icon_map_[download_id] = items[i];
+    }
+
     if (!RenderViewContextMenuBase::IsContentCustomCommandId(command_id)) {
       LOG(ERROR) << "Custom menu action value out of range.";
       return;
@@ -126,8 +155,6 @@ void AddCustomItemsToMenu(const std::vector<content::MenuItem>& items,
   }
 }
 
-}  // namespace
-
 // static
 void RenderViewContextMenuBase::SetContentCustomCommandIdRange(
     int first, int last) {
@@ -159,7 +186,8 @@ RenderViewContextMenuBase::RenderViewContextMenuBase(
       menu_model_(this),
       render_frame_id_(render_frame_host->GetRoutingID()),
       command_executed_(false),
-      render_process_id_(render_frame_host->GetProcess()->GetID()) {
+      render_process_id_(render_frame_host->GetProcess()->GetID()),
+      weak_ptr_factory_(this) {
 }
 
 RenderViewContextMenuBase::~RenderViewContextMenuBase() {
@@ -218,12 +246,14 @@ void RenderViewContextMenuBase::AddSubMenu(int command_id,
 void RenderViewContextMenuBase::UpdateMenuItem(int command_id,
                                            bool enabled,
                                            bool hidden,
-                                           const base::string16& label) {
+                                           const base::string16& label,
+                                           gfx::Image* icon) {
   if (toolkit_delegate_) {
     toolkit_delegate_->UpdateMenuItem(command_id,
                                       enabled,
                                       hidden,
-                                      label);
+                                      label,
+                                      icon);
   }
 }