[chromecast] Add cast_shell --accept-resource-provider switch

chromecast/base/chromecast_switches.cc

 // 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.
 
+#include "base/command_line.h"

[halliwell, 2016/03/05 01:01:50]

ordering nit: put the header corresponding to the cc file first, then everything
else in a separate group

[jyw, 2016/03/08 02:09:24]

Done.

 #include "chromecast/base/chromecast_switches.h"
 
 namespace switches {
 
+namespace {
+// Value indicating whether flag from command line switch is false.
+const char kSwitchValueFalse[] = "false";
+}  // namespace
+
 // Enable the CMA media pipeline.
 const char kEnableCmaMediaPipeline[] = "enable-cma-media-pipeline";
 
 // The bitmask of codecs (media_caps.h) supported by the current HDMI sink.
 const char kHdmiSinkSupportedCodecs[] = "hdmi-sink-supported-codecs";
 
 // Enable file accesses. It should not be enabled for most Cast devices.
 const char kEnableLocalFileAccesses[] = "enable-local-file-accesses";
 
 // Override the URL to which metrics logs are sent for debugging.
 const char kOverrideMetricsUploadUrl[] = "override-metrics-upload-url";
 
 // Disable features that require WiFi management.
 const char kNoWifi[] = "no-wifi";
 
 // Allows media playback for hidden WebContents
 const char kAllowHiddenMediaPlayback[] = "allow-hidden-media-playback";
 
 // Pass the app id information to the renderer process, to be used for logging.
 // last-launched-app should be the app that just launched and is spawning the
 // renderer.
 const char kLastLaunchedApp[] = "last-launched-app";
 // previous-app should be the app that was running when last-launched-app
 // started.
 const char kPreviousApp[] = "previous-app";
 
+// Flag indicating that a resource provider must be set up to provide cast
+// receiver with resources. Apps cannot start until provided resources.
+const char kAcceptResourceProvider[] = "accept-resource-provider";
+
 // Size of the ALSA output buffer in frames. This directly sets the latency of
 // the output device. Latency can be calculated by multiplying the sample rate
 // by the output buffer size.
 const char kAlsaOutputBufferSize[] = "alsa-output-buffer-size";
 
 // Size of the ALSA output period in frames. The period of an ALSA output device
 // determines how many frames elapse between hardware interrupts.
 const char kAlsaOutputPeriodSize[] = "alsa-output-period-size";
 
 // How many frames need to be in the output buffer before output starts.
 const char kAlsaOutputStartThreshold[] = "alsa-output-start-threshold";
 
 // Minimum number of available frames for scheduling a transfer.
 const char kAlsaOutputAvailMin[] = "alsa-output-avail-min";
 
 // Time in ms to wait before closing the PCM handle when no more mixer inputs
 // remain.
 const char kAlsaCheckCloseTimeout[] = "alsa-check-close-timeout";
 
 // Number of channels on the alsa output device that the stream mixer uses.
 // Default is 2 channels.
 const char kAlsaNumOutputChannels[] = "alsa-num-output-channels";
 
 }  // namespace switches
+
+namespace base {
+
+bool GetSwitchValueBoolean(const std::string& switch_string) {
+  const base::CommandLine* command_line =
+      base::CommandLine::ForCurrentProcess();
+  return command_line->HasSwitch(switch_string) &&
+         command_line->GetSwitchValueASCII(switch_string)
+             .compare(switches::kSwitchValueFalse);

[halliwell, 2016/03/05 01:01:50]

seems like this function accepts any old value that's not "false".  In
particular, 'FALSE', 'False', '0' and various other misleading things would
evaluate true.

Should we compare to an explicit 'true' value and name it IsSwitchExplicitTrue
for symmetry with the other function?

[byungchul, 2016/03/07 17:19:40]

These functions are used to disable a feature explicitly especially for OEMs
when it is enabled by default process.json. For example, when default
process.json has --accept-resource-provider=true and OEM wants to disable it,
they just give --accept-resource-provider=false instead of creating a separate
process.json. We don't encourage OEMs to have separate process.json files
because their own process.json may have command line flags obsoleted later, or
not enable some new features.

GetSwitchValueBoolean() is used to make a flag false by default. So,
no flag -> false,
flag without value -> true,
flag with a value except "false" -> true,
flag with a value "false" -> false

while IsSwitchExplicitFalse() makes a flag true by default. So,
no flag -> true,
flag without value -> true,
flag with a value except "false" -> true,
flag with a value "false" -> false

[halliwell, 2016/03/07 23:33:55]

Thanks for explanation.  It definitely sounds good to keep these functions for
consistency in argument handling.

However, this motivation is not clear from the CL as it stands.  I would
suggest:
1) Adding some comment similar to yours here.
2) Naming the two functions in a consistent/symmetric way, to reflect the fact
that they are the same thing but with different default value.

[kmackay, 2016/03/08 00:06:11]

Or, just have one function, and pass in the default value?

[kmackay, 2016/03/08 00:06:11]

I'd rather just accept 'true' or 'false' (or no value). If a flag is specified
with some other random thing, print an error and use the default value (as if no
flag was specified at all).

[jyw, 2016/03/08 02:09:24]

Done.

+}
+
+bool IsSwitchExplicitFalse(const std::string& switch_string) {
+  const base::CommandLine* command_line =
+      base::CommandLine::ForCurrentProcess();
+  return command_line->HasSwitch(switch_string) &&
+         command_line->GetSwitchValueASCII(switch_string)
+                 .compare(switches::kSwitchValueFalse) == 0;

[halliwell, 2016/03/05 01:10:58]

Also, just remembered: GetSwitchValueASCII returns an empty string if the switch
doesn't exist.  So it's redundant to call HasSwitch.

Which also makes this function have even less value for me as a utility :)

[byungchul, 2016/03/07 17:19:40]

One benefit of these functions is consistency in command line flag pattern.

+}
+
+}  // namespace base