Fix session restore/save with multiple tabs

BUGS: 335852
REVIEW: 122570

part.cpp

@@ -2744,26 +2744,6 @@ void Part::doPrint(QPrinter &printer)
     }
 }
 
-
-void Part::restoreDocument(const KConfigGroup &group)
-{
-    KUrl url ( group.readPathEntry( "URL", QString() ) );
-    if ( url.isValid() )
-    {
-        QString viewport = group.readEntry( "Viewport" );
-        if (!viewport.isEmpty()) m_document->setNextDocumentViewport( Okular::DocumentViewport( viewport ) );
-        openUrl( url );
-    }
-}
-
-
-void Part::saveDocumentRestoreInfo(KConfigGroup &group)
-{
-    group.writePathEntry( "URL", url().url() );
-    group.writeEntry( "Viewport", m_document->viewport().toString() );
-}
-
-
 void Part::psTransformEnded(int exit, QProcess::ExitStatus status)
 {
     Q_UNUSED( exit )

part.h

@@ -222,8 +222,6 @@ class OKULAR_PART_EXPORT Part : public KParts::ReadWritePart, public Okular::Doc
     public slots:
         // connected to Shell action (and browserExtension), not local one
         void slotPrint();
-        void restoreDocument(const KConfigGroup &group);
-        void saveDocumentRestoreInfo(KConfigGroup &group);
         void slotFileDirty( const QString& );
         void slotDoFileDirty();
         void psTransformEnded(int, QProcess::ExitStatus);

shell/main.cpp

@@ -39,7 +39,7 @@ int main(int argc, char** argv)
     // see if we are starting with session management
     if (app.isSessionRestored())
     {
-        RESTORE(Shell);
+        kRestoreMainWindows<Shell>();
     }
     else
     {

shell/shell.cpp

@@ -59,6 +59,9 @@
 static const char *shouldShowMenuBarComingFromFullScreen = "shouldShowMenuBarComingFromFullScreen";
 static const char *shouldShowToolBarComingFromFullScreen = "shouldShowToolBarComingFromFullScreen";
 
+static const char* const SESSION_URL_KEY = "Urls";
+static const char* const SESSION_TAB_KEY = "ActiveTab";
+
 Shell::Shell( const QString &serializedOptions )
   : KParts::MainWindow(), m_menuBarWasShown(true), m_toolBarWasShown(true)
 #ifdef KActivities_FOUND
@@ -66,7 +69,7 @@ Shell::Shell( const QString &serializedOptions )
 #endif
     , m_isValid(true)
 {
-  setObjectName( QLatin1String( "okular::Shell" ) );
+  setObjectName( QLatin1String( "okular::Shell#" ) );
   setContextMenuPolicy( Qt::NoContextMenu );
   // set the shell's ui resource file
   setXMLFile("shell.rc");
@@ -106,6 +109,7 @@ Shell::Shell( const QString &serializedOptions )
 
     // then, setup our actions
     setupActions();
+    connect( QCoreApplication::instance(), SIGNAL(aboutToQuit()), SLOT(deleteLater()) );
     // and integrate the part's GUI with the shell's
     setupGUI(Keys | ToolBar | Save);
     createGUI(firstPart);
@@ -156,6 +160,7 @@ Shell::~Shell()
         {
            it->part->closeUrl( false );
         }
+        m_tabs.clear();
     }
     if (m_unique)
         QDBusConnection::sessionBus().unregisterService("org.kde.okular");
@@ -321,19 +326,31 @@ void Shell::setupActions()
 
 void Shell::saveProperties(KConfigGroup &group)
 {
-  // the 'config' object points to the session managed
-  // config file.  anything you write here will be available
-  // later when this app is restored
-    emit saveDocumentRestoreInfo(group);
+    // Gather lists of settings to preserve
+    QStringList urls;
+    for( int i = 0; i < m_tabs.size(); ++i )
+    {
+        urls.append( m_tabs[i].part->url().url() );
+    }
+    group.writePathEntry( SESSION_URL_KEY, urls );
+    group.writeEntry( SESSION_TAB_KEY, m_tabWidget->currentIndex() );
 }
 
 void Shell::readProperties(const KConfigGroup &group)
 {
-  // the 'config' object points to the session managed
-  // config file.  this function is automatically called whenever
-  // the app is being restored.  read in here whatever you wrote
-  // in 'saveProperties'
-    emit restoreDocument(group);
+    // Reopen documents based on saved settings
+    QStringList urls = group.readPathEntry( SESSION_URL_KEY, QStringList() );
+
+    while( !urls.isEmpty() )
+    {
+        openUrl( urls.takeFirst() );
+    }
+
+    int desiredTab = group.readEntry<int>( SESSION_TAB_KEY, 0 );
+    if( desiredTab < m_tabs.size() )
+    {
+        setActiveTab( desiredTab );
+    }
 }
 
 QStringList Shell::fileFormats() const
@@ -399,11 +416,6 @@ void Shell::fileOpen()
     }
 }
 
