Extend contextMenus API to support browser/page actions

chrome/browser/extensions/menu_manager_factory.cc

Index: chrome/browser/extensions/menu_manager_factory.cc
diff --git a/chrome/browser/extensions/menu_manager_factory.cc b/chrome/browser/extensions/menu_manager_factory.cc
index c90d888a4f34656e3edfa2bd30569ddebf5970d4..b02f671666dcd0c90563a54cf78816880b9ef02f 100644
--- a/chrome/browser/extensions/menu_manager_factory.cc
+++ b/chrome/browser/extensions/menu_manager_factory.cc
@@ -25,6 +25,13 @@ MenuManagerFactory* MenuManagerFactory::GetInstance() {
   return Singleton<MenuManagerFactory>::get();
 }
 
+// static
+KeyedService* MenuManagerFactory::BuildInstanceFor(

[Devlin, 2014/07/24 21:51:28]

If you're gonna do this, why not just make BuildServiceInstanceFor public, or
(better yet) expose a ForTest() method so people get errors if they try to use
it in real code?

[gpdavis, 2014/07/28 21:08:22]

BuildServiceInstanceFor is an overridden non-static method, and BuildInstanceFor
must be static, so there's some conflict (unless I'm misunderstanding
something).

In what way can we ensure that a ForTest() method is only run in a testing
environment?  I'm a bit confused about what this method would be.  The only
thing I could find was this:

https://code.google.com/p/chromium/codesearch#chromium/src/extensions/browser...

but Associate is called in the code I've already added
(GetServiceForBrowserContext calls it).  Maybe a little explanation about what's
bad about the way I've implemented this would help steer me in the right
direction.

[Yoyo Zhou, 2014/07/28 23:23:25]

We have presubmits that warn about the use of methods named "...ForTesting" in
non-test code.

The goal here is not to have methods on the factory that can be accidentally
misused. Basically, it's better to define the method that creates the
MenuManager for tests in the test itself, even if your creating the MenuManager
looks a bit redundant with what's in here. (Sorry, in my earlier comment, I
hadn't realized we had ServiceIsNULLWhileTesting = true.) Here's a simpler
example of how one can use SetTestingFactoryAndUse:

https://code.google.com/p/chromium/codesearch#chromium/src/extensions/browser...

+    content::BrowserContext* context) {
+  Profile* profile = Profile::FromBrowserContext(context);
+  return new MenuManager(profile, ExtensionSystem::Get(profile)->state_store());
+}
+
 MenuManagerFactory::MenuManagerFactory()
     : BrowserContextKeyedServiceFactory(
         "MenuManager",
@@ -36,10 +43,7 @@ MenuManagerFactory::~MenuManagerFactory() {}
 
 KeyedService* MenuManagerFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
-  Profile* profile = Profile::FromBrowserContext(context);
-  return new MenuManager(
-      profile,
-      ExtensionSystem::Get(profile)->state_store());
+  return BuildInstanceFor(context);
 }
 
 content::BrowserContext* MenuManagerFactory::GetBrowserContextToUse(