Fix some issues with incognito split mode extensions.

- No longer create the background host if an extension has not been enabled
for incognito.
- Always open the options page in a normal window. Incognito options pages
aren't able to save state.
- Overrides (like NTP) now apply to incognito windows if the extension is
split mode.

BUG=54173
BUG=45879
TEST=Install chrome/common/extensions//docs/examples/api/override/blank_ntp. Allow it to run in incognito, and open an incognito window. The new tab page should say "Blank New Tab".

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

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

5 files changed, 56 insertions, 15 deletions

diff --git a/chrome/browser/extensions/extension_dom_ui.cc b/chrome/browser/extensions/extension_dom_ui.cc
index 62cfee0..659eb98 100644
--- a/chrome/browser/extensions/extension_dom_ui.cc
+++ b/chrome/browser/extensions/extension_dom_ui.cc
@@ -222,10 +222,6 @@ bool ExtensionDOMUI::HandleChromeURLOverride(GURL* url, Profile* profile) {
   if (!url->SchemeIs(chrome::kChromeUIScheme))
     return false;
 
-  // We can't handle chrome-extension URLs in incognito mode.
-  if (profile->IsOffTheRecord())
-    return false;
-
   const DictionaryValue* overrides =
       profile->GetPrefs()->GetDictionary(kExtensionURLOverrides);
   std::string page = url->host();
@@ -242,9 +238,10 @@ bool ExtensionDOMUI::HandleChromeURLOverride(GURL* url, Profile* profile) {
     return false;
   }
 
-  while (url_list->GetSize()) {
-    Value* val;
-    url_list->Get(0, &val);
+  size_t i = 0;
+  while (i < url_list->GetSize()) {
+    Value* val = NULL;
+    url_list->Get(i, &val);
 
     // Verify that the override value is good.  If not, unregister it and find
     // the next one.
@@ -272,6 +269,16 @@ bool ExtensionDOMUI::HandleChromeURLOverride(GURL* url, Profile* profile) {
       continue;
     }
 
+    // We can't handle chrome-extension URLs in incognito mode unless the
+    // extension uses split mode.
+    bool incognito_override_allowed =
+        extension->incognito_split_mode() &&
+        service->IsIncognitoEnabled(extension);
+    if (profile->IsOffTheRecord() && !incognito_override_allowed) {
+      ++i;
+      continue;
+    }
+
     *url = extension_url;
     return true;
   }
diff --git a/chrome/browser/extensions/extension_process_manager.cc b/chrome/browser/extensions/extension_process_manager.cc
index 1fff690..954f92d 100644
--- a/chrome/browser/extensions/extension_process_manager.cc
+++ b/chrome/browser/extensions/extension_process_manager.cc
@@ -50,6 +50,9 @@ class IncognitoExtensionProcessManager : public ExtensionProcessManager {
   // URL or a web app URL.
   Extension* GetExtensionOrAppByURL(const GURL& url);
 
+  // Returns true if the extension is allowed to run in incognito mode.
+  bool IsIncognitoEnabled(const Extension* extension);
+
   ExtensionProcessManager* original_manager_;
 };
 
@@ -183,10 +186,9 @@ void ExtensionProcessManager::OpenOptionsPage(Extension* extension,
                                               Browser* browser) {
   DCHECK(!extension->options_url().is_empty());
 
-  // We can't open extensions URLs in incognito windows, unless the extension
-  // uses "split" incognito mode.
-  if (!browser || (browser->profile()->IsOffTheRecord() &&
-                   !extension->incognito_split_mode())) {
+  // Force the options page to open in non-OTR window, because it won't be
+  // able to save settings from OTR.
+  if (!browser || browser->profile()->IsOffTheRecord()) {
     browser = Browser::GetOrCreateTabbedBrowser(
         browsing_instance_->profile()->GetOriginalProfile());
   }
@@ -373,8 +375,15 @@ ExtensionHost* IncognitoExtensionProcessManager::CreateView(
     Browser* browser,
     ViewType::Type view_type) {
   if (extension->incognito_split_mode()) {
-    return ExtensionProcessManager::CreateView(extension, url,
-                                               browser, view_type);
+    if (IsIncognitoEnabled(extension)) {
+      return ExtensionProcessManager::CreateView(extension, url,
+                                                 browser, view_type);
+    } else {
+      NOTREACHED() <<
+          "We shouldn't be trying to create an incognito extension view unless "
+          "it has been enabled for incognito.";
+      return NULL;
+    }
   } else {
     return original_manager_->CreateView(extension, url, browser, view_type);
   }
@@ -383,7 +392,8 @@ ExtensionHost* IncognitoExtensionProcessManager::CreateView(
 void IncognitoExtensionProcessManager::CreateBackgroundHost(
     Extension* extension, const GURL& url) {
   if (extension->incognito_split_mode()) {
-    ExtensionProcessManager::CreateBackgroundHost(extension, url);
+    if (IsIncognitoEnabled(extension))
+      ExtensionProcessManager::CreateBackgroundHost(extension, url);
   } else {
     // Do nothing. If an extension is spanning, then its original-profile
     // background page is shared with incognito, so we don't create another.
@@ -414,10 +424,19 @@ Extension* IncognitoExtensionProcessManager::GetExtensionOrAppByURL(
     const GURL& url) {
   ExtensionsService* service =
       browsing_instance_->profile()->GetExtensionsService();
+  if (!service)
+    return NULL;
   return (url.SchemeIs(chrome::kExtensionScheme)) ?
       service->GetExtensionByURL(url) : service->GetExtensionByWebExtent(url);
 }
 
+bool IncognitoExtensionProcessManager::IsIncognitoEnabled(
+    const Extension* extension) {
+  ExtensionsService* service =
+      browsing_instance_->profile()->GetExtensionsService();
+  return service && service->IsIncognitoEnabled(extension);
+}
+
 void IncognitoExtensionProcessManager::Observe(
     NotificationType type,
     const NotificationSource& source,
diff --git a/chrome/common/extensions/docs/examples/api/override/blank_ntp/manifest.json b/chrome/common/extensions/docs/examples/api/override/blank_ntp/manifest.json
index 3ae50ec..671075e 100644
--- a/chrome/common/extensions/docs/examples/api/override/blank_ntp/manifest.json
+++ b/chrome/common/extensions/docs/examples/api/override/blank_ntp/manifest.json
@@ -1,7 +1,8 @@
 {
   "name": "Blank new tab page",
   "version": "0.1",
+  "incognito": "split",
   "chrome_url_overrides": {
     "newtab": "blank.html"
   }
-}
\ No newline at end of file
+}
diff --git a/chrome/common/extensions/docs/override.html b/chrome/common/extensions/docs/override.html
index a23f419..bb2253a 100644
--- a/chrome/common/extensions/docs/override.html
+++ b/chrome/common/extensions/docs/override.html
@@ -367,6 +367,13 @@ A single extension can override
 only one page.
 </p>
 
+<p class="note">
+<b>Note:</b>
+If you want to override the page in incognito windows as well, make sure to
+specify "spanning" mode for the <a href="manifest.html#incognito">incognito</a>
+manifest property.
+</p>
+
 <p>
 The following screenshots show the default New Tab page
 next to a custom New Tab page.
diff --git a/chrome/common/extensions/docs/static/override.html b/chrome/common/extensions/docs/static/override.html
index 5bf20da0..a04b5de 100644
--- a/chrome/common/extensions/docs/static/override.html
+++ b/chrome/common/extensions/docs/static/override.html
@@ -63,6 +63,13 @@ A single extension can override
 only one page.
 </p>
 
+<p class="note">
+<b>Note:</b>
+If you want to override the page in incognito windows as well, make sure to
+specify "spanning" mode for the <a href="manifest.html#incognito">incognito</a>
+manifest property.
+</p>
+
 <p>
 The following screenshots show the default New Tab page
 next to a custom New Tab page.