Support Component Actions in the toolbar.

chrome/browser/ui/toolbar/component_toolbar_actions_factory.cc

Index: chrome/browser/ui/toolbar/component_toolbar_actions_factory.cc
diff --git a/chrome/browser/ui/toolbar/component_toolbar_actions_factory.cc b/chrome/browser/ui/toolbar/component_toolbar_actions_factory.cc
index 8f64c079393ae0495e3344bb9b04ca3610bcdd75..4f08cf2a052e7a3e29be1510c9401d0d87417f79 100644
--- a/chrome/browser/ui/toolbar/component_toolbar_actions_factory.cc
+++ b/chrome/browser/ui/toolbar/component_toolbar_actions_factory.cc
@@ -20,6 +20,11 @@ base::LazyInstance<ComponentToolbarActionsFactory> lazy_factory =
 
 }  // namespace
 
+// static
+const char ComponentToolbarActionsFactory::kMediaRouterActionId[] =
+    "media_router_action";
+const char ComponentToolbarActionsFactory::kActionIdForTest[] = "mock_action";
+
 ComponentToolbarActionsFactory::ComponentToolbarActionsFactory()
     : num_component_actions_(-1) {}
 ComponentToolbarActionsFactory::~ComponentToolbarActionsFactory() {}
@@ -29,6 +34,25 @@ ComponentToolbarActionsFactory* ComponentToolbarActionsFactory::GetInstance() {
   return testing_factory_ ? testing_factory_ : &lazy_factory.Get();
 }
 
+// static
+std::vector<std::string> ComponentToolbarActionsFactory::GetComponentIds() {
+  std::vector<std::string>* component_ids = new std::vector<std::string>;

[Peter Kasting, 2015/08/06 20:15:17]

This leaks memory.

[apacible, 2015/08/10 22:38:11]

Switched to scoped_ptr per mfoltz@'s suggestion.

+
+  if (testing_factory_) {
+    component_ids->push_back(ComponentToolbarActionsFactory::kActionIdForTest);
+  } else {
+#if defined(ENABLE_MEDIA_ROUTER)

[Peter Kasting, 2015/08/06 20:15:17]

Nit: If possible, make it so kEnableMediaRouter is defined in all cases, but the
switch value will always be false when ENABLE_MEDIA_ROUTER is not set.  Then
this code, and anywhere else trying to read this flag, can do so without an
ifdef and still get correct behavior.

[apacible, 2015/08/10 22:38:11]

From talking to some teammates more familiar with our choice of ifdef and switch
usage:

There are some builds of Chrome where we don't build Media Router at all, so the
ifdef prevents any references to Media Router symbols from hitting the compiler,
which would cause linker errors.

[Peter Kasting, 2015/08/11 19:10:49]

To be clear, I'm explicitly suggesting making sure switches::kEnableMediaRouter
and ComponentToolbarActionsFactory::kMediaRouterActionId are well-defined even
when ENABLE_MEDIA_ROUTER is false.

This should allow you to compile and link this code properly.  And the fewer
#ifs we can have in the code the better.

[apacible, 2015/08/12 05:26:44]

Updated chrome_switches.cc so kEnableMediaRouter isn't defined only within an
ifdef. I've removed the ifdefs in this file; other uses should be cleaned up in
a separate patch.

+    if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+            ::switches::kEnableMediaRouter)) {
+      component_ids->push_back(
+          ComponentToolbarActionsFactory::kMediaRouterActionId);
+    }
+#endif  // ENABLE_MEDIA_ROUTER
+  }
+
+  return *component_ids;
+}
+
 ScopedVector<ToolbarActionViewController>
 ComponentToolbarActionsFactory::GetComponentToolbarActions(Browser* browser) {
   ScopedVector<ToolbarActionViewController> component_actions;