chore: feedback from review

shell/common/api/electron_api_command_line.cc

@@ -10,68 +10,49 @@
 #include "shell/common/gin_converters/file_path_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/node_includes.h"
+#include "third_party/abseil-cpp/absl/strings/ascii.h"
 
 namespace {
+bool HasSwitch(const std::string& switch) {
+  auto switch_string = base::ToLowerASCII(switch_string);
 
-// Chromium will hard CHECK if the switch name is not lowercase.
-bool IsSwitchNameValid(std::string_view switch_name) {
-  return base::ToLowerASCII(switch_name) == switch_name;
-}
-
-bool HasSwitch(gin_helper::ErrorThrower thrower,
-               const std::string& switch_string) {
-  if (!IsSwitchNameValid(switch_string)) {
-    thrower.ThrowError("Switch name must be lowercase");
-    return false;
-  }
-
-  return base::CommandLine::ForCurrentProcess()->HasSwitch(switch_string);
+  auto* command_line = base::CommandLine::ForCurrentProcess();
+  return command_line->HasSwitch(switch_string);
 }
 
 base::CommandLine::StringType GetSwitchValue(gin_helper::ErrorThrower thrower,
                                              const std::string& switch_string) {
-  if (!IsSwitchNameValid(switch_string)) {
-    thrower.ThrowError("Switch name must be lowercase");
-    return base::CommandLine::StringType();
-  }
+  auto switch_str = base::ToLowerASCII(switch_string);
 
-  return base::CommandLine::ForCurrentProcess()->GetSwitchValueNative(
-      switch_string);
+  auto* command_line = base::CommandLine::ForCurrentProcess();
+  return command_line->GetSwitchValueNative(switch_str);
 }
 
 void AppendSwitch(const std::string& switch_string,
                   gin_helper::Arguments* args) {
-  if (!IsSwitchNameValid(switch_string)) {
-    args->ThrowError("Switch name must be lowercase");
-    return;
-  }
-
+  auto switch_str = base::ToLowerASCII(switch_string);
   auto* command_line = base::CommandLine::ForCurrentProcess();
   if (base::EndsWith(switch_string, "-path",
                      base::CompareCase::INSENSITIVE_ASCII) ||
       switch_string == network::switches::kLogNetLog) {
     base::FilePath path;
     args->GetNext(&path);
-    command_line->AppendSwitchPath(switch_string, path);
+    command_line->AppendSwitchPath(switch_str, path);
     return;
   }
 
   base::CommandLine::StringType value;
   if (args->GetNext(&value))
-    command_line->AppendSwitchNative(switch_string, value);
+    command_line->AppendSwitchNative(switch_str, value);
   else
-    command_line->AppendSwitch(switch_string);
+    command_line->AppendSwitch(switch_str);
 }
 
-void RemoveSwitch(gin_helper::ErrorThrower thrower,
-                  const std::string& switch_string) {
-  if (!IsSwitchNameValid(switch_string)) {
-    thrower.ThrowError("Switch name must be lowercase");
-    return;
-  }
+void RemoveSwitch(const std::string& switch_string) {
+  auto switch_str = base::ToLowerASCII(switch_string);
 
   auto* command_line = base::CommandLine::ForCurrentProcess();
-  command_line->RemoveSwitch(switch_string);
+  command_line->RemoveSwitch(switch_str);
 }
 
 void AppendArg(const std::string& arg) {

spec/api-app-spec.ts

@@ -1781,12 +1781,6 @@ describe('app module', () => {
   });
 
   describe('commandLine.hasSwitch', () => {
-    it('throws when an invalid switch is passed', () => {
-      expect(() => {
-        app.commandLine.getSwitchValue('FOOBAR');
-      }).to.throw(/Switch name must be lowercase/);
-    });
-
     it('returns true when present', () => {
       app.commandLine.appendSwitch('foobar1');
       expect(app.commandLine.hasSwitch('foobar1')).to.equal(true);
@@ -1810,12 +1804,6 @@ describe('app module', () => {
   });
 
   describe('commandLine.getSwitchValue', () => {
-    it('throws when an invalid switch is passed', () => {
-      expect(() => {
-        app.commandLine.getSwitchValue('FOOBAR');
-      }).to.throw(/Switch name must be lowercase/);
-    });
-
     it('returns the value when present', () => {
       app.commandLine.appendSwitch('foobar', 'æøåü');
       expect(app.commandLine.getSwitchValue('foobar')).to.equal('æøåü');
@@ -1849,12 +1837,6 @@ describe('app module', () => {
   });
 
   describe('commandLine.removeSwitch', () => {
-    it('throws when an invalid switch is passed', () => {
-      expect(() => {
-        app.commandLine.removeSwitch('FOOBAR');
-      }).to.throw(/Switch name must be lowercase/);
-    });
-
     it('no-ops a non-existent switch', async () => {
       expect(app.commandLine.hasSwitch('foobar3')).to.equal(false);
       app.commandLine.removeSwitch('foobar3');