[webkit-changes] [WebKit/WebKit] 878af5: [GLib] Need API for asynchronously handling WebKit...

[Michael Catanzaro]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 878af52cf35811d107d6bb6f38531ce3b73f211d
      https://github.com/WebKit/WebKit/commit/878af52cf35811d107d6bb6f38531ce3b73f211d
  Author: Michael Catanzaro <mcatanzaro at redhat.com>
  Date:   2023-03-02 (Thu, 02 Mar 2023)

  Changed paths:
    M Source/WebKit/UIProcess/API/glib/WebKitDownload.cpp
    M Source/WebKit/UIProcess/API/glib/WebKitDownloadClient.cpp
    M Source/WebKit/UIProcess/API/glib/WebKitDownloadPrivate.h
    M Tools/TestWebKitAPI/Tests/WebKitGLib/TestDownloads.cpp

  Log Message:
  -----------
  [GLib] Need API for asynchronously handling WebKitDownload::decide-destination
https://bugs.webkit.org/show_bug.cgi?id=238748

Reviewed by Carlos Garcia Campos.

Currently WebKitDownload::decide-destination is emitted when WebKit
wants to know the filesystem location to download the file to. The
application is expected to display a file chooser using
gtk_dialog_run(), then return the result to WebKit using
webkit_download_set_destination(). In GTK 4, gtk_dialog_run() no longer
exists. The only way to accomplish this is to hande it manually,
iterating the default GMainContext inside the signal handler callback.
This is called a "nested main loop" and it's very dangerous because it
frequently results in code being executed at unexpected times. For
example, the attached bug report shows an unintended but also
unavoidable example of a triple nested main loop that occurs where
a second download starts inside the first decide-destination signal
handler, then another, then another. Not good. (Note this is not a new
problem in GTK 4. gtk_dialog_run() no longer exists because it
internally ran a nested main loop, which had this exact same problem.)

To fix this, we need to allow applications to handle the
decide-destination signal asynchronously, so they can finish the signal
handler without immediately providing a destination, provide it later on
in a future main context iteration, and know that the download will not
actually begin until it has been provided. This is easy to do because
the download implementation is already internally asynchronous. A basic
test is added to make sure it even works.

This commit also contains a few drive-by changes in TestDownloads.cpp,
notably where two of the decide-destination callbacks were missing
return values and therefore triggering undefined behavior. It also fixes
a typo that got copy/pasted around and removes some references to URIs
that are now paths
* Source/WebKit/UIProcess/API/glib/WebKitDownload.cpp:
(_WebKitDownloadPrivate::~_WebKitDownloadPrivate):
(maybeFinishDecideDestination):
(webkitDownloadDecideDestination):
(webkit_download_class_init):
(webkitDownloadDecideDestinationWithSuggestedFilename):
(webkit_download_set_destination):
(webkit_download_cancel):
* Source/WebKit/UIProcess/API/glib/WebKitDownloadClient.cpp:
* Source/WebKit/UIProcess/API/glib/WebKitDownloadPrivate.h:
* Tools/TestWebKitAPI/Tests/WebKitGLib/TestDownloads.cpp:
(downloadLocalFileSuccessfully):
(testDownloadOverwriteDestinationDisallowed):
(testDownloadLocalFileError):
(testDownloadRemoteFile):
(testDownloadRemoteFileError):
(testDownloadAsyncDecideDestination):
(testDownloadAsyncDecideDestinationCancel):
(testDownloadMIMEType):
(testDownloadTextPlainMIMEType):
(testDownloadUserAgent):
(testDownloadEphemeralContext):
(testDownloadDestinationURI):
(beforeAll):

Canonical link: https://commits.webkit.org/261118@main