Optimize ShouldLaunchIncognito() since its on the startup path.

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 2e20d04d8d1230f2863471a010289aefa901f704..c8ac4bdb5faf3fdbcd568ba40cc4e2f89e152947 100644
--- a/chrome/browser/prefs/incognito_mode_prefs.cc
+++ b/chrome/browser/prefs/incognito_mode_prefs.cc
@@ -142,17 +142,7 @@ bool IncognitoModePrefs::IntToAvailability(int in_value,
 // 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 (result == IncognitoModePrefs::FORCED)
-      LOG(ERROR) << "Ignoring FORCED incognito. Parental control logging on";
-    return IncognitoModePrefs::DISABLED;
-  }
-  return result;
+  return GetAvailabilityInternal(pref_service, CHECK_PARENTAL_CONTROLS);
 }
 
 // static
@@ -172,10 +162,18 @@ void IncognitoModePrefs::RegisterProfilePrefs(
 bool IncognitoModePrefs::ShouldLaunchIncognito(
     const base::CommandLine& command_line,
     const PrefService* prefs) {
-  Availability incognito_avail = GetAvailability(prefs);
-  return incognito_avail != IncognitoModePrefs::DISABLED &&
-         (command_line.HasSwitch(switches::kIncognito) ||
-          incognito_avail == IncognitoModePrefs::FORCED);
+  // Note: We only absolutely need to check for parental controls in the
+  // startup path if the user requested to launch in incognito mode or
+  // it was forced via prefs, since computing parental controls can be

[gab, 2016/10/03 17:08:12]

We need to check parental controls (...) since parentals controls can be slow?

Don't think that's what you intended to say here?

[Alexei Svitkine (slow), 2016/10/03 18:57:39]

I think the comment wording did work - "we only do X in certain cases since X
can be slow" is what it's saying, which is a valid phrasing.

However, I agree it was a bit run-on - so I've reworded so it's two sentences
now and easier to read.

+  // slow. Otherwise, we can check later.
+  bool should_use_incognito =

[gab, 2016/10/03 17:08:12]

const

[Alexei Svitkine (slow), 2016/10/03 18:57:39]

Done.

+      command_line.HasSwitch(switches::kIncognito) ||
+      GetAvailabilityInternal(prefs, DONT_CHECK_PARENTAL_CONTROLS) ==
+          IncognitoModePrefs::FORCED;
+  // Only check parental controls only if |should_use_incognito| is true.

[gab, 2016/10/03 17:08:12]

double "only"

This comment is also probably redundant (i.e. it's merely stating what the
simple code below it does word for word).

[Alexei Svitkine (slow), 2016/10/03 18:57:39]

Done. After re-wording the comment above, I'm OK with not having this extra
comment.

+  return should_use_incognito &&
+         GetAvailabilityInternal(prefs, CHECK_PARENTAL_CONTROLS) !=
+             IncognitoModePrefs::DISABLED;
 }
 
 // static
@@ -220,3 +218,20 @@ bool IncognitoModePrefs::ArePlatformParentalControlsEnabled() {
 #endif
 }
 
+// static
+IncognitoModePrefs::Availability IncognitoModePrefs::GetAvailabilityInternal(
+    const PrefService* pref_service,
+    GetAvailabilityMode mode) {
+  DCHECK(pref_service);
+  int pref_value = pref_service->GetInteger(prefs::kIncognitoModeAvailability);
+  Availability result = IncognitoModePrefs::ENABLED;
+  bool valid = IntToAvailability(pref_value, &result);
+  DCHECK(valid);
+  if (result != IncognitoModePrefs::DISABLED &&
+      mode == CHECK_PARENTAL_CONTROLS && ArePlatformParentalControlsEnabled()) {
+    if (result == IncognitoModePrefs::FORCED)
+      LOG(ERROR) << "Ignoring FORCED incognito. Parental control logging on";
+    return IncognitoModePrefs::DISABLED;
+  }
+  return result;
+}