Add more polices to the protected list.

components/policy/core/common/policy_loader_win.cc

Index: components/policy/core/common/policy_loader_win.cc
diff --git a/components/policy/core/common/policy_loader_win.cc b/components/policy/core/common/policy_loader_win.cc
index c956a74a92913ba65e0614170388debc8c342b16..574b8080d7265b555ceea277729f5e08d3aaf4b5 100644
--- a/components/policy/core/common/policy_loader_win.cc
+++ b/components/policy/core/common/policy_loader_win.cc
@@ -30,6 +30,7 @@
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/histogram.h"
+#include "base/metrics/sparse_histogram.h"
 #include "base/scoped_native_library.h"
 #include "base/sequenced_task_runner.h"
 #include "base/stl_util.h"
@@ -72,6 +73,15 @@ const char kExpectedWebStoreUrl[] =
 // String to be prepended to each blocked entry.
 const char kBlockedExtensionPrefix[] = "[BLOCKED]";
 
+const char* kInsecurePolicies[] = {

[Joao da Silva, 2014/07/17 09:57:55]

Document

[pastarmovj, 2014/07/17 14:12:59]

Done.

+    policy::key::kMetricsReportingEnabled,
+    policy::key::kDefaultSearchProviderEnabled,

[Joao da Silva, 2014/07/17 09:57:55]

Shouldn't we include all of the search provider policies?

[pastarmovj, 2014/07/17 14:12:59]

They are gated on this one can be a useful lead if we get a problem reported if
they are present but this one isn't.

+    policy::key::kHomepageIsNewTabPage,
+    policy::key::kHomepageLocation,
+    policy::key::kRestoreOnStartup,
+    policy::key::kRestoreOnStartupURLs

[Joao da Silva, 2014/07/17 09:57:55]

policy:: isn't needed

[pastarmovj, 2014/07/17 14:12:59]

Done.

+};
+
 // The GUID of the registry settings group policy extension.
 GUID kRegistrySettingsCSEGUID = REGISTRY_EXTENSION_GUID;
 
@@ -147,10 +157,29 @@ void FilterUntrustedPolicy(PolicyMap* policy) {
 
       filtered_values->AppendString(entry);
     }
-    policy->Set(policy::key::kExtensionInstallForcelist,
-                map_entry->level, map_entry->scope,
-                filtered_values.release(),
-                map_entry->external_data_fetcher);
+    if (invalid_policies) {
+      policy->Set(policy::key::kExtensionInstallForcelist,
+                  map_entry->level, map_entry->scope,
+                  filtered_values.release(),
+                  map_entry->external_data_fetcher);
+
+      const PolicyDetails* details = policy::GetChromePolicyDetails(
+          policy::key::kExtensionInstallForcelist);
+      UMA_HISTOGRAM_SPARSE_SLOWLY("EnterpriseCheck.InvalidPolicies",
+                                  details->id);
+    }
+
+    for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kInsecurePolicies); ++i) {

[Joao da Silva, 2014/07/17 09:57:55]

Doesn't arraysize() work here?

[pastarmovj, 2014/07/17 14:12:59]

Done.

+      if (policy->Get(kInsecurePolicies[i])) {
+        policy->Erase(kInsecurePolicies[i]);

[Joao da Silva, 2014/07/17 09:57:55]

It would be cool to have this somehow surface in about:policy, so that admins
understand why these policies are being ignored. Unfortunately we don't have an
easy way to do that now... maybe leave a TODO here and file a bug for this?

[pastarmovj, 2014/07/17 14:12:59]

Done.

+        invalid_policies++;
+        const PolicyDetails* details =
+            policy::GetChromePolicyDetails(kInsecurePolicies[i]);
+        UMA_HISTOGRAM_SPARSE_SLOWLY("EnterpriseCheck.InvalidPolicies",
+                                    details->id);
+      }
+    }

[Joao da Silva, 2014/07/17 09:57:55]

This is in the wrong block: it shouldn't be conditional on the if() in line 137,
which is about the kExtensionInstallForcelist policy.

To make this code more clear, I suggest moving the special handling of
kExtensionInstallForcelist to its own separate function and call that from here.

[pastarmovj, 2014/07/17 14:12:59]

Done.

+
     UMA_HISTOGRAM_COUNTS("EnterpriseCheck.InvalidPoliciesDetected",
                          invalid_policies);
   }