Merge lp:~verzegnassi-stefano/ubuntu-docviewer-app/reboot-lok-error-detection into lp:ubuntu-docviewer-app

[Stefano Verzegnassi]

* [loviewer] Adding error detection
* [loviewer] Removed updateZoomIfAutomatic() function in LOView, its code was a bit cryptic
* Added a debug option in CMakeLists
* added '*.user.*' filter in .bzrignore file

* [loviewer] Adding error detection
* [loviewer] Removed updateZoomIfAutomatic() function in LOView, its code was a bit cryptic
* Added a debug option in CMakeLists
* added '*.user.*' filter in .bzrignore file

[lp:~verzegnassi-stefano/ubuntu-docviewer-app/reboot-lok-error-detection]

197. By Stefano Verzegnassi on 2015-11-13

Provide a more extended description of the error

[Roman Shchekin (mrqtros)]

See my inline note (not important).

review: Approve

[Jenkins Bot (ubuntu-core-apps-jenkins-bot)]

FAILED: Continuous integration, rev:197
https://core-apps-jenkins.ubuntu.com/job/docviewer-app-ci/11/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/148/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/docviewer-app-ci/11/rebuild

review: Needs Fixing (continuous-integration)

[Jenkins Bot (ubuntu-core-apps-jenkins-bot)]

PASSED: Continuous integration, rev:197
https://core-apps-jenkins.ubuntu.com/job/docviewer-app-ci/20/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/158/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/docviewer-app-ci/20/rebuild

review: Approve (continuous-integration)

=== modified file '.bzrignore'
--- .bzrignore	2014-10-28 22:10:16 +0000
+++ .bzrignore	2015-11-13 17:32:36 +0000
@@ -1,6 +1,7 @@
 Makefile
 ubuntu-docviewer-app
 *.user
+*.user.*
 moc_file.cpp
 launcher/build-docviewer-launcher-Desktop-Debug/
 launcher/build-docviewer-launcher-Desktop-Release/

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2015-10-03 12:37:24 +0000
+++ CMakeLists.txt	2015-11-13 17:32:36 +0000
@@ -14,6 +14,9 @@
 set(CMAKE_AUTOMOC ON)
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -fno-permissive -pedantic -Wall -Wextra -fPIC")
 
+# Debugging purpose. Keep commented unless you need it.
+# set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_DEBUG}")
+
 include(FindPkgConfig)
 # Standard install paths
 include(GNUInstallDirs)

=== renamed file 'src/app/qml/common/FileNotFoundDialog.qml' => 'src/app/qml/common/ErrorDialog.qml'
--- src/app/qml/common/FileNotFoundDialog.qml	2015-10-10 12:03:30 +0000
+++ src/app/qml/common/ErrorDialog.qml	2015-11-13 17:32:36 +0000
@@ -18,11 +18,9 @@
 import Ubuntu.Components 1.2
 import Ubuntu.Components.Popups 1.0
 
-// We may want to refactor this dialog for a more generic usage, when we'll need it.
 Dialog {
     id: errorDialog
     title: i18n.tr("Error")
-    text: i18n.tr("File does not exist")
 
     Button {
         text: i18n.tr("Close")

=== modified file 'src/app/qml/loView/LOViewPage.qml'
--- src/app/qml/loView/LOViewPage.qml	2015-10-19 11:44:11 +0000
+++ src/app/qml/loView/LOViewPage.qml	2015-11-13 17:32:36 +0000
@@ -164,6 +164,31 @@
                         loPageContent.forceActiveFocus()
                     }
 
+                    onErrorChanged: {
+                        var errorString;
+
+                        switch(error) {
+                        case LibreOffice.Error.LibreOfficeNotFound:
+                            errorString = i18n.tr("LibreOffice binaries not found.")
+                            break;
+                        case LibreOffice.Error.LibreOfficeNotInitialized:
+                            errorString = i18n.tr("Error while loading LibreOffice.")
+                            break;
+                        case LibreOffice.Error.DocumentNotLoaded:
+                            errorString = i18n.tr("Document not loaded.\nThe requested document may be corrupt.")
+                            break;
+                        }
+
+                        if (errorString) {
+                            loPage.pageStack.pop()
+
+                            // We create the dialog in the MainView, so that it isn't
+                            // initialized by 'loPage' and keep on working after the
+                            // page is destroyed.
+                            mainView.showErrorDialog(errorString);
+                        }
+                    }
+
                     Scrollbar { flickableItem: loView; parent: loView.parent }
                     Scrollbar { flickableItem: loView; parent: loView.parent; align: Qt.AlignBottom }
                 }

