Changeset View
Standalone View
src/main.cpp
Show All 39 Lines | |||||
40 | #include <QDBusAbstractInterface> | 40 | #include <QDBusAbstractInterface> | ||
41 | #include <QDBusConnectionInterface> | 41 | #include <QDBusConnectionInterface> | ||
42 | 42 | | |||
43 | #ifndef Q_OS_WIN | 43 | #ifndef Q_OS_WIN | ||
44 | #include <unistd.h> | 44 | #include <unistd.h> | ||
45 | #endif | 45 | #endif | ||
46 | #include <iostream> | 46 | #include <iostream> | ||
47 | 47 | | |||
48 | #ifdef Q_OS_MACOS | ||||
49 | class OpenFileEventHandler : public QObject | ||||
50 | { | ||||
51 | Q_OBJECT | ||||
52 | public: | ||||
53 | OpenFileEventHandler(QApplication *parent) | ||||
54 | : QObject(parent) | ||||
55 | { | ||||
56 | parent->installEventFilter(this); | ||||
57 | } | ||||
58 | | ||||
59 | bool eventFilter(QObject *obj, QEvent *event) override | ||||
60 | { | ||||
61 | if (event->type() == QEvent::FileOpen) { | ||||
62 | QFileOpenEvent *openEvent = static_cast<QFileOpenEvent*>(event); | ||||
63 | qCDebug(DolphinDebug) << "File open event:" << openEvent->url(); | ||||
broulik: This doesn't need to be a warning | |||||
64 | m_urls.append(Dolphin::validateUris(QStringList(openEvent->file()))); | ||||
65 | if (m_mainWindow && !m_urls.isEmpty()) { | ||||
66 | if (GeneralSettings::openExternallyCalledFolderInNewTab()) { | ||||
67 | const auto serviceName = QStringLiteral("org.kde.dolphin-%1").arg(QCoreApplication::applicationPid()); | ||||
68 | if (!Dolphin::attachToExistingInstance(m_urls, false, false, serviceName)) { | ||||
69 | qCWarning(DolphinDebug) << "Failed to open" << m_urls; | ||||
70 | } | ||||
71 | } else { | ||||
72 | Dolphin::openNewWindow(m_urls, m_mainWindow); | ||||
73 | } | ||||
74 | m_urls.clear(); | ||||
75 | } | ||||
76 | return true; | ||||
77 | } | ||||
78 | return QObject::eventFilter(obj, event); | ||||
79 | } | ||||
80 | | ||||
81 | QList<QUrl> popUrls() | ||||
82 | { | ||||
83 | const auto ret = m_urls; | ||||
84 | m_urls.clear(); | ||||
85 | return ret; | ||||
86 | } | ||||
87 | | ||||
88 | | ||||
meven: Remove | |||||
rjvbb: ??? | |||||
meven: The are two empty lines, one should suffice | |||||
89 | void setMainWindow(DolphinMainWindow *w) | ||||
broulik: Make those `private` and add proper getters | |||||
rjvbb: No need for getters beyond the already existing `popUrls()`, in fact. | |||||
90 | { | ||||
91 | m_mainWindow = w; | ||||
92 | } | ||||
93 | | ||||
94 | private: | ||||
95 | DolphinMainWindow* m_mainWindow = nullptr; | ||||
96 | QList<QUrl> m_urls; | ||||
97 | }; | ||||
98 | | ||||
99 | #endif | ||||
100 | | ||||
48 | extern "C" Q_DECL_EXPORT int kdemain(int argc, char **argv) | 101 | extern "C" Q_DECL_EXPORT int kdemain(int argc, char **argv) | ||
49 | { | 102 | { | ||
50 | #ifndef Q_OS_WIN | 103 | #ifndef Q_OS_WIN | ||
51 | // Prohibit using sudo or kdesu (but allow using the root user directly) | 104 | // Prohibit using sudo or kdesu (but allow using the root user directly) | ||
52 | if (getuid() == 0) { | 105 | if (getuid() == 0) { | ||
53 | if (!qEnvironmentVariableIsEmpty("SUDO_USER")) { | 106 | if (!qEnvironmentVariableIsEmpty("SUDO_USER")) { | ||
54 | std::cout << "Executing Dolphin with sudo is not possible due to unfixable security vulnerabilities." << std::endl; | 107 | std::cout << "Executing Dolphin with sudo is not possible due to unfixable security vulnerabilities." << std::endl; | ||
55 | return EXIT_FAILURE; | 108 | return EXIT_FAILURE; | ||
Show All 9 Lines | 114 | #endif | |||
65 | */ | 118 | */ | ||
66 | QCoreApplication::setAttribute(Qt::AA_UseHighDpiPixmaps, true); | 119 | QCoreApplication::setAttribute(Qt::AA_UseHighDpiPixmaps, true); | ||
67 | QCoreApplication::setAttribute(Qt::AA_EnableHighDpiScaling, true); | 120 | QCoreApplication::setAttribute(Qt::AA_EnableHighDpiScaling, true); | ||
68 | 121 | | |||
69 | QApplication app(argc, argv); | 122 | QApplication app(argc, argv); | ||
70 | app.setWindowIcon(QIcon::fromTheme(QStringLiteral("system-file-manager"), app.windowIcon())); | 123 | app.setWindowIcon(QIcon::fromTheme(QStringLiteral("system-file-manager"), app.windowIcon())); | ||
71 | 124 | | |||
72 | KCrash::initialize(); | 125 | KCrash::initialize(); | ||
126 | #ifdef Q_OS_MACOS | ||||
127 | // start processing Mac OS FileOpenEvents | ||||
128 | const auto openEventHandler = new OpenFileEventHandler(&app); | ||||
129 | #endif | ||||
73 | 130 | | |||
74 | Kdelibs4ConfigMigrator migrate(QStringLiteral("dolphin")); | 131 | Kdelibs4ConfigMigrator migrate(QStringLiteral("dolphin")); | ||
75 | migrate.setConfigFiles(QStringList() << QStringLiteral("dolphinrc")); | 132 | migrate.setConfigFiles(QStringList() << QStringLiteral("dolphinrc")); | ||
76 | migrate.setUiFiles(QStringList() << QStringLiteral("dolphinpart.rc") << QStringLiteral("dolphinui.rc")); | 133 | migrate.setUiFiles(QStringList() << QStringLiteral("dolphinpart.rc") << QStringLiteral("dolphinui.rc")); | ||
77 | migrate.migrate(); | 134 | migrate.migrate(); | ||
78 | 135 | | |||
79 | KLocalizedString::setApplicationDomain("dolphin"); | 136 | KLocalizedString::setApplicationDomain("dolphin"); | ||
80 | 137 | | |||
▲ Show 20 Lines • Show All 54 Lines • ▼ Show 20 Line(s) | |||||
135 | 192 | | |||
136 | parser.process(app); | 193 | parser.process(app); | ||
137 | aboutData.processCommandLine(&parser); | 194 | aboutData.processCommandLine(&parser); | ||
138 | 195 | | |||
139 | const bool splitView = parser.isSet(QStringLiteral("split")) || GeneralSettings::splitView(); | 196 | const bool splitView = parser.isSet(QStringLiteral("split")) || GeneralSettings::splitView(); | ||
140 | const bool openFiles = parser.isSet(QStringLiteral("select")); | 197 | const bool openFiles = parser.isSet(QStringLiteral("select")); | ||
141 | const QStringList args = parser.positionalArguments(); | 198 | const QStringList args = parser.positionalArguments(); | ||
142 | QList<QUrl> urls = Dolphin::validateUris(args); | 199 | QList<QUrl> urls = Dolphin::validateUris(args); | ||
143 | // We later mutate urls, so we need to store if it was empty originally | | |||
144 | const bool startedWithURLs = !urls.isEmpty(); | | |||
145 | | ||||
146 | 200 | | |||
147 | if (parser.isSet(QStringLiteral("daemon"))) { | 201 | if (parser.isSet(QStringLiteral("daemon"))) { | ||
148 | KDBusService dolphinDBusService; | 202 | KDBusService dolphinDBusService; | ||
149 | DBusInterface interface; | 203 | DBusInterface interface; | ||
150 | interface.setAsDaemon(); | 204 | interface.setAsDaemon(); | ||
205 | #ifdef Q_OS_MACOS | ||||
206 | // we probably shouldn't be accepting requests via Apple's LaunchServices | ||||
207 | app.removeEventFilter(openEventHandler); | ||||
208 | #endif | ||||
151 | return app.exec(); | 209 | return app.exec(); | ||
152 | } | 210 | } | ||
153 | 211 | | |||
212 | #ifdef Q_OS_MACOS | ||||
213 | // Get the file open events that have been queued; a priori those are only | ||||
214 | // for the documents or folders with which the user launched us. This has to | ||||
215 | // be done in "jit" fashion so we don't drop any events. Any such events \ | ||||
216 | // coming in from now until we call setMainWindow will get lost. | ||||
217 | // Since D11382 we also need to get the startup urls before setting startedWithURLs. | ||||
218 | QCoreApplication::sendPostedEvents(&app, QEvent::FileOpen); | ||||
219 | QCoreApplication::processEvents(); | ||||
rjvbb: Note to self: fix this, there's need for a getter after all. | |||||
220 | urls.append(openEventHandler->popUrls()); | ||||
221 | #endif | ||||
222 | // We later mutate urls, so we need to store if it was empty originally | ||||
223 | const bool startedWithURLs = !urls.isEmpty(); | ||||
224 | | ||||
154 | if (!parser.isSet(QStringLiteral("new-window"))) { | 225 | if (!parser.isSet(QStringLiteral("new-window"))) { | ||
155 | if (Dolphin::attachToExistingInstance(urls, openFiles, splitView)) { | 226 | if (Dolphin::attachToExistingInstance(urls, openFiles, splitView)) { | ||
156 | // Successfully attached to existing instance of Dolphin | 227 | // Successfully attached to existing instance of Dolphin | ||
157 | return 0; | 228 | return 0; | ||
158 | } | 229 | } | ||
159 | } | 230 | } | ||
160 | 231 | | |||
232 | // Open the main window now, so we can activate normal file open event | ||||
233 | // handling (on Mac) as soon as possible. | ||||
234 | DolphinMainWindow* mainWindow = new DolphinMainWindow(); | ||||
elvisangelaccio: I don't get it, why do we need to move this here? | |||||
To enable normal runtime processing of QFileOpen events as soon as possible after we've handled the first events. LaunchServices, the Mac's service that handles launching applications and handing off documents to be opened doesn't use argc,argv but sends ObjC messages (one per document) that Qt forwards as QFileOpenEvents. Opening the main window a bit earlier seems like the best way to prevent this and I don't see anything suggesting I can't move the operaton as I did. rjvbb: To enable normal runtime processing of QFileOpen events as soon as possible after we've handled… | |||||
235 | #ifdef Q_OS_MACOS | ||||
This goes against this diff: https://phabricator.kde.org/D23672 mainWindow needs to be created before these two classes are instantiated (well, in particular KDBusService but cleaner to do both at the same time). See the note in the class description: https://api.kde.org/frameworks/kdbusaddons/html/classKDBusService.html feverfew: This goes against this diff: https://phabricator.kde.org/D23672
`mainWindow` needs to be… | |||||
Not that I am against this change because it means the main window is created even quicker, but just how many tabs do you need to open (and how, exactly) to observe the hanging? FWIW, having the "open folders in tabs" settings under startup and not under general/behaviour is confusing and illogical to me... rjvbb: Not that I am against this change because it means the main window is created even quicker, but… | |||||
Please see the test plan in the diff that I mentioned. Should be readily reproducible in that scenario. Even if it isn't,my point still stands, the race exists. Feel free to open another patch regarding moving the location of the setting and add Dolphin and VDG as reviewers I guess. feverfew: Please see the test plan in the diff that I mentioned. Should be readily reproducible in that… | |||||
236 | openEventHandler->setMainWindow(mainWindow); | ||||
237 | #endif | ||||
238 | | ||||
161 | if (!startedWithURLs) { | 239 | if (!startedWithURLs) { | ||
162 | // We need at least one URL to open Dolphin | 240 | // We need at least one URL to open Dolphin | ||
163 | urls.append(Dolphin::homeUrl()); | 241 | urls.append(Dolphin::homeUrl()); | ||
164 | } | 242 | } | ||
165 | 243 | | |||
166 | if (splitView && urls.size() < 2) { | 244 | if (splitView && urls.size() < 2) { | ||
167 | // Split view does only make sense if we have at least 2 URLs | 245 | // Split view does only make sense if we have at least 2 URLs | ||
168 | urls.append(urls.last()); | 246 | urls.append(urls.last()); | ||
169 | } | 247 | } | ||
170 | 248 | | |||
171 | DolphinMainWindow* mainWindow = new DolphinMainWindow(); | | |||
172 | | ||||
173 | if (openFiles) { | 249 | if (openFiles) { | ||
174 | mainWindow->openFiles(urls, splitView); | 250 | mainWindow->openFiles(urls, splitView); | ||
175 | } else { | 251 | } else { | ||
176 | mainWindow->openDirectories(urls, splitView); | 252 | mainWindow->openDirectories(urls, splitView); | ||
177 | } | 253 | } | ||
178 | 254 | | |||
179 | mainWindow->show(); | 255 | mainWindow->show(); | ||
180 | 256 | | |||
Show All 19 Lines | 266 | if (!startedWithURLs && (app.isSessionRestored() || GeneralSettings::rememberOpenedTabs()) ) { | |||
200 | } | 276 | } | ||
201 | } | 277 | } | ||
202 | 278 | | |||
203 | KDBusService dolphinDBusService; | 279 | KDBusService dolphinDBusService; | ||
204 | DBusInterface interface; | 280 | DBusInterface interface; | ||
205 | 281 | | |||
206 | return app.exec(); // krazy:exclude=crash; | 282 | return app.exec(); // krazy:exclude=crash; | ||
207 | } | 283 | } | ||
284 | | ||||
285 | #include "main.moc" |
This doesn't need to be a warning