Emit error if unknown switch is passed to mojo_shell

mojo/shell/switches.cc

 // Copyright 2013 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.
 
 #include "mojo/shell/switches.h"
 
+#include "base/basictypes.h"
+
 namespace switches {
 
 // Specify configuration arguments for a Mojo application URL. For example:
 // --args-for='mojo:wget http://www.google.com'
 const char kArgsFor[] = "args-for";
 
 // Used to specify the type of child process (switch values from
 // |ChildProcess::Type|).
 const char kChildProcessType[] = "child-process-type";
 
@@ skipping 25 matching lines @@
 // Enables the mojo spy, which acts as a man-in-the-middle inspector for
 // message pipes and other activities. This is work in progress.
 const char kSpy[] = "spy";
 
 // Specifies a set of mappings to apply when resolving urls. The value is set of
 // ',' separated mappings, where each mapping consists of a pair of urls giving
 // the to/from url to map. For example, 'a=b,c=d' contains two mappings, the
 // first maps 'a' to 'b' and the second 'c' to 'd'.
 const char kURLMappings[] = "url-mappings";
 
+const char* switch_array[] = {
+  kArgsFor,

[Aaron Boodman, 2014/11/17 15:46:06]

This duplicates information. People are going to update the list of switches,
but forget to update this. In fact, it has already happened -- switch_array is
missing kContentHandlers.

I think if you want to make this change, you should find a way to avoid the
duplicate information. Somehow, defining a switch should add it to the list of
switches.

Sorry.

PS: naming-wise, it should be: kSwitchArray.

[DaveMoore, 2014/11/17 16:36:47]

Name changed. I didn't think you used kFoo for arrays.

It seems pretty unlikely that you'll add a switch and not add it to this array,
as then your switch can't work. kContentHandlers wasn't like that. It was simply
a failure of the cl (thanks).

[Aaron Boodman, 2014/11/17 16:44:45]

Sorry, I disagree. I think it is likely that people will add a switch and not
know or remember this all_switches variable. Especially if they aren't in the
same screenful of code.

Perhaps there is some way you can register the flag right below the line it is
defined... But I can't think of how to do that without significantly refactoring
how switches or adding a static vector.

+  kChildProcessType,
+  kDisableCache,
+  kEnableExternalApplications,
+  kEnableMultiprocess,
+  kHelp,
+  kOrigin,
+  kSpy,
+  kURLMappings
+};
+
+const std::set<std::string>& GetAllSwitches() {
+  static std::set<std::string> switch_set;

[hansmuller, 2014/11/17 15:33:19]

The switch_array and the cached set values aren't performance critical. It might
be better to just make switch_array[] a local in GetAllSwitches() and create the
set each time the function is called.

[Aaron Boodman, 2014/11/17 15:46:07]

It's against Chromium style to have static non-POD types anyway. We spent a lot
of time in Chromium trying to find all of them and kill them because they hurt
startup perf.

[DaveMoore, 2014/11/17 16:36:47]

I see now that you're right that even function statics can't be classes. That
surprised me because they don't invoke static initializers so they have no
impact on startup. Perhaps that's compiler specific.

I changed it to return a new map each time. It's only really going to be called
once at startup anyway.

+  static bool initialized = false;
+  if (!initialized) {
+    for (size_t i = 0; i < arraysize(switch_array); ++i)
+      switch_set.insert(switch_array[i]);
+    initialized = true;
+  }
+  return switch_set;
+}
+
 }  // namespace switches