IDocument: Simplify permission changes

Take care of handling file permission changes centrally.

TextDocument had its own, caching implementation of tracking the backing
file's read-only state. Move that into IDocument directly.

IDocument::reload with a permission-only change is not a very
interesting case, but every subclass needed to add handling of it.
Instead, remove TypePermission from the file-change types, and handle it
separately via the now unified checkPermissions() implementation.
IDocument::reloadBehavior already was never called with TypePermission.

Change-Id: I321d47ba6193bc878efa9bb50ba7a739fa492745
Reviewed-by: David Schulz <david.schulz@qt.io>

src/plugins/bineditor/bineditorplugin.cpp

@@ -322,22 +322,13 @@ public:
                                                  : m_widget->isModified();
     }
 
-    bool isFileReadOnly() const override {
-        const FilePath fn = filePath();
-        if (fn.isEmpty())
-            return false;
-        return !fn.toFileInfo().isWritable();
-    }
-
     bool isSaveAsAllowed() const override { return true; }
 
     bool reload(QString *errorString, ReloadFlag flag, ChangeType type) override
     {
+        Q_UNUSED(type)
         if (flag == FlagIgnore)
             return true;
-        if (type == TypePermissions) {
-            emit changed();
-        } else {
         emit aboutToReload();
         int cPos = m_widget->cursorPosition();
         m_widget->clear();
@@ -346,8 +337,6 @@ public:
         emit reloadFinished(success);
         return success;
     }
-        return true;
-    }
 
 private:
     BinEditorWidget *m_widget;

src/plugins/coreplugin/documentmanager.cpp

@@ -51,8 +51,9 @@
 #include <utils/globalfilechangeblocker.h>
 #include <utils/hostosinfo.h>
 #include <utils/mimetypes/mimedatabase.h>
-#include <utils/qtcassert.h>
+#include <utils/optional.h>
 #include <utils/pathchooser.h>
+#include <utils/qtcassert.h>
 #include <utils/reloadpromptutils.h>
 
 #include <QStringList>
@@ -1141,7 +1142,7 @@ void DocumentManager::checkForReload()
     QStringList filesToDiff;
     foreach (IDocument *document, changedIDocuments) {
         IDocument::ChangeTrigger trigger = IDocument::TriggerInternal;
-        IDocument::ChangeType type = IDocument::TypePermissions;
+        optional<IDocument::ChangeType> type;
         bool changed = false;
         // find out the type & behavior from the two possible files
         // behavior is internal if all changes are expected (and none removed)
@@ -1174,7 +1175,7 @@ void DocumentManager::checkForReload()
             IDocument::ChangeType fileChange = changeTypes.value(fileKey);
             if (fileChange == IDocument::TypeRemoved)
                 type = IDocument::TypeRemoved;
-            else if (fileChange == IDocument::TypeContents && type == IDocument::TypePermissions)
+            else if (fileChange == IDocument::TypeContents && !type)
                 type = IDocument::TypeContents;
         }
 
@@ -1196,35 +1197,36 @@ void DocumentManager::checkForReload()
         QString errorString;
         // we've got some modification
         // check if it's contents or permissions:
