Cache the Windows Parental Controls Platform Answer

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..7fbe112d4fc953b0c9e3d0a2812b4bbedd3270a7 100644
--- a/chrome/browser/prefs/incognito_mode_prefs.cc
+++ b/chrome/browser/prefs/incognito_mode_prefs.cc
@@ -23,6 +23,7 @@
 #if defined(OS_WIN)
 namespace {
 
+bool g_parental_control_checked = false;
 bool g_parental_control_on = false;

[gab, 2015/02/24 20:39:47]

How about an
enum class ParentalControlState {
  UNKNOWN,
  ENABLED,
  DISABLED,
};
rather than two variables?

[robliao, 2015/02/24 23:43:01]

sgtm. Done

 
 } // empty namespace
@@ -47,7 +48,7 @@ IncognitoModePrefs::Availability IncognitoModePrefs::GetAvailability(
   Availability result = IncognitoModePrefs::ENABLED;
   bool valid = IntToAvailability(pref_value, &result);
   DCHECK(valid);
-  if (ArePlatformParentalControlsEnabled()) {
+  if (ArePlatformParentalControlsEnabledCached()) {
     if (result == IncognitoModePrefs::FORCED)
       LOG(ERROR) << "Ignoring FORCED incognito. Parental control logging on";
     return IncognitoModePrefs::DISABLED;
@@ -101,27 +102,16 @@ 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() {

[gab, 2015/02/24 20:39:47]

Also, should we not instead lazily initialize this (or perhaps a mix of
lazy/delay initialization -- i.e. either initialize it asynchronously
post-startup with something like PostTaskAndReplyWithResult() to the
BlockingPool() or lazily when it's requested -- whichever kicks in first).

Doesn't make sense to me to call this unconditionally from
ChromeBrowserMainPartsWin::PreMainMessageLoopStart().

[robliao, 2015/02/24 23:43:01]

It's oddly enough the best place for it since we need the value right when we
fire off the renderers during startup. Separating I/O from the main thread at
startup will help as too many places check to see if incognito is available on
the UI thread.

[gab, 2015/02/25 20:23:51]

I see, since this state is const once initialized, we could consider using
LazyInstance here with an object which initializes this const state in its
constructor.

LazyInstance being thread-safe, we could kick it off the initial initialization
early on a background thread and it would only be waited upon by the main thread
if/when the main thread needs it for real?

[robliao, 2015/02/26 21:03:51]

I'm going to have to think about this some more to see if this makes sense in
the grand scheme things, especially with the plans of getting I/O out of the
init thread.

   g_parental_control_on = base::win::IsParentalControlActivityLoggingOn();

[gab, 2015/02/24 20:39:47]

Any idea why this call is even in base::win:: (and specifically in
base/win/metro.cc)? Seems to only be called from here.

[robliao, 2015/02/24 23:43:01]

In a change a long, long, time ago, this was going to be enabled for Win8 only,
hence metro.cc. Later on, Chrome decided to allow this for Vista and Win7.

+  g_parental_control_checked = true;
 }
 #endif // OS_WIN
 
 bool IncognitoModePrefs::ArePlatformParentalControlsEnabledCached() {
 #if defined(OS_WIN)
+  DCHECK(g_parental_control_checked);
   return g_parental_control_on;
 #elif defined(OS_ANDROID)
   return chrome::android::ChromiumApplication::AreParentalControlsEnabled();