Prevent incognito windows from opening when incognito mode is disabled.

Prevent normal windows from opening when incognito mode is forced.


BUG=97677
TEST=Added a test to chrome/browser/extensions/extension_tabs_test.cc.

Review URL: http://codereview.chromium.org/8373027

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@108058 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/extension_tabs_apitest.cc

@@ -222,3 +222,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, MAYBE_GetViewsOfCreatedWindow) {
   ASSERT_TRUE(RunExtensionSubtest("tabs/basics", "get_views_window.html"))
       << message_;
 }
+
+// Adding a new test? Awesome. But API tests are the old hotness. The
+// new hotness is extension_test_utils. See extension_tabs_test.cc for
+// an example. We are trying to phase out many uses of API tests as
+// they tend to be flaky.

chrome/browser/extensions/extension_tabs_module.cc

@@ -250,6 +250,57 @@ bool GetAllWindowsFunction::RunImpl() {
   return true;
 }
 
+bool CreateWindowFunction::ShouldOpenIncognitoWindow(
+    const base::DictionaryValue* args,
+    std::vector<GURL>* urls,
+    bool* is_error) {
+  *is_error = false;
+  const IncognitoModePrefs::Availability incognito_availability =
+      IncognitoModePrefs::GetAvailability(profile_->GetPrefs());
+  bool incognito = false;
+  if (args && args->HasKey(keys::kIncognitoKey)) {
+    EXTENSION_FUNCTION_VALIDATE(args->GetBoolean(keys::kIncognitoKey,
+                                                 &incognito));
+    if (incognito && incognito_availability == IncognitoModePrefs::DISABLED) {
+      error_ = keys::kIncognitoModeIsDisabled;
+      *is_error = true;
+      return false;
+    }
+    if (!incognito && incognito_availability == IncognitoModePrefs::FORCED) {
+      error_ = keys::kIncognitoModeIsForced;
+      *is_error = true;
+      return false;
+    }
+  } else if (incognito_availability == IncognitoModePrefs::FORCED) {
+    // If incognito argument is not specified explicitly, we default to
+    // incognito when forced so by policy.
+    incognito = true;
+  }
+
+  // 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++;
+      }
+    }
+    if (urls->empty() && !first_url_erased.empty()) {
+      error_ = ExtensionErrorUtils::FormatErrorMessage(
+          keys::kURLsNotAllowedInIncognitoError, first_url_erased);
+      *is_error = true;
+      return false;
+    }
+  }
+  return incognito;
+}
+
 bool CreateWindowFunction::RunImpl() {
   DictionaryValue* args = NULL;
   std::vector<GURL> urls;
@@ -352,6 +403,18 @@ bool CreateWindowFunction::RunImpl() {
   bool saw_focus_key = false;
   std::string extension_id;
 
+  // Decide whether we are opening a normal window or an incognito window.
+  bool is_error;
+  bool open_incognito_window = ShouldOpenIncognitoWindow(args, &urls,
+                                                         &is_error);
+  if (is_error) {
+    // error_ member variable is set inside of ShouldOpenIncognitoWindow.
+    return false;
+  }
+  if (open_incognito_window) {
+    window_profile = window_profile->GetOffTheRecordProfile();
+  }
+
   if (args) {
     // Any part of the bounds can optionally be set by the caller.
     int bounds_val;
@@ -387,38 +450,6 @@ bool CreateWindowFunction::RunImpl() {
       panel_bounds.set_height(bounds_val);
     }
 
-    bool incognito = false;
-    if (args->HasKey(keys::kIncognitoKey)) {
-      EXTENSION_FUNCTION_VALIDATE(args->GetBoolean(keys::kIncognitoKey,
-                                                   &incognito));
-      if (IncognitoModePrefs::GetAvailability(profile_->GetPrefs()) ==
-          IncognitoModePrefs::DISABLED) {
-        error_ = keys::kIncognitoModeIsDisabled;
-        return false;
-      }
-
-      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;
-        }
-        window_profile = window_profile->GetOffTheRecordProfile();
-      }
-    }
-
     if (args->HasKey(keys::kFocusedKey)) {
       EXTENSION_FUNCTION_VALIDATE(args->GetBoolean(keys::kFocusedKey,
                                                    &focused));

chrome/browser/extensions/extension_tabs_module.h

@@ -7,16 +7,22 @@
 #pragma once
 
 #include <string>
+#include <vector>
 
 #include "base/compiler_specific.h"
 #include "chrome/browser/extensions/extension_function.h"
 #include "content/browser/tab_contents/tab_contents_observer.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
+#include "googleurl/src/gurl.h"
 
 class BackingStore;
 class SkBitmap;
 
+namespace base {
+class DictionaryValue;
+}  // namespace base
+
 // Windows
 class GetWindowFunction : public SyncExtensionFunction {
   virtual ~GetWindowFunction() {}
@@ -41,6 +47,15 @@ class GetAllWindowsFunction : public SyncExtensionFunction {
 class CreateWindowFunction : public SyncExtensionFunction {
   virtual ~CreateWindowFunction() {}
   virtual bool RunImpl() OVERRIDE;
+  // Returns whether the window should be created in incognito mode.
+  // |urls| is the list of urls to open. If we are creating an incognito window,
+  // the function will remove these urls which may not be opened in incognito
+  // mode.  If window creation leads the browser into an erroneous state,
+  // |is_error| is set to true (also, error_ member variable is assigned
+  // the proper error message).
+  bool ShouldOpenIncognitoWindow(const base::DictionaryValue* args,
+                                 std::vector<GURL>* urls,
+                                 bool* is_error);
   DECLARE_EXTENSION_FUNCTION_NAME("windows.create")
 };
 class UpdateWindowFunction : public SyncExtensionFunction {

chrome/browser/extensions/extension_tabs_module_constants.cc

@@ -75,6 +75,8 @@ const char kTabStripNotEditableError[] =
 const char kNoSelectedTabError[] = "No selected tab";
 const char kNoHighlightedTabError[] = "No highlighted tab";
 const char kIncognitoModeIsDisabled[] = "Incognito mode is disabled.";
+const char kIncognitoModeIsForced[] = "Incognito mode is forced. "
+    "Cannot open normal windows.";
 const char kURLsNotAllowedInIncognitoError[] = "Cannot open URL \"*\" "
     "in an incognito window.";
 const char kInvalidUrlError[] = "Invalid url: \"*\".";

chrome/browser/extensions/extension_tabs_module_constants.h

@@ -75,6 +75,7 @@ extern const char kTabStripNotEditableError[];
 extern const char kNoHighlightedTabError[];
 extern const char kNoSelectedTabError[];
 extern const char kIncognitoModeIsDisabled[];
+extern const char kIncognitoModeIsForced[];
 extern const char kURLsNotAllowedInIncognitoError[];
 extern const char kInvalidUrlError[];
 extern const char kInternalVisibleTabCaptureError[];

chrome/browser/extensions/extension_tabs_test.cc

@@ -12,8 +12,10 @@
 #include "base/string_util.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/extension_function_test_utils.h"
-#include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/extensions/extension_tabs_module_constants.h"
+#include "chrome/browser/extensions/extension_tab_util.h"
+#include "chrome/browser/prefs/incognito_mode_prefs.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/test/base/in_process_browser_test.h"
@@ -126,3 +128,129 @@ IN_PROC_BROWSER_TEST_F(ExtensionTabsTest, UpdateNoPermissions) {
           browser()));
   EXPECT_EQ(base::Value::TYPE_NULL, result->GetType());
 }
+
+IN_PROC_BROWSER_TEST_F(ExtensionTabsTest,
+                       DefaultToIncognitoWhenItIsForced) {
+  static const char kArgsWithoutExplicitIncognitoParam[] =
+      "[{\"url\": \"about:blank\"}]";
+  // Force Incognito mode.
+  IncognitoModePrefs::SetAvailability(browser()->profile()->GetPrefs(),
+                                      IncognitoModePrefs::FORCED);
+  // Run without an explicit "incognito" param.
+  scoped_ptr<base::DictionaryValue> result(ToDictionary(
+      RunFunctionAndReturnResult(
+          new CreateWindowFunction(),
+          kArgsWithoutExplicitIncognitoParam,
+          browser(),
+          INCLUDE_INCOGNITO)));
+
+  // Make sure it is a new(different) window.
+  EXPECT_NE(ExtensionTabUtil::GetWindowId(browser()),
+            GetInteger(result.get(), "id"));
+  // ... and it is incognito.
+  EXPECT_TRUE(GetBoolean(result.get(), "incognito"));
+
+  // Now try creating a window from incognito window.
+  Browser* incognito_browser = CreateIncognitoBrowser();
+  // Run without an explicit "incognito" param.
+  result.reset(ToDictionary(
+      RunFunctionAndReturnResult(
+          new CreateWindowFunction(),
+          kArgsWithoutExplicitIncognitoParam,
+          incognito_browser,
+          INCLUDE_INCOGNITO)));
+  // Make sure it is a new(different) window.
+  EXPECT_NE(ExtensionTabUtil::GetWindowId(incognito_browser),
+            GetInteger(result.get(), "id"));
+  // ... and it is incognito.
+  EXPECT_TRUE(GetBoolean(result.get(), "incognito"));
+}
+
+IN_PROC_BROWSER_TEST_F(ExtensionTabsTest,
+                       DefaultToIncognitoWhenItIsForcedAndNoArgs) {
+  static const char kEmptyArgs[] = "[]";
+  // Force Incognito mode.
+  IncognitoModePrefs::SetAvailability(browser()->profile()->GetPrefs(),
+                                      IncognitoModePrefs::FORCED);
+  // Run without an explicit "incognito" param.
+  scoped_ptr<base::DictionaryValue> result(ToDictionary(
+      RunFunctionAndReturnResult(
+          new CreateWindowFunction(),
+          kEmptyArgs,
+          browser(),
+          INCLUDE_INCOGNITO)));
+
+  // Make sure it is a new(different) window.
+  EXPECT_NE(ExtensionTabUtil::GetWindowId(browser()),
+            GetInteger(result.get(), "id"));
+  // ... and it is incognito.
+  EXPECT_TRUE(GetBoolean(result.get(), "incognito"));
+
+  // Now try creating a window from incognito window.
+  Browser* incognito_browser = CreateIncognitoBrowser();
+  // Run without an explicit "incognito" param.
+  result.reset(ToDictionary(
+      RunFunctionAndReturnResult(
+          new CreateWindowFunction(),
+          kEmptyArgs,
+          incognito_browser,
+          INCLUDE_INCOGNITO)));
+  // Make sure it is a new(different) window.
+  EXPECT_NE(ExtensionTabUtil::GetWindowId(incognito_browser),
+            GetInteger(result.get(), "id"));
+  // ... and it is incognito.
+  EXPECT_TRUE(GetBoolean(result.get(), "incognito"));
+}
+
+IN_PROC_BROWSER_TEST_F(ExtensionTabsTest,
+                       DontCreateNormalWindowWhenIncognitoForced) {
+  static const char kArgsWithExplicitIncognitoParam[] =
+      "[{\"url\": \"about:blank\", \"incognito\": false }]";
+  // Force Incognito mode.
+  IncognitoModePrefs::SetAvailability(browser()->profile()->GetPrefs(),
+                                      IncognitoModePrefs::FORCED);
+
+  // Run with an explicit "incognito" param.
+  EXPECT_TRUE(MatchPattern(
+      RunFunctionAndReturnError(
+          new CreateWindowFunction(),
+          kArgsWithExplicitIncognitoParam,
+          browser()),
+      extension_tabs_module_constants::kIncognitoModeIsForced));
+
+  // Now try opening a normal window from incognito window.
+  Browser* incognito_browser = CreateIncognitoBrowser();
+  // Run with an explicit "incognito" param.
+  EXPECT_TRUE(MatchPattern(
+      RunFunctionAndReturnError(
+          new CreateWindowFunction(),
+          kArgsWithExplicitIncognitoParam,
+          incognito_browser),
+      extension_tabs_module_constants::kIncognitoModeIsForced));
+}
+
+IN_PROC_BROWSER_TEST_F(ExtensionTabsTest,
+                       DontCreateIncognitoWindowWhenIncognitoDisabled) {
+  static const char kArgs[] =
+      "[{\"url\": \"about:blank\", \"incognito\": true }]";
+
+  Browser* incognito_browser = CreateIncognitoBrowser();
+  // Disable Incognito mode.
+  IncognitoModePrefs::SetAvailability(browser()->profile()->GetPrefs(),
+                                      IncognitoModePrefs::DISABLED);
+  // Run in normal window.
+  EXPECT_TRUE(MatchPattern(
+      RunFunctionAndReturnError(
+          new CreateWindowFunction(),
+          kArgs,
+          browser()),
+      extension_tabs_module_constants::kIncognitoModeIsDisabled));
+
+  // Run in incognito window.
+  EXPECT_TRUE(MatchPattern(
+      RunFunctionAndReturnError(
+          new CreateWindowFunction(),
+          kArgs,
+          incognito_browser),
+      extension_tabs_module_constants::kIncognitoModeIsDisabled));
+}