pakjoy: Restore old QTWEBENGINE_RESOURCES_PATH value

Otherwise, doing :restart fails because it tries to copy quirk dir to quirk dir.

QtWebEngine reads the env var in resourcePath() in
src/core/web_engine_library_info.cpp. That only seems to be called from
WebEngineLibraryInfo::getPath(), and that in turn gets called from
ResourceBundle::LoadCommonResources() in src/core/resource_bundle_qt.cpp.

Finally, that only seems to be called during Chromium initialization, so it
seems alright for us to unset this as soon as we initialized the first profile.

It also seems to work fine in my testing on Google Meet, and indeed fixes
:restart.

3 files changed, 63 insertions, 19 deletions

diff --git a/qutebrowser/browser/webengine/webenginesettings.py b/qutebrowser/browser/webengine/webenginesettings.py
index 20657685e..1275edf0b 100644
--- a/qutebrowser/browser/webengine/webenginesettings.py
+++ b/qutebrowser/browser/webengine/webenginesettings.py
@@ -555,10 +555,10 @@ def init():
 
     log.init.debug("Initializing profiles...")
 
-    # Apply potential resource patches before initializing profiles.
-    pakjoy.patch_webengine()
+    # Apply potential resource patches while initializing profiles.
+    with pakjoy.patch_webengine():
+        _init_default_profile()
 
-    _init_default_profile()
     init_private_profile()
     config.instance.changed.connect(_update_settings)
 
diff --git a/qutebrowser/misc/pakjoy.py b/qutebrowser/misc/pakjoy.py
index b1e4b7884..a511034a2 100644
--- a/qutebrowser/misc/pakjoy.py
+++ b/qutebrowser/misc/pakjoy.py
@@ -29,7 +29,8 @@ import os
 import shutil
 import pathlib
 import dataclasses
-from typing import ClassVar, IO, Optional, Dict, Tuple
+import contextlib
+from typing import ClassVar, IO, Optional, Dict, Tuple, Iterator
 
 from qutebrowser.misc import binparsing, objects
 from qutebrowser.utils import qtutils, standarddir, version, utils, log
@@ -201,9 +202,6 @@ def copy_webengine_resources() -> Optional[pathlib.Path]:
     )
 
     shutil.copytree(resources_dir, work_dir)
-
-    os.environ[RESOURCES_ENV_VAR] = str(work_dir)
-
     return work_dir
 
 
@@ -227,10 +225,12 @@ def _patch(file_to_patch: pathlib.Path) -> None:
             log.misc.exception("Failed to apply quirk to resources pak.")
 
 
-def patch_webengine() -> None:
+@contextlib.contextmanager
+def patch_webengine() -> Iterator[None]:
     """Apply any patches to webengine resource pak files."""
     if os.environ.get(DISABLE_ENV_VAR):
         log.misc.debug(f"Not applying quirk due to {DISABLE_ENV_VAR}")
+        yield
         return
 
     try:
@@ -239,9 +239,22 @@ def patch_webengine() -> None:
         webengine_resources_path = copy_webengine_resources()
     except OSError:
         log.misc.exception("Failed to copy webengine resources, not applying quirk")
+        yield
         return
 
     if webengine_resources_path is None:
+        yield
         return
 
     _patch(webengine_resources_path / PAK_FILENAME)
+
+    old_value = os.environ.get(RESOURCES_ENV_VAR)
+    os.environ[RESOURCES_ENV_VAR] = str(webengine_resources_path)
+
+    yield
+
+    # Restore old value for subprocesses or :restart
+    if old_value is None:
+        del os.environ[RESOURCES_ENV_VAR]
+    else:
+        os.environ[RESOURCES_ENV_VAR] = old_value
diff --git a/tests/unit/misc/test_pakjoy.py b/tests/unit/misc/test_pakjoy.py
index a889fcf49..f5bb5b49c 100644
--- a/tests/unit/misc/test_pakjoy.py
+++ b/tests/unit/misc/test_pakjoy.py
@@ -8,6 +8,7 @@ import json
 import struct
 import pathlib
 import logging
+import shutil
 
 import pytest
 
@@ -72,7 +73,8 @@ def test_version_gate(cache_tmpdir, unaffected_version, mocker, workdir_exists):
         (workdir / "some_patched_file.pak").ensure()
     fake_open = mocker.patch("qutebrowser.misc.pakjoy.open")
 
