Bug 191583

Created attachment 354661 [details] Patch

The build errors came from https://bugs.webkit.org/show_bug.cgi?id=90342, in release+logging configurations, the others were some warnings I noticed in a wayland upgrade.

I had a dirty jhbuild for the wayland related changes, rebuilding that here to check the EWS failure.

Comment on attachment 354661 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354661&action=review > Source/WebCore/rendering/RenderLayerCompositor.cpp:654 > #endif I think the !LOG_DISABLED is for compositingLogEnabled() and ENABLE(TREE_DEBUGGING) for showPaintOrderTree. Does ENABLE(TREE_DEBUGGING) imply !LOG_DISABLED? > Source/WebCore/rendering/RenderLayerCompositor.cpp:730 > showPaintOrderTree(m_renderView.layer()); Ditto. > Source/WebCore/rendering/RenderLayerCompositor.cpp:765 > +#if ENABLE(TREE_DEBUGGING) Ditto. > Source/WebCore/rendering/RenderLayerCompositor.cpp:788 > +#endif Shouldn't it be #if !LOG_DISABLED if (compositingLogEnabled()) LOG(Compositing, ...) #endif ? > Source/WebCore/rendering/RenderLayerCompositor.cpp:1003 > +#endif Ditto. > Source/WebCore/rendering/RenderLayerCompositor.cpp:1019 > +#endif Ditto. > Tools/wpe/backends/WindowViewBackend.cpp:384 > + [](void*, struct wl_touch*, int32_t, wl_fixed_t) { } The WPE bot does not seem to like this... Apparently there was a trailing comma in the previous code.

Comment on attachment 354661 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354661&action=review >> Source/WebCore/rendering/RenderLayerCompositor.cpp:654 >> #endif > > I think the !LOG_DISABLED is for compositingLogEnabled() and ENABLE(TREE_DEBUGGING) for showPaintOrderTree. > Does ENABLE(TREE_DEBUGGING) imply !LOG_DISABLED? TREE_DEBUGGING -> !NDEBUG -> !ASSERTIONS_DISABLED_DEFAULT -> !LOG_DISABLED from my analysis. >> Source/WebCore/rendering/RenderLayerCompositor.cpp:788 >> +#endif > > Shouldn't it be > > #if !LOG_DISABLED > if (compositingLogEnabled()) > LOG(Compositing, ...) > #endif > > ? That seems more robust. I was relying on compositingState.depth only being defined in TREE_DEBUGGING mode, so this checked seemed reasonable. Will update to your suggestion. >> Tools/wpe/backends/WindowViewBackend.cpp:384 >> + [](void*, struct wl_touch*, int32_t, wl_fixed_t) { } > > The WPE bot does not seem to like this... Apparently there was a trailing comma in the previous code. Strange that my compiler didn't mind it :/ seems other inline structures in this file don't need the trailing comma, will double check here.

Comment on attachment 354661 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354661&action=review >>> Source/WebCore/rendering/RenderLayerCompositor.cpp:788 >>> +#endif >> >> Shouldn't it be >> >> #if !LOG_DISABLED >> if (compositingLogEnabled()) >> LOG(Compositing, ...) >> #endif >> >> ? > > That seems more robust. I was relying on compositingState.depth only being defined in TREE_DEBUGGING mode, so this checked seemed reasonable. Will update to your suggestion. Actually, that doesn't work properly. The issue is RenderLayerCompositor::CompositingState.depth is not defined when not building in TREE_DEBUGGING, so accessing it without this guard is incorrect.

Created attachment 354662 [details] Patch Add trailing comma to see if it helps the EWS bots in the wayland part of the patch. Can't reproduce that build failure locally with an updated jhbuild.

Created attachment 354663 [details] Patch There must be something out of sync with the trunk jhbuild and the wpe bot's jhbuild, rolling out the local warning fix.

Created attachment 354664 [details] Patch for landing

Comment on attachment 354664 [details] Patch for landing Clearing flags on attachment: 354664 Committed r238130: <https://trac.webkit.org/changeset/238130>

All reviewed patches have been landed. Closing bug.

<rdar://problem/46026592>