QT_TO_UTF8 returns a const char * that, in general, shouldn't be stored.
This is because QT_TO_UTF8(str) expands to str.toUtf8().constData():
toUtf8() returns a QByteArray, and constData() the pointer to its data
which is only valid until the QByteArray goes out of scope, which is
immediately after the call.
The original code that is changed here only works because in all of the
situations, the object that is stored to is actually a std::string that
gets constructed implicitly, so the constData() pointer is valid long
enough for the std::string constructor to copy the data.
The issue is that any "... = QT_TO_UTF8" code *looks* unsafe, and may
lead new or unfamiliar contributors to assume that they can also use it,
only to do "const char *... = QT_TO_UTF8(...)" which is dangerous.
Additionally, it introduces an unnecessary round of implicit conversions
and copies when QString.toStdString() already exists and copies into the
string buffer directly.
The strings (broadcast.id, stream.[id|name]) are stored as QString,
converted to const char * by QT_TO_UTF8 in OBSYoutubeActions, implicitly
converted back to QString because the OBSYoutubeActions::ok takes
const QString &, only to be converted back to const char * by QT_TO_UTF8
in OBSBasic_YouTube and immediately implicitly turned into
const std::strings, only to have .c_str() called on those to get their
const char * again which is needed for libobs. This is insane.
Let's just pass const std::string & and be happy.
RemoteTextThread and WhatsNewInfoThread explicitly convert their results
into QString, but many consumers need std::string, converting them
back. Let's just use std::string directly and only convert to QString
where actually needed.
While it is canonical to use the backspace key as an alternative to the
dedicated "delete" key (which is omitted on many smaller-sized Apple
keyboards), the delete key is still available on full-size Apple
keyboards and obviously third-party keyboards.
This change adds the delete key as an alternative to the backspace key
to allow removal of scene items from the scene list in the UI.
This issue was brought back by a somewhat recent change to fix slow
shutdown times in Linux.
To paraphrase tfo from the OBS Discord:
"I think the issue is that all the tray actions are parented to
trayIcon, so when it's deleted, Qt auto deletes them as children, then
delete trayMenu on the next line accesses those dead actions."
We now create the tray menu first, then setting the parent of the
actions (show / hide, stop / start streaming, etc.) to the tray menu.
With recent changes to the application shutdown logic, events had to
follow a very strict order as certain elements of shutdown code
depend on other elements not being deallocated prematurely.
This turned the (correct) order of events on macOS upside down and
lead to crashes either when the app was quit from within or when
terminated by the OS.
The fix incorporates multiple elements:
* Removal of the custom "Quit" menu item on macOS to use the default
implementation of Qt's platform plugin.
* Soft-revert (via preprocessor conditionals) parts of the updated
shutdown logic to prevent emitting recursive shutdown events.
* Handle main window close event by simply emitting a "quit" event
on the application instance.
* Update POSIX signal handlers to also simply emit a "quit" event.
In combination these changes reduce the number of different code paths
taken during shutdown:
* Closing the app via the menu item, menu item shortcut, or initiated
by AppKit (OS shutdown/reboot, or quit via Dock/Finder) will emit an
AppKit "terminate" event for orderly shutdown.
* Closing the main window or sending an appropriate POSIX signal
triggers the "terminate" event indirectly by emitting the "quit"
event on the application instance.
Either way a "close" event to the main window happens before the
event loop is terminated and the application instance is torn down
(either directly, or indirectly via Qt's "closeAllWindows" function in
response to "terminate"). The order of events thus is always:
0. Terminate event by AppKit (except when closing the main window)
1. Closing of main window
2. Termination of browser docks
3. Deallocation of main window
4. Termination of application
5. Deallocation of application
NOTE: All this only applies to macOS. The shutdown order and procedures
on Windows and Linux are unchanged.
When the main window is closed and with it the application state is torn
down, browser panels need to be explicitly removed before the the CEF
instance used by the application is shut down itself.
For service-based docks this happens as part of the "reset" of the
"auth" pointer (and thus its destructor), for user-created browser
panels this is achieved by the call to "ClearExtraBrowserDocks".
Because the Youtube app dock is a special browser panel that is created
conditionally, but potentially exists globally, it also has to be
closed this way (if it was created).
Otherwise CEF will force-close the underlying browser host instance as
part of its own shutdown and also deallocate the native window used
by the browser. When the QCefWidget then attempts to detach the
native window from the view hierarchy (to avoid this operation from
potentially closing the root window it is anchored to), it will either
attempt to access a CrFatZombie object (and crash) or access deallocated
memory (and also crash).
Implicitly capturing "this" with the capture default "=" is deprecated
with C++20. We fix this by either explicitly passing this, or by copying
the required members manually.
While this exposes some rather expensive copies like the QList
selectedItems in OBSBasic_Preview, it doesn't introduce them ("=" copies
implicitly).
