From a6307497c07b5033581a6641844f45cb56d34525 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 5 Jul 2016 11:58:31 +0200 Subject: [PATCH] Rewrite userscripts to work with async dumping --- qutebrowser/browser/commands.py | 16 ++-- qutebrowser/browser/hints.py | 9 +- qutebrowser/commands/userscripts.py | 111 ++++++++++++---------- tests/unit/commands/test_userscripts.py | 119 +++++++++++------------- 4 files changed, 127 insertions(+), 128 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 4e078b88f..d43116a1a 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1033,14 +1033,13 @@ class CommandDispatcher: if idx != -1: env['QUTE_TITLE'] = self._tabbed_browser.page_title(idx) - webview = self._tabbed_browser.currentWidget() - if webview is None: + tab = self._tabbed_browser.currentWidget() + if tab is None: mainframe = None else: - if webview.caret.has_selection(): - env['QUTE_SELECTED_TEXT'] = webview.caret.selection() - env['QUTE_SELECTED_HTML'] = webview.caret.selection(html=True) - mainframe = webview.page().mainFrame() + if tab.caret.has_selection(): + env['QUTE_SELECTED_TEXT'] = tab.caret.selection() + env['QUTE_SELECTED_HTML'] = tab.caret.selection(html=True) try: url = self._tabbed_browser.current_url() @@ -1049,9 +1048,8 @@ class CommandDispatcher: else: env['QUTE_URL'] = url.toString(QUrl.FullyEncoded) - env.update(userscripts.store_source(mainframe)) - userscripts.run(cmd, *args, win_id=self._win_id, env=env, - verbose=verbose) + userscripts.run_async(tab, cmd, *args, win_id=self._win_id, env=env, + verbose=verbose) @cmdutils.register(instance='command-dispatcher', scope='window') def quickmark_save(self): diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index df3688b89..d025f9385 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -84,6 +84,7 @@ class HintContext: args: Custom arguments for userscript/spawn rapid: Whether to do rapid hinting. mainframe: The main QWebFrame where we started hinting in. + tab: The WebTab object we started hinting in. group: The group of web elements to hint. """ @@ -98,6 +99,7 @@ class HintContext: self.destroyed_frames = [] self.args = [] self.mainframe = None + self.tab = None self.group = None def get_args(self, urlstr): @@ -569,7 +571,6 @@ class HintManager(QObject): """ cmd = context.args[0] args = context.args[1:] - frame = context.mainframe env = { 'QUTE_MODE': 'hints', 'QUTE_SELECTED_TEXT': str(elem), @@ -578,8 +579,9 @@ class HintManager(QObject): url = self._resolve_url(elem, context.baseurl) if url is not None: env['QUTE_URL'] = url.toString(QUrl.FullyEncoded) - env.update(userscripts.store_source(frame)) - userscripts.run(cmd, *args, win_id=self._win_id, env=env) + + userscripts.run_async(context.tab, cmd, *args, win_id=self._win_id, + env=env) def _spawn(self, url, context): """Spawn a simple command from a hint. @@ -837,6 +839,7 @@ class HintManager(QObject): self._check_args(target, *args) self._context = HintContext() + self._context.tab = tab self._context.target = target self._context.rapid = rapid try: diff --git a/qutebrowser/commands/userscripts.py b/qutebrowser/commands/userscripts.py index a8e62cf39..368ab2bf2 100644 --- a/qutebrowser/commands/userscripts.py +++ b/qutebrowser/commands/userscripts.py @@ -86,6 +86,10 @@ class _BaseUserscriptRunner(QObject): _proc: The GUIProcess which is being executed. _win_id: The window ID this runner is associated with. _cleaned_up: Whether temporary files were cleaned up. + _text_stored: Set when the page text was stored async. + _html_stored: Set when the page html was stored async. + _args: Arguments to pass to _run_process. + _kwargs: Keyword arguments to pass to _run_process. Signals: got_cmd: Emitted when a new command arrived and should be executed. @@ -101,9 +105,41 @@ class _BaseUserscriptRunner(QObject): self._win_id = win_id self._filepath = None self._proc = None - self._env = None + self._env = {} + self._text_stored = False + self._html_stored = False + self._args = None + self._kwargs = None - def _run_process(self, cmd, *args, env, verbose): + def store_text(self, text): + """Called as callback when the text is ready from the web backend.""" + with tempfile.NamedTemporaryFile(mode='w', encoding='utf-8', + suffix='.txt', + delete=False) as txt_file: + txt_file.write(text) + self._env['QUTE_TEXT'] = txt_file.name + + self._text_stored = True + log.procs.debug("Text stored from webview") + if self._text_stored and self._html_stored: + log.procs.debug("Both text/HTML stored, kicking off userscript!") + self._run_process(*self._args, **self._kwargs) + + def store_html(self, html): + """Called as callback when the html is ready from the web backend.""" + with tempfile.NamedTemporaryFile(mode='w', encoding='utf-8', + suffix='.html', + delete=False) as html_file: + html_file.write(html) + self._env['QUTE_HTML'] = html_file.name + + self._html_stored = True + log.procs.debug("HTML stored from webview") + if self._text_stored and self._html_stored: + log.procs.debug("Both text/HTML stored, kicking off userscript!") + self._run_process(*self._args, **self._kwargs) + + def _run_process(self, cmd, *args, env=None, verbose=False): """Start the given command. Args: @@ -112,7 +148,7 @@ class _BaseUserscriptRunner(QObject): env: A dictionary of environment variables to add. verbose: Show notifications when the command started/exited. """ - self._env = {'QUTE_FIFO': self._filepath} + self._env['QUTE_FIFO'] = self._filepath if env is not None: self._env.update(env) self._proc = guiprocess.GUIProcess(self._win_id, 'userscript', @@ -144,18 +180,19 @@ class _BaseUserscriptRunner(QObject): fn, e)) self._filepath = None self._proc = None - self._env = None + self._env = {} + self._text_stored = False + self._html_stored = False - def run(self, cmd, *args, env=None, verbose=False): - """Run the userscript given. + def prepare_run(self, *args, **kwargs): + """Prepare ruinning the userscript given. Needs to be overridden by subclasses. + The script will actually run after store_text and store_html have been + called. Args: - cmd: The command to be started. - *args: The arguments to hand to the command - env: A dictionary of environment variables to add. - verbose: Show notifications when the command started/exited. + Passed to _run_process. """ raise NotImplementedError @@ -190,7 +227,10 @@ class _POSIXUserscriptRunner(_BaseUserscriptRunner): super().__init__(win_id, parent) self._reader = None - def run(self, cmd, *args, env=None, verbose=False): + def prepare_run(self, *args, **kwargs): + self._args = args + self._kwargs = kwargs + try: # tempfile.mktemp is deprecated and discouraged, but we use it here # to create a FIFO since the only other alternative would be to @@ -209,8 +249,6 @@ class _POSIXUserscriptRunner(_BaseUserscriptRunner): self._reader = _QtFIFOReader(self._filepath) self._reader.got_line.connect(self.got_cmd) - self._run_process(cmd, *args, env=env, verbose=verbose) - @pyqtSlot() def on_proc_finished(self): self._cleanup() @@ -280,14 +318,16 @@ class _WindowsUserscriptRunner(_BaseUserscriptRunner): """Read back the commands when the process finished.""" self._cleanup() - def run(self, cmd, *args, env=None, verbose=False): + def prepare_run(self, *args, **kwargs): + self._args = args + self._kwargs = kwargs + try: self._oshandle, self._filepath = tempfile.mkstemp(text=True) except OSError as e: message.error(self._win_id, "Error while creating tempfile: " "{}".format(e)) return - self._run_process(cmd, *args, env=env, verbose=verbose) class _DummyUserscriptRunner(QObject): @@ -307,7 +347,7 @@ class _DummyUserscriptRunner(QObject): # pylint: disable=unused-argument super().__init__(parent) - def run(self, cmd, *args, env=None, verbose=False): + def prepare_run(self, *args, **kwargs): """Print an error as userscripts are not supported.""" # pylint: disable=unused-argument,unused-variable self.finished.emit() @@ -325,41 +365,11 @@ else: # pragma: no cover UserscriptRunner = _DummyUserscriptRunner -def store_source(frame): - """Store HTML/plaintext in files. - - This writes files containing the HTML/plaintext source of the page, and - returns a dict with the paths as QUTE_HTML/QUTE_TEXT. - - Args: - frame: The QWebFrame to get the info from, or None to do nothing. - - Return: - A dictionary with the needed environment variables. - - Warning: - The caller is responsible to delete the files after using them! - """ - if frame is None: - return {} - env = {} - with tempfile.NamedTemporaryFile(mode='w', encoding='utf-8', - suffix='.html', - delete=False) as html_file: - html_file.write(frame.toHtml()) - env['QUTE_HTML'] = html_file.name - with tempfile.NamedTemporaryFile(mode='w', encoding='utf-8', - suffix='.txt', - delete=False) as txt_file: - txt_file.write(frame.toPlainText()) - env['QUTE_TEXT'] = txt_file.name - return env - - -def run(cmd, *args, win_id, env, verbose=False): +def run_async(tab, cmd, *args, win_id, env, verbose=False): """Convenience method to run a userscript. Args: + tab: The WebKitTab/WebEngineTab to get the source from. cmd: The userscript binary to run. *args: The arguments to pass to the userscript. win_id: The window id the userscript is executed in. @@ -398,6 +408,9 @@ def run(cmd, *args, win_id, env, verbose=False): "userscripts", cmd) log.misc.debug("Userscript to run: {}".format(cmd_path)) - runner.run(cmd_path, *args, env=env, verbose=verbose) runner.finished.connect(commandrunner.deleteLater) runner.finished.connect(runner.deleteLater) + + runner.prepare_run(cmd_path, *args, env=env, verbose=verbose) + tab.dump_async(runner.store_html) + tab.dump_async(runner.store_text, plain=True) diff --git a/tests/unit/commands/test_userscripts.py b/tests/unit/commands/test_userscripts.py index 09e74d170..b19753745 100644 --- a/tests/unit/commands/test_userscripts.py +++ b/tests/unit/commands/test_userscripts.py @@ -80,7 +80,9 @@ def test_command(qtbot, py_proc, runner): f.write('foo\n') """) with qtbot.waitSignal(runner.got_cmd, timeout=10000) as blocker: - runner.run(cmd, *args) + runner.prepare_run(cmd, *args) + runner.store_html('') + runner.store_text('') assert blocker.args == ['foo'] @@ -100,7 +102,9 @@ def test_custom_env(qtbot, monkeypatch, py_proc, runner): """) with qtbot.waitSignal(runner.got_cmd, timeout=10000) as blocker: - runner.run(cmd, *args, env=env) + runner.prepare_run(cmd, *args, env=env) + runner.store_html('') + runner.store_text('') data = blocker.args[0] ret_env = json.loads(data) @@ -108,20 +112,16 @@ def test_custom_env(qtbot, monkeypatch, py_proc, runner): assert 'QUTEBROWSER_TEST_2' in ret_env -def test_temporary_files(qtbot, tmpdir, py_proc, runner): - """Make sure temporary files are passed and cleaned up correctly.""" - text_file = tmpdir / 'text' - text_file.write('This is text') - html_file = tmpdir / 'html' - html_file.write('This is HTML') - - env = {'QUTE_TEXT': str(text_file), 'QUTE_HTML': str(html_file)} - +def test_source(qtbot, py_proc, runner): + """Make sure the page source is read and cleaned up correctly.""" cmd, args = py_proc(r""" import os import json - data = {'html': None, 'text': None} + data = { + 'html_file': os.environ['QUTE_HTML'], + 'text_file': os.environ['QUTE_TEXT'], + } with open(os.environ['QUTE_HTML'], 'r') as f: data['html'] = f.read() @@ -136,76 +136,85 @@ def test_temporary_files(qtbot, tmpdir, py_proc, runner): with qtbot.waitSignal(runner.finished, timeout=10000): with qtbot.waitSignal(runner.got_cmd, timeout=10000) as blocker: - runner.run(cmd, *args, env=env) + runner.prepare_run(cmd, *args) + runner.store_html('This is HTML') + runner.store_text('This is text') data = blocker.args[0] parsed = json.loads(data) assert parsed['text'] == 'This is text' assert parsed['html'] == 'This is HTML' - assert not text_file.exists() - assert not html_file.exists() + assert not os.path.exists(parsed['text_file']) + assert not os.path.exists(parsed['html_file']) -def test_command_with_error(qtbot, tmpdir, py_proc, runner): - text_file = tmpdir / 'text' - text_file.write('This is text') - - env = {'QUTE_TEXT': str(text_file)} +def test_command_with_error(qtbot, py_proc, runner): cmd, args = py_proc(r""" - import sys + import sys, os, json + + with open(os.environ['QUTE_FIFO'], 'w') as f: + json.dump(os.environ['QUTE_TEXT'], f) + f.write('\n') + sys.exit(1) """) with qtbot.waitSignal(runner.finished, timeout=10000): - runner.run(cmd, *args, env=env) + with qtbot.waitSignal(runner.got_cmd, timeout=10000) as blocker: + runner.prepare_run(cmd, *args) + runner.store_text('Hello World') + runner.store_html('') - assert not text_file.exists() + data = json.loads(blocker.args[0]) + assert not os.path.exists(data) def test_killed_command(qtbot, tmpdir, py_proc, runner): - text_file = tmpdir / 'text' - text_file.write('This is text') - - pidfile = tmpdir / 'pid' + data_file = tmpdir / 'data' watcher = QFileSystemWatcher() watcher.addPath(str(tmpdir)) - env = {'QUTE_TEXT': str(text_file)} cmd, args = py_proc(r""" import os import time import sys + import json + + data = { + 'pid': os.getpid(), + 'text_file': os.environ['QUTE_TEXT'], + } # We can't use QUTE_FIFO to transmit the PID because that wouldn't work # on Windows, where QUTE_FIFO is only monitored after the script has # exited. with open(sys.argv[1], 'w') as f: - f.write(str(os.getpid())) + json.dump(data, f) time.sleep(30) """) - args.append(str(pidfile)) + args.append(str(data_file)) with qtbot.waitSignal(watcher.directoryChanged, timeout=10000): - runner.run(cmd, *args, env=env) + runner.prepare_run(cmd, *args) + runner.store_text('Hello World') + runner.store_html('') # Make sure the PID was written to the file, not just the file created time.sleep(0.5) + data = json.load(data_file) + with qtbot.waitSignal(runner.finished): - os.kill(int(pidfile.read()), signal.SIGTERM) + os.kill(int(data['pid']), signal.SIGTERM) - assert not text_file.exists() + assert not os.path.exists(data['text_file']) -def test_temporary_files_failed_cleanup(caplog, qtbot, tmpdir, py_proc, - runner): +def test_temporary_files_failed_cleanup(caplog, qtbot, py_proc, runner): """Delete a temporary file from the script so cleanup fails.""" - test_file = tmpdir / 'test' - test_file.write('foo') - cmd, args = py_proc(r""" import os os.remove(os.environ['QUTE_HTML']) @@ -213,10 +222,12 @@ def test_temporary_files_failed_cleanup(caplog, qtbot, tmpdir, py_proc, with caplog.at_level(logging.ERROR): with qtbot.waitSignal(runner.finished, timeout=10000): - runner.run(cmd, *args, env={'QUTE_HTML': str(test_file)}) + runner.prepare_run(cmd, *args) + runner.store_text('') + runner.store_html('') assert len(caplog.records) == 1 - expected = "Failed to delete tempfile {} (".format(test_file) + expected = "Failed to delete tempfile" assert caplog.records[0].message.startswith(expected) @@ -224,30 +235,4 @@ def test_dummy_runner(qtbot): runner = userscripts._DummyUserscriptRunner(0) with pytest.raises(cmdexc.CommandError): with qtbot.waitSignal(runner.finished): - runner.run('cmd', 'arg') - - -def test_store_source_none(): - assert userscripts.store_source(None) == {} - - -def test_store_source(stubs): - expected_text = 'This is text' - expected_html = 'This is HTML' - - frame = stubs.FakeWebFrame(plaintext=expected_text, html=expected_html) - env = userscripts.store_source(frame) - - with open(env['QUTE_TEXT'], 'r', encoding='utf-8') as f: - text = f.read() - with open(env['QUTE_HTML'], 'r', encoding='utf-8') as f: - html = f.read() - - os.remove(env['QUTE_TEXT']) - os.remove(env['QUTE_HTML']) - - assert set(env.keys()) == {'QUTE_TEXT', 'QUTE_HTML'} - assert text == expected_text - assert html == expected_html - assert env['QUTE_TEXT'].endswith('.txt') - assert env['QUTE_HTML'].endswith('.html') + runner.prepare_run('cmd', 'arg')