Hi, the attached fixed fixes #6597. Does it look alright?
--
Suggested log message:
Fix #6597: LyX Appears frozen if the process holding the clipboard is frozen
Implements CacheMimeData type so that we only need to query the
clipboard once on startup and once each time the contents of the
clipboard change.
This is important as Qt takes 5 seconds to time-out when the
clipboard is non-responsive.
--
I have considered a number of other alternatives, for example, instead
of implementing a CacheMimeData data type, we could just store the
formats as a QStringList and hardcode the various mimetype in
GuiClipboard.cpp, but all of these seemed to have serious drawbacks
(e.g. hardcoding is bad, and in this case more work than a "proper"
solution.)
--
John C. McCabe-Dansted
http://www.lyx.org/trac/ticket/6597
Index: frontends/qt4/GuiClipboard.cpp
===================================================================
--- frontends/qt4/GuiClipboard.cpp (revision 33816)
+++ frontends/qt4/GuiClipboard.cpp (working copy)
@@ -27,6 +27,7 @@
#include "support/filetools.h"
#include "support/gettext.h"
#include "support/lstrings.h"
+#include "support/lyxtime.h"
#ifdef Q_WS_MACX
#include "support/linkback/LinkBackProxy.h"
@@ -55,7 +56,71 @@
namespace frontend {
+QMimeData const * read_clipboard() {
+ LYXERR(Debug::ACTION, "Getting Clipboard\n");
+ QMimeData const * source =
+ qApp->clipboard()->mimeData(QClipboard::Clipboard);
+ if (!source) {
+ LYXERR(Debug::ACTION, "0 bytes (no QMimeData)");
+ return new QMimeData();
+ }
+ //It appears that doing IO between getting a mimeData object
+ //and using it can cause a crash (maybe Qt used IO
+ //as an excuse to free() it? Any lets not introduce
+ //any new IO here, so e.g. leave the following line commented.
+ //lyxerr << "Got Clipboard (" << (long) source << ")\n" ;
+ return source;
+}
+
+CacheMimeData::CacheMimeData()
+{
+ cached_formats_ = QStringList();
+ //LyX calls "on_dataChanged" on startup, so it is not necessary to
+ //query the clipboard here.
+ //update();
+}
+
+
+void CacheMimeData::update()
+{
+ time_t start_time = current_time();
+ LYXERR(Debug::ACTION, "Creating CacheMimeData object\n");
+ QMimeData const * const orig = read_clipboard();
+ cached_formats_ = orig->formats();
+
+ //Qt times out after 5 seconds if it does not recieve a response.
+ if (current_time() > (start_time + 3) ) {
+ string msg = "No timely response from clipboard, perhaps process holding clipboard is frozen?\n";
+ lyxerr << msg;
+ //We could perhaps emit a status message, but we are unlikely
+ //to get this unless starting up, and then there is no
+ //status bar to display it onto (resulting in a SIGSEGV).
+ //lyx::frontend::theGuiApp()->currentView()->message(_(msg));
+ }
+}
+
+
+// We can't really cache this so we will just use the original
+// QMimeData to retrieve the actual data... but thats protected
+// so we only use CacheMimeData fore MimeTypes, not
+// actual data.
+QVariant CacheMimeData::retrieveData(const QString &format,
+ QVariant::Type preferredType) const {
+ (void)format; (void)preferredType;
+ lyxerr << "Attempt to retrieveData from a CacheMimeData object\n" <<
+ "(should retrieve the data from the original QMimeData object instead)\n";
+ LASSERT(false, /**/);
+ return 0;
+}
+
+
+QStringList CacheMimeData::formats() const
+{
+ return this->cached_formats_;
+}
+
+
QString const lyxMimeType(){ return "application/x-lyx"; }
QString const pdfMimeType(){ return "application/pdf"; }
QString const emfMimeType(){ return "image/x-emf"; }
@@ -76,14 +141,13 @@
LYXERR(Debug::ACTION, "GuiClipboard::getAsLyX(): `");
// We don't convert encodings here since the encoding of the
// clipboard contents is specified in the data itself
- QMimeData const * source =
- qApp->clipboard()->mimeData(QClipboard::Clipboard);
+ QMimeData const * source = read_clipboard();
if (!source) {
LYXERR(Debug::ACTION, "' (no QMimeData)");
return string();
}
- if (source->hasFormat(lyxMimeType())) {
+ if (cache_.hasFormat(lyxMimeType())) {
// data from ourself or some other LyX instance
QByteArray const ar = source->data(lyxMimeType());
string const s(ar.data(), ar.count());
@@ -248,8 +312,7 @@
}
// get mime data
- QMimeData const * source =
- qApp->clipboard()->mimeData(QClipboard::Clipboard);
+ QMimeData const * source = read_clipboard();
if (!source) {
LYXERR(Debug::ACTION, "0 bytes (no QMimeData)");
return FileName();
@@ -266,7 +329,7 @@
}
// get data
- if (!source->hasFormat(mime))
+ if (!cache_.hasFormat(mime))
return FileName();
// data from ourself or some other LyX instance
QByteArray const ar = source->data(mime);
@@ -336,17 +399,13 @@
bool GuiClipboard::hasLyXContents() const
{
- QMimeData const * const source =
- qApp->clipboard()->mimeData(QClipboard::Clipboard);
- return source && source->hasFormat(lyxMimeType());
+ return cache_.hasFormat(lyxMimeType());
}
bool GuiClipboard::hasTextContents() const
{
- QMimeData const * const source =
- qApp->clipboard()->mimeData(QClipboard::Clipboard);
- return source && source->hasText();
+ return cache_.hasText();
}
@@ -361,12 +420,9 @@
|| hasGraphicsContents(LinkBackGraphicsType);
}
- QMimeData const * const source =
- qApp->clipboard()->mimeData(QClipboard::Clipboard);
-
// handle image cases first
if (type == PngGraphicsType || type == JpegGraphicsType)
- return source->hasImage();
+ return cache_.hasImage();
// handle LinkBack for Mac
if (type == LinkBackGraphicsType)
@@ -377,7 +433,7 @@
#endif // Q_WS_MACX
// get mime data
- QStringList const & formats = source->formats();
+ QStringList const & formats = cache_.formats();
LYXERR(Debug::ACTION, "We found " << formats.size() << " formats");
for (int i = 0; i < formats.size(); ++i)
LYXERR(Debug::ACTION, "Found format " << formats[i]);
@@ -391,7 +447,7 @@
default: LASSERT(false, /**/);
}
- return source && source->hasFormat(mime);
+ return cache_.hasFormat(mime);
}
@@ -420,9 +476,14 @@
void GuiClipboard::on_dataChanged()
{
- QMimeData const * const source =
- qApp->clipboard()->mimeData(QClipboard::Clipboard);
- QStringList l = source->formats();
+ //Note: we do not really need to run cache_.update() unless the
+ //data has been changed *and* the GuiClipboard has been queried.
+ //However if run cache_.update() the moment a process grabs the
+ //clipboard, the process presumably won't yet be frozen, and so
+ //we will not need to wait 5 seconds for Qt to time-out waiting
+ //for the clipboard and won't cache the invalid empty QMimeData.
+ cache_.update();
+ QStringList l = cache_.formats();
LYXERR(Debug::ACTION, "Qt Clipboard changed. We found the following mime types:");
for (int i = 0; i < l.count(); i++)
LYXERR(Debug::ACTION, l.value(i));
Index: frontends/qt4/GuiClipboard.h
===================================================================
--- frontends/qt4/GuiClipboard.h (revision 33816)
+++ frontends/qt4/GuiClipboard.h (working copy)
@@ -17,12 +17,34 @@
#include "frontends/Clipboard.h"
#include <QObject>
+#include <QMimeData>
+#include <QStringList>
namespace lyx {
namespace frontend {
class QMacPasteboardMimeGraphics;
+class CacheMimeData : public QMimeData
+{
+
+public:
+ CacheMimeData();
+ void update();
+ virtual QStringList formats() const;
+
+protected:
+ // A stub, do not use.
+ virtual QVariant retrieveData(const QString &format,
+ QVariant::Type preferredType) const;
+
+private:
+ QStringList cached_formats_;
+ //Not needed as retrieveData is just a stub:
+ //const QMimeData * actualMimeData;
+
+};
+
/**
* The Qt4 version of the Clipboard.
*/
@@ -54,9 +76,22 @@
void on_dataChanged();
private:
+ FileName getPastedGraphicsFileName(QMimeData const * const cache,
+ Cursor const & cur, Clipboard::GraphicsType & type) const;
+ bool hasLyXContents(QMimeData const * const source) const;
+ bool hasGraphicsContents(QMimeData const * const source,
+ GraphicsType type = AnyGraphicsType) const;
+ bool hasTextContents(QMimeData const * const source) const;
+
bool text_clipboard_empty_;
+
+ // We could probably do away with the next two bools now that we have
+ // the CacheMimeData type. However it is still presumably slightly
+ // faster to read a bool than walk a QStringList, so its not urgent.
bool has_lyx_contents_;
bool has_graphics_contents_;
+ CacheMimeData cache_;
+
};
QString const lyxMimeType();
