Disable the Developer Tools when Javascript is disabled.

chrome/browser/policy/configuration_policy_handler.cc

Index: chrome/browser/policy/configuration_policy_handler.cc
diff --git a/chrome/browser/policy/configuration_policy_handler.cc b/chrome/browser/policy/configuration_policy_handler.cc
index 38f9e69bab074dcd714408887a6ba230617d5eee..c06d13daf1ca1c73e904071ff9067269184d4c20 100644
--- a/chrome/browser/policy/configuration_policy_handler.cc
+++ b/chrome/browser/policy/configuration_policy_handler.cc
@@ -35,6 +35,36 @@ 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
@@ -370,8 +400,6 @@ const PolicyToPreferenceMapEntry kSimplePolicyMap[] = {
     prefs::kSavingBrowserHistoryDisabled },
   { Value::TYPE_BOOLEAN, kPolicyClearSiteDataOnExit,
     prefs::kClearSiteDataOnExit },
-  { Value::TYPE_BOOLEAN, kPolicyDeveloperToolsDisabled,
-    prefs::kDevToolsDisabled },
   { Value::TYPE_BOOLEAN, kPolicyBlockThirdPartyCookies,
     prefs::kBlockThirdPartyCookies },
   { Value::TYPE_INTEGER, kPolicyDefaultCookiesSetting,
@@ -518,24 +546,6 @@ const ProxyModeValidationEntry kProxyModeValidationMap[] = {
 };
 
 
-// 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(
@@ -1235,35 +1245,61 @@ JavascriptPolicyHandler::~JavascriptPolicyHandler() {
 
 bool JavascriptPolicyHandler::CheckPolicySettings(const PolicyMap* policies,
                                                   PolicyErrorMap* errors) {
-  const Value* javascript_enabled = policies->Get(kPolicyJavascriptEnabled);
-  const Value* default_setting = policies->Get(kPolicyDefaultJavaScriptSetting);
-
-  if (javascript_enabled && !javascript_enabled->IsType(Value::TYPE_BOOLEAN)) {
-    errors->AddError(kPolicyJavascriptEnabled,
-                     IDS_POLICY_TYPE_ERROR,
-                     ValueTypeToString(Value::TYPE_BOOLEAN));
-    javascript_enabled = NULL;
+  // Whether Javascript is globally disabled.
+  bool disabled = false;
+  ConfigurationPolicyType disabling_policy;
+
+  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;
 }
 
@@ -1271,24 +1307,41 @@ 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.

+  }
 }