DOM UI options handler cleanup:

chrome/browser/dom_ui/options/advanced_options_handler.cc

 // Copyright (c) 2011 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 "chrome/browser/dom_ui/options/advanced_options_handler.h"
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/callback.h"
@@ skipping 39 matching lines @@
 #endif
 
 AdvancedOptionsHandler::AdvancedOptionsHandler() {
 #if defined(GOOGLE_CHROME_BUILD) && defined(OS_WIN)
   cloud_print_proxy_ui_enabled_ = true;
 #elif !defined(OS_CHROMEOS)
   cloud_print_proxy_ui_enabled_ =
       CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableCloudPrintProxy);
 #endif
+  cloud_print_proxy_ui_enabled_ = true;
 }
 
 AdvancedOptionsHandler::~AdvancedOptionsHandler() {
 }
 
 void AdvancedOptionsHandler::GetLocalizedValues(
     DictionaryValue* localized_strings) {
   DCHECK(localized_strings);
 
-  OptionsStringResource resources[] = {
+  static OptionsStringResource resources[] = {
     { "downloadLocationGroupName",
       IDS_OPTIONS_DOWNLOADLOCATION_GROUP_NAME },
     { "downloadLocationChangeButton",
       IDS_OPTIONS_DOWNLOADLOCATION_CHANGE_BUTTON },
     { "downloadLocationBrowseTitle",
       IDS_OPTIONS_DOWNLOADLOCATION_BROWSE_TITLE },
     { "downloadLocationBrowseWindowTitle",
       IDS_OPTIONS_DOWNLOADLOCATION_BROWSE_WINDOW_TITLE },
     { "downloadLocationAskForSaveLocation",
       IDS_OPTIONS_DOWNLOADLOCATION_ASKFORSAVELOCATION },
@@ skipping 100 matching lines @@
   RegisterStrings(localized_strings, resources, arraysize(resources));
   RegisterTitle(localized_strings, "advancedPage",
                 IDS_OPTIONS_ADVANCED_TAB_LABEL);
 
   localized_strings->SetString("privacyLearnMoreURL",
       google_util::AppendGoogleLocaleParam(
           GURL(chrome::kPrivacyLearnMoreURL)).spec());
 #if !defined(OS_CHROMEOS)
   // Add the cloud print proxy management ui section if it's been runtime
   // enabled.
-  localized_strings->SetString(
-      "enable-cloud-print-proxy",
-      cloud_print_proxy_ui_enabled_ ? "true" : "false" );
+  localized_strings->SetBoolean("enable-cloud-print-proxy",
+                                cloud_print_proxy_ui_enabled_);
 #endif
 }
 
 void AdvancedOptionsHandler::Initialize() {
   DCHECK(dom_ui_);
   SetupMetricsReportingCheckbox();
   SetupMetricsReportingSettingVisibility();
   SetupFontSizeLabel();
   SetupDownloadLocationPath();
   SetupAutoOpenFileTypesDisabledAttribute();
@@ skipping 166 matching lines @@
   UserMetricsRecordAction(UserMetricsAction("Options_ResetAutoOpenFiles"));
   DownloadManager* manager = dom_ui_->GetProfile()->GetDownloadManager();
   if (manager)
     manager->download_prefs()->ResetAutoOpen();
 }
 
 void AdvancedOptionsHandler::HandleMetricsReportingCheckbox(
     const ListValue* args) {
 #if defined(GOOGLE_CHROME_BUILD) && !defined(OS_CHROMEOS)
   std::string checked_str = WideToUTF8(ExtractStringValue(args));
-  bool enabled = (checked_str == "true");

[Mike Mammarella, 2011/02/11 18:38:48]

Personally I like these parens, because they make it clear that I'm not
assigning checked_str to enabled and then comparing the result to "true" and
throwing that away. In this case the types make that mostly clear, but you never
can tell what C++ might do with implicit conversions and such...

Does clang really not like them usually?

[Evan Stade, 2011/02/11 19:23:33]

well, I think clang would complain, but I guess I'm not positive. When an
assignment is used as a truth value, g++ warns unless you put extra parens
around it (to prove you really mean it). Transversely, extra parens around a
comparison could indicate that you meant to do an assignment, so clang warns.

What I'm not positive about is if assigning to a bool variable counts the same
as a truth value in the context of if().

p.s. what makes this clear for me is operator precedence, not the types

[Mike Mammarella, 2011/02/11 20:56:19]

Yeah, there are just some operators that I don't always remember the precedence
of, and that I've gotten wrong in the past, so I tend to try and avoid counting
on anything other than the simple ones that everybody remembers. Maybe I'm just
weird in not remembering = vs ==...

Overall LGTM in any event.

+  bool enabled = checked_str == "true";
   UserMetricsRecordAction(
       enabled ?
           UserMetricsAction("Options_MetricsReportingCheckbox_Enable") :
           UserMetricsAction("Options_MetricsReportingCheckbox_Disable"));
   bool is_enabled = OptionsUtil::ResolveMetricsReportingEnabled(enabled);
   enable_metrics_recording_.SetValue(is_enabled);
   SetupMetricsReportingCheckbox();
 #endif
 }
 
 void AdvancedOptionsHandler::HandleDefaultFontSize(const ListValue* args) {
   int font_size;
   if (ExtractIntegerValue(args, &font_size)) {
     if (font_size > 0) {
       default_font_size_.SetValue(font_size);
       default_fixed_font_size_.SetValue(font_size);
       SetupFontSizeLabel();
     }
   }
 }
 
 #if defined(OS_WIN)
 void AdvancedOptionsHandler::HandleCheckRevocationCheckbox(
     const ListValue* args) {
   std::string checked_str = WideToUTF8(ExtractStringValue(args));
-  bool enabled = (checked_str == "true");
+  bool enabled = checked_str == "true";
   std::string metric =
       (enabled ? "Options_CheckCertRevocation_Enable"
                : "Options_CheckCertRevocation_Disable");
   UserMetricsRecordAction(UserMetricsAction(metric.c_str()));
   net::SSLConfigServiceWin::SetRevCheckingEnabled(enabled);
 }
 
 void AdvancedOptionsHandler::HandleUseSSL3Checkbox(const ListValue* args) {
   std::string checked_str = WideToUTF8(ExtractStringValue(args));
-  bool enabled = (checked_str == "true");
+  bool enabled = checked_str == "true";
   std::string metric =
       (enabled ? "Options_SSL3_Enable" : "Options_SSL3_Disable");
   UserMetricsRecordAction(UserMetricsAction(metric.c_str()));
   net::SSLConfigServiceWin::SetSSL3Enabled(enabled);
 }
 
 void AdvancedOptionsHandler::HandleUseTLS1Checkbox(const ListValue* args) {
   std::string checked_str = WideToUTF8(ExtractStringValue(args));
-  bool enabled = (checked_str == "true");
+  bool enabled = checked_str == "true";
   std::string metric =
       (enabled ? "Options_TLS1_Enable" : "Options_TLS1_Disable");
   UserMetricsRecordAction(UserMetricsAction(metric.c_str()));
   net::SSLConfigServiceWin::SetTLS1Enabled(enabled);
 }
 
 void AdvancedOptionsHandler::HandleShowGearsSettings(const ListValue* args) {
   UserMetricsRecordAction(UserMetricsAction("Options_GearsSettings"));
   GearsSettingsPressed(
       dom_ui_->tab_contents()->view()->GetTopLevelNativeWindow());
@@ skipping 201 matching lines @@
   FundamentalValue useSSL3Value(useSSL3Setting);
   dom_ui_->CallJavascriptFunction(
       L"options.AdvancedOptions.SetUseSSL3CheckboxState",
       useSSL3Value, disabledValue);
   FundamentalValue useTLS1Value(useTLS1Setting);
   dom_ui_->CallJavascriptFunction(
       L"options.AdvancedOptions.SetUseTLS1CheckboxState",
       useTLS1Value, disabledValue);
 }
 #endif