From cb71ec8344ed8fc0cd595099a5f831ebd17f4e60 Mon Sep 17 00:00:00 2001
From: Rob Wu <rob@robwu.nl>
Date: Tue, 30 Apr 2024 14:54:10 +0000
Subject: [PATCH] Bug 1876924 - Accept manifest with "incognito":"split" r=rpl

Before this patch, extensions with "incognito":"split" could not be
loaded at all. With this patch, we accept such extensions, but default
to "not_allowed". This prevents users from installing the extension in
private browsing mode (including permanent private browsing mode), but
at least it enables users to use the extension in regular browsing mode.

Differential Revision: https://phabricator.services.mozilla.com/D199800
---
 toolkit/components/extensions/Schemas.sys.mjs | 21 ++++++++++
 .../extensions/schemas/manifest.json          |  4 +-
 .../xpcshell/test_ext_manifest_incognito.js   | 39 ++++++++++++++++---
 .../extensions/test/xpcshell/xpcshell.toml    |  1 +
 4 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/toolkit/components/extensions/Schemas.sys.mjs b/toolkit/components/extensions/Schemas.sys.mjs
index b107036355cb..e95cbd6bb51f 100644
--- a/toolkit/components/extensions/Schemas.sys.mjs
+++ b/toolkit/components/extensions/Schemas.sys.mjs
@@ -316,6 +316,27 @@ const POSTPROCESSORS = {
     }
     return value;
   },
+
+  incognitoSplitUnsupportedAndFallback(value, context) {
+    if (value === "split") {
+      // incognito:split has not been implemented (bug 1380812). There are two
+      // alternatives: "spanning" and "not_allowed".
+      //
+      // "incognito":"split" is required by Chrome when extensions want to load
+      // any extension page in a tab in Chrome. In Firefox that is not required,
+      // so extensions could replace "split" with "spanning".
+      // Another (poorly documented) effect of "incognito":"split" is separation
+      // of some state between some extension APIs. Because this can in theory
+      // result in unwanted mixing of state between private and non-private
+      // browsing, we fall back to "not_allowed", which prevents the user from
+      // enabling the extension in private browsing windows.
+      value = "not_allowed";
+      context.logWarning(
+        `incognito "split" is unsupported. Falling back to incognito "${value}".`
+      );
+    }
+    return value;
+  },
 };
 
 // Parses a regular expression, with support for the Python extended
diff --git a/toolkit/components/extensions/schemas/manifest.json b/toolkit/components/extensions/schemas/manifest.json
index 384b168e39c6..9701eda25a4d 100644
--- a/toolkit/components/extensions/schemas/manifest.json
+++ b/toolkit/components/extensions/schemas/manifest.json
@@ -121,7 +121,9 @@
 
           "incognito": {
             "type": "string",
-            "enum": ["not_allowed", "spanning"],
+            "description": "The 'split' value is not supported.",
+            "enum": ["not_allowed", "spanning", "split"],
+            "postprocess": "incognitoSplitUnsupportedAndFallback",
             "default": "spanning",
             "optional": true
           },
diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_manifest_incognito.js b/toolkit/components/extensions/test/xpcshell/test_ext_manifest_incognito.js
index 4330e1b681b6..6c2c0f65f94f 100644
--- a/toolkit/components/extensions/test/xpcshell/test_ext_manifest_incognito.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_manifest_incognito.js
@@ -27,19 +27,48 @@ add_task(async function test_manifest_incognito() {
     "Should have the expected incognito string"
   );
 
+  ExtensionTestUtils.failOnSchemaWarnings(false);
   normalized = await ExtensionTestUtils.normalizeManifest({
-    incognito: "split",
+    incognito: "whatisthis",
   });
+  ExtensionTestUtils.failOnSchemaWarnings(true);
 
   equal(
     normalized.error,
-    'Error processing incognito: Invalid enumeration value "split"',
+    'Error processing incognito: Invalid enumeration value "whatisthis"',
     "Should have an error"
   );
   Assert.deepEqual(normalized.errors, [], "Should not have a warning");
+
+  // Sanity check: Default value of "incognito" is "spanning".
+  normalized = await ExtensionTestUtils.normalizeManifest({});
+  equal(normalized.error, undefined, "Should not have an error");
+  equal(normalized.errors.length, 0, "Should not have warnings");
   equal(
-    normalized.value,
-    undefined,
-    "Invalid incognito string should be undefined"
+    normalized.value.incognito,
+    "spanning",
+    "Should have the expected default value for incognito when unspecified"
+  );
+});
+
+add_task(async function test_manifest_incognito_split_fallback_not_allowed() {
+  ExtensionTestUtils.failOnSchemaWarnings(false);
+  let normalized = await ExtensionTestUtils.normalizeManifest({
+    incognito: "split",
+  });
+  ExtensionTestUtils.failOnSchemaWarnings(true);
+
+  equal(normalized.error, undefined, "Should not have an error");
+  Assert.deepEqual(
+    normalized.errors,
+    [
+      `Warning processing incognito: incognito "split" is unsupported. Falling back to incognito "not_allowed".`,
+    ],
+    "Should have a warning"
+  );
+  equal(
+    normalized.value.incognito,
+    "not_allowed",
+    "incognito:split should fall back to not_allowed by default"
   );
 });
diff --git a/toolkit/components/extensions/test/xpcshell/xpcshell.toml b/toolkit/components/extensions/test/xpcshell/xpcshell.toml
index 1d08d855f0cd..5ab8cc7ca25a 100644
--- a/toolkit/components/extensions/test/xpcshell/xpcshell.toml
+++ b/toolkit/components/extensions/test/xpcshell/xpcshell.toml
@@ -75,6 +75,7 @@ skip-if = ["os == 'android'"] # Not shipped on Android
 ["test_ext_manifest_content_security_policy.js"]
 
 ["test_ext_manifest_incognito.js"]
+skip-if = ["condprof"] # remove in a few days - see bug 1891740
 
 ["test_ext_manifest_minimum_chrome_version.js"]