Unify the Windows Parental Controls Platform Caching

chrome/browser/prefs/incognito_mode_prefs.cc

Index: chrome/browser/prefs/incognito_mode_prefs.cc
diff --git a/chrome/browser/prefs/incognito_mode_prefs.cc b/chrome/browser/prefs/incognito_mode_prefs.cc
index e0255d4947d11a90c3d2e5fbbd56f707459c811b..f08a76feca3231b5c42ab6c7fcc8769a23ea49f2 100644
--- a/chrome/browser/prefs/incognito_mode_prefs.cc
+++ b/chrome/browser/prefs/incognito_mode_prefs.cc
@@ -7,23 +7,65 @@
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/prefs/pref_service.h"
+#include "base/threading/thread_restrictions.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "components/pref_registry/pref_registry_syncable.h"
+#include "content/public/browser/browser_thread.h"
 
 #if defined(OS_WIN)
-#include "base/win/metro.h"
+#include <windows.h>
+#include <wpcapi.h>
+#include "base/win/scoped_comptr.h"
+#include "base/win/windows_version.h"
 #endif  // OS_WIN
 
 #if defined(OS_ANDROID)
 #include "chrome/browser/android/chromium_application.h"
 #endif  // OS_ANDROID
 
+using content::BrowserThread;
+
 #if defined(OS_WIN)
 namespace {
 
-bool g_parental_control_on = false;
+// Possible values for the parental controls state.
+enum class ParentalControlsState {
+  // Parental controls state unknown. We have not queried for this value yet.
+  UNKNOWN = 0,
+  // Parental controls activity logging disabled.
+  ACTIVITY_LOGGING_DISABLED = 1,
+  // Parental controls activity logging enabled.
+  ACTIVITY_LOGGING_ENABLED = 2,
+} g_parental_controls_state = ParentalControlsState::UNKNOWN;
+
+// Returns true if Windows Parental control activity logging is enabled. This
+// feature is available on Windows Vista and beyond.

[gab, 2015/03/03 20:05:03]

Is it? You only query it for Win7+?

[robliao, 2015/03/03 20:20:35]

Looks like this comment wasn't updated in
https://codereview.chromium.org/671273003

Fixed.

+// This function should be called on a COM Initialized thread.

[gab, 2015/03/03 20:05:03]

Also mention that this can be blocking? And move the wait/IO assertions here?

[robliao, 2015/03/03 20:20:35]

Done.

+bool IsParentalControlActivityLoggingOn() {
+  using namespace base::win;

[gab, 2015/03/03 20:05:03]

Hmmm, I wouldn't do this, what does it simplify? I like explicit everywhere
unless it feels like clutter.

(and if you insist on doing it, at least get rid of base::win:: instances below
;-)!)

[robliao, 2015/03/03 20:20:35]

This simplifies the ScopedComPtr uses below. Unfortunately, there's ambiguity
for base::win::GetVersion with GetVersion from Windows, so that has to stay.

+
+  // Query this info on Windows 7 and above.
+  if (base::win::GetVersion() < base::win::VERSION_WIN7)
+    return false;
+
+  ScopedComPtr<IWindowsParentalControlsCore> parent_controls;
+  HRESULT hr = parent_controls.CreateInstance(
+      __uuidof(WindowsParentalControls));
+  if (FAILED(hr))
+    return false;
+
+  ScopedComPtr<IWPCSettings> settings;
+  hr = parent_controls->GetUserSettings(NULL, settings.Receive());

[gab, 2015/03/03 20:05:03]

s/NULL/nullptr

[robliao, 2015/03/03 20:20:35]

Done.

+  if (FAILED(hr))
+    return false;
+
+  unsigned long restrictions = 0;
+  settings->GetRestrictions(&restrictions);
+
+  return (restrictions & WPCFLAG_LOGGING_REQUIRED) == WPCFLAG_LOGGING_REQUIRED;
+}
 
 } // empty namespace
 #endif  // OS_WIN
@@ -104,25 +146,22 @@ bool IncognitoModePrefs::CanOpenBrowser(Profile* profile) {
 // static
 bool IncognitoModePrefs::ArePlatformParentalControlsEnabled() {
 #if defined(OS_WIN)
-  // Disable incognito mode windows if parental controls are on. This is only
-  // for Windows Vista and above.
-  return base::win::IsParentalControlActivityLoggingOn();
-#elif defined(OS_ANDROID)
-  return chrome::android::ChromiumApplication::AreParentalControlsEnabled();
-#else
-  return false;
-#endif
-}
-
-#if defined(OS_WIN)
-void IncognitoModePrefs::InitializePlatformParentalControls() {
-  g_parental_control_on = base::win::IsParentalControlActivityLoggingOn();
-}
-#endif // OS_WIN
-
-bool IncognitoModePrefs::ArePlatformParentalControlsEnabledCached() {
-#if defined(OS_WIN)
-  return g_parental_control_on;
+  if (g_parental_controls_state == ParentalControlsState::UNKNOWN) {

[grt (UTC plus 2), 2015/03/03 20:28:54]

g_parental_controls_state should be a local static since it isn't accessed
outside of this function.

[robliao, 2015/03/03 21:07:12]

Done.

+    // Production: The thread isn't initialized, so we're the only thread with
+    // IO and waiting allowed.
+    // Test: The thread may be initialized, so check that it's the UI thread.
+    DCHECK(
+        !BrowserThread::IsThreadInitialized(BrowserThread::UI) ||
+        BrowserThread::CurrentlyOn(BrowserThread::UI));
+    base::ThreadRestrictions::AssertIOAllowed();
+    base::ThreadRestrictions::AssertWaitAllowed();
+    g_parental_controls_state =
+        IsParentalControlActivityLoggingOn() ?
+            ParentalControlsState::ACTIVITY_LOGGING_ENABLED :
+            ParentalControlsState::ACTIVITY_LOGGING_DISABLED;
+  }
+  return g_parental_controls_state ==
+      ParentalControlsState::ACTIVITY_LOGGING_ENABLED;
 #elif defined(OS_ANDROID)
   return chrome::android::ChromiumApplication::AreParentalControlsEnabled();
 #else