greasemonkey: move 5.7.1 injection method into _WebEngineScripts

Moves the 5.8 check to `_WebEngineScripts.init()`.

Changes `_inject_userscripts` to allow for the two code paths. With
5.7.1 we need to specify the injection point and not clear all scripts
for each call, since we have to call it three times.

Change the 5.8+ hook to call a new method which passes all the scripts
into `_inject_userscripts` so that doesn't have to have a fallback
conditional inside it because thats an inversion of responsibility!

Pulling the remove scripts part into a seperate function and making it
the callers responsibilty to call that first would tidy it up a little
more but meh.

I was worried about just doing `_widget.page().urlChanged.connect()`
once at tab init, where before it was connected at page init, because I
was under the impression that the child page can be replaced at any
time, eg when navigating to a new origin. But under manual testing I
didn't see that at all. Maybe I was mistaken or maybe that only started
in a later Qt version.

2 files changed, 55 insertions, 79 deletions

diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py
index a62524496..d3d497ec7 100644
--- a/qutebrowser/browser/webengine/webenginetab.py
+++ b/qutebrowser/browser/webengine/webenginetab.py
@@ -854,9 +854,16 @@ class _WebEngineScripts(QObject):
         self._inject_early_js('js', js_code, subframes=True)
         self._init_stylesheet()
 
