chromiumembedded / cef

Chromium Embedded Framework (CEF). A simple framework for embedding Chromium-based browsers in other applications.
https://bitbucket.org/chromiumembedded/cef/
Other
3.25k stars 455 forks source link

Use Chrome file dialogs on all platforms and runtimes #3314

Closed magreenblatt closed 2 years ago

magreenblatt commented 2 years ago

Original report by me.


The current file dialog implementation in CEF has problems that can be summarized as follows:

All of the above problems can (hopefully) be resolved by sharing the Chrome file dialog implementation on all platforms and runtimes (e.g. routing all file dialog requests through CefDialogManager). This will allow us to:

magreenblatt commented 2 years ago

Issue #3295 was marked as a duplicate of this issue.

magreenblatt commented 2 years ago

Issue #3256 was marked as a duplicate of this issue.

magreenblatt commented 2 years ago

This will need to be manually tested on all platforms and modes (details).

Unit tests should pass:

Manual tests:

Manual tests expected behavior:

Manual test for: “Start Logging to Disk” from chrome://net-export/ (see issue #2613):

  1. Run the above cefclient commands with --url=chrome://net-export/
  2. Click the “Start Logging to Disk” button
  3. Get a “Save As” file dialog with “JSON File” and “All Files” type filters.
  4. Clicking the “Save” button on the file dialog; starts logging.

