Extend contextMenus API to support browser/page actions

chrome/browser/extensions/api/context_menus/context_menus_api_helpers.h

 // Copyright 2014 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.
 
 // Definition of helper functions for the ContextMenus API.
 
 #ifndef CHROME_BROWSER_EXTENSIONS_API_CONTEXT_MENUS_CONTEXT_MENUS_API_HELPERS_H_
 #define CHROME_BROWSER_EXTENSIONS_API_CONTEXT_MENUS_CONTEXT_MENUS_API_HELPERS_H_
 
 #include "chrome/browser/extensions/menu_manager.h"
@@ skipping 68 matching lines @@
       case PropertyWithEnumT::CONTEXTS_TYPE_VIDEO:
         contexts.Add(extensions::MenuItem::VIDEO);
         break;
       case PropertyWithEnumT::CONTEXTS_TYPE_AUDIO:
         contexts.Add(extensions::MenuItem::AUDIO);
         break;
       case PropertyWithEnumT::CONTEXTS_TYPE_FRAME:
         contexts.Add(extensions::MenuItem::FRAME);
         break;
       case PropertyWithEnumT::CONTEXTS_TYPE_LAUNCHER:
         // Not available for <webview>.

[gpdavis, 2014/07/22 18:52:53]

Should I add this comment in for the browser and page action types as well?  It
looks like the launcher type was added in for the same reason I added browser
and page actions to the web view json file (to maintain the generic nature of
the methods in this class).

[Yoyo Zhou, 2014/07/23 23:37:21]

Yes, these are also not available for webview.

[gpdavis, 2014/07/23 23:57:58]

Done.

         contexts.Add(extensions::MenuItem::LAUNCHER);
         break;
+      case PropertyWithEnumT::CONTEXTS_TYPE_BROWSER_ACTION:
+        contexts.Add(extensions::MenuItem::BROWSER_ACTION);
+        break;
+      case PropertyWithEnumT::CONTEXTS_TYPE_PAGE_ACTION:
+        contexts.Add(extensions::MenuItem::PAGE_ACTION);
+        break;
       case PropertyWithEnumT::CONTEXTS_TYPE_NONE:
         NOTREACHED();
     }
   }
   return contexts;
 }
 
 template<typename PropertyWithEnumT>
 MenuItem::Type GetType(const PropertyWithEnumT& property,
                        MenuItem::Type default_type) {
@@ skipping 34 matching lines @@
     return false;
   }
 
   // Contexts.
   MenuItem::ContextList contexts;
   if (create_properties.contexts.get())
     contexts = GetContexts(create_properties);
   else
     contexts.Add(MenuItem::PAGE);
 
   if (contexts.Contains(MenuItem::LAUNCHER)) {

[gpdavis, 2014/07/23 23:09:52]

@yoyo,

Since you pointed out the bit about webviews, should this conditional also check
for browser_action or page_action contexts being added for <webview>?

Also, this is the function that I ought to be adding this top level limitation
check to, correct?

[Yoyo Zhou, 2014/07/23 23:37:21]

Likewise, yes, this is where you should be returning an error for webviews.

[gpdavis, 2014/07/23 23:57:58]

Done.

     // Launcher item is not allowed for <webview>.
     if (!extension->is_platform_app() || is_webview) {
       *error = kLauncherNotAllowedError;
       return false;
     }
   }
 
   // Title.
   std::string title;
   if (create_properties.title.get())
@@ skipping 145 matching lines @@
     return false;
 
   menu_manager->WriteToStorage(extension, item_id.extension_key);
   return true;
 }
 
 }  // namespace context_menus_api_helpers
 }  // namespace extensions
 
 #endif  // CHROME_BROWSER_EXTENSIONS_API_CONTEXT_MENUS_CONTEXT_MENUS_API_HELPERS_H_