-        greasemonkey = objreg.get('greasemonkey')
-        greasemonkey.scripts_reloaded.connect(self._inject_userscripts)
-        self._inject_userscripts()
+        # The Greasemonkey metadata block support in QtWebEngine only starts at
+        # Qt 5.8. With 5.7.1, we need to inject the scripts ourselves in
+        # response to urlChanged.
+        if not qtutils.version_check('5.8'):
+            self._widget.page().urlChanged.connect(
+                self._inject_userscripts_for_url)
+        else:
+            greasemonkey = objreg.get('greasemonkey')
+            greasemonkey.scripts_reloaded.connect(self._inject_all_userscripts)
+            self._inject_all_userscripts()
 
     def _init_stylesheet(self):
         """Initialize custom stylesheets.
@@ -873,32 +880,52 @@ class _WebEngineScripts(QObject):
         )
         self._inject_early_js('stylesheet', js_code, subframes=True)
 
-    def _inject_userscripts(self):
-        """Register user JavaScript files with the current tab."""
-        # The Greasemonkey metadata block support in QtWebEngine only starts at
-        # Qt 5.8. With 5.7.1, we need to inject the scripts ourselves in
-        # response to urlChanged.
-        if not qtutils.version_check('5.8'):
-            return
+    def _inject_userscripts_for_url(self, url):
+        greasemonkey = objreg.get('greasemonkey')
+        matching_scripts = greasemonkey.scripts_for(url)
+        self._inject_userscripts(
+            matching_scripts.start, QWebEngineScript.DocumentCreation, True)
+        self._inject_userscripts(
+            matching_scripts.end, QWebEngineScript.DocumentReady, False)
+        self._inject_userscripts(
+            matching_scripts.idle, QWebEngineScript.Deferred, False)
+
+    def _inject_all_userscripts(self):
+        greasemonkey = objreg.get('greasemonkey')
+        scripts = greasemonkey.all_scripts()
+        self._inject_userscripts(scripts)
 
+    def _inject_userscripts(self, scripts=None, injection_point=None,
+                            remove_first=True):
+        """Register user JavaScript files with the current tab.
+
+        Args:
+            scripts: A list of GreasemonkeyScripts, or None to add all
+                     known by the Greasemonkey subsystem.
+            injection_point: The QWebEngineScript::InjectionPoint stage
+                             to inject the script into, None to use
+                             auto-detection.
+            remove_first: Whether to remove all previously injected
+                          scripts before adding these ones.
+        """
         # Since we are inserting scripts into a per-tab collection,
         # rather than just injecting scripts on page load, we need to
         # make sure we replace existing scripts, not just add new ones.
         # While, taking care not to remove any other scripts that might
         # have been added elsewhere, like the one for stylesheets.
-        greasemonkey = objreg.get('greasemonkey')
-        scripts = self._widget.page().scripts()
-        for script in scripts.toList():
-            if script.name().startswith("GM-"):
-                log.greasemonkey.debug('Removing script: {}'
-                                       .format(script.name()))
-                removed = scripts.remove(script)
-                assert removed, script.name()
-
-        # Then add the new scripts.
-        for script in greasemonkey.all_scripts():
-            # @run-at (and @include/@exclude/@match) is parsed by
-            # QWebEngineScript.
+        page_scripts = self._widget.page().scripts()
+        if remove_first:
+            for script in page_scripts.toList():
+                if script.name().startswith("GM-"):
+                    log.greasemonkey.debug('Removing script: {}'
+                                           .format(script.name()))
+                    removed = page_scripts.remove(script)
+                    assert removed, script.name()
+
+        if not scripts:
+            return
+
+        for script in scripts:
             new_script = QWebEngineScript()
             try:
                 world = int(script.jsworld)
@@ -915,9 +942,12 @@ class _WebEngineScripts(QObject):
             new_script.setSourceCode(script.code())
             new_script.setName("GM-{}".format(script.name))
             new_script.setRunsOnSubFrames(script.runs_on_sub_frames)
+            # Override the @run-at value parsed by QWebEngineScript if desired.
+            if injection_point:
+                new_script.setInjectionPoint(injection_point)
             log.greasemonkey.debug('adding script: {}'
                                    .format(new_script.name()))
-            scripts.insert(new_script)
+            page_scripts.insert(new_script)
 
 
 class WebEngineTab(browsertab.AbstractTab):
diff --git a/qutebrowser/browser/webengine/webview.py b/qutebrowser/browser/webengine/webview.py
index 2eea5d837..75b5df354 100644
--- a/qutebrowser/browser/webengine/webview.py
+++ b/qutebrowser/browser/webengine/webview.py
@@ -20,11 +20,10 @@
 """The main browser widget for QtWebEngine."""
 
 import sip
-from PyQt5.QtCore import pyqtSignal, pyqtSlot, QUrl, PYQT_VERSION
+from PyQt5.QtCore import pyqtSignal, QUrl, PYQT_VERSION
 from PyQt5.QtGui import QPalette
 from PyQt5.QtWidgets import QWidget
 from PyQt5.QtWebEngineWidgets import (QWebEngineView, QWebEnginePage,
-                                      QWebEngineScript,
                                       QWebEngineCertificateError)
 
 from qutebrowser.browser import shared
@@ -169,7 +168,6 @@ class WebEnginePage(QWebEnginePage):
         self._theme_color = theme_color
         self._set_bg_color()
         config.instance.changed.connect(self._set_bg_color)
-        self.urlChanged.connect(self._inject_userjs)
 
     @config.change_filter('colors.webpage.bg')
     def _set_bg_color(self):
@@ -264,55 +262,3 @@ class WebEnginePage(QWebEnginePage):
                                                  is_main_frame=is_main_frame)
         self.navigation_request.emit(navigation)
         return navigation.accepted
-
-    @pyqtSlot('QUrl')
-    def _inject_userjs(self, url):
-        """Inject userscripts registered for `url` into the current page."""
-        if qtutils.version_check('5.8'):
-            # Handled in webenginetab with the builtin Greasemonkey
-            # support.
-            return
-
-        # Using QWebEnginePage.scripts() to hold the user scripts means
-        # we don't have to worry ourselves about where to inject the
-        # page but also means scripts hang around for the tab lifecycle.
-        # So clear them here.
-        scripts = self.scripts()
-        for script in scripts.toList():
-            if script.name().startswith("GM-"):
-                log.greasemonkey.debug("Removing script: {}"
-                                       .format(script.name()))
-                removed = scripts.remove(script)
-                assert removed, script.name()
-
-        def _add_script(script, injection_point):
-            new_script = QWebEngineScript()
-            new_script.setInjectionPoint(injection_point)
-            try:
-                world = int(script.jsworld)
-            except ValueError:
-                try:
-                    from qutebrowser.browser.webengine.webenginetab import _JS_WORLD_MAP
-                    world = _JS_WORLD_MAP[usertypes.JsWorld[
-                        script.jsworld.lower()]]
-                except KeyError:
-                    log.greasemonkey.error(
-                        "script {} has invalid value for '@qute-js-world'"
-                        ": {}".format(script.name, script.jsworld))
-                    return
-            new_script.setWorldId(world)
-            new_script.setSourceCode(script.code())
-            new_script.setName("GM-{}".format(script.name))
-            new_script.setRunsOnSubFrames(script.runs_on_sub_frames)
-            log.greasemonkey.debug("Adding script: {}"
-                                   .format(new_script.name()))
-            scripts.insert(new_script)
-
-        greasemonkey = objreg.get('greasemonkey')
-        matching_scripts = greasemonkey.scripts_for(url)
-        for script in matching_scripts.start:
-            _add_script(script, QWebEngineScript.DocumentCreation)
-        for script in matching_scripts.end:
-            _add_script(script, QWebEngineScript.DocumentReady)
-        for script in matching_scripts.idle:
-            _add_script(script, QWebEngineScript.Deferred)