-void Shell::slotQuit()
-{
-    close();
-}
-
 void Shell::tryRaise()
 {
     KWindowSystem::forceActiveWindow( window()->effectiveWinId() );
@@ -553,8 +565,6 @@ void Shell::applyOptionsToPart( QObject* part, const QString &serializedOptions
 
 void Shell::connectPart( QObject* part )
 {
-    connect( this, SIGNAL(restoreDocument(KConfigGroup)), part, SLOT(restoreDocument(KConfigGroup)));
-    connect( this, SIGNAL(saveDocumentRestoreInfo(KConfigGroup&)), part, SLOT(saveDocumentRestoreInfo(KConfigGroup&)));
     connect( part, SIGNAL(enablePrintAction(bool)), this, SLOT(setPrintEnabled(bool)));
     connect( part, SIGNAL(enableCloseAction(bool)), this, SLOT(setCloseEnabled(bool)));
     connect( part, SIGNAL(mimeTypeChanged(KMimeType::Ptr)), this, SLOT(setTabIcon(KMimeType::Ptr)));
@@ -655,6 +665,4 @@ void Shell::moveTabData( int from, int to )
    m_tabs.move( from, to );
 }
 
-#include "shell.moc"
-
 /* kate: replace-tabs on; indent-width 4; */

shell/shell.h

@@ -67,8 +67,6 @@ public:
   bool isValid() const;
 
 public slots:
-  void slotQuit();
-  
   Q_SCRIPTABLE Q_NOREPLY void tryRaise();
   Q_SCRIPTABLE bool openDocument( const QString& url, const QString &serializedOptions = QString() );
   Q_SCRIPTABLE bool canOpenDocs( int numDocs, int desktop );
@@ -86,6 +84,13 @@ protected:
    * with @ref saveProperties
    */
   void readProperties(const KConfigGroup&);
+
+  /**
+   * Expose internal functions for session restore testing
+   */
+  void savePropertiesInternal(KConfig* config, int num) {KMainWindow::savePropertiesInternal(config,num);}
+  void readPropertiesInternal(KConfig* config, int num) {KMainWindow::readPropertiesInternal(config,num);}
+
   void readSettings();
   void writeSettings();
   void setFullScreen( bool );
@@ -117,10 +122,6 @@ private slots:
   void handleTabDrop( QDropEvent* event );
   void moveTabData( int from, int to );
 
-signals:
-  void restoreDocument(const KConfigGroup &group);
-  void saveDocumentRestoreInfo(KConfigGroup &group);
-
 private:
   void setupAccel();
   void setupActions();

tests/mainshelltest.cpp

@@ -11,6 +11,7 @@
 #include <qprintdialog.h>
 #include <qwidget.h>
 #include <ktabwidget.h>
+#include <kconfiggroup.h>
 
 #include "../shell/okular_main.h"
 #include "../shell/shell.h"
@@ -73,10 +74,26 @@ private slots:
     void testFileRemembersPagePosition();
     void test2FilesError_data();
     void test2FilesError();
+    void testSessionRestore_data();
+    void testSessionRestore();
 
 private:
 };
 
+QList<Shell*> getShells()
+{
+    QList<Shell*> shells;
+    foreach( KMainWindow* kmw, KMainWindow::memberList() )
+    {
+        Shell* shell = qobject_cast<Shell*>( kmw );
+        if( shell )
+        {
+            shells.append( shell );
+        }
+    }
+    return shells;
+}
+
 Shell *findShell(Shell *ignore = 0)
 {
     foreach (QWidget *widget, QApplication::topLevelWidgets())
@@ -467,6 +484,108 @@ void MainShellTest::test2FilesError()
     QVERIFY(!s);
 }
 
+void MainShellTest::testSessionRestore_data()
+{
+    QTest::addColumn<QStringList>("paths");
+    QTest::addColumn<QString>("options");
+    QTest::addColumn<bool>("useTabsOpen");
+    QTest::addColumn<bool>("useTabsRestore");
+
+    QStringList oneDocPaths( KDESRCDIR "data/file1.pdf" );
+    QStringList twoDocPaths( oneDocPaths );
+    twoDocPaths << KDESRCDIR "data/formSamples.pdf";
+
+    const QString options = ShellUtils::serializeOptions(false, false, false, false, QString());
+
+    QTest::newRow("1 doc, 1 window, tabs")      << oneDocPaths << options << true  << true;
+    QTest::newRow("2 docs, 1 window, tabs")     << twoDocPaths << options << true  << true;
+    QTest::newRow("2 docs, 2 windows, tabs")    << twoDocPaths << options << false << true;
+    QTest::newRow("2 docs, 2 windows, no tabs") << twoDocPaths << options << false << false;
+    QTest::newRow("2 docs, 1 window, no tabs")  << twoDocPaths << options << true  << false;
+}
+
+void MainShellTest::testSessionRestore()
+{
+    QFETCH( QStringList, paths );
+    QFETCH( QString, options );
+    QFETCH( bool, useTabsOpen );
+    QFETCH( bool, useTabsRestore );
+
+    Okular::Settings::self()->setShellOpenFileInTabs( useTabsOpen );
+
+    Okular::Status status = Okular::main( paths, options );
+    QCOMPARE( status, Okular::Success );
+
+    // Gather some information about the state
+    // Verify that the correct number of windows/tabs were opened
+    QList<Shell*> shells = getShells();
+    QVERIFY( !shells.isEmpty() );
+    int numDocs = 0;
+    foreach( Shell* shell, shells )
+    {
+        QTest::qWaitForWindowShown( shell );
+        numDocs += shell->m_tabs.size();
+    }
+
+    QCOMPARE( numDocs, paths.size() );
+    QCOMPARE( shells.size(), useTabsOpen ? 1 : paths.size() );
+    QTest::qWait( 100 );
+
+    // Simulate session shutdown. The actual shutdown path comes through
+    // QSessionManager XSMP handlers, then KApplication::commitData/saveState,
+    // then KMWSessionManager::commitData/saveState. Without simulating an X
+    // session manager, the best we can do here is to make a temporary Config
+    // and call KMainWindows save functions directly.
+    QTemporaryFile configFile;
+    QVERIFY( configFile.open() );
+
+    int numWindows = 0;
+    {   // Scope for config so that we can reconstruct from file
+        KConfig config( configFile.fileName(), KConfig::SimpleConfig );
+        foreach( Shell* shell, shells )
+        {
+            shell->savePropertiesInternal( &config, ++numWindows );
+            // Windows aren't necessarily closed on shutdown, but we'll use
+            // this as a way to trigger the destructor code, which is normally
+            // connected to the aboutToQuit signal
+            shell->close();
+        }
+    }
+
+    // Wait for shells to delete themselves. QTest::qWait doesn't do deferred
+    // deletions so we'll set up a full event loop to do that.
+    QEventLoop eventLoop;
+    QTimer::singleShot( 100, &eventLoop, SLOT(quit()) );
+    eventLoop.exec( QEventLoop::AllEvents | QEventLoop::DeferredDeletion );
+    shells = getShells();
+    QVERIFY( shells.isEmpty() );
+
+    Okular::Settings::self()->setShellOpenFileInTabs( useTabsRestore );
+
+    // Simulate session restore. We can't call KMainWindow::restore() directly
+    // because it asks for info from the session manager, which doesn't know
+    // about our temporary config. But the logic here mostly mirrors restore().
+    KConfig config( configFile.fileName(), KConfig::SimpleConfig );
+    for( int i = 1; i <= numWindows; ++i )
+    {
+        Shell* shell = new Shell;
+        shell->readPropertiesInternal( &config, i );
+        shell->show();
+    }
+
+    // Verify that the restore state is reasonable
+    shells = getShells();
+    QVERIFY( !shells.isEmpty() );
+    numDocs = 0;
+    foreach( Shell* shell, shells )
+    {
+        QTest::qWaitForWindowShown( shell );
+        numDocs += shell->m_tabs.size();
+    }
+
+    QCOMPARE( numDocs, paths.size() );
+    QCOMPARE( shells.size(), useTabsRestore ? numWindows : paths.size() );
+}
 
 QTEST_KDEMAIN( MainShellTest, GUI )
 #include "mainshelltest.moc"