Fix exception thrown due to a form not existing yet

- Fix an issue where an exception was thrown because the theme progress form didn't exist yet
- Refactor a few things
- Fix other tests
- Add a test for wait_for

openlp/core/common/utils.py

@@ -0,0 +1,49 @@
+# -*- coding: utf-8 -*-
+
+##########################################################################
+# OpenLP - Open Source Lyrics Projection                                 #
+# ---------------------------------------------------------------------- #
+# Copyright (c) 2008-2019 OpenLP Developers                              #
+# ---------------------------------------------------------------------- #
+# This program is free software: you can redistribute it and/or modify   #
+# it under the terms of the GNU General Public License as published by   #
+# the Free Software Foundation, either version 3 of the License, or      #
+# (at your option) any later version.                                    #
+#                                                                        #
+# This program is distributed in the hope that it will be useful,        #
+# but WITHOUT ANY WARRANTY; without even the implied warranty of         #
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the          #
+# GNU General Public License for more details.                           #
+#                                                                        #
+# You should have received a copy of the GNU General Public License      #
+# along with this program.  If not, see <https://www.gnu.org/licenses/>. #
+##########################################################################
+"""
+The :mod:`~openlp.core.display.window` module contains the display window
+"""
+import logging
+import time
+
+from openlp.core.common.registry import Registry
+
+log = logging.getLogger(__name__)
+
+
+def wait_for(check_func, error_message='Timed out waiting for completion', timeout=10):
+    """
+    Wait until web engine page loaded
+    :return boolean: True on success, False on timeout
+    """
+    # Timeout in 10 seconds
+    end_time = time.time() + timeout
+    app = Registry().get('application')
+    success = True
+    while not check_func():
+        now = time.time()
+        if now > end_time:
+            log.error(error_message)
+            success = False
+            break
+        time.sleep(0.001)
+        app.process_events()
+    return success

openlp/core/display/html/display.js

