refactor: prefer inline constexpr string_view for string constants (#44405)

* refactor: BaseWindow::OnExecuteAppCommand() now takes a std::string_view

* refactor: NativeWindow::NotifyWindowExecuteAppCommand() takes a std::string_view

* refactor: AppCommandToString() returns a std::string_view, is now constexpr

* refactor: make kBrowserBackward, kBrowserForward inline constexpr std::string_view

Xref: https://abseil.io/tips/140

https://groups.google.com/a/chromium.org/g/chromium-dev/c/jROTxMo_m2Q/m/HgciN2KsAgAJ

* refactor: use inline constexpr string_view for kDevice*Key constants

Xref: https://abseil.io/tips/140

https://groups.google.com/a/chromium.org/g/chromium-dev/c/jROTxMo_m2Q/m/HgciN2KsAgAJ

* refactor: IsEnvSet now takes a base::cstring_view

* refactor: use inline constexpr cstring_view for kRunAsNode

* refactor: use inline constexpr string_view for kPDF*PluginName

* refactor: use base::FilePath::FromASCII() since "internal-pdf-viewer" is ascii

* chore: remove unused shell/common/electron_constants.cc

* fixup! refactor: IsEnvSet now takes a base::cstring_view

BUILD.gn

@@ -920,7 +920,6 @@ if (is_mac) {
         "shell/app/electron_main_mac.cc",
         "shell/app/uv_stdio_fix.cc",
         "shell/app/uv_stdio_fix.h",
-        "shell/common/electron_constants.cc",
       ]
       include_dirs = [ "." ]
       info_plist = "shell/renderer/resources/mac/Info.plist"

filenames.gni

@@ -582,7 +582,6 @@ filenames = {
     "shell/common/crash_keys.h",
     "shell/common/electron_command_line.cc",
     "shell/common/electron_command_line.h",
-    "shell/common/electron_constants.cc",
     "shell/common/electron_constants.h",
     "shell/common/electron_paths.h",
     "shell/common/gin_converters/accelerator_converter.cc",

shell/app/electron_content_client.cc

@@ -187,7 +187,8 @@ void ElectronContentClient::AddPlugins(
   pdf_info.name = kPDFInternalPluginName;
   pdf_info.description = kPDFPluginDescription;
   // This isn't a real file path; it's just used as a unique identifier.
-  pdf_info.path = base::FilePath(kPdfPluginPath);
+  static constexpr std::string_view kPdfPluginPath = "internal-pdf-viewer";
+  pdf_info.path = base::FilePath::FromASCII(kPdfPluginPath);
   content::WebPluginMimeType pdf_mime_type(
       pdf::kInternalPluginMimeType, kPDFPluginExtension, kPDFPluginDescription);
   pdf_info.mime_types.push_back(pdf_mime_type);

shell/app/electron_main_linux.cc

@@ -8,6 +8,7 @@
 #include "base/at_exit.h"
 #include "base/command_line.h"
 #include "base/i18n/icu_util.h"
+#include "base/strings/cstring_view.h"
 #include "content/public/app/content_main.h"
 #include "electron/fuses.h"
 #include "shell/app/electron_main_delegate.h"  // NOLINT
@@ -18,9 +19,9 @@
 
 namespace {
 
-bool IsEnvSet(const char* name) {
+[[nodiscard]] bool IsEnvSet(const base::cstring_view name) {
-  char* indicator = getenv(name);
+  const char* const indicator = getenv(name.c_str());
-  return indicator && indicator[0] != '\0';
+  return indicator && *indicator;
 }
 
 }  // namespace

shell/app/electron_main_mac.cc

@@ -5,10 +5,12 @@
 #include <cstdlib>
 #include <memory>
 
+#include "base/strings/cstring_view.h"
 #include "electron/fuses.h"
 #include "electron/mas.h"
 #include "shell/app/electron_library_main.h"
 #include "shell/app/uv_stdio_fix.h"
+#include "shell/common/electron_constants.h"
 
 #if defined(HELPER_EXECUTABLE) && !IS_MAS_BUILD()
 #include <mach-o/dyld.h>
@@ -27,9 +29,9 @@ void abort_report_np(const char* fmt, ...);
 
 namespace {
 
-bool IsEnvSet(const char* name) {
+[[nodiscard]] bool IsEnvSet(const base::cstring_view name) {
-  char* indicator = getenv(name);
+  const char* const indicator = getenv(name.c_str());
-  return indicator && indicator[0] != '\0';
+  return indicator && *indicator;
 }
 
 #if defined(HELPER_EXECUTABLE) && !IS_MAS_BUILD()
@@ -51,8 +53,7 @@ bool IsEnvSet(const char* name) {
 int main(int argc, char* argv[]) {
   FixStdioStreams();
 
-  if (electron::fuses::IsRunAsNodeEnabled() &&
+  if (electron::fuses::IsRunAsNodeEnabled() && IsEnvSet(electron::kRunAsNode)) {
-      IsEnvSet("ELECTRON_RUN_AS_NODE")) {
     return ElectronInitializeICUandStartNode(argc, argv);
   }
 

shell/app/electron_main_win.cc

@@ -21,6 +21,7 @@
 #include "base/i18n/icu_util.h"
 #include "base/memory/raw_ptr_exclusion.h"
 #include "base/process/launch.h"
+#include "base/strings/cstring_view.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/win/dark_mode_support.h"
 #include "chrome/app/exit_code_watcher_win.h"
@@ -45,9 +46,9 @@ namespace {
 const char kUserDataDir[] = "user-data-dir";
 const char kProcessType[] = "type";
 
-bool IsEnvSet(const char* name) {
+[[nodiscard]] bool IsEnvSet(const base::cstring_view name) {
-  size_t required_size;
+  size_t required_size = 0;
-  getenv_s(&required_size, nullptr, 0, name);
+  getenv_s(&required_size, nullptr, 0, name.c_str());
   return required_size != 0;
 }
 
@@ -139,7 +140,7 @@ int APIENTRY wWinMain(HINSTANCE instance, HINSTANCE, wchar_t* cmd, int) {
 
 #ifdef _DEBUG
   // Don't display assert dialog boxes in CI test runs
-  static const char kCI[] = "CI";
+  static constexpr base::cstring_view kCI = "CI";
   if (IsEnvSet(kCI)) {
     _CrtSetReportMode(_CRT_ERROR, _CRTDBG_MODE_DEBUG | _CRTDBG_MODE_FILE);
     _CrtSetReportFile(_CRT_ERROR, _CRTDBG_FILE_STDERR);

shell/browser/api/electron_api_base_window.cc

@@ -285,7 +285,7 @@ void BaseWindow::OnWindowAlwaysOnTopChanged() {
   Emit("always-on-top-changed", IsAlwaysOnTop());
 }
 
-void BaseWindow::OnExecuteAppCommand(const std::string& command_name) {
+void BaseWindow::OnExecuteAppCommand(const std::string_view command_name) {
   Emit("app-command", command_name);
 }
 

shell/browser/api/electron_api_base_window.h

@@ -84,7 +84,7 @@ class BaseWindow : public gin_helper::TrackableObject<BaseWindow>,
   void OnWindowEnterHtmlFullScreen() override;
   void OnWindowLeaveHtmlFullScreen() override;
   void OnWindowAlwaysOnTopChanged() override;
-  void OnExecuteAppCommand(const std::string& command_name) override;
+  void OnExecuteAppCommand(std::string_view command_name) override;
   void OnTouchBarItemResult(const std::string& item_id,
                             const base::Value::Dict& details) override;
   void OnNewWindowForTab() override;

shell/browser/native_window.cc

@@ -665,9 +665,10 @@ void NativeWindow::NotifyWindowAlwaysOnTopChanged() {
     observer.OnWindowAlwaysOnTopChanged();
 }
 
-void NativeWindow::NotifyWindowExecuteAppCommand(const std::string& command) {
+void NativeWindow::NotifyWindowExecuteAppCommand(
+    const std::string_view command_name) {
   for (NativeWindowObserver& observer : observers_)
-    observer.OnExecuteAppCommand(command);
+    observer.OnExecuteAppCommand(command_name);
 }
 
 void NativeWindow::NotifyTouchBarItemInteraction(const std::string& item_id,

shell/browser/native_window.h

@@ -326,7 +326,7 @@ class NativeWindow : public base::SupportsUserData,
   void NotifyWindowEnterHtmlFullScreen();
   void NotifyWindowLeaveHtmlFullScreen();
   void NotifyWindowAlwaysOnTopChanged();
-  void NotifyWindowExecuteAppCommand(const std::string& command);
+  void NotifyWindowExecuteAppCommand(std::string_view command_name);
   void NotifyTouchBarItemInteraction(const std::string& item_id,
                                      base::Value::Dict details);
   void NotifyNewWindowForTab();

shell/browser/native_window_observer.h

@@ -6,6 +6,7 @@
 #define ELECTRON_SHELL_BROWSER_NATIVE_WINDOW_OBSERVER_H_
 
 #include <string>
+#include <string_view>
 
 #include "base/observer_list_types.h"
 #include "base/values.h"
@@ -102,7 +103,7 @@ class NativeWindowObserver : public base::CheckedObserver {
 
   // Called on Windows when App Commands arrive (WM_APPCOMMAND)
   // Some commands are implemented on on other platforms as well
-  virtual void OnExecuteAppCommand(const std::string& command_name) {}
+  virtual void OnExecuteAppCommand(std::string_view command_name) {}
 
   virtual void UpdateWindowControlsOverlay(const gfx::Rect& bounding_rect) {}
 };

shell/browser/native_window_views_win.cc

@@ -28,7 +28,7 @@ namespace electron {
 namespace {
 
 // Convert Win32 WM_APPCOMMANDS to strings.
-const char* AppCommandToString(int command_id) {
+constexpr std::string_view AppCommandToString(int command_id) {
   switch (command_id) {
     case APPCOMMAND_BROWSER_BACKWARD:
       return kBrowserBackward;
@@ -227,8 +227,8 @@ void NativeWindowViews::Maximize() {
 }
 
 bool NativeWindowViews::ExecuteWindowsCommand(int command_id) {
-  std::string command = AppCommandToString(command_id);
+  const auto command_name = AppCommandToString(command_id);
-  NotifyWindowExecuteAppCommand(command);
+  NotifyWindowExecuteAppCommand(command_name);
 
   return false;
 }

shell/common/electron_constants.cc

@@ -1,25 +0,0 @@
-// Copyright (c) 2015 GitHub, Inc.
-// Use of this source code is governed by the MIT license that can be
-// found in the LICENSE file.
-
-#include "shell/common/electron_constants.h"
-
-namespace electron {
-
-const char kBrowserForward[] = "browser-forward";
-const char kBrowserBackward[] = "browser-backward";
-
-const char kDeviceVendorIdKey[] = "vendorId";
-const char kDeviceProductIdKey[] = "productId";
-const char kDeviceSerialNumberKey[] = "serialNumber";
-
-const char kRunAsNode[] = "ELECTRON_RUN_AS_NODE";
-
-#if BUILDFLAG(ENABLE_PDF_VIEWER)
-const char kPDFExtensionPluginName[] = "Chromium PDF Viewer";
-const char kPDFInternalPluginName[] = "Chromium PDF Plugin";
-const base::FilePath::CharType kPdfPluginPath[] =
-    FILE_PATH_LITERAL("internal-pdf-viewer");
-#endif  // BUILDFLAG(ENABLE_PDF_VIEWER)
-
-}  // namespace electron

shell/common/electron_constants.h

@@ -5,26 +5,29 @@
 #ifndef ELECTRON_SHELL_COMMON_ELECTRON_CONSTANTS_H_
 #define ELECTRON_SHELL_COMMON_ELECTRON_CONSTANTS_H_
 
-#include "base/files/file_path.h"
+#include <string_view>
+
+#include "base/strings/cstring_view.h"
 #include "electron/buildflags/buildflags.h"
 
 namespace electron {
 
 // The app-command in NativeWindow.
-extern const char kBrowserForward[];
+inline constexpr std::string_view kBrowserForward = "browser-forward";
-extern const char kBrowserBackward[];
+inline constexpr std::string_view kBrowserBackward = "browser-backward";
 
 // Keys for Device APIs
-extern const char kDeviceVendorIdKey[];
+inline constexpr std::string_view kDeviceVendorIdKey = "vendorId";
-extern const char kDeviceProductIdKey[];
+inline constexpr std::string_view kDeviceProductIdKey = "productId";
-extern const char kDeviceSerialNumberKey[];
+inline constexpr std::string_view kDeviceSerialNumberKey = "serialNumber";
 
-extern const char kRunAsNode[];
+inline constexpr base::cstring_view kRunAsNode = "ELECTRON_RUN_AS_NODE";
 
 #if BUILDFLAG(ENABLE_PDF_VIEWER)
-extern const char kPDFExtensionPluginName[];
+inline constexpr std::string_view kPDFExtensionPluginName =
-extern const char kPDFInternalPluginName[];
+    "Chromium PDF Viewer";
-extern const base::FilePath::CharType kPdfPluginPath[];
+inline constexpr std::string_view kPDFInternalPluginName =
+    "Chromium PDF Plugin";
 #endif  // BUILDFLAG(ENABLE_PDF_VIEWER)
 
 }  // namespace electron