Disable the Developer Tools when Javascript is disabled.

chrome/browser/policy/configuration_policy_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/policy/configuration_policy_handler.h"
 
 #include <string>
 #include <vector>
 
 #include "base/file_path.h"
@@ skipping 17 matching lines @@
 #include "chrome/common/content_settings.h"
 #include "chrome/common/pref_names.h"
 #include "grit/generated_resources.h"
 #include "policy/configuration_policy_type.h"
 #include "policy/policy_constants.h"
 
 namespace policy {
 
 namespace {
 
+// Helper functions ------------------------------------------------------------
+
+std::string ValueTypeToString(Value::Type type) {
+  static const char* strings[] = {
+    "null",
+    "boolean",
+    "integer",
+    "double",
+    "string",
+    "binary",
+    "dictionary",
+    "list"
+  };
+  DCHECK(static_cast<size_t>(type) < arraysize(strings));
+  return std::string(strings[type]);
+}
+
+const Value* GetTypeCheckedPolicy(const PolicyMap* policies,
+                                  PolicyErrorMap* errors,
+                                  ConfigurationPolicyType policy,
+                                  Value::Type type) {
+  const Value* value = policies->Get(policy);
+  if (value && !value->IsType(type)) {
+    errors->AddError(policy, IDS_POLICY_TYPE_ERROR, ValueTypeToString(type));
+    return NULL;
+  }
+  return value;
+}
+
+
 // Implementations of ConfigurationPolicyHandler -------------------------------
 
 // Abstract class derived from ConfigurationPolicyHandler that should be
 // subclassed to handle a single policy (not a combination of policies).
 class TypeCheckingPolicyHandler : public ConfigurationPolicyHandler {
  public:
   TypeCheckingPolicyHandler(ConfigurationPolicyType policy_type,
                             base::Value::Type value_type);
 
   // ConfigurationPolicyHandler methods:
@@ skipping 315 matching lines @@
   { Value::TYPE_LIST, kPolicyDisabledPluginsExceptions,
     prefs::kPluginsDisabledPluginsExceptions},
   { Value::TYPE_LIST, kPolicyEnabledPlugins,
     prefs::kPluginsEnabledPlugins},
   { Value::TYPE_BOOLEAN, kPolicyShowHomeButton,
     prefs::kShowHomeButton },
   { Value::TYPE_BOOLEAN, kPolicySavingBrowserHistoryDisabled,
     prefs::kSavingBrowserHistoryDisabled },
   { Value::TYPE_BOOLEAN, kPolicyClearSiteDataOnExit,
     prefs::kClearSiteDataOnExit },
-  { Value::TYPE_BOOLEAN, kPolicyDeveloperToolsDisabled,
-    prefs::kDevToolsDisabled },
   { Value::TYPE_BOOLEAN, kPolicyBlockThirdPartyCookies,
     prefs::kBlockThirdPartyCookies },
   { Value::TYPE_INTEGER, kPolicyDefaultCookiesSetting,
     prefs::kManagedDefaultCookiesSetting },
   { Value::TYPE_INTEGER, kPolicyDefaultImagesSetting,
     prefs::kManagedDefaultImagesSetting },
   { Value::TYPE_INTEGER, kPolicyDefaultPluginsSetting,
     prefs::kManagedDefaultPluginsSetting },
   { Value::TYPE_INTEGER, kPolicyDefaultPopupsSetting,
     prefs::kManagedDefaultPopupsSetting },
@@ skipping 126 matching lines @@
     false, false, false, IDS_POLICY_PROXY_MODE_AUTO_DETECT_ERROR },
   { ProxyPrefs::kPacScriptProxyModeName,
     true, false, false, IDS_POLICY_PROXY_MODE_PAC_URL_ERROR },
   { ProxyPrefs::kFixedServersProxyModeName,
     false, true, true, IDS_POLICY_PROXY_MODE_FIXED_SERVERS_ERROR },
   { ProxyPrefs::kSystemProxyModeName,
     false, false, false, IDS_POLICY_PROXY_MODE_SYSTEM_ERROR },
 };
 
 
-// Helper functions ------------------------------------------------------------
-
-std::string ValueTypeToString(Value::Type type) {
-  static const char* strings[] = {
-    "null",
-    "boolean",
-    "integer",
-    "double",
-    "string",
-    "binary",
-    "dictionary",
-    "list"
-  };
-  DCHECK(static_cast<size_t>(type) < arraysize(strings));
-  return std::string(strings[type]);
-}
-
-
 // TypeCheckingPolicyHandler implementation ------------------------------------
 
 TypeCheckingPolicyHandler::TypeCheckingPolicyHandler(
     ConfigurationPolicyType policy_type,
     Value::Type value_type)
     : policy_type_(policy_type),
       value_type_(value_type) {
 }
 
 TypeCheckingPolicyHandler::~TypeCheckingPolicyHandler() {
 }
 
 ConfigurationPolicyType TypeCheckingPolicyHandler::policy_type() const {
   return policy_type_;
 }
 
 bool TypeCheckingPolicyHandler::CheckPolicySettings(const PolicyMap* policies,
                                                     PolicyErrorMap* errors) {
   const Value* value = policies->Get(policy_type_);
   if (value && value_type_ != value->GetType()) {

[Mattias Nissler (ping if slow), 2011/10/26 15:09:28]

This is almost GetTypeCheckedPolicy(). Share code?

     errors->AddError(policy_type_,
                      IDS_POLICY_TYPE_ERROR,
                      ValueTypeToString(value_type_));
     return false;
   }
   return true;
 }
 
 
 // SimplePolicyHandler implementation ------------------------------------------
@@ skipping 659 matching lines @@
 // JavascriptPolicyHandler implementation --------------------------------------
 
 JavascriptPolicyHandler::JavascriptPolicyHandler() {
 }
 
 JavascriptPolicyHandler::~JavascriptPolicyHandler() {
 }
 
 bool JavascriptPolicyHandler::CheckPolicySettings(const PolicyMap* policies,
                                                   PolicyErrorMap* errors) {
-  const Value* javascript_enabled = policies->Get(kPolicyJavascriptEnabled);
-  const Value* default_setting = policies->Get(kPolicyDefaultJavaScriptSetting);
+  // Whether Javascript is globally disabled.
+  bool disabled = false;
+  ConfigurationPolicyType disabling_policy;
 
-  if (javascript_enabled && !javascript_enabled->IsType(Value::TYPE_BOOLEAN)) {
-    errors->AddError(kPolicyJavascriptEnabled,
-                     IDS_POLICY_TYPE_ERROR,
-                     ValueTypeToString(Value::TYPE_BOOLEAN));
-    javascript_enabled = NULL;
+  const Value* javascript_enabled =
+      GetTypeCheckedPolicy(policies,
+                           errors,
+                           kPolicyJavascriptEnabled,
+                           Value::TYPE_BOOLEAN);
+  if (javascript_enabled) {
+    bool value;
+    if (javascript_enabled->GetAsBoolean(&value) && !value) {
+      disabled = true;
+      disabling_policy = kPolicyJavascriptEnabled;
+    }
   }
 
-  if (default_setting && !default_setting->IsType(Value::TYPE_INTEGER)) {
-    errors->AddError(kPolicyDefaultJavaScriptSetting,
-                     IDS_POLICY_TYPE_ERROR,
-                     ValueTypeToString(Value::TYPE_INTEGER));
-    default_setting = NULL;
+  // DefaultJavaScriptSetting is forced to CONTENT_SETTING_BLOCK if
+  // JavascriptEnabled is false.
+  const Value* default_setting =
+      GetTypeCheckedPolicy(policies,
+                           errors,
+                           kPolicyDefaultJavaScriptSetting,
+                           Value::TYPE_INTEGER);
+  if (default_setting) {
+    int value;
+    if (default_setting->GetAsInteger(&value)) {
+      if (disabled && value != CONTENT_SETTING_BLOCK) {
+        errors->AddError(kPolicyDefaultJavaScriptSetting,
+                         IDS_POLICY_OVERRIDDEN,
+                         GetPolicyName(kPolicyJavascriptEnabled));
+      }
+      if (!disabled && value == CONTENT_SETTING_BLOCK) {
+        disabled = true;
+        disabling_policy = kPolicyDefaultJavaScriptSetting;
+      }
+    }
   }
 
-  bool enabled;
-  int setting;
-  if (javascript_enabled &&
-      default_setting &&
-      javascript_enabled->GetAsBoolean(&enabled) &&
-      default_setting->GetAsInteger(&setting) &&
-      !enabled &&
-      setting != CONTENT_SETTING_BLOCK) {
-    errors->AddError(kPolicyDefaultJavaScriptSetting,
-                     IDS_POLICY_OVERRIDDEN,
-                     GetPolicyName(kPolicyJavascriptEnabled));
+  // DeveloperToolsDisabled is forced to true if javascript is disabled by
+  // JavascriptEnabled or DefaultJavaScriptSetting.
+  const Value* devtools_disabled =
+      GetTypeCheckedPolicy(policies,
+                           errors,
+                           kPolicyDeveloperToolsDisabled,
+                           Value::TYPE_BOOLEAN);
+  if (devtools_disabled) {
+    bool value;
+    if (devtools_disabled->GetAsBoolean(&value) && !value && disabled) {
+      errors->AddError(kPolicyDeveloperToolsDisabled,
+                       IDS_POLICY_OVERRIDDEN,
+                       GetPolicyName(disabling_policy));
+    }
   }
+
   return true;
 }
 
 void JavascriptPolicyHandler::ApplyPolicySettings(const PolicyMap* policies,
                                                   PrefValueMap* prefs) {
   const Value* javascript_enabled = policies->Get(kPolicyJavascriptEnabled);
   const Value* default_setting = policies->Get(kPolicyDefaultJavaScriptSetting);
+  const Value* devtools_disabled = policies->Get(kPolicyDeveloperToolsDisabled);
 
   int setting = CONTENT_SETTING_DEFAULT;
   if (default_setting)
     default_setting->GetAsInteger(&setting);
 
-  bool enabled = true;
-  if (javascript_enabled && javascript_enabled->GetAsBoolean(&enabled)) {
-    prefs->SetValue(prefs::kWebKitJavascriptEnabled,
-                    javascript_enabled->DeepCopy());
-    // Force the javascript content setting to BLOCK when this policy disables
-    // javascript globally.
-    if (!enabled)
-      setting = CONTENT_SETTING_BLOCK;
+  if (javascript_enabled) {
+    bool enabled;
+    if (javascript_enabled->GetAsBoolean(&enabled)) {
+      prefs->SetValue(prefs::kWebKitJavascriptEnabled,
+                      javascript_enabled->DeepCopy());
+      // Force the javascript content setting to BLOCK when this policy disables
+      // javascript globally.
+      if (!enabled)
+        setting = CONTENT_SETTING_BLOCK;
+    }
   }
 
-  if (setting != CONTENT_SETTING_DEFAULT)
+  if (setting != CONTENT_SETTING_DEFAULT) {
     prefs->SetValue(prefs::kManagedDefaultJavaScriptSetting,
                     Value::CreateIntegerValue(setting));
+  }
+
+  // Disable the developer tools if javascript is disabled by JavascriptEnabled
+  // or by DefaultJavaScriptSetting.
+  bool disabled = false;
+  if (devtools_disabled)
+    devtools_disabled->GetAsBoolean(&disabled);
+  if (setting == CONTENT_SETTING_BLOCK)
+    disabled = true;
+  // Only set the preference if there is a policy or if it's forced to true.
+  if (devtools_disabled || setting == CONTENT_SETTING_BLOCK) {
+    prefs->SetValue(prefs::kDevToolsDisabled,
+                    Value::CreateBooleanValue(disabled));

[Mattias Nissler (ping if slow), 2011/10/26 15:09:28]

I'm not sure this is the way to go. It only fixes the problem of DevTools being
inaccessible when Javascript is disabled through policy. If a user disables
Javascript through content settings, they can still try and open DevTools and
they don't get an indication why that fails. I think this is rather a bug on the
DevTools side, and one solution would be to just disable all DevTools shortcuts
and menu items if we detect Javascript is not available. Can you talk to the
DevTools team? If they agree, this block eher is actually not necessary.

[Joao da Silva, 2011/11/03 18:00:10]

It turns out that content settings never disable the DevTools. So deprecating
JavascriptEnabled and wiring it to the content setting behind the scenes will
allow the DevTools to keep working when Javascript is disabled.

The discussion on bug 102668 argues that this is the right thing to do, since
the policy is meant to disable javascript from internet sites; chrome pages
always have javascript enabled anyways, and the DevTools are part of chrome.
They run in the renderer however, and that's why completely disabling javascript
in the renderer led to the DevTool glitch. Admins can still disable the DevTools
through its own policy.

+  }
 }
 
 
 }  // namespace
 
 
 // ConfigurationPolicyHandler implementation -----------------------------------
 
 // static
 ConfigurationPolicyHandler::HandlerList*
@@ skipping 17 matching lines @@
   list->push_back(new SyncPolicyHandler());
 
 #if !defined(OS_CHROMEOS)
   list->push_back(new DownloadDirPolicyHandler());
 #endif  // !defined(OS_CHROME0S)
 
   return list;
 }
 
 }  // namespace policy