-    pakjoy.patch_webengine()
+    with pakjoy.patch_webengine():
+        pass
 
     assert not fake_open.called
     assert not workdir.exists()
@@ -82,7 +84,8 @@ def test_escape_hatch(affected_version, mocker, monkeypatch):
     fake_open = mocker.patch("qutebrowser.misc.pakjoy.open")
     monkeypatch.setenv(pakjoy.DISABLE_ENV_VAR, "1")
 
-    pakjoy.patch_webengine()
+    with pakjoy.patch_webengine():
+        pass
 
     assert not fake_open.called
 
@@ -196,7 +199,8 @@ class TestWithRealResourcesFile:
         # Go through the full patching processes with the real resources file from
         # the current installation. Make sure our replacement string is in it
         # afterwards.
-        pakjoy.patch_webengine()
+        with pakjoy.patch_webengine():
+            pass
 
         json_manifest = read_patched_manifest()
         assert (
@@ -220,9 +224,6 @@ class TestWithRealResourcesFile:
         tmpfile = work_dir / "tmp.txt"
         tmpfile.touch()
 
-        # Set by first call to copy_webengine_resources()
-        del os.environ[pakjoy.RESOURCES_ENV_VAR]
-
         pakjoy.copy_webengine_resources()
         assert not tmpfile.exists()
 
@@ -239,7 +240,9 @@ class TestWithRealResourcesFile:
             pakjoy.shutil, osfunc, lambda *_args: raiseme(PermissionError(osfunc))
         )
         with caplog.at_level(logging.ERROR, "misc"):
-            pakjoy.patch_webengine()
+            with pakjoy.patch_webengine():
+                pass
+
         assert caplog.messages == [
             "Failed to copy webengine resources, not applying quirk"
         ]
@@ -393,8 +396,10 @@ class TestWithConstructedResourcesFile:
 
         assert caplog.messages == ["Failed to apply quirk to resources pak."]
 
-    def test_patching(self, monkeypatch: pytest.MonkeyPatch, tmp_path: pathlib.Path):
-        """Go through the full patching processes with a fake resources file."""
+    @pytest.fixture
+    def resources_path(
+        self, monkeypatch: pytest.MonkeyPatch, tmp_path: pathlib.Path
+    ) -> pathlib.Path:
         resources_path = tmp_path / "resources"
         resources_path.mkdir()
 
@@ -403,10 +408,36 @@ class TestWithConstructedResourcesFile:
             fd.write(buffer.read())
 
         monkeypatch.setattr(pakjoy.qtutils, "library_path", lambda _which: tmp_path)
-        pakjoy.patch_webengine()
+        return resources_path
+
+    @pytest.fixture
+    def quirk_dir_path(self, tmp_path: pathlib.Path) -> pathlib.Path:
+        return tmp_path / "cache" / pakjoy.CACHE_DIR_NAME
+
+    def test_patching(self, resources_path: pathlib.Path, quirk_dir_path: pathlib.Path):
+        """Go through the full patching processes with a fake resources file."""
+
+        with pakjoy.patch_webengine():
+            assert os.environ[pakjoy.RESOURCES_ENV_VAR] == str(quirk_dir_path)
+            json_manifest = read_patched_manifest()
 
-        json_manifest = read_patched_manifest()
         assert (
             pakjoy.REPLACEMENT_URL.decode("utf-8")
             in json_manifest["externally_connectable"]["matches"]
         )
+        assert pakjoy.RESOURCES_ENV_VAR not in os.environ
+
+    def test_preset_env_var(
+        self,
+        resources_path: pathlib.Path,
+        monkeypatch: pytest.MonkeyPatch,
+        quirk_dir_path: pathlib.Path,
+    ):
+        new_resources_path = resources_path.with_name(resources_path.name + "_moved")
+        shutil.move(resources_path, new_resources_path)
+        monkeypatch.setenv(pakjoy.RESOURCES_ENV_VAR, str(new_resources_path))
+
+        with pakjoy.patch_webengine():
+            assert os.environ[pakjoy.RESOURCES_ENV_VAR] == str(quirk_dir_path)
+
+        assert os.environ[pakjoy.RESOURCES_ENV_VAR] == str(new_resources_path)