=== modified file 'src/app/qml/ubuntu-docviewer-app.qml'
--- src/app/qml/ubuntu-docviewer-app.qml	2015-10-20 18:21:17 +0000
+++ src/app/qml/ubuntu-docviewer-app.qml	2015-11-13 17:32:36 +0000
@@ -98,6 +98,11 @@
         mainView.pickMode = true
     }
 
+    function showErrorDialog(message) {
+        PopupUtils.open(Qt.resolvedUrl("common/ErrorDialog.qml"),
+                        mainView, { parent: mainView, text: message });
+    }
+
     // On screen rotation, force updating of header/U8 indicators panel visibility
     onIsLandscapeChanged: setHeaderVisibility(true);
 
@@ -122,8 +127,7 @@
         onMimetypeChanged: LoadComponent.load(mimetype.name)
         onErrorChanged: {
             if (error == -1)
-                PopupUtils.open(Qt.resolvedUrl("common/FileNotFoundDialog.qml"),
-                                mainView, { parent: mainView });
+                mainView.showErrorDialog(i18n.tr("File does not exist."));
         }
     }
 

=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/CMakeLists.txt'
--- src/plugin/libreofficetoolkit-qml-plugin/CMakeLists.txt	2015-10-18 20:58:32 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/CMakeLists.txt	2015-11-13 17:32:36 +0000
@@ -28,6 +28,7 @@
 set(libreofficetoolkitqmlplugin_HDRS
     twips.h
     config.h
+    loerror.h
 )
 
 add_library(libreofficetoolkitqmlplugin MODULE

=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/lodocument.cpp'
--- src/plugin/libreofficetoolkit-qml-plugin/lodocument.cpp	2015-10-27 13:27:27 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/lodocument.cpp	2015-11-13 17:32:36 +0000
@@ -26,8 +26,6 @@
 #include <LibreOfficeKit/LibreOfficeKitInit.h>
 #include <LibreOfficeKit/LibreOfficeKit.hxx>
 
-// TODO: Error management
-
 #ifdef DEBUG_TILE_BENCHMARK
 #include <QElapsedTimer>
 #endif
@@ -37,6 +35,7 @@
 LODocument::LODocument()
   : m_path("")
   , m_currentPart(-1)
+  , m_error(LibreOfficeError::NoError)
   , m_document(nullptr)
 {
     // This space is intentionally empty.
@@ -58,7 +57,7 @@
     Q_EMIT pathChanged();
 
     // Load the new document
-    this->loadDocument(m_path);
+    loadDocument(m_path);
 }
 
 int LODocument::currentPart() {
@@ -78,21 +77,48 @@
 }
 
 // Load the document
-bool LODocument::loadDocument(const QString &pathName)
+void LODocument::loadDocument(const QString &pathName)
 {
     qDebug() << "Loading document...";
+    setError(LibreOfficeError::NoError);
 
     if (pathName.isEmpty()) {
         qDebug() << "Can't load the document, path is empty.";
-        return false;
-    }
-
+        return;
+    }
+
+
+    /* Get LibreOffice path */
+    const char* loPath = Config::getLibreOfficePath();
+
+    if (loPath == NULL) {
+        setError(LibreOfficeError::LibreOfficeNotFound);
+        return;
+    }
+
+
+    /* Load LibreOffice */
     if (!s_office)
-        s_office = lok::lok_cpp_init(Config::getLibreOfficePath(),
-                                     Config::getLibreOfficeProfilePath());
-
+        s_office = lok::lok_cpp_init(loPath, Config::getLibreOfficeProfilePath());
+
+    if (s_office == NULL) {
+        setError(LibreOfficeError::LibreOfficeNotInitialized);
+        qDebug() << "[lok-qml]: LibreOffice not initialized.";
+        return;
+    }
+
+
+    /* Load the document */
     m_document = s_office->documentLoad(m_path.toUtf8().constData());
 
+    if (m_document == NULL) {
+        setError(LibreOfficeError::DocumentNotLoaded);
+        qDebug() << "[lok-qml]: Document not loaded.";
+        return;
+    }
+
+
+    /* Do the further initialization */
     m_docType = DocumentType(m_document->getDocumentType());
     Q_EMIT documentTypeChanged();
 
@@ -101,7 +127,16 @@
     m_document->initializeForRendering();
     qDebug() << "Document loaded successfully !";
 
-    return true;
+    return;
+}
+
+void LODocument::setError(const LibreOfficeError::Error &error)
+{
+    if (m_error == error)
+        return;
+
+    m_error = error;
+    Q_EMIT errorChanged();
 }
 
 // Return the type of the loaded document (e.g. text document,
