Prevent incognito windows from opening when incognito mode is disabled.

chrome/browser/extensions/extension_tabs_module.cc

Index: chrome/browser/extensions/extension_tabs_module.cc
diff --git a/chrome/browser/extensions/extension_tabs_module.cc b/chrome/browser/extensions/extension_tabs_module.cc
index bf40f57dfd0080acbe758e307eb7221d9f0c08c1..3d4cc67ea432b74968bda2d22eb11e1f6b8ed546 100644
--- a/chrome/browser/extensions/extension_tabs_module.cc
+++ b/chrome/browser/extensions/extension_tabs_module.cc
@@ -387,36 +387,46 @@ bool CreateWindowFunction::RunImpl() {
       panel_bounds.set_height(bounds_val);
     }
 
+    const IncognitoModePrefs::Availability incognito_availability =

[Joao da Silva, 2011/10/28 11:05:29]

These checks are scoped inside an "if (args)" block; but should't the incognito
availability checks be done even if there are no args?

I'd move this block, from here down to the "window_profile = .." line, to
outside the "if (args)" block. The line that checks the args here could be
rewritten as "if (args && args->HasKey(...))".

[rustema, 2011/10/29 06:43:21]

Done.

+        IncognitoModePrefs::GetAvailability(profile_->GetPrefs());
     bool incognito = false;
     if (args->HasKey(keys::kIncognitoKey)) {
       EXTENSION_FUNCTION_VALIDATE(args->GetBoolean(keys::kIncognitoKey,
                                                    &incognito));
-      if (IncognitoModePrefs::GetAvailability(profile_->GetPrefs()) ==
-          IncognitoModePrefs::DISABLED) {
+      if (incognito && incognito_availability == IncognitoModePrefs::DISABLED) {
         error_ = keys::kIncognitoModeIsDisabled;
         return false;
       }
+      if (!incognito && incognito_availability == IncognitoModePrefs::FORCED) {
+        error_ = keys::kIncognitoModeIsForced;
+        return false;
+      }
+    } else if (incognito_availability == IncognitoModePrefs::FORCED) {
+      // If incognito argument is not specified explicitly, we default to
+      // incognito.

[pastarmovj, 2011/10/27 09:14:31]

maybe extend this sentence to include "...when forced so by policy". Because it
sounds like this is the default always to me. The code does the right thing but
only by reading the comments one gets the wrong idea.

[rustema, 2011/10/29 06:43:21]

Done.

+      incognito = true;
+    }
 
-      if (incognito) {
-        std::string first_url_erased;
-        // Guest session is an exception as it always opens in incognito mode.
-        for (size_t i = 0; i < urls.size();) {
-          if (browser::IsURLAllowedInIncognito(urls[i]) &&
-              !Profile::IsGuestSession()) {
-            if (first_url_erased.empty())
-              first_url_erased = urls[i].spec();
-            urls.erase(urls.begin() + i);
-          } else {
-            i++;
-          }
-        }
-        if (urls.empty() && !first_url_erased.empty()) {
-          error_ = ExtensionErrorUtils::FormatErrorMessage(
-              keys::kURLsNotAllowedInIncognitoError, first_url_erased);
-          return false;
+    // If we are opening an incognito window.
+    if (incognito) {
+      std::string first_url_erased;
+      // Guest session is an exception as it always opens in incognito mode.
+      for (size_t i = 0; i < urls.size();) {
+        if (browser::IsURLAllowedInIncognito(urls[i]) &&
+            !Profile::IsGuestSession()) {
+          if (first_url_erased.empty())
+            first_url_erased = urls[i].spec();
+          urls.erase(urls.begin() + i);
+        } else {
+          i++;
         }
-        window_profile = window_profile->GetOffTheRecordProfile();
       }
+      if (urls.empty() && !first_url_erased.empty()) {
+        error_ = ExtensionErrorUtils::FormatErrorMessage(
+            keys::kURLsNotAllowedInIncognitoError, first_url_erased);
+        return false;
+      }
+      window_profile = window_profile->GetOffTheRecordProfile();
     }
 
     if (args->HasKey(keys::kFocusedKey)) {