RESOLVED FIXED 196825
REGRESSION (r244182): RemoteLayerTreeDrawingArea::flushLayers() should not be reentrant 
https://bugs.webkit.org/show_bug.cgi?id=196825
Summary REGRESSION (r244182): RemoteLayerTreeDrawingArea::flushLayers() should not be...
Said Abou-Hallawa
Reported 2019-04-11 12:25:49 PDT
After r244182, RemoteLayerTreeDrawingArea::flushLayers() can be reentrant when running run-webkit-tests. This can happen when notifyDone() is called from the rAF callback which forces repaint. Here is the problematic call stack: 3 0x1033b3acd WebKit::RemoteLayerTreeDrawingArea::flushLayers() 4 0x1033b64be WebKit::RemoteLayerTreeDrawingArea::forceRepaint() 5 0x104462f85 WebKit::WebPage::forceRepaintWithoutCallback() 6 0x10413ddbd WKBundlePageForceRepaint 7 0x4ec12346e WTR::InjectedBundlePage::dump() 8 0x4ec146afd WTR::TestRunner::notifyDone() 9 0x4ec1390a7 WTR::JSTestRunner::notifyDone(OpaqueJSContext const*, OpaqueJSValue*, OpaqueJSValue*, unsigned long, OpaqueJSValue const* const*, OpaqueJSValue const**) 10 0x4cb554d51 long long JSC::APICallbackFunction::call<JSC::JSCallbackFunction>(JSC::ExecState*) 11 0x23a354601027 12 0x4cb4d18b1 llint_entry 13 0x4cb4d18b1 llint_entry 14 0x4cb4d18b1 llint_entry 15 0x4cb4be500 vmEntryToJavaScript 16 0x4cbe3dace JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) 17 0x4cbe3e0ff JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) 18 0x4cc115c4c JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) 19 0x4cc115d3a JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) 20 0x4cc11602e JSC::profiledCall(JSC::ExecState*, JSC::ProfilingReason, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) 21 0x4d0ec3fdb WebCore::JSExecState::profiledCall(JSC::ExecState*, JSC::ProfilingReason, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) 22 0x4d0ec3e8f WebCore::JSCallbackData::invokeCallback(WebCore::JSDOMGlobalObject&, JSC::JSObject*, JSC::JSValue, JSC::MarkedArgumentBuffer&, WebCore::JSCallbackData::CallbackType, JSC::PropertyName, WTF::NakedPtr<JSC::Exception>&) 23 0x4cf65d332 WebCore::JSCallbackDataStrong::invokeCallback(JSC::JSValue, JSC::MarkedArgumentBuffer&, WebCore::JSCallbackData::CallbackType, JSC::PropertyName, WTF::NakedPtr<JSC::Exception>&) 24 0x4d020deb9 WebCore::JSRequestAnimationFrameCallback::handleEvent(double) 25 0x4d15d8344 WebCore::ScriptedAnimationController::serviceRequestAnimationFrameCallbacks(double) 26 0x4d14180d6 WebCore::Document::serviceRequestAnimationFrameCallbacks(double) 27 0x4d1f29387 WebCore::Page::updateRendering() 28 0x1044642d4 WebKit::WebPage::updateRendering() 29 0x1033b3ae9 WebKit::RemoteLayerTreeDrawingArea::flushLayers() 30 0x1033bcf91 WTF::Function<void ()>::CallableWrapper<std::__1::__bind<void (WebKit::RemoteLayerTreeDrawingArea::*&)(), WebKit::RemoteLayerTreeDrawingArea*> >::call() 31 0x1032f665d WTF::Function<void ()>::operator()() const This call stack was caught by the iOS simulator layout tests because RemoteLayerTreeDrawingAreaProxy::commitLayerTree() asserts the transition IDs are sequential.
Attachments
Patch (5.44 KB, patch)
2019-04-11 12:31 PDT, Said Abou-Hallawa 		no flags
DetailsFormatted DiffDiff
Patch (2.85 KB, patch)
2019-04-11 14:18 PDT, Said Abou-Hallawa 		no flags
DetailsFormatted DiffDiff
Patch (2.60 KB, patch)
2019-04-11 14:29 PDT, Said Abou-Hallawa 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Said Abou-Hallawa
Comment 1 2019-04-11 12:31:22 PDT
Created attachment 367233 [details] Patch
Tim Horton
Comment 2 2019-04-11 13:11:40 PDT
Comment on attachment 367233 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=367233&action=review > Source/WebKit/WebProcess/WebPage/WebPage.cpp:3659 > +void WebPage::scheduleCompositingLayerFlush() I don't think you need this > Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:350 > + m_webPage.scheduleCompositingLayerFlush(); Why doesn't this just call scheduleCOmpositingLayerFlush like the above?
Simon Fraser (smfr)
Comment 3 2019-04-11 13:54:16 PDT
Comment on attachment 367233 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=367233&action=review > Source/WebKit/ChangeLog:9 > + detected. But scheduleCompositingLayerFlush() to make a final repaint. But that will be async, which breaks testing. Why bother scheduling it at all? >> Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:350 >> + m_webPage.scheduleCompositingLayerFlush(); > > Why doesn't this just call scheduleCOmpositingLayerFlush like the above? Is it really necessary?
Said Abou-Hallawa
Comment 4 2019-04-11 14:18:28 PDT
Created attachment 367239 [details] Patch
Said Abou-Hallawa
Comment 5 2019-04-11 14:22:33 PDT
Comment on attachment 367233 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=367233&action=review >>> Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:350 >>> + m_webPage.scheduleCompositingLayerFlush(); >> >> Why doesn't this just call scheduleCOmpositingLayerFlush like the above? > > Is it really necessary? It will not be necessary if notifyDone() is the only reason to re-enter this function. I will give it a try and upload another patch.
Said Abou-Hallawa
Comment 6 2019-04-11 14:29:21 PDT
Created attachment 367240 [details] Patch
WebKit Commit Bot
Comment 7 2019-04-11 15:51:49 PDT
Comment on attachment 367240 [details] Patch Clearing flags on attachment: 367240 Committed r244198: <https://trac.webkit.org/changeset/244198>
WebKit Commit Bot
Comment 8 2019-04-11 15:51:50 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 9 2019-04-11 15:52:19 PDT
<rdar://problem/49831390>
Note You need to log in before you can comment on or make changes to this bug.