[office] Add regression tests for PDFDocumentPage.

CMakeLists.txt

@@ -75,3 +75,7 @@ install(FILES ${CMAKE_BINARY_DIR}/sailfish-office.ts DESTINATION ${CMAKE_INSTALL
 install(FILES org.sailfish.office.service DESTINATION ${CMAKE_INSTALL_PREFIX}/share/dbus-1/services)
 install(FILES org.sailfish.office.xml DESTINATION ${CMAKE_INSTALL_PREFIX}/share/dbus-1/interfaces)
 
+install(FILES tests/tst_PDFDocumentPage.qml DESTINATION /opt/tests/sailfish-office)
+install(FILES tests/data/broken.pdf DESTINATION /opt/tests/sailfish-office/data)
+install(FILES tests/data/protected.pdf DESTINATION /opt/tests/sailfish-office/data)
+install(FILES tests/data/sample.pdf DESTINATION /opt/tests/sailfish-office/data)

pdf/pdfjob.cpp

@@ -20,6 +20,7 @@
 
 #include <QtMath>
 #include <QUrlQuery>
+#include <QDebug>
 #include <poppler-qt5.h>
 
 LoadDocumentJob::LoadDocumentJob(const QString &source)
@@ -115,6 +116,15 @@ void RenderPageJob::run()
 {
     Q_ASSERT(m_document);
 
+    if (m_document->isLocked()) {
+        qWarning() << QStringLiteral("Rendering of a locked document.");
+        return;
+    }
+    if (m_index >= m_document->numPages()) {
+        qWarning() << QStringLiteral("Rendering of page %1 in a doument with %2 pages.").arg(m_index).arg(m_document->numPages());
+        return;
+    }
+
     Poppler::Page *page = m_document->page(m_index);
     QSizeF size = page->pageSizeF();
     float scale = 72.0f * (float(m_width) / size.width());

pdf/pdfrenderthread.cpp

@@ -445,7 +445,16 @@ void PDFRenderThreadQueue::processPendingJob()
     case PDFJob::LoadDocumentJob:
         d->loadFailure = false;
         break;
+    case PDFJob::UnLockDocumentJob:
+        job->m_document = d->document;
+        break;
     default:
+        // Avoid treating rendering jobs of a locked document
+        // because of a race condition when reusing pdfcanvas.cpp
+        // with the same PdfDocument instance while changing the actual
+        // PDF it is working on.
+        if (d->document->isLocked())
+            return;
         job->m_document = d->document;
         break;
     }

plugin/PDFDocumentPage.qml

@@ -27,6 +27,12 @@ import "PDFStorage.js" as PDFStorage
 DocumentPage {
     id: base
 
+    // Group of accessors properties. Can be used to control the PDFDocumentPage
+    // from outside or for testing purposes.
+    property alias document: pdfDocument
+    property alias placeHolder: placeHolderLoader
+    property alias toolbar: toolbar
+
     property var _settings // Handle save and restore the view settings using PDFStorage
     property ContextMenu contextMenuLinks
     property ContextMenu contextMenuText
@@ -90,6 +96,7 @@ DocumentPage {
     }
 
     Loader {
+        id: placeHolderLoader
         parent: base
         sourceComponent: (pdfDocument.failure || pdfDocument.locked) ? placeholderComponent : null
         anchors.verticalCenter: parent.verticalCenter
@@ -171,6 +178,11 @@ DocumentPage {
 
         property Notification notice
 
+        // Group of accessors properties. Can be used to control the toolbar
+        // from outside or for testing purposes.
+        property alias searchIconized: search.iconized
+        property alias searchText: search.text
+
         width: parent.width
         height: base.orientation == Orientation.Portrait
                 || base.orientation == Orientation.InvertedPortrait

plugin/PDFView.qml

@@ -36,6 +36,9 @@ SilicaFlickable {
     property alias selectionDraggable: selectionView.draggable
     property bool canMoveBack: (_contentYAtGotoLink >= 0)
 
+    // Accessor currently used for testing purposes.
+    property bool scrolling: scrollAnimation.running || quickScrollAnimation.running
+
     property bool scaled: pdfCanvas.width != width
     property QtObject _feedbackEffect
 
@@ -97,12 +100,8 @@ SilicaFlickable {
         _searchIndex = index
 
         var match = searchDisplay.itemAt(index)
-        var cX = match.x + match.width / 2. - width / 2.
-        cX = Math.max(0, Math.min(cX, pdfCanvas.width - width))
-        var cY = match.y + match.height / 2. - height / 2.
-        cY = Math.max(0, Math.min(cY, pdfCanvas.height - height))
-
-        scrollTo(Qt.point(cX, cY), match.page, match)
+        scrollTo(Qt.point(match.x + match.width / 2. - width / 2.,
+                          match.y + match.height / 2. - height / 2.), match.page, match)
     }
 
     function nextSearchMatch() {
@@ -122,24 +121,27 @@ SilicaFlickable {
     }
 
     function scrollTo(pt, pageId, focusItem) {
+        // Ensure that pt can be reached.
+        var ctX = Math.max(0, Math.min(contentWidth - width, pt.x))
+        var ctY = Math.max(0, Math.min(contentHeight - height, pt.y))
         if ((pt.y < base.contentY + base.height && pt.y > base.contentY - base.height)
             && (pt.x < base.contentX + base.width && pt.x > base.contentX - base.width)) {
-            scrollX.to = pt.x
-            scrollY.to = pt.y
+            scrollX.to = ctX
+            scrollY.to = ctY
             scrollAnimation.focusItem = (focusItem !== undefined) ? focusItem : null
             scrollAnimation.start()
         } else {
-            var deltaY = pt.y - base.contentY
+            var deltaY = ctY - base.contentY
             if (deltaY < 0) {
                 deltaY = Math.max(deltaY / 2., -base.height / 2.)
             } else {
                 deltaY = Math.min(deltaY / 2., base.height / 2.)
             }
-            leaveX.to = (base.contentX + pt.x) / 2
+            leaveX.to = (base.contentX + ctX) / 2
             leaveY.to = base.contentY + deltaY
-            returnX.to = pt.x
-            returnY.from = pt.y - deltaY
-            returnY.to = pt.y
+            returnX.to = ctX
+            returnY.from = ctY - deltaY
+            returnY.to = ctY
             quickScrollAnimation.pageTo = pageId
             quickScrollAnimation.focusItem = (focusItem !== undefined) ? focusItem : null
             quickScrollAnimation.start()
@@ -439,20 +441,14 @@ SilicaFlickable {
         if (left !== undefined && left >= 0.) {
             scrollX = rect.x + left * rect.width - ( leftSpacing !== undefined ? leftSpacing : 0.)
         }
-        if (scrollX > contentWidth - width) {
-            scrollX = contentWidth - width
-        }
         // Adjust vertical position.
         scrollY = rect.y + (top === undefined ? 0. : top * rect.height) - ( topSpacing !== undefined ? topSpacing : 0.)
-        if (scrollY > contentHeight - height) {
-            scrollY = contentHeight - height
-        }
-        return Qt.point(Math.max(0, scrollX), Math.max(0, scrollY))
+        return Qt.point(scrollX, scrollY)
     }
     function goToPage(pageNumber, top, left, topSpacing, leftSpacing) {
         var pt = contentAt(pageNumber, top, left, topSpacing, leftSpacing)
-        contentX = pt.x
-        contentY = pt.y
+        contentX = Math.max(0, Math.min(contentWidth - width, pt.x))
+        contentY = Math.max(0, Math.min(contentHeight - height, pt.y))
     }
     // This function is the inverse of goToPage(), returning (pageNumber, top, left).
     function getPagePosition() {

plugin/SearchBarItem.qml

@@ -47,6 +47,7 @@ BackgroundItem {
     property real iconizedWidth
     property bool searching
     property alias searchProgress: progressBar.progress
+    property alias text: searchField._searchText
 
     property real _margin: Math.max((iconizedWidth - searchIcon.width) / 2., 0.)
 

rpm/sailfish-office.spec

@@ -31,13 +31,22 @@ Summary: Translation source for %{name}
 License: GPLv2
 Group: System/Base
 
+%package tests
+Summary: Unitary tests for %{name}
+Requires: qt5-qtdeclarative-import-qttest
+Requires: qt5-qtdeclarative-devel-tools
+Requires: %{name} = %{version}-%{release}
+
 
 %description
 %{summary}.
 
 %description ts-devel
 %{summary}.
 
+%description tests
+%{summary}.
+
 
 %files
 %defattr(-,root,root,-)
@@ -52,6 +61,9 @@ Group: System/Base
 %files ts-devel
 %{_datadir}/translations/source/*.ts
 
+%files tests
+/opt/tests/sailfish-office/tst_PDFDocumentPage.qml
+/opt/tests/sailfish-office/data/*.pdf
 
 %prep
 %setup -q -n %{name}-%{version}