-        if (type == IDocument::TypePermissions) {
+        if (!type) {
             // Only permission change
-            success = document->reload(&errorString, IDocument::FlagReload, IDocument::TypePermissions);
+            document->checkPermissions();
+            success = true;
             // now we know it's a content change or file was removed
-        } else if (defaultBehavior == IDocument::ReloadUnmodified
+        } else if (defaultBehavior == IDocument::ReloadUnmodified && type == IDocument::TypeContents
-                   && type == IDocument::TypeContents && !document->isModified()) {
+                   && !document->isModified()) {
             // content change, but unmodified (and settings say to reload in this case)
-            success = document->reload(&errorString, IDocument::FlagReload, type);
+            success = document->reload(&errorString, IDocument::FlagReload, *type);
             // file was removed or it's a content change and the default behavior for
             // unmodified files didn't kick in
-        } else if (defaultBehavior == IDocument::ReloadUnmodified
+        } else if (defaultBehavior == IDocument::ReloadUnmodified && type == IDocument::TypeRemoved
-                   && type == IDocument::TypeRemoved && !document->isModified()) {
+                   && !document->isModified()) {
             // file removed, but unmodified files should be reloaded
             // so we close the file
             documentsToClose << document;
         } else if (defaultBehavior == IDocument::IgnoreAll) {
             // content change or removed, but settings say ignore
-            success = document->reload(&errorString, IDocument::FlagIgnore, type);
+            success = document->reload(&errorString, IDocument::FlagIgnore, *type);
             // either the default behavior is to always ask,
             // or the ReloadUnmodified default behavior didn't kick in,
             // so do whatever the IDocument wants us to do
         } else {
             // check if IDocument wants us to ask
-            if (document->reloadBehavior(trigger, type) == IDocument::BehaviorSilent) {
+            if (document->reloadBehavior(trigger, *type) == IDocument::BehaviorSilent) {
                 // content change or removed, IDocument wants silent handling
                 if (type == IDocument::TypeRemoved)
                     documentsToClose << document;
                 else
-                    success = document->reload(&errorString, IDocument::FlagReload, type);
+                    success = document->reload(&errorString, IDocument::FlagReload, *type);
             // IDocument wants us to ask
             } else if (type == IDocument::TypeContents) {
                 // content change, IDocument wants to ask user

src/plugins/coreplugin/idocument.cpp

@@ -27,6 +27,7 @@
 
 #include <utils/fileutils.h>
 #include <utils/infobar.h>
+#include <utils/optional.h>
 #include <utils/qtcassert.h>
 
 #include <QFile>
@@ -129,8 +130,6 @@
 
     \value TypeContents
            The contents of the file changed.
-    \value TypePermissions
-           The file permissions changed.
     \value TypeRemoved
            The file was removed.
 
@@ -228,6 +227,7 @@ public:
     QString autoSaveName;
     Utils::InfoBar *infoBar = nullptr;
     Id id;
+    optional<bool> fileIsReadOnly;
     bool temporary = false;
     bool hasWriteWarning = false;
     bool restored = false;
@@ -403,8 +403,6 @@ const Utils::FilePath &IDocument::filePath() const
 */
 IDocument::ReloadBehavior IDocument::reloadBehavior(ChangeTrigger trigger, ChangeType type) const
 {
-    if (type == TypePermissions)
-        return BehaviorSilent;
     if (type == TypeContents && trigger == TriggerInternal && !isModified())
         return BehaviorSilent;
     return BehaviorAsk;
@@ -443,10 +441,18 @@ bool IDocument::reload(QString *errorString, ReloadFlag flag, ChangeType type)
 }
 
 /*!
-    \internal
+    Updates the cached information about the read-only status of the backing file.
 */
 void IDocument::checkPermissions()
 {
+    bool previousReadOnly = d->fileIsReadOnly.value_or(false);
+    if (!filePath().isEmpty()) {
+        d->fileIsReadOnly = !filePath().toFileInfo().isWritable();
+    } else {
+        d->fileIsReadOnly = false;
+    }
+    if (previousReadOnly != *(d->fileIsReadOnly))
+        emit changed();
 }
 
 /*!
@@ -524,7 +530,9 @@ bool IDocument::isFileReadOnly() const
 {
     if (filePath().isEmpty())
         return false;
-    return !filePath().toFileInfo().isWritable();
+    if (!d->fileIsReadOnly)
+        const_cast<IDocument *>(this)->checkPermissions();
+    return d->fileIsReadOnly.value_or(false);
 }
 
 /*!

src/plugins/coreplugin/idocument.h

@@ -68,7 +68,6 @@ public:
 
     enum ChangeType {
         TypeContents,
-        TypePermissions,
         TypeRemoved
     };
 
@@ -104,7 +103,7 @@ public:
     void setUniqueDisplayName(const QString &name);
     QString uniqueDisplayName() const;
 
-    virtual bool isFileReadOnly() const;
+    bool isFileReadOnly() const;
     bool isTemporary() const;
     void setTemporary(bool temporary);
 
@@ -123,7 +122,7 @@ public:
     virtual ReloadBehavior reloadBehavior(ChangeTrigger state, ChangeType type) const;
     virtual bool reload(QString *errorString, ReloadFlag flag, ChangeType type);
 
-    virtual void checkPermissions();
+    void checkPermissions();
 
     bool autoSave(QString *errorString, const QString &filePath);
     void setRestoredFrom(const QString &name);

src/plugins/designer/formwindowfile.cpp

@@ -225,9 +225,6 @@ bool FormWindowFile::reload(QString *errorString, ReloadFlag flag, ChangeType ty
         if (!wasModified)
             updateIsModified();
         return true;
-    }
-    if (type == TypePermissions) {
-        emit changed();
     } else {
         emit aboutToReload();
         const bool success
@@ -235,7 +232,6 @@ bool FormWindowFile::reload(QString *errorString, ReloadFlag flag, ChangeType ty
         emit reloadFinished(success);
         return success;
     }
-    return true;
 }
 
 void FormWindowFile::setFallbackSaveAsFileName(const QString &fn)

src/plugins/genericprojectmanager/genericproject.cpp

@@ -642,9 +642,7 @@ bool GenericProjectFile::reload(QString *errorString, IDocument::ReloadFlag flag
 {
     Q_UNUSED(errorString)
     Q_UNUSED(flag)
-    if (type == TypePermissions)
+    Q_UNUSED(type)
-        return true;
-
     if (Target *t = m_project->activeTarget())
         static_cast<GenericBuildSystem *>(t->buildSystem())->refresh(m_options);
 

src/plugins/imageviewer/imageviewerfile.cpp

@@ -148,7 +148,7 @@ Core::IDocument::OpenResult ImageViewerFile::openImpl(QString *errorString, cons
 
 Core::IDocument::ReloadBehavior ImageViewerFile::reloadBehavior(ChangeTrigger state, ChangeType type) const
 {
-    if (type == TypeRemoved || type == TypePermissions)
+    if (type == TypeRemoved)
         return BehaviorSilent;
     if (type == TypeContents && state == TriggerInternal && !isModified())
         return BehaviorSilent;
@@ -159,12 +159,9 @@ bool ImageViewerFile::reload(QString *errorString,
                              Core::IDocument::ReloadFlag flag,
                              Core::IDocument::ChangeType type)
 {
+    Q_UNUSED(type)
     if (flag == FlagIgnore)
         return true;
-    if (type == TypePermissions) {
-        emit changed();
-        return true;
-    }
     emit aboutToReload();
     bool success = (openImpl(errorString, filePath().toString()) == OpenResult::Success);
     emit reloadFinished(success);

src/plugins/modeleditor/modeldocument.cpp

@@ -121,12 +121,9 @@ bool ModelDocument::isSaveAsAllowed() const
 bool ModelDocument::reload(QString *errorString, Core::IDocument::ReloadFlag flag,
                            Core::IDocument::ChangeType type)
 {
+    Q_UNUSED(type)
     if (flag == FlagIgnore)
         return true;
-    if (type == TypePermissions) {
-        emit changed();
-        return true;
-    }
     try {
         d->documentController->loadProject(filePath().toString());
     } catch (const qmt::FileNotFoundException &ex) {

src/plugins/qmakeprojectmanager/qmakeproject.cpp

@@ -121,8 +121,7 @@ public:
     {
         Q_UNUSED(errorString)
         Q_UNUSED(flag)
-        if (type == TypePermissions)
+        Q_UNUSED(type)
-            return true;
         m_priFile->scheduleUpdate();
         return true;
     }

src/plugins/resourceeditor/resourceeditorw.cpp

@@ -268,19 +268,15 @@ bool ResourceEditorDocument::isSaveAsAllowed() const
 
 bool ResourceEditorDocument::reload(QString *errorString, ReloadFlag flag, ChangeType type)
 {
+    Q_UNUSED(type)
     if (flag == FlagIgnore)
         return true;
-    if (type == TypePermissions) {
-        emit changed();
-    } else {
     emit aboutToReload();
     QString fn = filePath().toString();
     const bool success = (open(errorString, fn, fn) == OpenResult::Success);
     emit reloadFinished(success);
     return success;
 }
-    return true;
-}
 
 void ResourceEditorDocument::dirtyChanged(bool dirty)
 {

src/plugins/resourceeditor/resourcenode.cpp

@@ -70,8 +70,7 @@ public:
 
     bool reload(QString *, ReloadFlag, ChangeType type) final
     {
-        if (type == TypePermissions)
+        Q_UNUSED(type)
-            return true;
         FolderNode *parent = m_node->parentFolderNode();
         QTC_ASSERT(parent, return false);
         parent->replaceSubtree(m_node, std::make_unique<ResourceTopLevelNode>(

src/plugins/scxmleditor/scxmleditordocument.cpp

@@ -136,11 +136,9 @@ bool ScxmlEditorDocument::isModified() const
 
 bool ScxmlEditorDocument::reload(QString *errorString, ReloadFlag flag, ChangeType type)
 {
-    if (flag == FlagIgnore) {
+    Q_UNUSED(type)
+    if (flag == FlagIgnore)
         return true;
-    } if (type == TypePermissions) {
-        emit changed();
-    } else {
     emit aboutToReload();
     emit reloadRequested(errorString, filePath().toString());
     const bool success = errorString->isEmpty();
@@ -148,9 +146,6 @@ bool ScxmlEditorDocument::reload(QString *errorString, ReloadFlag flag, ChangeTy
     return success;
 }
 
-    return true;
-}
-
 QString ScxmlEditorDocument::designWidgetContents() const
 {
     return m_designWidget->contents();

src/plugins/tasklist/taskfile.cpp

@@ -52,8 +52,6 @@ bool TaskFile::reload(QString *errorString, ReloadFlag flag, ChangeType type)
 {
     Q_UNUSED(flag)
 
-    if (type == TypePermissions)
-        return true;
     if (type == TypeRemoved) {
         deleteLater();
         return true;

src/plugins/texteditor/textdocument.cpp

@@ -106,7 +106,6 @@ public:
     QScopedPointer<Indenter> m_indenter;
     QScopedPointer<Formatter> m_formatter;
 
-    bool m_fileIsReadOnly = false;
     int m_autoSaveRevision = -1;
 
     TextMarks m_marksCache; // Marks not owned
@@ -702,30 +701,11 @@ void TextDocument::setFilePath(const Utils::FilePath &newName)
     IDocument::setFilePath(Utils::FilePath::fromUserInput(newName.toFileInfo().absoluteFilePath()));
 }
 
-bool TextDocument::isFileReadOnly() const
-{
-    if (filePath().isEmpty()) //have no corresponding file, so editing is ok
-        return false;
-    return d->m_fileIsReadOnly;
-}
-
 bool TextDocument::isModified() const
 {
     return d->m_document.isModified();
 }
 
-void TextDocument::checkPermissions()
-{
-    bool previousReadOnly = d->m_fileIsReadOnly;
-    if (!filePath().isEmpty()) {
-        d->m_fileIsReadOnly = !filePath().toFileInfo().isWritable();
-    } else {
-        d->m_fileIsReadOnly = false;
-    }
-    if (previousReadOnly != d->m_fileIsReadOnly)
-        emit changed();
-}
-
 Core::IDocument::OpenResult TextDocument::open(QString *errorString, const QString &fileName,
                                                const QString &realFileName)
 {
@@ -747,7 +727,6 @@ Core::IDocument::OpenResult TextDocument::openImpl(QString *errorString, const Q
 
     if (!fileName.isEmpty()) {
         const QFileInfo fi(fileName);
-        d->m_fileIsReadOnly = !fi.isWritable();
         readResult = read(realFileName, &content, errorString);
         const int chunks = content.size();
 
@@ -864,13 +843,8 @@ bool TextDocument::reload(QString *errorString, ReloadFlag flag, ChangeType type
             modificationChanged(true);
         return true;
     }
-    if (type == TypePermissions) {
-        checkPermissions();
-        return true;
-    } else {
     return reload(errorString);
 }
-}
 
 void TextDocument::setSyntaxHighlighter(SyntaxHighlighter *highlighter)
 {

src/plugins/texteditor/textdocument.h

@@ -115,10 +115,8 @@ public:
     QByteArray contents() const override;
     bool setContents(const QByteArray &contents) override;
     bool shouldAutoSave() const override;
-    bool isFileReadOnly() const override;
     bool isModified() const override;
     bool isSaveAsAllowed() const override;
-    void checkPermissions() override;
     bool reload(QString *errorString, ReloadFlag flag, ChangeType type) override;
     void setFilePath(const Utils::FilePath &newName) override;