@@ -383,6 +383,7 @@ var Display = {
   init: function (doTransitions=false) {
     Display._doTransitions = doTransitions;
     Reveal.initialize(Display._revealConfig);
+    displayWatcher.setInitialised(true);
   },
   /**
    * Reinitialise Reveal
@@ -1134,4 +1135,5 @@ var Display = {
 };
 new QWebChannel(qt.webChannelTransport, function (channel) {
   window.mediaWatcher = channel.objects.mediaWatcher;
+  window.displayWatcher = channel.objects.displayWatcher;
 });

openlp/core/display/render.py

@@ -36,6 +36,7 @@ from openlp.core.common.i18n import translate
 from openlp.core.common.mixins import LogMixin
 from openlp.core.common.registry import Registry, RegistryBase
 from openlp.core.common.settings import Settings
+from openlp.core.common.utils import wait_for
 from openlp.core.display.screens import ScreenList
 from openlp.core.display.window import DisplayWindow
 from openlp.core.lib import ItemCapabilities
@@ -486,35 +487,6 @@ class ThemePreviewRenderer(LogMixin, DisplayWindow):
             footer_html = 'Dummy footer text'
         return footer_html
 
-    def wait_till_loaded(self):
-        """
-        Wait until web engine page loaded
-        :return boolean: True on success, False on timeout
-        """
-        # Timeout in 10 seconds
-        end_time = time.time() + 10
-        app = Registry().get('application')
-        success = True
-        while not self._is_initialised:
-            if time.time() > end_time:
-                log.error('Timed out waiting for web engine page to load')
-                success = False
-                break
-            time.sleep(0.1)
-            app.process_events()
-        return success
-
-    def _wait_and_process(self, delay):
-        """
-        Wait while allowing things to process
-
-        :param delay: The amount of time in seconds to delay, can be a float
-        """
-        end_time = time.time() + delay
-        app = Registry().get('application')
-        while time.time() < end_time:
-            app.process_events()
-
     def generate_preview(self, theme_data, force_page=False, generate_screenshot=True):
         """
         Generate a preview of a theme.
@@ -535,7 +507,8 @@ class ThemePreviewRenderer(LogMixin, DisplayWindow):
             verses['verse'] = 'V1'
             verses['footer'] = self.generate_footer()
             self.load_verses([verses], is_sync=True)
-            self._wait_and_process(1)
+            # Wait for a second
+            wait_for(lambda: False, timeout=1)
             self.force_page = False
             if generate_screenshot:
                 return self.grab()
@@ -550,8 +523,7 @@ class ThemePreviewRenderer(LogMixin, DisplayWindow):
         :param item: The :class:`~openlp.core.lib.serviceitem.ServiceItem` item object.
 
         """
-        while not self._is_initialised:
+        wait_for(lambda: self._is_initialised)
-            QtWidgets.QApplication.instance().processEvents()
         self.log_debug('format slide')
         if item:
             # Set theme for preview
@@ -771,8 +743,10 @@ class ThemePreviewRenderer(LogMixin, DisplayWindow):
         :param text:  The text to check. It may contain HTML tags.
         """
         self.clear_slides()
+        self.log_debug('_text_fits_on_slide: 1\n{text}'.format(text=text))
         self.run_javascript('Display.addTextSlide("v1", "{text}", "Dummy Footer");'
                             .format(text=text.replace('"', '\\"')), is_sync=True)
+        self.log_debug('_text_fits_on_slide: 2')
         does_text_fits = self.run_javascript('Display.doesContentFit();', is_sync=True)
         return does_text_fits
 

openlp/core/display/window.py

@@ -25,18 +25,18 @@ import json
 import logging
 import os
 import copy
-import time
 
 from PyQt5 import QtCore, QtWebChannel, QtWidgets
 
-from openlp.core.common.i18n import translate
-from openlp.core.common.path import path_to_str
-from openlp.core.common.settings import Settings
-from openlp.core.common.registry import Registry
 from openlp.core.common.applocation import AppLocation
-from openlp.core.ui import HideMode
+from openlp.core.common.i18n import translate
-from openlp.core.display.screens import ScreenList
 from openlp.core.common.mixins import RegistryProperties
+from openlp.core.common.path import path_to_str
+from openlp.core.common.registry import Registry
+from openlp.core.common.settings import Settings
+from openlp.core.common.utils import wait_for
+from openlp.core.display.screens import ScreenList
+from openlp.core.ui import HideMode
 
 log = logging.getLogger(__name__)
 
@@ -102,6 +102,21 @@ class MediaWatcher(QtCore.QObject):
         self.muted.emit(is_muted)
 
 
+class DisplayWatcher(QtCore.QObject):
+    """
+    This facilitates communication from the Display object in the browser back to the Python
+    """
+    initialised = QtCore.pyqtSignal(bool)
+
+    @QtCore.pyqtSlot(bool)
+    def setInitialised(self, is_initialised):
+        """
+        This method is called from the JS in the browser to set the _is_initialised attribute
+        """
+        log.info('Display is initialised: {init}'.format(init=is_initialised))
+        self.initialised.emit(is_initialised)
+
+
 class DisplayWindow(QtWidgets.QWidget, RegistryProperties):
     """
     This is a window to show the output
@@ -137,10 +152,13 @@ class DisplayWindow(QtWidgets.QWidget, RegistryProperties):
         self.checkerboard_path = display_base_path / 'checkerboard.png'
         self.openlp_splash_screen_path = display_base_path / 'openlp-splash-screen.png'
         self.set_url(QtCore.QUrl.fromLocalFile(path_to_str(self.display_path)))
-        self.media_watcher = MediaWatcher(self)
         self.channel = QtWebChannel.QWebChannel(self)
+        self.media_watcher = MediaWatcher(self)
         self.channel.registerObject('mediaWatcher', self.media_watcher)
+        self.display_watcher = DisplayWatcher(self)
+        self.channel.registerObject('displayWatcher', self.display_watcher)
         self.webview.page().setWebChannel(self.channel)
+        self.display_watcher.initialised.connect(self.on_initialised)
         self.is_display = False
         self.scale = 1
         self.hide_mode = None
@@ -155,6 +173,16 @@ class DisplayWindow(QtWidgets.QWidget, RegistryProperties):
             if len(ScreenList()) > 1 or Settings().value('core/display on monitor'):
                 self.show()
 
+    @property
+    def is_initialised(self):
+        return self._is_initialised
+
+    def on_initialised(self, is_initialised):
+        """
+        Update the initialised status
+        """
+        self._is_initialised = is_initialised
+
     def update_from_screen(self, screen):
         """
         Update the number and the geometry from the screen.
@@ -208,7 +236,7 @@ class DisplayWindow(QtWidgets.QWidget, RegistryProperties):
         """
         js_is_display = str(self.is_display).lower()
         self.run_javascript('Display.init({do_transitions});'.format(do_transitions=js_is_display))
-        self._is_initialised = True
+        wait_for(lambda: self._is_initialised)
         if self.scale != 1:
             self.set_scale(self.scale)
         if self._can_show_startup_screen:
@@ -222,14 +250,8 @@ class DisplayWindow(QtWidgets.QWidget, RegistryProperties):
         :param is_sync: Run the script synchronously. Defaults to False
         """
         log.debug(script)
-        # Wait for other scripts to finish
+        # Wait for previous scripts to finish
-        end_time = time.time() + 10
+        wait_for(lambda: self.__script_done)
-        while not self.__script_done:
-            if time.time() > end_time:
-                log.error('Timed out waiting for preivous javascript script to finish')
-                break
-            time.sleep(0.1)
-            self.application.process_events()
         if not is_sync:
             self.webview.page().runJavaScript(script)
         else:
@@ -244,14 +266,9 @@ class DisplayWindow(QtWidgets.QWidget, RegistryProperties):
                 self.__script_result = result
 
             self.webview.page().runJavaScript(script, handle_result)
-            end_time = time.time() + 10
+            # Wait for script to finish
-            while not self.__script_done:
+            if not wait_for(lambda: self.__script_done):
-                if time.time() > end_time:
+                self.__script_done = True
-                    self.__script_done = True
-                    log.error('Timed out waiting for javascript script to finish')
-                    break
-                time.sleep(0.001)
-                self.application.process_events()
             return self.__script_result
 
     def go_to_slide(self, verse):

openlp/core/ui/thememanager.py

@@ -37,6 +37,7 @@ from openlp.core.common.mixins import LogMixin, RegistryProperties
 from openlp.core.common.path import create_paths
 from openlp.core.common.registry import Registry, RegistryBase
 from openlp.core.common.settings import Settings
+from openlp.core.common.utils import wait_for
 from openlp.core.lib import build_icon, check_item_selected, create_thumb, get_text_file_string, validate_thumb
 from openlp.core.lib.exceptions import ValidationError
 from openlp.core.lib.theme import Theme
@@ -165,7 +166,6 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
         self.setup_ui(self)
         self.global_theme = Settings().value(self.settings_section + '/global theme')
         self.build_theme_path()
-        self.upgrade_themes()  # TODO: Can be removed when upgrade path from OpenLP 2.4 no longer needed
 
     def bootstrap_post_set_up(self):
         """
@@ -175,8 +175,14 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
         self.theme_form = ThemeForm(self)
         self.theme_form.path = self.theme_path
         self.file_rename_form = FileRenameForm()
-        Registry().register_function('theme_update_global', self.change_global_from_tab)
+
+    def bootstrap_completion(self):
+        """
+        process the bootstrap completion request
+        """
+        self.upgrade_themes()  # TODO: Can be removed when upgrade path from OpenLP 2.4 no longer needed
         self.load_themes()
+        Registry().register_function('theme_update_global', self.change_global_from_tab)
 
     def upgrade_themes(self):
         """
@@ -184,6 +190,8 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
 
         :rtype: None
         """
+        # Wait for 2 seconds to allow some other things to start processing first
+        wait_for(lambda: False, timeout=1)
         xml_file_paths = AppLocation.get_section_data_path('themes').glob('*/*.xml')
         for xml_file_path in xml_file_paths:
             theme_data = get_text_file_string(xml_file_path)
@@ -722,7 +730,6 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
         theme_name_list = theme_name_list or self.get_theme_names()
         self.progress_form.theme_list = theme_name_list
         self.progress_form.show()
-        self.progress_form.theme_display.wait_till_loaded()
         for theme_name in theme_name_list:
             theme_data = self._get_theme_data(theme_name)
             preview_pixmap = self.progress_form.get_preview(theme_name, theme_data)

openlp/core/ui/themeprogressform.py

@@ -23,12 +23,13 @@ The theme regeneration progress dialog
 """
 from PyQt5 import QtWidgets
 
+from openlp.core.common.mixins import RegistryProperties, LogMixin
+from openlp.core.common.utils import wait_for
 from openlp.core.display.screens import ScreenList
 from openlp.core.ui.themeprogressdialog import UiThemeProgressDialog
-from openlp.core.common.mixins import RegistryProperties
 
 
-class ThemeProgressForm(QtWidgets.QDialog, UiThemeProgressDialog, RegistryProperties):
+class ThemeProgressForm(QtWidgets.QDialog, UiThemeProgressDialog, RegistryProperties, LogMixin):
     """
     The theme regeneration progress dialog
     """
@@ -38,9 +39,9 @@ class ThemeProgressForm(QtWidgets.QDialog, UiThemeProgressDialog, RegistryProper
         self._theme_list = []
 
     def show(self):
+        self.progress_bar.setValue(0)
         self.progress_bar.setMinimum(0)
         self.progress_bar.setMaximum(0)
-        self.progress_bar.setValue(0)
         try:
             screens = ScreenList()
             self.ratio = screens.current.display_geometry.width() / screens.current.display_geometry.height()
@@ -52,6 +53,7 @@ class ThemeProgressForm(QtWidgets.QDialog, UiThemeProgressDialog, RegistryProper
     def get_preview(self, theme_name, theme_data):
         self.label.setText(theme_name)
         self.progress_bar.setValue(self.progress_bar.value() + 1)
+        wait_for(lambda: self.theme_display.is_initialised)
         self.theme_display.set_scale(float(self.theme_display.width()) / self.renderer.width())
         return self.theme_display.generate_preview(theme_data, generate_screenshot=True)
 

tests/interfaces/openlp_core/ui/test_thememanager.py

@@ -58,7 +58,6 @@ class TestThemeManager(TestCase, TestMixin):
         # GIVEN: A new a call to initialise
         self.theme_manager.setup_ui = MagicMock()
         self.theme_manager.build_theme_path = MagicMock()
-        self.theme_manager.upgrade_themes = MagicMock()
         Settings().setValue('themes/global theme', 'my_theme')
 
         # WHEN: the initialisation is run
@@ -68,7 +67,6 @@ class TestThemeManager(TestCase, TestMixin):
         self.theme_manager.setup_ui.assert_called_once_with(self.theme_manager)
         assert self.theme_manager.global_theme == 'my_theme'
         self.theme_manager.build_theme_path.assert_called_once_with()
-        self.theme_manager.upgrade_themes.assert_called_once_with()
 
     @patch('openlp.core.ui.thememanager.create_paths')
     @patch('openlp.core.ui.thememanager.AppLocation.get_section_data_path')
@@ -110,7 +108,6 @@ class TestThemeManager(TestCase, TestMixin):
         Test the functions of bootstrap_post_setup are called.
         """
         # GIVEN:
-        self.theme_manager.load_themes = MagicMock()
         self.theme_manager.theme_path = MagicMock()
 
         # WHEN:
@@ -118,4 +115,21 @@ class TestThemeManager(TestCase, TestMixin):
             self.theme_manager.bootstrap_post_set_up()
 
         # THEN:
-        assert 1 == self.theme_manager.load_themes.call_count, "load_themes should have been called once"
+        assert self.theme_manager.progress_form is not None
+        assert self.theme_manager.theme_form is not None
+        assert self.theme_manager.file_rename_form is not None
+
+    def test_bootstrap_completion(self):
+        """
+        Test the functions of bootstrap_post_setup are called.
+        """
+        # GIVEN:
+        self.theme_manager.load_themes = MagicMock()
+        self.theme_manager.upgrade_themes = MagicMock()
+
+        # WHEN:
+        self.theme_manager.bootstrap_completion()
+
+        # THEN:
+        self.theme_manager.upgrade_themes.assert_called_once()
+        self.theme_manager.load_themes.assert_called_once()

tests/js/fake_webchannel.js

@@ -1,5 +1,10 @@
 // This is a mock QWebChannel
 var qt = {webChannelTransport: 1};
+var displayWatcher = {
+  setInitialised: function (is_initialised) {
+    // do nothing
+  }
+}
 var QWebChannel = function (transport, callback) {
-  callback({objects: {mediaWatcher: {}}});
+  callback({objects: {mediaWatcher: {}, displayWatcher: displayWatcher}});
 };

tests/openlp_core/common/test_utils.py

@@ -0,0 +1,44 @@
+# -*- coding: utf-8 -*-
+
+##########################################################################
+# OpenLP - Open Source Lyrics Projection                                 #
+# ---------------------------------------------------------------------- #
+# Copyright (c) 2008-2019 OpenLP Developers                              #
+# ---------------------------------------------------------------------- #
+# This program is free software: you can redistribute it and/or modify   #
+# it under the terms of the GNU General Public License as published by   #
+# the Free Software Foundation, either version 3 of the License, or      #
+# (at your option) any later version.                                    #
+#                                                                        #
+# This program is distributed in the hope that it will be useful,        #
+# but WITHOUT ANY WARRANTY; without even the implied warranty of         #
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the          #
+# GNU General Public License for more details.                           #
+#                                                                        #
+# You should have received a copy of the GNU General Public License      #
+# along with this program.  If not, see <https://www.gnu.org/licenses/>. #
+##########################################################################
+"""
+Interface tests to test the themeManager class and related methods.
+"""
+from unittest.mock import MagicMock
+
+from openlp.core.common.registry import Registry
+from openlp.core.common.utils import wait_for
+
+
+def test_wait_for(registry):
+    """
+    Test the wait_for function
+    """
+    # GIVEN: Mocked app and Registry
+    mock_app = MagicMock()
+    Registry().register('application', mock_app)
+    mock_func = MagicMock()
+    mock_func.side_effect = [False, True]
+
+    # WHEN: wait_for is run
+    wait_for(mock_func)
+
+    # THEN: the functions got called
+    assert mock_func.call_count == 2