Generic version of caching cc switches to avoid searching of switches on each function call

cc/base/switches.cc

 // Copyright (c) 2012 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 "cc/base/switches.h"
 
 #include "base/command_line.h"
 
 namespace cc {
 namespace switches {
@@ skipping 133 matching lines @@
 // Makes pixel tests write their output instead of read it.
 const char kCCRebaselinePixeltests[] = "cc-rebaseline-pixeltests";
 
 // Disable textures using RGBA_4444 layout.
 const char kDisable4444Textures[] = "disable-4444-textures";
 
 // Disable touch hit testing in the compositor.
 const char kDisableCompositorTouchHitTesting[] =
     "disable-compositor-touch-hit-testing";
 
-bool IsLCDTextEnabled() {
-  const CommandLine* command_line = CommandLine::ForCurrentProcess();
-  if (command_line->HasSwitch(cc::switches::kDisableLCDText))
-    return false;
-  else if (command_line->HasSwitch(cc::switches::kEnableLCDText))
-    return true;
+class CCSwitchHandler {

[danakj, 2013/11/13 19:37:40]

you're already in the cc:: namespace. How about just "SwitchesHandler"

I don't really see the point of the class here TBH, I think the static
file-local variables are the same as some class-static variables, and this just
adds more code. I'd move all these static class methods back into the public
methods that now do nothing but call them.

My last objection was just that the global variables needed a g_ prefix to their
names.

+ public:
+  static bool isImplSidePaintingEnabled();

[danakj, 2013/11/13 19:37:40]

IsImplSidePaintingEnabled <- capitalized

for all these methods

+  static bool isMapImageEnabled();
+  static bool isLCDTextEnabled();
 
-#if defined(OS_ANDROID)
-  return false;
-#else
-  return true;
-#endif
+ private:
+  static void initializeCCSwitchesIfRequired();
+
+ private:
+  static bool s_has_initialized_switches_;

[danakj, 2013/11/13 19:37:40]

no trailing _ on static variables. they are not instance/member vars.

+  static bool s_impl_side_painting_enabled_;
+  static bool s_map_image_enabled_;
+  static bool s_lcd_text_enabled_;
+};
+
+bool CCSwitchHandler::isImplSidePaintingEnabled() {
+  initializeCCSwitchesIfRequired();
+  return s_impl_side_painting_enabled_;
 }
 
-namespace {
-bool CheckImplSidePaintingStatus() {
+bool CCSwitchHandler::isMapImageEnabled() {
+  initializeCCSwitchesIfRequired();
+  return s_map_image_enabled_;
+}
+
+bool CCSwitchHandler::isLCDTextEnabled() {
+  initializeCCSwitchesIfRequired();
+  return s_lcd_text_enabled_;
+}
+
+void CCSwitchHandler::initializeCCSwitchesIfRequired() {
+  if (s_has_initialized_switches_)
+    return;
+
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
+  // Impl-side painting.
+  if (command_line.HasSwitch(cc::switches::kDisableImplSidePainting)) {
+    s_impl_side_painting_enabled_ = false;
+  } else if (command_line.HasSwitch(cc::switches::kEnableImplSidePainting)) {
+    s_impl_side_painting_enabled_ = true;
+  } else {
+#if defined(OS_ANDROID)
+    s_impl_side_painting_enabled_ = true;
+#else
+    s_impl_side_painting_enabled_ = false;
+#endif
+  }
 
-  if (command_line.HasSwitch(cc::switches::kDisableImplSidePainting))
-    return false;
-  else if (command_line.HasSwitch(cc::switches::kEnableImplSidePainting))
-    return true;
+  // Map-Image.
+  if (command_line.HasSwitch(cc::switches::kDisableMapImage))
+    s_map_image_enabled_ = false;
+  else if (command_line.HasSwitch(cc::switches::kEnableMapImage))
+    s_map_image_enabled_ = true;
+  else
+    s_map_image_enabled_ = false;
 
+  // LCD-Text.
+  if (command_line.HasSwitch(cc::switches::kDisableLCDText)) {
+    s_lcd_text_enabled_ = false;
+  } else if (command_line.HasSwitch(cc::switches::kEnableLCDText)) {
+    s_lcd_text_enabled_ = true;
+  } else {
 #if defined(OS_ANDROID)
-  return true;
+    s_lcd_text_enabled_ = false;
 #else
-  return false;
+    s_lcd_text_enabled_ = true;
 #endif
+  }
+
+  s_has_initialized_switches_ = true;
 }
-}  // namespace
+
+bool CCSwitchHandler::s_has_initialized_switches_ = false;
+bool CCSwitchHandler::s_impl_side_painting_enabled_ = false;
+bool CCSwitchHandler::s_map_image_enabled_ = false;
+bool CCSwitchHandler::s_lcd_text_enabled_ = false;
 
 bool IsImplSidePaintingEnabled() {
-  static bool enabled = CheckImplSidePaintingStatus();
-  return enabled;
+  return CCSwitchHandler::isImplSidePaintingEnabled();
 }
 
 bool IsMapImageEnabled() {
-  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
+  return CCSwitchHandler::isMapImageEnabled();
+}
 
-  if (command_line.HasSwitch(cc::switches::kDisableMapImage))
-    return false;
-  else if (command_line.HasSwitch(cc::switches::kEnableMapImage))
-    return true;
-
-  return false;
+bool IsLCDTextEnabled() {
+  return CCSwitchHandler::isLCDTextEnabled();
 }
 
 }  // namespace switches
 }  // namespace cc