Cache the Windows Parental Controls Platform Answer

chrome/browser/prefs/incognito_mode_prefs.cc

 // Copyright (c) 2012 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/prefs/incognito_mode_prefs.h"
 
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/prefs/pref_service.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"
 
 #if defined(OS_WIN)
 #include "base/win/metro.h"
 #endif  // OS_WIN
 
 #if defined(OS_ANDROID)
 #include "chrome/browser/android/chromium_application.h"
 #endif  // OS_ANDROID
 
 #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
 #endif  // OS_WIN
 
 // static
 bool IncognitoModePrefs::IntToAvailability(int in_value,
                                            Availability* out_value) {
   if (in_value < 0 || in_value >= AVAILABILITY_NUM_TYPES) {
     *out_value = ENABLED;
     return false;
   }
   *out_value = static_cast<Availability>(in_value);
   return true;
 }
 
 // static
 IncognitoModePrefs::Availability IncognitoModePrefs::GetAvailability(
     const PrefService* pref_service) {
   DCHECK(pref_service);
   int pref_value = pref_service->GetInteger(prefs::kIncognitoModeAvailability);
   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;
   }
   return result;
 }
 
 // static
 void IncognitoModePrefs::SetAvailability(PrefService* prefs,
                                          const Availability availability) {
@@ skipping 33 matching lines @@
 
     case IncognitoModePrefs::FORCED:
       return profile->IsOffTheRecord();
 
     default:
       NOTREACHED();
       return false;
   }
 }
 
-// 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();
 #else
   return false;
 #endif
 }