From b836a597c63520374a17771ce6a4628c7968e8cece1e42b02f385c7baeed1a90 Mon Sep 17 00:00:00 2001 From: Luca Beltrame Date: Sat, 22 Feb 2020 14:07:32 +0000 Subject: [PATCH] Accepting request 777988 from home:Vogtinator:qt5.14 - Fix a deadlock causing audio/video playback to fail (boo#1163744): * QTBUG-82186.patch - Fix an issue with selections breaking replying in KMail: * QTBUG-81574.patch OBS-URL: https://build.opensuse.org/request/show/777988 OBS-URL: https://build.opensuse.org/package/show/KDE:Qt:5.14/libqt5-qtwebengine?expand=0&rev=12 --- QTBUG-81574.patch | 197 +++++++++++++++++++++++++++++++++++++ QTBUG-82186.patch | 48 +++++++++ libqt5-qtwebengine.changes | 12 +++ libqt5-qtwebengine.spec | 4 + 4 files changed, 261 insertions(+) create mode 100644 QTBUG-81574.patch create mode 100644 QTBUG-82186.patch diff --git a/QTBUG-81574.patch b/QTBUG-81574.patch new file mode 100644 index 0000000..2ca70e3 --- /dev/null +++ b/QTBUG-81574.patch @@ -0,0 +1,197 @@ +From 7d56bbb4c1708f00f729bdfe2e8951c644c83194 Mon Sep 17 00:00:00 2001 +From: Kirill Burtsev +Date: Wed, 12 Feb 2020 16:15:34 +0100 +Subject: [PATCH] Clear previous page text selection on new navigation unconditionally + +Remove code duplication on triggering new url load, and use direct +code to clear SelectedText instead of CollapseSelection as it assumes +focused frame and might be ignored. + +Fixes: QTBUG-81574 +Change-Id: I01cf02967e118f407c8a3997e176d5b258478a5a +--- + +diff --git a/src/core/web_contents_adapter.cpp b/src/core/web_contents_adapter.cpp +index 8cc8179..a7579f9 100644 +--- a/src/core/web_contents_adapter.cpp ++++ b/src/core/web_contents_adapter.cpp +@@ -67,6 +67,7 @@ + #include "base/task/sequence_manager/thread_controller_with_message_pump_impl.h" + #include "base/values.h" + #include "content/browser/renderer_host/render_view_host_impl.h" ++#include "content/browser/renderer_host/text_input_manager.h" + #include "content/browser/web_contents/web_contents_impl.h" + #include "content/public/browser/browser_task_traits.h" + #include "content/public/browser/child_process_security_policy.h" +@@ -369,6 +370,23 @@ + } + } + ++static void Navigate(WebContentsAdapter *adapter, const content::NavigationController::LoadURLParams ¶ms) ++{ ++ Q_ASSERT(adapter); ++ adapter->webContents()->GetController().LoadURLWithParams(params); ++ adapter->focusIfNecessary(); ++ adapter->resetSelection(); ++} ++ ++static void NavigateTask(QWeakPointer weakAdapter, const content::NavigationController::LoadURLParams ¶ms) ++{ ++ DCHECK_CURRENTLY_ON(content::BrowserThread::UI); ++ const auto adapter = weakAdapter.toStrongRef(); ++ if (!adapter) ++ return; ++ Navigate(adapter.get(), params); ++} ++ + namespace { + static QList recursive_guard_loading_adapters; + +@@ -705,21 +723,12 @@ + } + } + +- auto navigate = [](QWeakPointer weakAdapter, const content::NavigationController::LoadURLParams ¶ms) { +- const auto adapter = weakAdapter.toStrongRef(); +- if (!adapter) +- return; +- adapter->webContents()->GetController().LoadURLWithParams(params); +- adapter->focusIfNecessary(); +- }; +- +- QWeakPointer weakThis(sharedFromThis()); + if (resizeNeeded) { + // Schedule navigation on the event loop. + base::PostTaskWithTraits(FROM_HERE, {content::BrowserThread::UI}, +- base::BindOnce(navigate, std::move(weakThis), std::move(params))); ++ base::BindOnce(&NavigateTask, sharedFromThis().toWeakRef(), std::move(params))); + } else { +- navigate(std::move(weakThis), params); ++ Navigate(this, params); + } + } + +@@ -752,9 +761,7 @@ + params.can_load_local_resources = true; + params.transition_type = ui::PageTransitionFromInt(ui::PAGE_TRANSITION_TYPED | ui::PAGE_TRANSITION_FROM_API); + params.override_user_agent = content::NavigationController::UA_OVERRIDE_TRUE; +- m_webContents->GetController().LoadURLWithParams(params); +- focusIfNecessary(); +- m_webContents->CollapseSelection(); ++ Navigate(this, params); + } + + void WebContentsAdapter::save(const QString &filePath, int savePageFormat) +@@ -1676,6 +1683,17 @@ + return m_webContents->GetFocusedFrame() != nullptr; + } + ++void WebContentsAdapter::resetSelection() ++{ ++ CHECK_INITIALIZED(); ++ // unconditionally clears the selection in contrast to CollapseSelection, which checks focus state first ++ if (auto rwhv = static_cast(m_webContents->GetRenderWidgetHostView())) { ++ if (auto mgr = rwhv->GetTextInputManager()) ++ if (auto selection = const_cast(mgr->GetTextSelection(rwhv))) ++ selection->SetSelection(base::string16(), 0, gfx::Range(), false); ++ } ++} ++ + WebContentsAdapterClient::RenderProcessTerminationStatus + WebContentsAdapterClient::renderProcessExitStatus(int terminationStatus) { + auto status = WebContentsAdapterClient::RenderProcessTerminationStatus(-1); +diff --git a/src/core/web_contents_adapter.h b/src/core/web_contents_adapter.h +index 11f8f9c..f8f147f 100644 +--- a/src/core/web_contents_adapter.h ++++ b/src/core/web_contents_adapter.h +@@ -229,6 +229,7 @@ + void focusIfNecessary(); + bool isFindTextInProgress() const; + bool hasFocusedFrame() const; ++ void resetSelection(); + + // meant to be used within WebEngineCore only + void initialize(content::SiteInstance *site); +diff --git a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp +index 8cdcc9f..94b3f16 100644 +--- a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp ++++ b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp +@@ -700,7 +700,7 @@ + CursorTrackedPage(QWidget *parent = 0): QWebEnginePage(parent) { + } + +- QString selectedText() { ++ QString jsSelectedText() { + return evaluateJavaScriptSync(this, "window.getSelection().toString()").toString(); + } + +@@ -716,42 +716,52 @@ + int isSelectionCollapsed() { + return evaluateJavaScriptSync(this, "window.getSelection().getRangeAt(0).collapsed").toBool(); + } +- bool hasSelection() +- { +- return !selectedText().isEmpty(); +- } + }; + + void tst_QWebEnginePage::textSelection() + { +- QWebEngineView view; +- CursorTrackedPage *page = new CursorTrackedPage(&view); +- QString content("

The quick brown fox

" \ ++ CursorTrackedPage page; ++ ++ QString textToSelect("The quick brown fox"); ++ QString content = QString("

%1

" \ + "

jumps over the lazy dog

" \ +- "

May the source
be with you!

"); +- page->setView(&view); +- QSignalSpy loadSpy(&view, SIGNAL(loadFinished(bool))); +- page->setHtml(content); ++ "

May the source
be with you!

").arg(textToSelect); ++ ++ QSignalSpy loadSpy(&page, SIGNAL(loadFinished(bool))); ++ page.setHtml(content); + QTRY_COMPARE_WITH_TIMEOUT(loadSpy.count(), 1, 20000); + + // these actions must exist +- QVERIFY(page->action(QWebEnginePage::SelectAll) != 0); ++ QVERIFY(page.action(QWebEnginePage::SelectAll) != 0); + + // ..but SelectAll is disabled because the page has no focus due to disabled FocusOnNavigationEnabled. +- QCOMPARE(page->action(QWebEnginePage::SelectAll)->isEnabled(), false); ++ QCOMPARE(page.action(QWebEnginePage::SelectAll)->isEnabled(), false); + + // Verify hasSelection returns false since there is no selection yet... +- QCOMPARE(page->hasSelection(), false); ++ QVERIFY(!page.hasSelection()); ++ QVERIFY(page.jsSelectedText().isEmpty()); + + // this will select the first paragraph + QString selectScript = "var range = document.createRange(); " \ + "var node = document.getElementById(\"one\"); " \ + "range.selectNode(node); " \ + "getSelection().addRange(range);"; +- evaluateJavaScriptSync(page, selectScript); +- QCOMPARE(page->selectedText().trimmed(), QString::fromLatin1("The quick brown fox")); ++ evaluateJavaScriptSync(&page, selectScript); ++ + // Make sure hasSelection returns true, since there is selected text now... +- QCOMPARE(page->hasSelection(), true); ++ QTRY_VERIFY(page.hasSelection()); ++ QCOMPARE(page.selectedText().trimmed(), textToSelect); ++ ++ QCOMPARE(page.jsSelectedText().trimmed(), textToSelect); ++ ++ // navigate away and check that selection is cleared ++ page.load(QUrl("about:blank")); ++ QTRY_COMPARE(loadSpy.count(), 2); ++ ++ QVERIFY(!page.hasSelection()); ++ QVERIFY(page.selectedText().isEmpty()); ++ ++ QVERIFY(page.jsSelectedText().isEmpty()); + } + + diff --git a/QTBUG-82186.patch b/QTBUG-82186.patch new file mode 100644 index 0000000..9af1d43 --- /dev/null +++ b/QTBUG-82186.patch @@ -0,0 +1,48 @@ +From c729361f9f8f6c0602d401d5e230ba63ab11a682 Mon Sep 17 00:00:00 2001 +From: Jüri Valdmann +Date: Wed, 19 Feb 2020 14:15:34 +0100 +Subject: [PATCH] Fix recursive deadlock in sandbox::InitLibcLocaltimeFunctions + +QtWebEngineProcess overrides the C library's localtime* functions by redefining +the symbols in src/process/main.cpp and then using dlsym(RTLD_NEXT, ...) to +fetch the original symbols in //sandbox/linux/services/libc_interceptor.cc. The +functions InitLibcLocaltimeFunctions{,Impl} use pthread_once to guarantee that +this symbol resolution happens only once. + +If dlsym fails, for example because the C library is earlier in the search path +than QtWebEngineCore, then InitLibcLocaltimeFunctionsImpl tries to print an +error message with LOG(ERROR). However, printing a log message involves also +printing the timestamp in the local time zone, using, of course, localtime_r. +Thus, InitLibcLocaltimeFunctions depends on localtime_r depends on +InitLibcLocaltimeFunctions, and we get a deadlock due to the recursive use of +pthread_once. + +This deadlock happens only for utility processes and not for zygotes or +renderers, since the latter proxy the localtime* calls back to the main process. +(See service_manager::ZygoteMain, where the first function call is to +sandbox::SetAmZygoteOrRenderer, and compare with content::UtilityMain) + +Task-number: QTBUG-82186 +Change-Id: I32009e8482b2634c47082a4c89393dc61c22507e +--- + +diff --git a/src/3rdparty/chromium/sandbox/linux/services/libc_interceptor.cc b/chromium/sandbox/linux/services/libc_interceptor.cc +index ed4dd02..fad77f9 100644 +--- a/src/3rdparty/chromium/sandbox/linux/services/libc_interceptor.cc ++++ b/src/3rdparty/chromium/sandbox/linux/services/libc_interceptor.cc +@@ -199,6 +199,7 @@ + g_libc_funcs->localtime64_r = + reinterpret_cast(dlsym(RTLD_NEXT, "localtime64_r")); + ++#if !defined(TOOLKIT_QT) + if (!g_libc_funcs->localtime || !g_libc_funcs->localtime_r) { + // https://bugs.chromium.org/p/chromium/issues/detail?id=16800 + // +@@ -210,6 +211,7 @@ + " time related functions to misbehave. " + "https://bugs.chromium.org/p/chromium/issues/detail?id=16800"; + } ++#endif + + if (!g_libc_funcs->localtime) + g_libc_funcs->localtime = gmtime; diff --git a/libqt5-qtwebengine.changes b/libqt5-qtwebengine.changes index f51adf7..7b94e83 100644 --- a/libqt5-qtwebengine.changes +++ b/libqt5-qtwebengine.changes @@ -1,3 +1,15 @@ +------------------------------------------------------------------- +Fri Feb 21 13:36:31 UTC 2020 - Fabian Vogt + +- Fix a deadlock causing audio/video playback to fail (boo#1163744): + * QTBUG-82186.patch + +------------------------------------------------------------------- +Fri Feb 21 09:25:44 UTC 2020 - Fabian Vogt + +- Fix an issue with selections breaking replying in KMail: + * QTBUG-81574.patch + ------------------------------------------------------------------- Mon Jan 27 13:14:47 UTC 2020 - Fabian Vogt diff --git a/libqt5-qtwebengine.spec b/libqt5-qtwebengine.spec index 06f0559..6d5d41a 100644 --- a/libqt5-qtwebengine.spec +++ b/libqt5-qtwebengine.spec @@ -70,6 +70,10 @@ Patch1: armv6-ffmpeg-no-thumb.patch Patch2: disable-gpu-when-using-nouveau-boo-1005323.diff # PATCH-FIX-UPSTREAM 0001-fix-build-after-y2038-changes-in-glibc.patch Patch3: 0001-fix-build-after-y2038-changes-in-glibc.patch +# PATCH-FIX-UPSTREAM https://codereview.qt-project.org/c/qt/qtwebengine/+/290321 +Patch4: QTBUG-81574.patch +# PATCH-FIX-UPSTREAM https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/291216 +Patch5: QTBUG-82186.patch # http://www.chromium.org/blink not ported to PowerPC ExcludeArch: ppc ppc64 ppc64le s390 s390x # Try to fix i586 MemoryErrors with rpmlint