@@ -210,7 +245,12 @@
  
     return QString::fromUtf8(m_document->getPartName(index));
 }
- 
+
+LibreOfficeError::Error LODocument::error() const
+{
+    return m_error;
+}
+
 // TODO: Is there some documentation on safe formats or filterOptions that can
 // be used?
 bool LODocument::saveAs(QString url, QString format = QString(), QString filterOptions = QString())

=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/lodocument.h'
--- src/plugin/libreofficetoolkit-qml-plugin/lodocument.h	2015-10-04 16:11:47 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/lodocument.h	2015-11-13 17:32:36 +0000
@@ -20,6 +20,8 @@
 
 #include <QObject>
 
+#include "loerror.h"
+
 namespace lok {
 class Office;
 class Document;
@@ -30,12 +32,13 @@
     Q_OBJECT
     Q_DISABLE_COPY(LODocument)
 
-    Q_PROPERTY(QString      path         READ path         WRITE setPath         NOTIFY pathChanged)
-    Q_PROPERTY(int          currentPart  READ currentPart  WRITE setCurrentPart  NOTIFY currentPartChanged)
+    Q_PROPERTY(QString                 path         READ path         WRITE setPath         NOTIFY pathChanged)
+    Q_PROPERTY(int                     currentPart  READ currentPart  WRITE setCurrentPart  NOTIFY currentPartChanged)
     // Declare partsCount as constant at the moment, since LOK-plugin is just a viewer for now.
-    Q_PROPERTY(int          partsCount   READ partsCount                                                    CONSTANT)
-    Q_PROPERTY(int          documentPart READ documentPart WRITE setDocumentPart NOTIFY documentPartChanged)
-    Q_PROPERTY(DocumentType documentType READ documentType                       NOTIFY documentTypeChanged)
+    Q_PROPERTY(int                     partsCount   READ partsCount                                                    CONSTANT)
+    Q_PROPERTY(int                     documentPart READ documentPart WRITE setDocumentPart NOTIFY documentPartChanged)
+    Q_PROPERTY(DocumentType            documentType READ documentType                       NOTIFY documentTypeChanged)
+    Q_PROPERTY(LibreOfficeError::Error error        READ error                              NOTIFY errorChanged)
     Q_ENUMS(DocumentType)
 
 public:
@@ -70,6 +73,8 @@
     QString getPartName(int index) const;
     void setPart(int index);
 
+    LibreOfficeError::Error error() const;
+
     Q_INVOKABLE bool saveAs(QString url, QString format, QString filterOptions);
 
 Q_SIGNALS:
@@ -77,13 +82,17 @@
     void currentPartChanged();
     void documentTypeChanged();
     void documentPartChanged();
+    void errorChanged();
 
 private:
     QString m_path;
     int m_currentPart;
     DocumentType m_docType;
-
-    bool loadDocument(const QString &pathNAme);
+    LibreOfficeError::Error m_error;
+
+    void loadDocument(const QString &pathNAme);
+
+    void setError(const LibreOfficeError::Error &error);
 
     lok::Document *m_document;
 

=== added file 'src/plugin/libreofficetoolkit-qml-plugin/loerror.h'
--- src/plugin/libreofficetoolkit-qml-plugin/loerror.h	1970-01-01 00:00:00 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/loerror.h	2015-11-13 17:32:36 +0000
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2015 Canonical, Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3, as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY, 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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef LOERROR_H
+#define LOERROR_H
+
+#include <QObject>
+
+class LibreOfficeError : public QObject
+{
+    Q_OBJECT
+    Q_ENUMS(Error)
+
+public:
+    enum Error {
+        NoError                   = 0,
+        LibreOfficeNotFound       = 1,
+        LibreOfficeNotInitialized = 2,
+        DocumentNotLoaded         = 3
+    };
+};
+
+#endif // LOERROR_H

=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/loview.cpp'
--- src/plugin/libreofficetoolkit-qml-plugin/loview.cpp	2015-10-18 21:27:16 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/loview.cpp	2015-11-13 17:32:36 +0000
@@ -40,6 +40,7 @@
     , m_cacheBuffer(TILE_SIZE * 3)
     , m_visibleArea(0, 0, 0, 0)
     , m_bufferArea(0, 0, 0, 0)
+    , m_error(LibreOfficeError::NoError)
 {
     Q_UNUSED(parent)   
 
@@ -86,9 +87,22 @@
     if (m_document)
         m_document->disconnect(this);
 
+    setError(LibreOfficeError::NoError);
+
     m_document = QSharedPointer<LODocument>(new LODocument());
     m_document->setPath(path);
 
+    /* A lot of things happens when we set the path property in
+     * m_document. Need to check if an error has been emitted. */
+    if (m_document->error() != LibreOfficeError::NoError) {
+        setError(m_document->error());
+
+        m_document.clear();
+
+        // Stop doing anything below.
+        return;
+    }
+
     // TODO MOVE
     m_partsModel = new LOPartsModel(m_document);
     Q_EMIT partsModelChanged();
@@ -164,8 +178,16 @@
     Q_EMIT cacheBufferChanged();
 }
 
+LibreOfficeError::Error LOView::error() const
+{
+    return m_error;
+}
+
 void LOView::adjustZoomToWidth()
- {
+{
+    if (!m_document)
+        return;
+
     setZoomMode(LOView::FitToWidth);
 
     zoomValueToFitWidth = getZoomToFitWidth(m_parentFlickable->width(),
@@ -173,26 +195,6 @@
 
     setZoomFactor(zoomValueToFitWidth);
     qDebug() << "Adjust zoom to width - value:" << zoomValueToFitWidth;
- }
-
-bool LOView::updateZoomIfAutomatic()
-{
-    // This function is only used in LOView::updateVisibleRect()
-    // It returns a bool, so that we can stop the execution of that function,
-    // which will be triggered again when we'll automatically update the zoom value.
-    if (m_zoomMode == LOView::FitToWidth) {
-        zoomValueToFitWidth = getZoomToFitWidth(m_parentFlickable->width(),
-                                                m_document->documentSize().width());
-
-        if (m_zoomFactor != zoomValueToFitWidth) {
-            setZoomFactor(zoomValueToFitWidth);
-
-            qDebug() << "Adjust automatic zoom to width - value:" << zoomValueToFitWidth;
-            return true;
-        }
-    }
-
-    return false;
 }
 
 void LOView::updateViewSize()
@@ -210,7 +212,7 @@
 
 void LOView::updateVisibleRect()
 {
-    if (!m_parentFlickable)
+    if (!m_parentFlickable || !m_document)
         return;
 
     // Changes in parentFlickable width/height trigger directly LOView::updateVisibleRect(),
@@ -222,8 +224,17 @@
     // If that happens, stop the execution of this function, since the change of
     // zoomFactor will trigger the updateViewSize() function, which triggers this
     // function again.
-    if (this->updateZoomIfAutomatic())
-        return;
+    if (m_zoomMode == LOView::FitToWidth) {
+        zoomValueToFitWidth = getZoomToFitWidth(m_parentFlickable->width(),
+                                                m_document->documentSize().width());
+
+        if (m_zoomFactor != zoomValueToFitWidth) {
+            setZoomFactor(zoomValueToFitWidth);
+
+            qDebug() << "Adjust automatic zoom to width - value:" << zoomValueToFitWidth;
+            return;
+        }
+    }
 
     // Check if current tiles have a different zoom value
     if (!m_tiles.isEmpty()) {
@@ -381,6 +392,15 @@
     }
 }
 
+void LOView::setError(const LibreOfficeError::Error &error)
+{
+    if (m_error == error)
+        return;
+
+    m_error = error;
+    Q_EMIT errorChanged();
+}
+
 LOView::~LOView()
 {
     delete m_partsModel;

=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/loview.h'
--- src/plugin/libreofficetoolkit-qml-plugin/loview.h	2015-10-11 11:27:29 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/loview.h	2015-11-13 17:32:36 +0000
@@ -23,6 +23,7 @@
 #include <QQmlContext>
 #include <QQmlEngine>
 
+#include "loerror.h"
 #include "renderengine.h"
 #include "lopartsmodel.h"
 #include "lopartsimageprovider.h"
@@ -34,12 +35,13 @@
 {
     Q_OBJECT
     Q_ENUMS(ZoomMode)
-    Q_PROPERTY(QQuickItem*   parentFlickable READ parentFlickable WRITE setParentFlickable NOTIFY parentFlickableChanged)
-    Q_PROPERTY(LODocument*   document        READ document        /*WRITE setDocument*/    NOTIFY documentChanged)
-    Q_PROPERTY(LOPartsModel* partsModel      READ partsModel                               NOTIFY partsModelChanged)
-    Q_PROPERTY(qreal         zoomFactor      READ zoomFactor      WRITE setZoomFactor      NOTIFY zoomFactorChanged)
-    Q_PROPERTY(ZoomMode      zoomMode        READ zoomMode                                 NOTIFY zoomModeChanged)
-    Q_PROPERTY(int           cacheBuffer     READ cacheBuffer     WRITE setCacheBuffer     NOTIFY cacheBufferChanged)
+    Q_PROPERTY(QQuickItem*              parentFlickable READ parentFlickable WRITE setParentFlickable NOTIFY parentFlickableChanged)
+    Q_PROPERTY(LODocument*              document        READ document        /*WRITE setDocument*/    NOTIFY documentChanged)
+    Q_PROPERTY(LOPartsModel*            partsModel      READ partsModel                               NOTIFY partsModelChanged)
+    Q_PROPERTY(qreal                    zoomFactor      READ zoomFactor      WRITE setZoomFactor      NOTIFY zoomFactorChanged)
+    Q_PROPERTY(ZoomMode                 zoomMode        READ zoomMode                                 NOTIFY zoomModeChanged)
+    Q_PROPERTY(int                      cacheBuffer     READ cacheBuffer     WRITE setCacheBuffer     NOTIFY cacheBufferChanged)
+    Q_PROPERTY(LibreOfficeError::Error  error           READ error                                    NOTIFY errorChanged)
 
 public:
     LOView(QQuickItem *parent = 0);
@@ -66,6 +68,8 @@
     int         cacheBuffer() const;
     void        setCacheBuffer(int cacheBuffer);
 
+    LibreOfficeError::Error error() const;
+
     Q_INVOKABLE void adjustZoomToWidth();
 
 Q_SIGNALS:
@@ -75,6 +79,7 @@
     void zoomFactorChanged();
     void zoomModeChanged();
     void cacheBufferChanged();
+    void errorChanged();
 
 private Q_SLOTS:
     void updateViewSize();
@@ -99,6 +104,8 @@
     QRect                       m_visibleArea;
     QRect                       m_bufferArea;
 
+    LibreOfficeError::Error     m_error;
+
     QTimer                      m_updateTimer;
 
     QMap<int, SGTileItem*>      m_tiles;
@@ -106,8 +113,9 @@
     void generateTiles(int x1, int y1, int x2, int y2, int tilesPerWidth);
     void createTile(int index, QRect rect);
     void setZoomMode(const ZoomMode zoomMode);
-    bool updateZoomIfAutomatic();
     void clearView();
+
+    void setError(const LibreOfficeError::Error &error);
 };
 
 #endif // LOVIEW_H

=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/plugin.cpp'
--- src/plugin/libreofficetoolkit-qml-plugin/plugin.cpp	2015-10-05 20:53:25 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/plugin.cpp	2015-11-13 17:32:36 +0000
@@ -22,15 +22,17 @@
 #include "lodocument.h"
 #include "loview.h"
 #include "lopartsmodel.h"
+#include "loerror.h"
 
 void LOPlugin::registerTypes(const char *uri)
 {
     Q_ASSERT(uri == QLatin1String("DocumentViewer.LibreOffice"));
     
     //@uri DocumentViewer.LibreOffice
-    qmlRegisterType<LODocument>(uri, 1, 0, "Document");
-    qmlRegisterType<LOView>(uri, 1, 0, "View");
-    qmlRegisterUncreatableType<LOPartsModel>(uri, 1, 0, "PartsModel", "You shouldn't create LOPartsModel in QML");
+    qmlRegisterType             <LODocument>        (uri, 1, 0, "Document");
+    qmlRegisterType             <LOView>            (uri, 1, 0, "View");
+    qmlRegisterUncreatableType  <LOPartsModel>      (uri, 1, 0, "PartsModel", "You shouldn't create LOPartsModel in QML");
+    qmlRegisterUncreatableType  <LibreOfficeError>  (uri, 1, 0, "Error",      "Not creatable as an object, use only to retrieve error enums (e.g. LibreOffice.Error.DocumentNotFound)");
 }
 
 void LOPlugin::initializeEngine(QQmlEngine *engine, const char *uri)

=== modified file 'src/plugin/libreofficetoolkit-qml-plugin/qml/Viewer.qml'
--- src/plugin/libreofficetoolkit-qml-plugin/qml/Viewer.qml	2015-10-05 06:42:33 +0000
+++ src/plugin/libreofficetoolkit-qml-plugin/qml/Viewer.qml	2015-11-13 17:32:36 +0000
@@ -23,8 +23,9 @@
     property alias document:    view.document
     property alias zoomFactor:  view.zoomFactor
     property alias cacheBuffer: view.cacheBuffer
-    property alias partsModel: view.partsModel
+    property alias partsModel:  view.partsModel
     property alias zoomMode:    view.zoomMode
+    property alias error:       view.error
 
     property string documentPath: ""