Manual test for: Print preview “Save as PDF” (see issue #2867):

  1. Run the above cefclient commands with --enable-print-preview --url=https://www.nap.edu/resource/12161/origin_and_evolution_of_earth_final.pdf
  2. Click the print button in the PDF viewer
  3. Select “Save as PDF” destination in the print preview dialog
  4. Click the “Save” button
  5. Get a “Save As” dialog with “Adobe Acrobat Document” and “All Files” type filters.
  6. Click the “Save” button on the file dialog; the PDF file is written to disk and the print preview dialog is dismissed.

Manual test for: PDF viewer download original and “With your changes” (see issue #3169)

  1. Run the above cefclient commands with --url=https://www.hetcak.nl/HETCAK/media/HetCAK/formulieren/klant/wmo/SEPA-formulier-automatisch-betalen-Wmo.pdf

  2. Enter some text in the fields

  3. Select the download button > “With your changes” option

    1. Get a “Save As” dialog with “*.pdf” and “All Files” type filters.
    2. Click the “Save” button on the file dialog; the PDF file is written to disk.
  4. Select the download button > “Without your changes” option

    1. Get a “Save As” dialog with “Adobe Acrobat Document” and “All Files” type filters.
    2. Click the “Save” button on the file dialog; the PDF file is written to disk.

Manual test for: Multiple simultaneous downloads (see issue #3154)

Save the following code snippet to a test.html file on disk:

<html>
<body>
<script>
for (let i = 0; i < 3; i++) {
    let blobFile = new Blob(["file"+i])
    var link = document.createElement('a');
    link.href = URL.createObjectURL(blobFile)
    link.download = 'Download.txt';
    document.body.appendChild(link);
    link.click();
    document.body.removeChild(link);
}
</script>
</body>
</html>
  1. Run the above cefclient commands with --url=file:///C:/temp/test.html

    1. With Chrome runtime the first file will download, and the following files will show a user confirmation prompt.
    2. With Alloy runtime the first file will show a single “Save As” dialog with “Text Document” and “All FIles” filters. Click the “Save” button on the file dialog and the file will be written to disk.
    3. The additional file downloads will be canceled with this log message:

      ERROR:file_dialog_manager.cc(404)] Multiple simultaneous dialogs are not supported; canceling the file dialog

magreenblatt commented 2 years ago

Some existing CEF API configuration options will be removed because they are not supported by blink::mojom::FileChooserParams:

magreenblatt commented 2 years ago

CEF currently build with GTK3 enabled on Linux and we have access to the default GTK dialog implementations in Chrome. However, some work will be required in CefPrintDialogLinux to support both the default GTK dialog implementation and the delegation to CefPrintHandler.

magreenblatt commented 2 years ago

Multiple simultaneous file dialogs will not be supported with the default platform implementation, but CefDialogHandler will still be called. See issue #3154 for related test and commentary.

magreenblatt commented 2 years ago

On Linux there are some preexisting issues with ceftests --gtest_filter=DialogTest.* --use-views --multi-threaded-message-loop. Looks like the ResourceBundle is being accessed unexpectedly during startup on the main thread:

#0  0x00007fffd7b9623d in logging::LogMessage::~LogMessage() ()
    at /home/marshall/code/chromium_git/chromium/src/out/Release_GN_x64/libcef.so
#1  0x00007fffd7b96f0e in logging::LogMessage::~LogMessage() ()
    at /home/marshall/code/chromium_git/chromium/src/out/Release_GN_x64/libcef.so
#2  0x00007fffdcfee910 in ui::ResourceBundle::GetFontListForDetails(ui::ResourceBundle::FontDetails const&) ()
    at /home/marshall/code/chromium_git/chromium/src/out/Release_GN_x64/libcef.so
#3  0x00007fffdcff0eb0 in ui::ResourceBundle::GetFontList(ui::ResourceBundle::FontStyle) ()
    at /home/marshall/code/chromium_git/chromium/src/out/Release_GN_x64/libcef.so
#4  0x00007fffdcff1094 in ui::ResourceBundle::GetFont(ui::ResourceBundle::FontStyle) ()
    at /home/marshall/code/chromium_git/chromium/src/out/Release_GN_x64/libcef.so
#5  0x00007fffdd008246 in webui::GetFontFamily() ()
    at /home/marshall/code/chromium_git/chromium/src/out/Release_GN_x64/libcef.so
#6  0x00007fffdd007e5b in webui::SetLoadTimeDataDefaults(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, base::Value*) () at /home/marshall/code/chromium_git/chromium/src/out/Release_GN_x64/libcef.so
#7  0x00007fffe1f8ec46 in pdf_extension_util::AddStrings(pdf_extension_util::PdfViewerContext, base::Value*) ()
    at /home/marshall/code/chromium_git/chromium/src/out/Release_GN_x64/libcef.so
#8  0x00007fffd764c612 in extensions::CefComponentExtensionResourceManager::CefComponentExtensionResourceManager() ()
    at /home/marshall/code/chromium_git/chromium/src/out/Release_GN_x64/libcef.so
#9  0x00007fffd764f623 in extensions::CefExtensionsBrowserClient::CefExtensionsBrowserClient() ()
    at /home/marshall/code/chromium_git/chromium/src/out/Release_GN_x64/libcef.so
#10 0x00007fffd75448d8 in ChromeBrowserProcessAlloy::Initialize() ()
    at /home/marshall/code/chromium_git/chromium/src/out/Release_GN_x64/libcef.so
#11 0x00007fffd76ae5a7 in CefMainRunner::ContentMainRun(bool*, base::OnceCallback<void ()>) ()
    at /home/marshall/code/chromium_git/chromium/src/out/Release_GN_x64/libcef.so
#12 0x00007fffd76add55 in CefMainRunner::Initialize(CefStructBase<CefSettingsTraits>*, scoped_refptr<CefApp>, CefMainArgs const&, void*, bool*, base::OnceCallback<void ()>) ()
    at /home/marshall/code/chromium_git/chromium/src/out/Release_GN_x64/libcef.so
#13 0x00007fffd75f11b4 in CefContext::Initialize(CefMainArgs const&, CefStructBase<CefSettingsTraits> const&, scoped_refptr<CefApp>, void*) () at /home/marshall/code/chromium_git/chromium/src/out/Release_GN_x64/libcef.so
#14 0x00007fffd75f0808 in CefInitialize(CefMainArgs const&, CefStructBase<CefSettingsTraits> const&, scoped_refptr<CefApp>, void*) () at /home/marshall/code/chromium_git/chromium/src/out/Release_GN_x64/libcef.so
#15 0x00007fffc5a6be71 in cef_initialize () at /home/marshall/code/chromium_git/chromium/src/out/Release_GN_x64/libcef.so
#16 0x0000555557afc47e in CefInitialize(CefMainArgs const&, CefStructBase<CefSettingsTraits> const&, scoped_refptr<CefApp>, void*) ()
#17 0x0000555556e6d9f0 in main ()

magreenblatt commented 2 years ago

Use Chrome file dialogs on all platforms and runtimes (fixes issue #3314)

All file dialogs irrespective of source, platform and runtime will now be routed through CefFileDialogManager and trigger CefDialogHandler callbacks (see issue #3293).

Adds Chrome runtime support for CefBrowserHost::RunFileDialog and CefDialogHandler callbacks.

Adds Alloy runtime support for internal GTK file and print dialogs on Linux subject to the following limitations:

  1. Internal GTK implementation:
    • Cannot be used with multi-threaded-message-loop because Chromium's internal GTK implementation is not thread-safe (does not use GDK threads).
    • Dialogs will not be modal to application windows when used with off-screen rendering due to lack of access to the client's top-level GtkWindow.
  2. Cefclient CefDialogHandler implementation:
    • Cannot be used with Views because it requires a top-level GtkWindow.

Due to the above limitations no dialog implementation is currently provided for Views + multi-threaded-message-loop on Linux. In cases where both implementations are supported the cefclient version is now behind an optional --use-client-dialogs command-line flag.

Expressly forbids multiple simultaneous file dialogs with the internal platform implementation which uses modal dialogs. CefDialogHandler will still be notified and can optionally handle each request without a modal dialog (see issue #3154).

Removes some RunFileDialog parameters that are not supported by the Chrome file dialog implementation (selected_accept_filter parameter, cef_file_dialog_mode_t overwrite/read-only flags).

→ <<cset 2ea7459a89fb (bb)>>

magreenblatt commented 2 years ago

alloy: Delay creation of CefComponentExtensionResourceManager (see issue #3314)

This was causing early access to ResourceBundle on the main thread (via webui::GetFontFamily) which resulted in crashes on Linux when running with multi-threaded-message-loop.

→ <<cset c1b06ccee874 (bb)>>

magreenblatt commented 2 years ago
magreenblatt commented 2 years ago